Properly force-clean for shortening bytesXX conversions.

[chriseth]

Fixes #3867

It seems like the proper thing is done for integers, but not for bytesXX types.

[chriseth]

This is weird, because we actually have a test for this: https://github.com/ethereum/solidity/blob/develop/test/libsolidity/SolidityEndToEndTest.cpp#L1580

[chriseth]

Ok, I think I can see why this is still fine in most cases: Whenever we store in storage, we chop additional bits, so storage is safe. Whenever we store in memory, we do an additional cleanup, and for that additional cleanup, we either widen (implicit conversion) or we do not change the size at all. In both cases, the typeOnStack is the shorter one, so we do proper cleanup.

The only case where this can be problematic that I can think of is:

contract test {
    function f() public pure returns (bytes32 r) {
        bytes32 x = bytes32(-1);
        bytes8 a = bytes8(x);
        assembly { r := a }
    }
}

[axic]

Doesn't the ABI encoder does cleaning on its own?

A better test would be:

function f(bytes4 in) returns (bytes2 out) {
  out = in;
  assembly {
    if eq(in, out) { invalid() }
  }
}

[axic]

unsigned?

[chriseth]

numBytes() returns int.

[axic]

Hm, what case can numBytes be negative? Is that another oversight?

[axic]

It seems to be an oversight since there's an assert forcing it to be unsigned: https://github.com/ethereum/solidity/blob/develop/libsolidity/ast/Types.cpp#L1146

[chriseth]

Yes, this is a very old line in the code. I guess the idea was that it is easier to subtract bytes or something.

[axic]

Need to be rebased and remove the extra masking in abi.encode.

[chriseth]

Finished.

[chriseth]

Will add documentation to inline assembly that states when accessing local variables, short types are not guaranteed to have clean higher order bits.

[chriseth]

Updated.

[erak]

Looks good to me.

[axic]

Should have given an example.

[axic]

I actually wonder why shouldn't we properly apply the cleanup before the assembly block?

[chriseth]

We can actually check how the optimizer copes with cleanup done all the time.