solidity icon indicating copy to clipboard operation
solidity copied to clipboard

[TypeChecker] ICE in memoryMemberTypes()

Open bshastry opened this issue 1 year ago • 2 comments

Description

pragma solidity >= 0.0.0;
contract C0 {
  struct St0 {
    uint40 el0;
    mapping(uint224 => int64) el1;
  }
  type T0 is uint40;
  function f0(C0.T0 i0,bytes12 i1) external    returns(bool o0,bool o1)  {
  }
  fallback() external virtual  
  {
    (bool  l0, C0.St0[8] memory l1, C0.St0 memory l2) = abi.decode(abi.encodeCall(this.f0, (C0.T0.wrap(uint40(1099511627775)), bytes12(bytes29(0x0000000000000000000000000000000000000000000000000000000000)))), (bool, C0.St0[8], C0.St0));
  }
}

throws

https://github.com/ethereum/solidity/blob/4577aebfd24a8dec47f7ce085d4e8887f5b8b3af/libsolidity/ast/Types.cpp#L2553

Environment

  • Compiler version: 4edbaf1e1
  • Target EVM version (as per compiler settings): shanghai

Steps to Reproduce

solc test.sol

bshastry avatar Feb 02 '24 05:02 bshastry

Thanks @bshastry. I reduced the example to this:

pragma solidity >=0.0;

contract C {
  struct S {
    mapping(uint224 => int64) b;
  }
  function f() external returns(bool)  {}
  fallback() external virtual
  {
    (, C.S[2] memory ret) = abi.decode(abi.encodeCall(this.f, ()), (bool, C.S[2]));
  }
}

I'm wondering whether this issue might be related to the fact that if a struct includes a map, it's restricted to storage use only. So, this might not being detected by the compiler when doing low-level calls, and if that is the case, we may need to remove that assert and properly handle this case. For instance, if you do a direct call:

C.S[2] memory ret = this.f();

It will throw the error:

Error: Type struct C.S[2] memory is only valid in storage because it contains a (nested) mapping.

Maybe @ekpyron or @cameel has a better idea of the issue?

r0qs avatar Feb 05 '24 13:02 r0qs

It's just a fluke of type-checking. The order in which we check things is a mess in our current analysis, so it happens easily like in this case that we assert against something assuming that it ought to already have triggered an error, while the error is only actually emitted later. So in the end this boils down to "this should be a better, more graceful error", but it's not particularly high priority to fix.

ekpyron avatar Feb 05 '24 13:02 ekpyron