diff --git a/Changelog.md b/Changelog.md index 571c1011d..510feae9f 100644 --- a/Changelog.md +++ b/Changelog.md @@ -16,6 +16,8 @@ Bugfixes: * Code Generator: Fix ICE when doing an explicit conversion from ``string calldata`` to ``bytes``. * Control Flow Graph: Perform proper virtual lookup for modifiers for uninitialized variable and unreachable code analysis. * Immutables: Fix wrong error when the constructor of a base contract uses ``return`` and the parent contract contains immutable variables. + * IR Generator: Add missing cleanup during the conversion of fixed bytes types to smaller fixed bytes types. + * IR Generator: Add missing cleanup for indexed event arguments of value type. * IR Generator: Fix IR syntax error when copying storage arrays of structs containing functions. * Natspec: Fix ICE when overriding a struct getter with a Natspec-documented return value and the name in the struct is different. * TypeChecker: Fix ICE when a constant variable declaration forward references a struct. diff --git a/libsolidity/codegen/YulUtilFunctions.cpp b/libsolidity/codegen/YulUtilFunctions.cpp index 217ec14b0..84f63a9e1 100644 --- a/libsolidity/codegen/YulUtilFunctions.cpp +++ b/libsolidity/codegen/YulUtilFunctions.cpp @@ -3400,11 +3400,11 @@ string YulUtilFunctions::conversionFunction(Type const& _from, Type const& _to) .render(); else { - // clear for conversion to longer bytes solAssert(toCategory == Type::Category::FixedBytes, "Invalid type conversion requested."); + FixedBytesType const& to = dynamic_cast(_to); body = Whiskers("converted := (value)") - ("clean", cleanupFunction(from)) + ("clean", cleanupFunction((to.numBytes() <= from.numBytes()) ? to : from)) .render(); } break; diff --git a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp index 420c48baa..cf48df470 100644 --- a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp +++ b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp @@ -1035,7 +1035,10 @@ void IRGeneratorForStatements::endVisit(FunctionCall const& _functionCall) ")\n"; } else - indexedArgs.emplace_back(convert(arg, *paramTypes[i])); + { + solAssert(parameterTypes[i]->sizeOnStack() == 1, ""); + indexedArgs.emplace_back(convert(arg, *paramTypes[i], true)); + } } else { @@ -2724,14 +2727,14 @@ void IRGeneratorForStatements::assignInternalFunctionIDIfNotCalledDirectly( m_context.addToInternalDispatch(_referencedFunction); } -IRVariable IRGeneratorForStatements::convert(IRVariable const& _from, Type const& _to) +IRVariable IRGeneratorForStatements::convert(IRVariable const& _from, Type const& _to, bool _forceCleanup) { - if (_from.type() == _to) + if (_from.type() == _to && !_forceCleanup) return _from; else { IRVariable converted(m_context.newYulVariable(), _to); - define(converted, _from); + define(converted, _from, _forceCleanup); return converted; } } @@ -2763,10 +2766,10 @@ void IRGeneratorForStatements::declare(IRVariable const& _var) appendCode() << "let " << _var.commaSeparatedList() << "\n"; } -void IRGeneratorForStatements::declareAssign(IRVariable const& _lhs, IRVariable const& _rhs, bool _declare) +void IRGeneratorForStatements::declareAssign(IRVariable const& _lhs, IRVariable const& _rhs, bool _declare, bool _forceCleanup) { string output; - if (_lhs.type() == _rhs.type()) + if (_lhs.type() == _rhs.type() && !_forceCleanup) for (auto const& [stackItemName, stackItemType]: _lhs.type().stackItems()) if (stackItemType) declareAssign(_lhs.part(stackItemName), _rhs.part(stackItemName), _declare); diff --git a/libsolidity/codegen/ir/IRGeneratorForStatements.h b/libsolidity/codegen/ir/IRGeneratorForStatements.h index 74b7def54..10e8f0b5e 100644 --- a/libsolidity/codegen/ir/IRGeneratorForStatements.h +++ b/libsolidity/codegen/ir/IRGeneratorForStatements.h @@ -86,7 +86,11 @@ public: IRVariable evaluateExpression(Expression const& _expression, Type const& _to); /// Defines @a _var using the value of @a _value while performing type conversions, if required. - void define(IRVariable const& _var, IRVariable const& _value) { declareAssign(_var, _value, true); } + /// If @a _forceCleanup is set to true, it also cleans the value of the variable after the conversion. + void define(IRVariable const& _var, IRVariable const& _value, bool _forceCleanup = false) + { + declareAssign(_var, _value, true, _forceCleanup); + } /// @returns the name of a function that computes the value of the given constant /// and also generates the function. @@ -162,8 +166,8 @@ private: ); /// Generates the required conversion code and @returns an IRVariable referring to the value of @a _variable - /// converted to type @a _to. - IRVariable convert(IRVariable const& _variable, Type const& _to); + /// If @a _forceCleanup is set to true, it also cleans the value of the variable after the conversion. + IRVariable convert(IRVariable const& _variable, Type const& _to, bool _forceCleanup = false); /// @returns a Yul expression representing the current value of @a _expression, /// converted to type @a _to if it does not yet have that type. @@ -179,7 +183,7 @@ private: /// Declares variable @a _var. void declare(IRVariable const& _var); - void declareAssign(IRVariable const& _var, IRVariable const& _value, bool _define); + void declareAssign(IRVariable const& _var, IRVariable const& _value, bool _define, bool _forceCleanup = false); /// @returns an IRVariable with the zero /// value of @a _type. diff --git a/test/libsolidity/semanticTests/cleanup/cleanup_bytes_types_shortening.sol b/test/libsolidity/semanticTests/cleanup/cleanup_bytes_types_shortening_OldCodeGen.sol similarity index 77% rename from test/libsolidity/semanticTests/cleanup/cleanup_bytes_types_shortening.sol rename to test/libsolidity/semanticTests/cleanup/cleanup_bytes_types_shortening_OldCodeGen.sol index edae83729..9ca68e6d0 100644 --- a/test/libsolidity/semanticTests/cleanup/cleanup_bytes_types_shortening.sol +++ b/test/libsolidity/semanticTests/cleanup/cleanup_bytes_types_shortening_OldCodeGen.sol @@ -11,7 +11,6 @@ contract C { } } // ==== -// compileToEwasm: also -// compileViaYul: also +// compileViaYul: false // ---- -// f() -> "\xff\xff\xff\xff" +// f() -> 0xffffffff00000000000000000000000000000000000000000000000000000000 diff --git a/test/libsolidity/semanticTests/cleanup/cleanup_bytes_types_shortening_newCodeGen.sol b/test/libsolidity/semanticTests/cleanup/cleanup_bytes_types_shortening_newCodeGen.sol new file mode 100644 index 000000000..a99b82a08 --- /dev/null +++ b/test/libsolidity/semanticTests/cleanup/cleanup_bytes_types_shortening_newCodeGen.sol @@ -0,0 +1,14 @@ +contract C { + function f() public pure returns (bytes32 r) { + bytes4 x = 0xffffffff; + bytes2 y = bytes2(x); + assembly { + r := y + } + } +} +// ==== +// compileToEwasm: also +// compileViaYul: true +// ---- +// f() -> 0xffff000000000000000000000000000000000000000000000000000000000000 diff --git a/test/libsolidity/semanticTests/cleanup/indexed_log_topic_during_explicit_downcast.sol b/test/libsolidity/semanticTests/cleanup/indexed_log_topic_during_explicit_downcast.sol new file mode 100644 index 000000000..f0aa239ba --- /dev/null +++ b/test/libsolidity/semanticTests/cleanup/indexed_log_topic_during_explicit_downcast.sol @@ -0,0 +1,23 @@ +contract C { + function f() public pure returns (uint32 y) { + uint8 x = uint8(uint256(0x31313131313131313131313131313131)); + assembly { y := x } + } + + function g() public pure returns (bytes32 y) { + bytes1 x = bytes1(bytes16(0x31313131313131313131313131313131)); + assembly { y := x } + } + + function h() external returns (bytes32 y) { + bytes1 x; + assembly { x := sub(0,1) } + y = x; + } +} +// ==== +// compileViaYul: true +// ---- +// f() -> 0x31 +// g() -> 0x3100000000000000000000000000000000000000000000000000000000000000 +// h() -> 0xff00000000000000000000000000000000000000000000000000000000000000 diff --git a/test/libsolidity/semanticTests/cleanup/indexed_log_topic_during_explicit_downcast_during_emissions.sol b/test/libsolidity/semanticTests/cleanup/indexed_log_topic_during_explicit_downcast_during_emissions.sol new file mode 100644 index 000000000..df6bc2192 --- /dev/null +++ b/test/libsolidity/semanticTests/cleanup/indexed_log_topic_during_explicit_downcast_during_emissions.sol @@ -0,0 +1,19 @@ +contract C { + event ev0(bytes1 indexed); + constructor() { + emit ev0(bytes1(bytes16(0x31313131313131313131313131313131))); + } + function j() external { + bytes1 x; + assembly { x := 0x3131313131313131313131313131313131313131313131313131313131313131 } + emit ev0(x); + } +} +// ==== +// compileViaYul: also +// ---- +// constructor() -> +// ~ emit ev0(bytes1): #"1" +// gas legacy: 168735 +// j() -> +// ~ emit ev0(bytes1): #"1" diff --git a/test/libsolidity/semanticTests/userDefinedValueType/erc20.sol b/test/libsolidity/semanticTests/userDefinedValueType/erc20.sol index efe2ee114..220d9d9a2 100644 --- a/test/libsolidity/semanticTests/userDefinedValueType/erc20.sol +++ b/test/libsolidity/semanticTests/userDefinedValueType/erc20.sol @@ -115,7 +115,7 @@ contract ERC20 { // ---- // constructor() // ~ emit Transfer(address,address,uint256): #0x00, #0x1212121212121212121212121212120000000012, 0x14 -// gas irOptimized: 442239 +// gas irOptimized: 447831 // gas legacy: 861559 // gas legacyOptimized: 420959 // totalSupply() -> 20 diff --git a/test/libsolidity/semanticTests/various/erc20.sol b/test/libsolidity/semanticTests/various/erc20.sol index 204e620d6..64a3286a9 100644 --- a/test/libsolidity/semanticTests/various/erc20.sol +++ b/test/libsolidity/semanticTests/various/erc20.sol @@ -98,7 +98,7 @@ contract ERC20 { // ---- // constructor() // ~ emit Transfer(address,address,uint256): #0x00, #0x1212121212121212121212121212120000000012, 0x14 -// gas irOptimized: 437697 +// gas irOptimized: 443295 // gas legacy: 833310 // gas legacyOptimized: 416135 // totalSupply() -> 20