CodeGen: Avoid double cleanup when copying to memory

This commit is contained in:
Mathias Baumann 2020-06-08 17:52:40 +02:00 committed by Mathias Baumann
parent f47981d484
commit d3af598c49
4 changed files with 20 additions and 9 deletions

View File

@ -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.

View File

@ -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<ReferenceType const*>(&_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<StringLiteralType const*>(&_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<ArrayType const*>(type))
ArrayUtils(m_context).copyArrayToMemory(*arrayType, _padToWordBoundaries);
else if (auto arraySliceType = dynamic_cast<ArraySliceType const*>(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);

View File

@ -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);

View File

@ -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()