diff --git a/Changelog.md b/Changelog.md index 4953a4e49..0f0e533b9 100644 --- a/Changelog.md +++ b/Changelog.md @@ -12,6 +12,7 @@ Bugfixes: * SMTChecker: Fix internal error when using bitwise operators on fixed bytes type. * Type Checker: Fix overload resolution in combination with ``{value: ...}``. * Type Checker: Fix internal compiler error related to oversized types. + * Code Generator: Avoid double cleanup when copying to memory. Compiler Features: * Build System: Update internal dependency of jsoncpp to 1.9.3. diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp index b3fc9da2e..2d7b38b25 100644 --- a/libsolidity/codegen/CompilerUtils.cpp +++ b/libsolidity/codegen/CompilerUtils.cpp @@ -180,7 +180,7 @@ void CompilerUtils::storeInMemory(unsigned _offset) m_context << u256(_offset) << Instruction::MSTORE; } -void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBoundaries) +void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBoundaries, bool _cleanup) { // process special types (Reference, StringLiteral, Function) if (auto ref = dynamic_cast(&_type)) @@ -189,7 +189,7 @@ void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBound ref->location() == DataLocation::Memory, "Only in-memory reference type can be stored." ); - storeInMemoryDynamic(*TypeProvider::uint256(), _padToWordBoundaries); + storeInMemoryDynamic(*TypeProvider::uint256(), _padToWordBoundaries, _cleanup); } else if (auto str = dynamic_cast(&_type)) { @@ -212,7 +212,7 @@ void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBound } else if (_type.isValueType()) { - unsigned numBytes = prepareMemoryStore(_type, _padToWordBoundaries); + unsigned numBytes = prepareMemoryStore(_type, _padToWordBoundaries, _cleanup); m_context << Instruction::DUP2 << Instruction::MSTORE; m_context << u256(numBytes) << Instruction::ADD; } @@ -463,6 +463,7 @@ void CompilerUtils::encodeToMemory( } else { + bool needCleanup = true; copyToStackTop(argSize - stackPos + dynPointers + 2, _givenTypes[i]->sizeOnStack()); solAssert(!!targetType, "Externalable type expected."); TypePointer type = targetType; @@ -481,7 +482,11 @@ void CompilerUtils::encodeToMemory( ) type = _givenTypes[i]; // delay conversion else + { convertType(*_givenTypes[i], *targetType, true); + needCleanup = false; + } + if (auto arrayType = dynamic_cast(type)) ArrayUtils(m_context).copyArrayToMemory(*arrayType, _padToWordBoundaries); else if (auto arraySliceType = dynamic_cast(type)) @@ -495,7 +500,7 @@ void CompilerUtils::encodeToMemory( ArrayUtils(m_context).copyArrayToMemory(arraySliceType->arrayType(), _padToWordBoundaries); } else - storeInMemoryDynamic(*type, _padToWordBoundaries); + storeInMemoryDynamic(*type, _padToWordBoundaries, needCleanup); } stackPos += _givenTypes[i]->sizeOnStack(); } @@ -1499,7 +1504,7 @@ void CompilerUtils::rightShiftNumberOnStack(unsigned _bits) m_context << (u256(1) << _bits) << Instruction::SWAP1 << Instruction::DIV; } -unsigned CompilerUtils::prepareMemoryStore(Type const& _type, bool _padToWords) +unsigned CompilerUtils::prepareMemoryStore(Type const& _type, bool _padToWords, bool _cleanup) { solAssert( _type.sizeOnStack() == 1, @@ -1522,7 +1527,9 @@ unsigned CompilerUtils::prepareMemoryStore(Type const& _type, bool _padToWords) bool leftAligned = _type.category() == Type::Category::FixedBytes; - convertType(_type, _type, true); + if (_cleanup) + convertType(_type, _type, true); + if (numBytes != 32 && !leftAligned && !_padToWords) // shift the value accordingly before storing leftShiftNumberOnStack((32 - numBytes) * 8); diff --git a/libsolidity/codegen/CompilerUtils.h b/libsolidity/codegen/CompilerUtils.h index a3c266bde..a65edb8f2 100644 --- a/libsolidity/codegen/CompilerUtils.h +++ b/libsolidity/codegen/CompilerUtils.h @@ -107,10 +107,11 @@ public: /// and also updates that. For reference types, only copies the data pointer. Fails for /// non-memory-references. /// @param _padToWords if true, adds zeros to pad to multiple of 32 bytes. Array elements + /// @param _cleanup if true, adds code to cleanup the value before storing it. /// are always padded (except for byte arrays), regardless of this parameter. /// Stack pre: memory_offset value... /// Stack post: (memory_offset+length) - void storeInMemoryDynamic(Type const& _type, bool _padToWords = true); + void storeInMemoryDynamic(Type const& _type, bool _padToWords = true, bool _cleanup = true); /// Creates code that unpacks the arguments according to their types specified by a vector of TypePointers. /// From memory if @a _fromMemory is true, otherwise from call data. @@ -309,7 +310,8 @@ private: void cleanHigherOrderBits(IntegerType const& _typeOnStack); /// Prepares the given type for storing in memory by shifting it if necessary. - unsigned prepareMemoryStore(Type const& _type, bool _padToWords); + /// @param _cleanup if true, also cleanup the value when preparing to store it in memory + unsigned prepareMemoryStore(Type const& _type, bool _padToWords, bool _cleanup = true); /// Loads type from memory assuming memory offset is on stack top. unsigned loadFromMemoryHelper(Type const& _type, bool _fromCalldata, bool _padToWords); diff --git a/test/libsolidity/SolidityOptimizer.cpp b/test/libsolidity/SolidityOptimizer.cpp index 68c500e8c..c4f7a2e71 100644 --- a/test/libsolidity/SolidityOptimizer.cpp +++ b/test/libsolidity/SolidityOptimizer.cpp @@ -718,7 +718,8 @@ BOOST_AUTO_TEST_CASE(avoid_double_cleanup) } )"; compileBothVersions(sourceCode, 0, "C", 50); - BOOST_CHECK_EQUAL(numInstructions(m_nonOptimizedBytecode, Instruction::AND), 2); + // Check that there is no double AND instruction in the resulting code + BOOST_CHECK_EQUAL(numInstructions(m_nonOptimizedBytecode, Instruction::AND), 1); } BOOST_AUTO_TEST_SUITE_END()