ethereum / solidity

Solidity, the Smart Contract Programming Language
https://soliditylang.org
GNU General Public License v3.0
22.91k stars 5.68k forks source link

[TypeChecker] ICE in spurious dragon due to unset return parameter decoding type in recursive library function call with a return function type #12305

Open bshastry opened 2 years ago

bshastry commented 2 years ago
library L {
  function f() returns (function() r) {
    L.f();
  }
}

throws

https://github.com/ethereum/solidity/blob/defc74c8a248585b9fe1ddc8a10d10f11c92c052/libsolidity/ast/Types.cpp#L2877

Repro:

$ solc --evm-version spuriousDragon test.sol
cameel commented 2 years ago

This started happening in 0.7.4.

Slightly simpler repro (with no syntax errors):

library L {
    function f() external returns (function() r) {}
}

function g() {
    L.f();
}
Internal compiler error:
/solidity/libsolidity/ast/Types.cpp(2877): Throw in function solidity::frontend::TypePointers solidity::frontend::FunctionType::returnParameterTypesWithoutDynamicTypes() const
Dynamic exception type: boost::wrapexcept<solidity::langutil::InternalCompilerError>
std::exception::what: Solidity assertion failed
[solidity::util::tag_comment*] = Solidity assertion failed

Using an internal type as return value here is still an error though. I don't think it's actually reproducible on something that would compile.

Also, this is only broken for libraries, contracts are fine:

contract C {
    function f() external returns (function() r) {}
}

function g() {
    C c;
    c.f();
}
Error: Internal type is not allowed for public or external functions.
 --> test.sol:2:36:
  |
2 |     function f() external returns (function() r) {}
  |                                    ^^^^^^^^^^^^

Error: Member "f" not found or not visible after argument-dependent lookup in contract C.
 --> test.sol:7:5:
  |
7 |     c.f();
  |     ^^^
chriseth commented 2 years ago

It looks like we need to distinguish between internal types like storage references and internal types like internal function pointers. The latter should only be invalid in non-internal library functions...