Moved storage size assert to TypeChecker from DeclarationTypeChecker #12186
Conversation
hrkrshnn
force-pushed
the
fix-userdefined-ice
branch
from
da127a1 to
0f0d5ca
Compare
Changelog.md
Outdated
| @@ -20,6 +20,7 @@ Bugfixes: | |||
| * Commandline Interface: Disallow ``--error-recovery`` option outside of the compiler mode. | |||
| * SMTChecker: Fix internal error in magic type access (``block``, ``msg``, ``tx``). | |||
| * TypeChecker: Fix internal error when using user defined value types in public library functions. | |||
| * TypeChecker: Fix internal error when using arrays with user defined value types before declaration. | |||
There was a problem hiding this comment.
Looks like it's not just arrays. I just found a new failing case (and it fails even with this fix):
struct S { U u; }
contract C { S s; }
type U is address;Internal compiler error:
/solidity/libsolidity/ast/Types.cpp(2538): Throw in function const solidity::frontend::Type& solidity::frontend::UserDefinedValueType::underlyingType() const
Dynamic exception type: boost::wrapexcept<solidity::langutil::InternalCompilerError>
std::exception::what: Solidity assertion failed
[solidity::util::tag_comment*] = Solidity assertion failed
There was a problem hiding this comment.
Will take a look now. Thanks for noticing!
There was a problem hiding this comment.
@bshastry Any idea why the fuzzer missed this?
There was a problem hiding this comment.
This basically opened a bag of bugs. Basically any call to underlyingType() from DeclarationTypeChecker can be converted into an ICE. Will fix all of them.
There was a problem hiding this comment.
Wasn't as bad as I thought, other than canBeStored (from the struct example), only isValueType() will cause issues:
MyInt constant x;
type MyInt is int;
There was a problem hiding this comment.
@bshastry Any idea why the fuzzer missed this?
Because, the grammar-based generator does not do a second pass over type declarations i.e., forward references are not created.
There was a problem hiding this comment.
@hrkrshnn are the things you mention (canBeStored...) already in this PR? Could not find any changes.
There was a problem hiding this comment.
Not yet, moving https://github.com/ethereum/solidity/blob/develop/libsolidity/analysis/DeclarationTypeChecker.cpp#L442 to TypeChecker created additional issues in other tests. Will keep this as draft for now.
hrkrshnn
force-pushed
the
fix-userdefined-ice
branch
from
0f0d5ca to
cbee4fc
Compare
This is to avoid a assert from failing for forward declared user defined value types.
And added a proper error message when constant types containing (nested) mapping types are used.
hrkrshnn
force-pushed
the
fix-userdefined-ice
branch
from
06c87cf to
8815d6f
Compare
| m_errorReporter.fatalDeclarationError( | ||
| 3530_error, | ||
| _variable.location(), | ||
| "The type contains a (nested) mapping and therefore cannot be a constant." |
There was a problem hiding this comment.
See existing tests with this message. They were throwing, without this additional check.
Replaces: #12163
Fixes: #12159