diff options
-rw-r--r-- | libevmasm/Assembly.cpp | 4 | ||||
-rw-r--r-- | libevmasm/CommonSubexpressionEliminator.h | 8 | ||||
-rw-r--r-- | libevmasm/Instruction.cpp | 2 | ||||
-rw-r--r-- | libevmasm/SemanticInformation.cpp | 7 | ||||
-rw-r--r-- | libevmasm/SemanticInformation.h | 3 | ||||
-rw-r--r-- | libsolidity/ast/Types.h | 13 | ||||
-rw-r--r-- | libsolidity/codegen/CompilerUtils.cpp | 36 | ||||
-rw-r--r-- | libsolidity/codegen/ExpressionCompiler.cpp | 21 | ||||
-rw-r--r-- | test/libevmasm/Optimiser.cpp | 5 | ||||
-rw-r--r-- | test/libsolidity/SolidityEndToEndTest.cpp | 23 |
10 files changed, 87 insertions, 35 deletions
diff --git a/libevmasm/Assembly.cpp b/libevmasm/Assembly.cpp index bd4ebf59..b71bc80c 100644 --- a/libevmasm/Assembly.cpp +++ b/libevmasm/Assembly.cpp @@ -438,13 +438,15 @@ map<u256, u256> Assembly::optimiseInternal( // function types that can be stored in storage. AssemblyItems optimisedItems; + bool usesMSize = (find(m_items.begin(), m_items.end(), AssemblyItem(Instruction::MSIZE)) != m_items.end()); + auto iter = m_items.begin(); while (iter != m_items.end()) { KnownState emptyState; CommonSubexpressionEliminator eliminator(emptyState); auto orig = iter; - iter = eliminator.feedItems(iter, m_items.end()); + iter = eliminator.feedItems(iter, m_items.end(), usesMSize); bool shouldReplace = false; AssemblyItems optimisedChunk; try diff --git a/libevmasm/CommonSubexpressionEliminator.h b/libevmasm/CommonSubexpressionEliminator.h index 0b957a0e..b20de246 100644 --- a/libevmasm/CommonSubexpressionEliminator.h +++ b/libevmasm/CommonSubexpressionEliminator.h @@ -65,8 +65,9 @@ public: /// Feeds AssemblyItems into the eliminator and @returns the iterator pointing at the first /// item that must be fed into a new instance of the eliminator. + /// @param _msizeImportant if false, do not consider modification of MSIZE a side-effect template <class _AssemblyItemIterator> - _AssemblyItemIterator feedItems(_AssemblyItemIterator _iterator, _AssemblyItemIterator _end); + _AssemblyItemIterator feedItems(_AssemblyItemIterator _iterator, _AssemblyItemIterator _end, bool _msizeImportant); /// @returns the resulting items after optimization. AssemblyItems getOptimizedItems(); @@ -168,11 +169,12 @@ private: template <class _AssemblyItemIterator> _AssemblyItemIterator CommonSubexpressionEliminator::feedItems( _AssemblyItemIterator _iterator, - _AssemblyItemIterator _end + _AssemblyItemIterator _end, + bool _msizeImportant ) { assertThrow(!m_breakingItem, OptimizerException, "Invalid use of CommonSubexpressionEliminator."); - for (; _iterator != _end && !SemanticInformation::breaksCSEAnalysisBlock(*_iterator); ++_iterator) + for (; _iterator != _end && !SemanticInformation::breaksCSEAnalysisBlock(*_iterator, _msizeImportant); ++_iterator) feedItem(*_iterator); if (_iterator != _end) m_breakingItem = &(*_iterator++); diff --git a/libevmasm/Instruction.cpp b/libevmasm/Instruction.cpp index a677a631..f9bbad2c 100644 --- a/libevmasm/Instruction.cpp +++ b/libevmasm/Instruction.cpp @@ -199,7 +199,7 @@ static const std::map<Instruction, InstructionInfo> c_instructionInfo = { Instruction::ADDMOD, { "ADDMOD", 0, 3, 1, false, Tier::Mid } }, { Instruction::MULMOD, { "MULMOD", 0, 3, 1, false, Tier::Mid } }, { Instruction::SIGNEXTEND, { "SIGNEXTEND", 0, 2, 1, false, Tier::Low } }, - { Instruction::KECCAK256, { "KECCAK256", 0, 2, 1, false, Tier::Special } }, + { Instruction::KECCAK256, { "KECCAK256", 0, 2, 1, true, Tier::Special } }, { Instruction::ADDRESS, { "ADDRESS", 0, 0, 1, false, Tier::Base } }, { Instruction::BALANCE, { "BALANCE", 0, 1, 1, false, Tier::Balance } }, { Instruction::ORIGIN, { "ORIGIN", 0, 0, 1, false, Tier::Base } }, diff --git a/libevmasm/SemanticInformation.cpp b/libevmasm/SemanticInformation.cpp index 03870f7c..71267ee8 100644 --- a/libevmasm/SemanticInformation.cpp +++ b/libevmasm/SemanticInformation.cpp @@ -28,7 +28,7 @@ using namespace std; using namespace dev; using namespace dev::eth; -bool SemanticInformation::breaksCSEAnalysisBlock(AssemblyItem const& _item) +bool SemanticInformation::breaksCSEAnalysisBlock(AssemblyItem const& _item, bool _msizeImportant) { switch (_item.type()) { @@ -59,6 +59,11 @@ bool SemanticInformation::breaksCSEAnalysisBlock(AssemblyItem const& _item) return false; if (_item.instruction() == Instruction::MSTORE) return false; + if (!_msizeImportant && ( + _item.instruction() == Instruction::MLOAD || + _item.instruction() == Instruction::KECCAK256 + )) + return false; //@todo: We do not handle the following memory instructions for now: // calldatacopy, codecopy, extcodecopy, mstore8, // msize (note that msize also depends on memory read access) diff --git a/libevmasm/SemanticInformation.h b/libevmasm/SemanticInformation.h index 83656252..8bdc70be 100644 --- a/libevmasm/SemanticInformation.h +++ b/libevmasm/SemanticInformation.h @@ -38,7 +38,8 @@ class AssemblyItem; struct SemanticInformation { /// @returns true if the given items starts a new block for common subexpression analysis. - static bool breaksCSEAnalysisBlock(AssemblyItem const& _item); + /// @param _msizeImportant if false, consider an operation non-breaking if its only side-effect is that it modifies msize. + static bool breaksCSEAnalysisBlock(AssemblyItem const& _item, bool _msizeImportant); /// @returns true if the item is a two-argument operation whose value does not depend on the /// order of its arguments. static bool isCommutativeOperation(AssemblyItem const& _item); diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index b7e64891..2c392705 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -229,6 +229,9 @@ public: /// i.e. it behaves differently in lvalue context and in value context. virtual bool isValueType() const { return false; } virtual unsigned sizeOnStack() const { return 1; } + /// If it is possible to initialize such a value in memory by just writing zeros + /// of the size memoryHeadSize(). + virtual bool hasSimpleZeroValueInMemory() const { return true; } /// @returns the mobile (in contrast to static) type corresponding to the given type. /// This returns the corresponding IntegerType or FixedPointType for RationalNumberType /// and the pointer type for storage reference types. @@ -568,6 +571,7 @@ public: virtual TypePointer mobileType() const override { return copyForLocation(m_location, true); } virtual bool dataStoredIn(DataLocation _location) const override { return m_location == _location; } + virtual bool hasSimpleZeroValueInMemory() const override { return false; } /// Storage references can be pointers or bound references. In general, local variables are of /// pointer type, state variables are bound references. Assignments to pointers or deleting @@ -855,6 +859,7 @@ public: virtual u256 storageSize() const override; virtual bool canLiveOutsideStorage() const override { return false; } virtual unsigned sizeOnStack() const override; + virtual bool hasSimpleZeroValueInMemory() const override { return false; } virtual TypePointer mobileType() const override; /// Converts components to their temporary types and performs some wildcard matching. virtual TypePointer closestTemporaryType(TypePointer const& _targetType) const override; @@ -999,6 +1004,7 @@ public: virtual bool isValueType() const override { return true; } virtual bool canLiveOutsideStorage() const override { return m_kind == Kind::Internal || m_kind == Kind::External; } virtual unsigned sizeOnStack() const override; + virtual bool hasSimpleZeroValueInMemory() const override { return false; } virtual MemberList::MemberMap nativeMembers(ContractDefinition const* _currentScope) const override; virtual TypePointer encodingType() const override; virtual TypePointer interfaceType(bool _inLibrary) const override; @@ -1104,6 +1110,8 @@ public: return _inLibrary ? shared_from_this() : TypePointer(); } virtual bool dataStoredIn(DataLocation _location) const override { return _location == DataLocation::Storage; } + /// Cannot be stored in memory, but just in case. + virtual bool hasSimpleZeroValueInMemory() const override { solAssert(false, ""); } TypePointer const& keyType() const { return m_keyType; } TypePointer const& valueType() const { return m_valueType; } @@ -1132,6 +1140,7 @@ public: virtual u256 storageSize() const override; virtual bool canLiveOutsideStorage() const override { return false; } virtual unsigned sizeOnStack() const override; + virtual bool hasSimpleZeroValueInMemory() const override { solAssert(false, ""); } virtual std::string toString(bool _short) const override { return "type(" + m_actualType->toString(_short) + ")"; } virtual MemberList::MemberMap nativeMembers(ContractDefinition const* _currentScope) const override; @@ -1154,6 +1163,7 @@ public: virtual u256 storageSize() const override; virtual bool canLiveOutsideStorage() const override { return false; } virtual unsigned sizeOnStack() const override { return 0; } + virtual bool hasSimpleZeroValueInMemory() const override { solAssert(false, ""); } virtual std::string richIdentifier() const override; virtual bool operator==(Type const& _other) const override; virtual std::string toString(bool _short) const override; @@ -1179,6 +1189,7 @@ public: virtual bool operator==(Type const& _other) const override; virtual bool canBeStored() const override { return false; } virtual bool canLiveOutsideStorage() const override { return true; } + virtual bool hasSimpleZeroValueInMemory() const override { solAssert(false, ""); } virtual unsigned sizeOnStack() const override { return 0; } virtual MemberList::MemberMap nativeMembers(ContractDefinition const*) const override; @@ -1209,6 +1220,7 @@ public: virtual bool operator==(Type const& _other) const override; virtual bool canBeStored() const override { return false; } virtual bool canLiveOutsideStorage() const override { return true; } + virtual bool hasSimpleZeroValueInMemory() const override { solAssert(false, ""); } virtual unsigned sizeOnStack() const override { return 0; } virtual MemberList::MemberMap nativeMembers(ContractDefinition const*) const override; @@ -1238,6 +1250,7 @@ public: virtual bool canLiveOutsideStorage() const override { return false; } virtual bool isValueType() const override { return true; } virtual unsigned sizeOnStack() const override { return 1; } + virtual bool hasSimpleZeroValueInMemory() const override { solAssert(false, ""); } virtual std::string toString(bool) const override { return "inaccessible dynamic type"; } virtual TypePointer decodingType() const override { return std::make_shared<IntegerType>(256); } }; diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp index 676d5d4e..deaef017 100644 --- a/libsolidity/codegen/CompilerUtils.cpp +++ b/libsolidity/codegen/CompilerUtils.cpp @@ -495,14 +495,34 @@ void CompilerUtils::abiDecodeV2(TypePointers const& _parameterTypes, bool _fromM void CompilerUtils::zeroInitialiseMemoryArray(ArrayType const& _type) { - auto repeat = m_context.newTag(); - m_context << repeat; - pushZeroValue(*_type.baseType()); - storeInMemoryDynamic(*_type.baseType()); - m_context << Instruction::SWAP1 << u256(1) << Instruction::SWAP1; - m_context << Instruction::SUB << Instruction::SWAP1; - m_context << Instruction::DUP2; - m_context.appendConditionalJumpTo(repeat); + if (_type.baseType()->hasSimpleZeroValueInMemory()) + { + solAssert(_type.baseType()->isValueType(), ""); + Whiskers templ(R"({ + let size := mul(length, <element_size>) + // cheap way of zero-initializing a memory range + codecopy(memptr, codesize(), size) + memptr := add(memptr, size) + })"); + templ("element_size", to_string(_type.baseType()->memoryHeadSize())); + m_context.appendInlineAssembly(templ.render(), {"length", "memptr"}); + } + else + { + // TODO: Potential optimization: + // When we create a new multi-dimensional dynamic array, each element + // is initialized to an empty array. It actually does not hurt + // to re-use exactly the same empty array for all elements. Currently, + // a new one is created each time. + auto repeat = m_context.newTag(); + m_context << repeat; + pushZeroValue(*_type.baseType()); + storeInMemoryDynamic(*_type.baseType()); + m_context << Instruction::SWAP1 << u256(1) << Instruction::SWAP1; + m_context << Instruction::SUB << Instruction::SWAP1; + m_context << Instruction::DUP2; + m_context.appendConditionalJumpTo(repeat); + } m_context << Instruction::SWAP1 << Instruction::POP; } diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index 9e2d30d5..76aa6843 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -850,8 +850,6 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) } case FunctionType::Kind::ObjectCreation: { - // Will allocate at the end of memory (MSIZE) and not write at all unless the base - // type is dynamically sized. ArrayType const& arrayType = dynamic_cast<ArrayType const&>(*_functionCall.annotation().type); _functionCall.expression().accept(*this); solAssert(arguments.size() == 1, ""); @@ -861,15 +859,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) utils().convertType(*arguments[0]->annotation().type, IntegerType(256)); // Stack: requested_length - // Allocate at max(MSIZE, freeMemoryPointer) utils().fetchFreeMemoryPointer(); - m_context << Instruction::DUP1 << Instruction::MSIZE; - m_context << Instruction::LT; - auto initialise = m_context.appendConditionalJump(); - // Free memory pointer does not point to empty memory, use MSIZE. - m_context << Instruction::POP; - m_context << Instruction::MSIZE; - m_context << initialise; // Stack: requested_length memptr m_context << Instruction::SWAP1; @@ -894,13 +884,10 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) // Check if length is zero m_context << Instruction::DUP1 << Instruction::ISZERO; auto skipInit = m_context.appendConditionalJump(); - - // We only have to initialise if the base type is a not a value type. - if (dynamic_cast<ReferenceType const*>(arrayType.baseType().get())) - { - m_context << Instruction::DUP2 << u256(32) << Instruction::ADD; - utils().zeroInitialiseMemoryArray(arrayType); - } + // Always initialize because the free memory pointer might point at + // a dirty memory area. + m_context << Instruction::DUP2 << u256(32) << Instruction::ADD; + utils().zeroInitialiseMemoryArray(arrayType); m_context << skipInit; m_context << Instruction::POP; break; diff --git a/test/libevmasm/Optimiser.cpp b/test/libevmasm/Optimiser.cpp index f630b304..28f6b8c0 100644 --- a/test/libevmasm/Optimiser.cpp +++ b/test/libevmasm/Optimiser.cpp @@ -69,8 +69,9 @@ namespace { AssemblyItems input = addDummyLocations(_input); + bool usesMsize = (find(_input.begin(), _input.end(), AssemblyItem{Instruction::MSIZE}) != _input.end()); eth::CommonSubexpressionEliminator cse(_state); - BOOST_REQUIRE(cse.feedItems(input.begin(), input.end()) == input.end()); + BOOST_REQUIRE(cse.feedItems(input.begin(), input.end(), usesMsize) == input.end()); AssemblyItems output = cse.getOptimizedItems(); for (AssemblyItem const& item: output) @@ -124,7 +125,7 @@ BOOST_AUTO_TEST_CASE(cse_intermediate_swap) Instruction::SLOAD, Instruction::SWAP1, u256(100), Instruction::EXP, Instruction::SWAP1, Instruction::DIV, u256(0xff), Instruction::AND }; - BOOST_REQUIRE(cse.feedItems(input.begin(), input.end()) == input.end()); + BOOST_REQUIRE(cse.feedItems(input.begin(), input.end(), false) == input.end()); AssemblyItems output = cse.getOptimizedItems(); BOOST_CHECK(!output.empty()); } diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index f5813aed..7fd65c14 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -9119,7 +9119,7 @@ BOOST_AUTO_TEST_CASE(calling_uninitialized_function_in_detail) int mutex; function t() returns (uint) { if (mutex > 0) - return 7; + { assembly { mstore(0, 7) return(0, 0x20) } } mutex = 1; // Avoid re-executing this function if we jump somewhere. x(); @@ -9132,6 +9132,27 @@ BOOST_AUTO_TEST_CASE(calling_uninitialized_function_in_detail) ABI_CHECK(callContractFunction("t()"), encodeArgs()); } +BOOST_AUTO_TEST_CASE(calling_uninitialized_function_through_array) +{ + char const* sourceCode = R"( + contract C { + int mutex; + function t() returns (uint) { + if (mutex > 0) + { assembly { mstore(0, 7) return(0, 0x20) } } + mutex = 1; + // Avoid re-executing this function if we jump somewhere. + function() internal returns (uint)[200] x; + x[0](); + return 2; + } + } + )"; + + compileAndRun(sourceCode, 0, "C"); + ABI_CHECK(callContractFunction("t()"), encodeArgs()); +} + BOOST_AUTO_TEST_CASE(pass_function_types_internally) { char const* sourceCode = R"( |