From 36bedf6a7998b0dbd48800998e9179789d0a3e1c Mon Sep 17 00:00:00 2001 From: Marek Kotewicz Date: Sat, 21 Feb 2015 13:41:25 +0100 Subject: [PATCH 01/23] checking jsonrpccpp version when looking for library. version 0.4 exact required --- cmake/EthDependencies.cmake | 9 +----- ...sonRpcCpp.cmake => Findjson_rpc_cpp.cmake} | 29 +++++++++++++++++-- 2 files changed, 27 insertions(+), 11 deletions(-) rename cmake/{FindJsonRpcCpp.cmake => Findjson_rpc_cpp.cmake} (65%) diff --git a/cmake/EthDependencies.cmake b/cmake/EthDependencies.cmake index 3f6cbcd77..1b21280cb 100644 --- a/cmake/EthDependencies.cmake +++ b/cmake/EthDependencies.cmake @@ -45,16 +45,9 @@ find_package (Jsoncpp 0.60 REQUIRED) message(" - Jsoncpp header: ${JSONCPP_INCLUDE_DIRS}") message(" - Jsoncpp lib : ${JSONCPP_LIBRARIES}") -# TODO the JsonRpcCpp package does not yet check for correct version number -# json-rpc-cpp support is currently not mandatory -# TODO make headless client optional # TODO get rid of -DETH_JSONRPC if (JSONRPC) - - find_package (JsonRpcCpp 0.3.2) - if (NOT JSON_RPC_CPP_FOUND) - message (FATAL_ERROR "JSONRPC 0.3.2. not found") - endif() + find_package (json_rpc_cpp 0.4 EXACT REQUIRED) message (" - json-rpc-cpp header: ${JSON_RPC_CPP_INCLUDE_DIRS}") message (" - json-rpc-cpp lib : ${JSON_RPC_CPP_LIBRARIES}") add_definitions(-DETH_JSONRPC) diff --git a/cmake/FindJsonRpcCpp.cmake b/cmake/Findjson_rpc_cpp.cmake similarity index 65% rename from cmake/FindJsonRpcCpp.cmake rename to cmake/Findjson_rpc_cpp.cmake index 2ca176f68..8b705aeeb 100644 --- a/cmake/FindJsonRpcCpp.cmake +++ b/cmake/Findjson_rpc_cpp.cmake @@ -10,6 +10,11 @@ # JSON_RPC_CPP_SERVER_LIBRARIES, the libraries needed to use json-rpc-cpp-server # JSON_RPC_CPP_CLIENT_LIBRARIES, the libraries needed to use json-rpc-cpp-client # JSON_RCP_CPP_FOUND, If false, do not try to use json-rpc-cpp. +# JSON_RPC_CPP_VERSION, version of library +# JSON_RPC_CPP_VERSION_MAJOR +# JSON_RPC_CPP_VERSION_MINOR +# JSON_RPC_CPP_VERSION_PATCH + # only look in default directories find_path( @@ -90,10 +95,28 @@ if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC") endif() +if (JSON_RPC_CPP_INCLUDE_DIR) + set (JSON_RPC_CPP_VERSION_HEADER "${JSON_RPC_CPP_INCLUDE_DIR}/jsonrpccpp/version.h") + if (EXISTS ${JSON_RPC_CPP_VERSION_HEADER}) + file (STRINGS ${JSON_RPC_CPP_VERSION_HEADER} JSON_RPC_CPP_VERSION_MAJOR REGEX "^#define JSONRPC_CPP_MAJOR_VERSION[ \t]+[0-9]+$") + file (STRINGS ${JSON_RPC_CPP_VERSION_HEADER} JSON_RPC_CPP_VERSION_MINOR REGEX "^#define JSONRPC_CPP_MINOR_VERSION[ \t]+[0-9]+$") + file (STRINGS ${JSON_RPC_CPP_VERSION_HEADER} JSON_RPC_CPP_VERSION_PATCH REGEX "^#define JSONRPC_CPP_PATCH_VERSION[ \t]+[0-9]+$") + string (REGEX REPLACE "^#define JSONRPC_CPP_MAJOR_VERSION[ \t]+([0-9]+)" "\\1" JSON_RPC_CPP_VERSION_MAJOR ${JSON_RPC_CPP_VERSION_MAJOR}) + string (REGEX REPLACE "^#define JSONRPC_CPP_MINOR_VERSION[ \t]+([0-9]+)" "\\1" JSON_RPC_CPP_VERSION_MINOR ${JSON_RPC_CPP_VERSION_MINOR}) + string (REGEX REPLACE "^#define JSONRPC_CPP_PATCH_VERSION[ \t]+([0-9]+)" "\\1" JSON_RPC_CPP_VERSION_PATCH ${JSON_RPC_CPP_VERSION_PATCH}) + set (JSON_RPC_CPP_VERSION ${JSON_RPC_CPP_VERSION_MAJOR}.${JSON_RPC_CPP_VERSION_MINOR}.${JSON_RPC_CPP_VERSION_PATCH}) + endif() +endif() + # handle the QUIETLY and REQUIRED arguments and set JSON_RPC_CPP_FOUND to TRUE # if all listed variables are TRUE, hide their existence from configuration view include(FindPackageHandleStandardArgs) -find_package_handle_standard_args(json_rpc_cpp DEFAULT_MSG - JSON_RPC_CPP_COMMON_LIBRARY JSON_RPC_CPP_SERVER_LIBRARY JSON_RPC_CPP_CLIENT_LIBRARY JSON_RPC_CPP_INCLUDE_DIR) -mark_as_advanced (JSON_RPC_CPP_COMMON_LIBRARY JSON_RPC_CPP_SERVER_LIBRARY JSON_RPC_CPP_CLIENT_LIBRARY JSON_RPC_CPP_INCLUDE_DIR) + +find_package_handle_standard_args( + json_rpc_cpp + REQUIRED_VARS JSON_RPC_CPP_INCLUDE_DIR JSON_RPC_CPP_COMMON_LIBRARY JSON_RPC_CPP_SERVER_LIBRARY JSON_RPC_CPP_CLIENT_LIBRARY + VERSION_VAR JSON_RPC_CPP_VERSION +) + +mark_as_advanced (JSON_RPC_CPP_INCLUDE_DIR JSON_RPC_CPP_COMMON_LIBRARY JSON_RPC_CPP_SERVER_LIBRARY JSON_RPC_CPP_CLIENT_LIBRARY) From 42177bc187df8ebeb8a10ec891ee73d12624d82f Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Tue, 17 Mar 2015 14:21:22 +0100 Subject: [PATCH 02/23] added getABIType() to types --- libsolidity/Types.h | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/libsolidity/Types.h b/libsolidity/Types.h index c90aabda1..3d3a263d5 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -187,6 +187,9 @@ public: "for type without literals.")); } + /// Returns address of type for ABI interface + virtual TypePointer ABIType() const { return TypePointer(); } + protected: /// Convenience object used when returning an empty member list. static const MemberList EmptyMemberList; @@ -217,10 +220,12 @@ public: virtual unsigned getStorageBytes() const override { return m_bits / 8; } virtual bool isValueType() const override { return true; } - virtual MemberList const& getMembers() const { return isAddress() ? AddressMemberList : EmptyMemberList; } + virtual MemberList const& getMembers() const override { return isAddress() ? AddressMemberList : EmptyMemberList; } virtual std::string toString() const override; + virtual TypePointer ABIType() const override { return shared_from_this(); } + int getNumBits() const { return m_bits; } bool isAddress() const { return m_modifier == Modifier::Address; } bool isSigned() const { return m_modifier == Modifier::Signed; } @@ -258,6 +263,7 @@ public: virtual std::string toString() const override; virtual u256 literalValue(Literal const* _literal) const override; virtual TypePointer getRealType() const override; + virtual TypePointer ABIType() const override { return shared_from_this(); } /// @returns the smallest integer type that can hold the value or an empty pointer if not possible. std::shared_ptr getIntegerType() const; @@ -311,7 +317,7 @@ public: virtual TypePointer unaryOperatorResult(Token::Value _operator) const override; virtual TypePointer binaryOperatorResult(Token::Value _operator, TypePointer const& _other) const override; - virtual unsigned getCalldataEncodedSize(bool _padded) const { return _padded ? 32 : 1; } + virtual unsigned getCalldataEncodedSize(bool _padded) const override{ return _padded ? 32 : 1; } virtual unsigned getStorageBytes() const override { return 1; } virtual bool isValueType() const override { return true; } @@ -361,7 +367,7 @@ public: virtual unsigned getSizeOnStack() const override; virtual std::string toString() const override; virtual MemberList const& getMembers() const override { return s_arrayTypeMemberList; } - + virtual TypePointer ABIType() const override { return (m_baseType->ABIType() ? m_baseType->ABIType() : ABIType()); } Location getLocation() const { return m_location; } bool isByteArray() const { return m_isByteArray; } TypePointer const& getBaseType() const { solAssert(!!m_baseType, ""); return m_baseType;} @@ -400,6 +406,7 @@ public: virtual std::string toString() const override; virtual MemberList const& getMembers() const override; + virtual TypePointer ABIType() const override { return std::make_shared(160, IntegerType::Modifier::Address); } bool isSuper() const { return m_super; } ContractDefinition const& getContractDefinition() const { return m_contract; } @@ -468,6 +475,7 @@ public: virtual bool isValueType() const override { return true; } virtual bool isExplicitlyConvertibleTo(Type const& _convertTo) const override; + virtual TypePointer ABIType() const override { return std::make_shared(8 * int(getStorageBytes())); } EnumDefinition const& getEnumDefinition() const { return m_enum; } /// @returns the value that the string has in the Enum From 3d18c02f3697d4399d50962f31fd11c2523c0371 Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Thu, 19 Mar 2015 16:48:54 +0100 Subject: [PATCH 03/23] added externalType for ArrayType --- libsolidity/Types.cpp | 17 +++++++++++++++++ libsolidity/Types.h | 13 +++++++------ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 016f0b236..6fe8a6410 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -742,6 +742,23 @@ string ArrayType::toString() const return ret + "]"; } +TypePointer ArrayType::externalType() const +{ + if (m_location != Location::CallData) + return TypePointer(); + if (m_isByteArray) + return shared_from_this(); + if (!(m_baseType->externalType())) + return TypePointer(); + if (dynamic_cast(m_baseType.get()) && m_baseType->isDynamicallySized()) + return TypePointer(); + + if (m_baseType->isDynamicallySized()) + return std::make_shared(Location::CallData, m_baseType->externalType()); + else + return std::make_shared(Location::CallData, m_baseType->externalType(), m_length); +} + shared_ptr ArrayType::copyForLocation(ArrayType::Location _location) const { auto copy = make_shared(_location); diff --git a/libsolidity/Types.h b/libsolidity/Types.h index 3d3a263d5..013c65893 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -188,7 +188,7 @@ public: } /// Returns address of type for ABI interface - virtual TypePointer ABIType() const { return TypePointer(); } + virtual TypePointer externalType() const { return TypePointer(); } protected: /// Convenience object used when returning an empty member list. @@ -224,7 +224,7 @@ public: virtual std::string toString() const override; - virtual TypePointer ABIType() const override { return shared_from_this(); } + virtual TypePointer externalType() const override { return shared_from_this(); } int getNumBits() const { return m_bits; } bool isAddress() const { return m_modifier == Modifier::Address; } @@ -263,7 +263,7 @@ public: virtual std::string toString() const override; virtual u256 literalValue(Literal const* _literal) const override; virtual TypePointer getRealType() const override; - virtual TypePointer ABIType() const override { return shared_from_this(); } + virtual TypePointer externalType() const override { return shared_from_this(); } /// @returns the smallest integer type that can hold the value or an empty pointer if not possible. std::shared_ptr getIntegerType() const; @@ -367,7 +367,8 @@ public: virtual unsigned getSizeOnStack() const override; virtual std::string toString() const override; virtual MemberList const& getMembers() const override { return s_arrayTypeMemberList; } - virtual TypePointer ABIType() const override { return (m_baseType->ABIType() ? m_baseType->ABIType() : ABIType()); } + virtual TypePointer externalType() const override; + Location getLocation() const { return m_location; } bool isByteArray() const { return m_isByteArray; } TypePointer const& getBaseType() const { solAssert(!!m_baseType, ""); return m_baseType;} @@ -406,7 +407,7 @@ public: virtual std::string toString() const override; virtual MemberList const& getMembers() const override; - virtual TypePointer ABIType() const override { return std::make_shared(160, IntegerType::Modifier::Address); } + virtual TypePointer externalType() const override { return std::make_shared(160, IntegerType::Modifier::Address); } bool isSuper() const { return m_super; } ContractDefinition const& getContractDefinition() const { return m_contract; } @@ -475,7 +476,7 @@ public: virtual bool isValueType() const override { return true; } virtual bool isExplicitlyConvertibleTo(Type const& _convertTo) const override; - virtual TypePointer ABIType() const override { return std::make_shared(8 * int(getStorageBytes())); } + virtual TypePointer externalType() const override { return std::make_shared(8 * int(getStorageBytes())); } EnumDefinition const& getEnumDefinition() const { return m_enum; } /// @returns the value that the string has in the Enum From a7eccfaa978b8f1e5ea7d3eb4d89b276bdf626d7 Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Thu, 19 Mar 2015 17:33:10 +0100 Subject: [PATCH 04/23] added check for valid externalType to checkTypeRequirements for function --- libsolidity/AST.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index d05eaf83e..3ecf79eb6 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -305,8 +305,12 @@ TypePointer FunctionDefinition::getType(ContractDefinition const*) const void FunctionDefinition::checkTypeRequirements() { for (ASTPointer const& var: getParameters() + getReturnParameters()) + { if (!var->getType()->canLiveOutsideStorage()) BOOST_THROW_EXCEPTION(var->createTypeError("Type is required to live outside storage.")); + if (!var->getType()->externalType() && getVisibility() >= Visibility::Internal) + BOOST_THROW_EXCEPTION(var->createTypeError("Type is required to have an external address.")); + } for (ASTPointer const& modifier: m_functionModifiers) modifier->checkTypeRequirements(isConstructor() ? dynamic_cast(*getScope()).getBaseContracts() : @@ -653,6 +657,10 @@ void MemberAccess::checkTypeRequirements() if (!m_type) BOOST_THROW_EXCEPTION(createTypeError("Member \"" + *m_memberName + "\" not found or not " "visible in " + type.toString())); + //todo check for visibility +// else if (!m_type->externalType()) +// BOOST_THROW_EXCEPTION(createTypeError("Type is required to have an external address.")); + // This should probably move somewhere else. if (type.getCategory() == Type::Category::Struct) m_isLValue = true; From 1d15c09e5fa457d201968aa1a85b517113a7f689 Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Fri, 20 Mar 2015 13:30:00 +0100 Subject: [PATCH 05/23] - added externalType to BooleanType. - fixed the error message --- libsolidity/AST.cpp | 6 +++--- libsolidity/Types.cpp | 2 ++ libsolidity/Types.h | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 3ecf79eb6..0827f7941 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -308,8 +308,8 @@ void FunctionDefinition::checkTypeRequirements() { if (!var->getType()->canLiveOutsideStorage()) BOOST_THROW_EXCEPTION(var->createTypeError("Type is required to live outside storage.")); - if (!var->getType()->externalType() && getVisibility() >= Visibility::Internal) - BOOST_THROW_EXCEPTION(var->createTypeError("Type is required to have an external address.")); + if (!var->getType()->externalType() && getVisibility() >= Visibility::Public) + BOOST_THROW_EXCEPTION(var->createTypeError("Internal type is not allowed for function with external visibility")); } for (ASTPointer const& modifier: m_functionModifiers) modifier->checkTypeRequirements(isConstructor() ? @@ -659,7 +659,7 @@ void MemberAccess::checkTypeRequirements() "visible in " + type.toString())); //todo check for visibility // else if (!m_type->externalType()) -// BOOST_THROW_EXCEPTION(createTypeError("Type is required to have an external address.")); +// BOOST_THROW_EXCEPTION(createTypeError("Internal type not allowed for function with external visibility")); // This should probably move somewhere else. if (type.getCategory() == Type::Category::Struct) diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 6fe8a6410..82c28a394 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -749,7 +749,9 @@ TypePointer ArrayType::externalType() const if (m_isByteArray) return shared_from_this(); if (!(m_baseType->externalType())) + { return TypePointer(); + } if (dynamic_cast(m_baseType.get()) && m_baseType->isDynamicallySized()) return TypePointer(); diff --git a/libsolidity/Types.h b/libsolidity/Types.h index 013c65893..5941646aa 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -263,7 +263,6 @@ public: virtual std::string toString() const override; virtual u256 literalValue(Literal const* _literal) const override; virtual TypePointer getRealType() const override; - virtual TypePointer externalType() const override { return shared_from_this(); } /// @returns the smallest integer type that can hold the value or an empty pointer if not possible. std::shared_ptr getIntegerType() const; @@ -298,6 +297,7 @@ public: virtual std::string toString() const override { return "bytes" + dev::toString(m_bytes); } virtual u256 literalValue(Literal const* _literal) const override; + virtual TypePointer externalType() const override { return shared_from_this(); } int getNumBytes() const { return m_bytes; } @@ -323,6 +323,7 @@ public: virtual std::string toString() const override { return "bool"; } virtual u256 literalValue(Literal const* _literal) const override; + virtual TypePointer externalType() const override { return shared_from_this(); } }; /** From 44c7da426293fef7a816c72e05ee07b5d287cdc6 Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Fri, 20 Mar 2015 17:02:24 +0100 Subject: [PATCH 06/23] added check for events and stat variables --- libsolidity/AST.cpp | 13 +++++++++---- libsolidity/Types.cpp | 8 +++----- libsolidity/Types.h | 3 ++- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 0827f7941..63f3d772b 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -346,8 +346,11 @@ void VariableDeclaration::checkTypeRequirements() if (!m_value) return; if (m_type) + { m_value->expectType(*m_type); - else + if (m_isStateVariable && !m_type->externalType() && getVisibility() >= Visibility::Public) + BOOST_THROW_EXCEPTION(createTypeError("Internal type is not allowed for state variables.")); + } else { // no type declared and no previous assignment, infer the type m_value->checkTypeRequirements(); @@ -426,6 +429,8 @@ void EventDefinition::checkTypeRequirements() numIndexed++; if (!var->getType()->canLiveOutsideStorage()) BOOST_THROW_EXCEPTION(var->createTypeError("Type is required to live outside storage.")); + if (!var->getType()->externalType() && getVisibility() >= Visibility::Public) + BOOST_THROW_EXCEPTION(var->createTypeError("Internal type is not allowed for Events")); } if (numIndexed > 3) BOOST_THROW_EXCEPTION(createTypeError("More than 3 indexed arguments for event.")); @@ -657,9 +662,9 @@ void MemberAccess::checkTypeRequirements() if (!m_type) BOOST_THROW_EXCEPTION(createTypeError("Member \"" + *m_memberName + "\" not found or not " "visible in " + type.toString())); - //todo check for visibility -// else if (!m_type->externalType()) -// BOOST_THROW_EXCEPTION(createTypeError("Internal type not allowed for function with external visibility")); +// if (auto f = dynamic_cast(m_expression->getType().get())) +// if (f->getLocation() == FunctionType::Location::External && !m_type->externalType()) +// BOOST_THROW_EXCEPTION(createTypeError(*m_memberName + " member has an internal type.")); // This should probably move somewhere else. if (type.getCategory() == Type::Category::Struct) diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 82c28a394..7b4d1de26 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -748,14 +748,12 @@ TypePointer ArrayType::externalType() const return TypePointer(); if (m_isByteArray) return shared_from_this(); - if (!(m_baseType->externalType())) - { + if (!m_baseType->externalType()) return TypePointer(); - } - if (dynamic_cast(m_baseType.get()) && m_baseType->isDynamicallySized()) + if (m_baseType->getCategory() == Category::Array && m_baseType->isDynamicallySized()) return TypePointer(); - if (m_baseType->isDynamicallySized()) + if (isDynamicallySized()) return std::make_shared(Location::CallData, m_baseType->externalType()); else return std::make_shared(Location::CallData, m_baseType->externalType(), m_length); diff --git a/libsolidity/Types.h b/libsolidity/Types.h index 5941646aa..17264dc59 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -187,7 +187,8 @@ public: "for type without literals.")); } - /// Returns address of type for ABI interface + /// @returns a type suitable for outside of Solidity, i.e. for contract types it returns address. + /// If there is no such type, returns an empty shared pointer. virtual TypePointer externalType() const { return TypePointer(); } protected: From 0ca313ec85d8a094eeab175c20603175b0d1c26c Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Mon, 23 Mar 2015 18:08:45 +0100 Subject: [PATCH 07/23] renamed getCanonicalSignature added externalTypes instead of types for interface functions added simple test todo testing --- libsolidity/AST.cpp | 11 ++++---- libsolidity/AST.h | 2 +- libsolidity/ExpressionCompiler.cpp | 2 +- libsolidity/InterfaceHandler.cpp | 4 +-- libsolidity/Types.cpp | 8 +++--- libsolidity/Types.h | 4 +-- mix/QFunctionDefinition.cpp | 2 +- test/SolidityNameAndTypeResolution.cpp | 36 +++++++++++++++++++++++--- test/SolidityTypes.cpp | 1 + 9 files changed, 52 insertions(+), 18 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 63f3d772b..2e24d4f9d 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -88,7 +88,7 @@ void ContractDefinition::checkTypeRequirements() if (hashes.count(hash)) BOOST_THROW_EXCEPTION(createTypeError( std::string("Function signature hash collision for ") + - it.second->getCanonicalSignature())); + it.second->externalTypes())); hashes.insert(hash); } } @@ -192,7 +192,7 @@ vector, FunctionTypePointer>> const& ContractDefinition::getIn if (functionsSeen.count(f->getName()) == 0 && f->isPartOfExternalInterface()) { functionsSeen.insert(f->getName()); - FixedHash<4> hash(dev::sha3(f->getCanonicalSignature())); + FixedHash<4> hash(dev::sha3(f->externalTypes())); m_interfaceFunctionList->push_back(make_pair(hash, make_shared(*f, false))); } @@ -200,8 +200,9 @@ vector, FunctionTypePointer>> const& ContractDefinition::getIn if (functionsSeen.count(v->getName()) == 0 && v->isPartOfExternalInterface()) { FunctionType ftype(*v); + solAssert(v->getType().get(), ""); functionsSeen.insert(v->getName()); - FixedHash<4> hash(dev::sha3(ftype.getCanonicalSignature(v->getName()))); + FixedHash<4> hash(dev::sha3(ftype.externalTypes(v->getName()))); m_interfaceFunctionList->push_back(make_pair(hash, make_shared(*v))); } } @@ -319,9 +320,9 @@ void FunctionDefinition::checkTypeRequirements() m_body->checkTypeRequirements(); } -string FunctionDefinition::getCanonicalSignature() const +string FunctionDefinition::externalTypes() const { - return FunctionType(*this).getCanonicalSignature(getName()); + return FunctionType(*this).externalTypes(getName()); } bool VariableDeclaration::isLValue() const diff --git a/libsolidity/AST.h b/libsolidity/AST.h index 335b9a880..c5cd2e5b1 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -424,7 +424,7 @@ public: /// @returns the canonical signature of the function /// That consists of the name of the function followed by the types of the /// arguments separated by commas all enclosed in parentheses without any spaces. - std::string getCanonicalSignature() const; + std::string externalTypes() const; private: bool m_isConstructor; diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index da762147a..288af3983 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -544,7 +544,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) } if (!event.isAnonymous()) { - m_context << u256(h256::Arith(dev::sha3(function.getCanonicalSignature(event.getName())))); + m_context << u256(h256::Arith(dev::sha3(function.externalTypes(event.getName())))); ++numIndexed; } solAssert(numIndexed <= 4, "Too many indexed arguments."); diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp index 406d1e24a..e09e55cbc 100644 --- a/libsolidity/InterfaceHandler.cpp +++ b/libsolidity/InterfaceHandler.cpp @@ -129,7 +129,7 @@ std::unique_ptr InterfaceHandler::getUserDocumentation(ContractDefi if (!m_notice.empty()) {// since @notice is the only user tag if missing function should not appear user["notice"] = Json::Value(m_notice); - methods[it.second->getCanonicalSignature()] = user; + methods[it.second->externalTypes()] = user; } } } @@ -185,7 +185,7 @@ std::unique_ptr InterfaceHandler::getDevDocumentation(ContractDefin method["return"] = m_return; if (!method.empty()) // add the function, only if we have any documentation to add - methods[it.second->getCanonicalSignature()] = method; + methods[it.second->externalTypes()] = method; } } doc["methods"] = methods; diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 7b4d1de26..28a3af33a 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -1127,7 +1127,7 @@ MemberList const& FunctionType::getMembers() const } } -string FunctionType::getCanonicalSignature(std::string const& _name) const +string FunctionType::externalTypes(std::string const& _name) const { std::string funcName = _name; if (_name == "") @@ -1138,8 +1138,10 @@ string FunctionType::getCanonicalSignature(std::string const& _name) const string ret = funcName + "("; for (auto it = m_parameterTypes.cbegin(); it != m_parameterTypes.cend(); ++it) - ret += (*it)->toString() + (it + 1 == m_parameterTypes.cend() ? "" : ","); - + { + solAssert(!!(*it)->externalType(), "Parameter should have external type"); + ret += (*it)->externalType()->toString() + (it + 1 == m_parameterTypes.cend() ? "" : ","); + } return ret + ")"; } diff --git a/libsolidity/Types.h b/libsolidity/Types.h index 17264dc59..599d80cc6 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -550,10 +550,10 @@ public: virtual MemberList const& getMembers() const override; Location const& getLocation() const { return m_location; } - /// @returns the canonical signature of this function type given the function name + /// @returns the external type of this function type given the function name /// If @a _name is not provided (empty string) then the @c m_declaration member of the /// function type is used - std::string getCanonicalSignature(std::string const& _name = "") const; + std::string externalTypes(std::string const& _name = "") const; Declaration const& getDeclaration() const { solAssert(m_declaration, "Requested declaration from a FunctionType that has none"); diff --git a/mix/QFunctionDefinition.cpp b/mix/QFunctionDefinition.cpp index fed4e8add..d8cc63dee 100644 --- a/mix/QFunctionDefinition.cpp +++ b/mix/QFunctionDefinition.cpp @@ -28,7 +28,7 @@ using namespace dev::solidity; using namespace dev::mix; -QFunctionDefinition::QFunctionDefinition(QObject* _parent, dev::solidity::FunctionTypePointer const& _f): QBasicNodeDefinition(_parent, &_f->getDeclaration()), m_hash(dev::sha3(_f->getCanonicalSignature())) +QFunctionDefinition::QFunctionDefinition(QObject* _parent, dev::solidity::FunctionTypePointer const& _f): QBasicNodeDefinition(_parent, &_f->getDeclaration()), m_hash(dev::sha3(_f->externalTypes())) { auto paramNames = _f->getParameterNames(); auto paramTypes = _f->getParameterTypes(); diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index 591cf053a..4e8166963 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -359,7 +359,7 @@ BOOST_AUTO_TEST_CASE(function_canonical_signature) if (ContractDefinition* contract = dynamic_cast(node.get())) { auto functions = contract->getDefinedFunctions(); - BOOST_CHECK_EQUAL("foo(uint256,uint64,bool)", functions[0]->getCanonicalSignature()); + BOOST_CHECK_EQUAL("foo(uint256,uint64,bool)", functions[0]->externalTypes()); } } @@ -376,10 +376,41 @@ BOOST_AUTO_TEST_CASE(function_canonical_signature_type_aliases) if (ContractDefinition* contract = dynamic_cast(node.get())) { auto functions = contract->getDefinedFunctions(); - BOOST_CHECK_EQUAL("boo(uint256,bytes32,address)", functions[0]->getCanonicalSignature()); + BOOST_CHECK_EQUAL("boo(uint256,bytes32,address)", functions[0]->externalTypes()); } } +BOOST_AUTO_TEST_CASE(function_external_types) +{ + ASTPointer sourceUnit; + char const* text = R"( + contract Test { + function boo(uint arg2, bool arg3, bytes8 arg4) returns (uint ret) { + ret = 5; + } + })"; + ETH_TEST_REQUIRE_NO_THROW(sourceUnit = parseTextAndResolveNames(text), "Parsing and name Resolving failed"); + for (ASTPointer const& node: sourceUnit->getNodes()) + if (ContractDefinition* contract = dynamic_cast(node.get())) + { + auto functions = contract->getDefinedFunctions(); + BOOST_CHECK_EQUAL("boo(uint256,bool,bytes8)", functions[0]->externalTypes()); + } +} + +//todo should check arrays and contract. also event +//BOOST_AUTO_TEST_CASE(function_external_types_throw) +//{ +// ASTPointer sourceUnit; +// char const* text = R"( +// contract Test { +// function boo(uint32[] arg5) returns (uint ret) { +// ret = 5; +// } +// })"; +// BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); +//} + BOOST_AUTO_TEST_CASE(hash_collision_in_interface) { char const* text = "contract test {\n" @@ -635,7 +666,6 @@ BOOST_AUTO_TEST_CASE(state_variable_accessors) "mapping(uint=>bytes4) public map;\n" "mapping(uint=>mapping(uint=>bytes4)) public multiple_map;\n" "}\n"; - ASTPointer source; ContractDefinition const* contract; ETH_TEST_CHECK_NO_THROW(source = parseTextAndResolveNames(text), "Parsing and Resolving names failed"); diff --git a/test/SolidityTypes.cpp b/test/SolidityTypes.cpp index 6b6306479..4133ce7b7 100644 --- a/test/SolidityTypes.cpp +++ b/test/SolidityTypes.cpp @@ -86,6 +86,7 @@ BOOST_AUTO_TEST_CASE(storage_layout_arrays) BOOST_CHECK(ArrayType(ArrayType::Location::Storage, make_shared(32), 9).getStorageSize() == 9); } + BOOST_AUTO_TEST_SUITE_END() } From ba8d0f615c2fee7b08f83cac07b4bacc23f6fd5d Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Tue, 24 Mar 2015 11:11:27 +0100 Subject: [PATCH 08/23] renamed externalTypes to externalSignature --- libsolidity/AST.cpp | 12 ++++++------ libsolidity/AST.h | 6 +++--- libsolidity/ExpressionCompiler.cpp | 2 +- libsolidity/InterfaceHandler.cpp | 4 ++-- libsolidity/Types.cpp | 2 +- libsolidity/Types.h | 4 ++-- mix/QFunctionDefinition.cpp | 2 +- test/SolidityNameAndTypeResolution.cpp | 27 ++++++++++++++++++++++---- test/SolidityTypes.cpp | 1 - 9 files changed, 39 insertions(+), 21 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 2e24d4f9d..9a2c1be07 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -88,7 +88,7 @@ void ContractDefinition::checkTypeRequirements() if (hashes.count(hash)) BOOST_THROW_EXCEPTION(createTypeError( std::string("Function signature hash collision for ") + - it.second->externalTypes())); + it.second->externalSignature())); hashes.insert(hash); } } @@ -192,7 +192,7 @@ vector, FunctionTypePointer>> const& ContractDefinition::getIn if (functionsSeen.count(f->getName()) == 0 && f->isPartOfExternalInterface()) { functionsSeen.insert(f->getName()); - FixedHash<4> hash(dev::sha3(f->externalTypes())); + FixedHash<4> hash(dev::sha3(f->externalSignature())); m_interfaceFunctionList->push_back(make_pair(hash, make_shared(*f, false))); } @@ -202,7 +202,7 @@ vector, FunctionTypePointer>> const& ContractDefinition::getIn FunctionType ftype(*v); solAssert(v->getType().get(), ""); functionsSeen.insert(v->getName()); - FixedHash<4> hash(dev::sha3(ftype.externalTypes(v->getName()))); + FixedHash<4> hash(dev::sha3(ftype.externalSignature(v->getName()))); m_interfaceFunctionList->push_back(make_pair(hash, make_shared(*v))); } } @@ -320,9 +320,9 @@ void FunctionDefinition::checkTypeRequirements() m_body->checkTypeRequirements(); } -string FunctionDefinition::externalTypes() const +string FunctionDefinition::externalSignature() const { - return FunctionType(*this).externalTypes(getName()); + return FunctionType(*this).externalSignature(getName()); } bool VariableDeclaration::isLValue() const @@ -430,7 +430,7 @@ void EventDefinition::checkTypeRequirements() numIndexed++; if (!var->getType()->canLiveOutsideStorage()) BOOST_THROW_EXCEPTION(var->createTypeError("Type is required to live outside storage.")); - if (!var->getType()->externalType() && getVisibility() >= Visibility::Public) + if (!var->getType()->externalType()) BOOST_THROW_EXCEPTION(var->createTypeError("Internal type is not allowed for Events")); } if (numIndexed > 3) diff --git a/libsolidity/AST.h b/libsolidity/AST.h index c5cd2e5b1..937c2ceab 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -421,10 +421,10 @@ public: /// Checks that all parameters have allowed types and calls checkTypeRequirements on the body. void checkTypeRequirements(); - /// @returns the canonical signature of the function - /// That consists of the name of the function followed by the types of the + /// @returns the external signature of the function + /// That consists of the name of the function followed by the external types of the /// arguments separated by commas all enclosed in parentheses without any spaces. - std::string externalTypes() const; + std::string externalSignature() const; private: bool m_isConstructor; diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index 288af3983..90568767b 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -544,7 +544,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) } if (!event.isAnonymous()) { - m_context << u256(h256::Arith(dev::sha3(function.externalTypes(event.getName())))); + m_context << u256(h256::Arith(dev::sha3(function.externalSignature(event.getName())))); ++numIndexed; } solAssert(numIndexed <= 4, "Too many indexed arguments."); diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp index e09e55cbc..2f35a96f6 100644 --- a/libsolidity/InterfaceHandler.cpp +++ b/libsolidity/InterfaceHandler.cpp @@ -129,7 +129,7 @@ std::unique_ptr InterfaceHandler::getUserDocumentation(ContractDefi if (!m_notice.empty()) {// since @notice is the only user tag if missing function should not appear user["notice"] = Json::Value(m_notice); - methods[it.second->externalTypes()] = user; + methods[it.second->externalSignature()] = user; } } } @@ -185,7 +185,7 @@ std::unique_ptr InterfaceHandler::getDevDocumentation(ContractDefin method["return"] = m_return; if (!method.empty()) // add the function, only if we have any documentation to add - methods[it.second->externalTypes()] = method; + methods[it.second->externalSignature()] = method; } } doc["methods"] = methods; diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 28a3af33a..6344d128d 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -1127,7 +1127,7 @@ MemberList const& FunctionType::getMembers() const } } -string FunctionType::externalTypes(std::string const& _name) const +string FunctionType::externalSignature(std::string const& _name) const { std::string funcName = _name; if (_name == "") diff --git a/libsolidity/Types.h b/libsolidity/Types.h index 599d80cc6..e00e6b983 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -550,10 +550,10 @@ public: virtual MemberList const& getMembers() const override; Location const& getLocation() const { return m_location; } - /// @returns the external type of this function type given the function name + /// @returns the external signature of this function type given the function name /// If @a _name is not provided (empty string) then the @c m_declaration member of the /// function type is used - std::string externalTypes(std::string const& _name = "") const; + std::string externalSignature(std::string const& _name = "") const; Declaration const& getDeclaration() const { solAssert(m_declaration, "Requested declaration from a FunctionType that has none"); diff --git a/mix/QFunctionDefinition.cpp b/mix/QFunctionDefinition.cpp index d8cc63dee..13dbd4821 100644 --- a/mix/QFunctionDefinition.cpp +++ b/mix/QFunctionDefinition.cpp @@ -28,7 +28,7 @@ using namespace dev::solidity; using namespace dev::mix; -QFunctionDefinition::QFunctionDefinition(QObject* _parent, dev::solidity::FunctionTypePointer const& _f): QBasicNodeDefinition(_parent, &_f->getDeclaration()), m_hash(dev::sha3(_f->externalTypes())) +QFunctionDefinition::QFunctionDefinition(QObject* _parent, dev::solidity::FunctionTypePointer const& _f): QBasicNodeDefinition(_parent, &_f->getDeclaration()), m_hash(dev::sha3(_f->externalSignature())) { auto paramNames = _f->getParameterNames(); auto paramTypes = _f->getParameterTypes(); diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index 4e8166963..a6d1920a1 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -359,7 +359,7 @@ BOOST_AUTO_TEST_CASE(function_canonical_signature) if (ContractDefinition* contract = dynamic_cast(node.get())) { auto functions = contract->getDefinedFunctions(); - BOOST_CHECK_EQUAL("foo(uint256,uint64,bool)", functions[0]->externalTypes()); + BOOST_CHECK_EQUAL("foo(uint256,uint64,bool)", functions[0]->externalSignature()); } } @@ -376,7 +376,7 @@ BOOST_AUTO_TEST_CASE(function_canonical_signature_type_aliases) if (ContractDefinition* contract = dynamic_cast(node.get())) { auto functions = contract->getDefinedFunctions(); - BOOST_CHECK_EQUAL("boo(uint256,bytes32,address)", functions[0]->externalTypes()); + BOOST_CHECK_EQUAL("boo(uint256,bytes32,address)", functions[0]->externalSignature()); } } @@ -385,7 +385,7 @@ BOOST_AUTO_TEST_CASE(function_external_types) ASTPointer sourceUnit; char const* text = R"( contract Test { - function boo(uint arg2, bool arg3, bytes8 arg4) returns (uint ret) { + function boo(uint arg2, bool arg3, bytes8 arg4, bool[2] pairs) public returns (uint ret) { ret = 5; } })"; @@ -394,10 +394,28 @@ BOOST_AUTO_TEST_CASE(function_external_types) if (ContractDefinition* contract = dynamic_cast(node.get())) { auto functions = contract->getDefinedFunctions(); - BOOST_CHECK_EQUAL("boo(uint256,bool,bytes8)", functions[0]->externalTypes()); + BOOST_CHECK_EQUAL("boo(uint256,bool,bytes8)", functions[0]->externalSignature()); } } +//BOOST_AUTO_TEST_CASE(function_external_types_throw) +//{ +// ASTPointer sourceUnit; +// char const* text = R"( +// contract ArrayContract { +// bool[2][] m_pairsOfFlags; +// function setAllFlagPairs(bool[2][] newPairs) { +// // assignment to array replaces the complete array +// m_pairsOfFlags = newPairs; +// } +// })"; +// ETH_TEST_REQUIRE_NO_THROW(sourceUnit = parseTextAndResolveNames(text), "Parsing and name Resolving failed"); +// for (ASTPointer const& node: sourceUnit->getNodes()) +// if (ContractDefinition* contract = dynamic_cast(node.get())) +// { +// auto functions = contract->getDefinedFunctions(); +// BOOST_CHECK_EQUAL("boo(uint256,bool,bytes8)", functions[0]->externalSigniture()); +// } //todo should check arrays and contract. also event //BOOST_AUTO_TEST_CASE(function_external_types_throw) //{ @@ -666,6 +684,7 @@ BOOST_AUTO_TEST_CASE(state_variable_accessors) "mapping(uint=>bytes4) public map;\n" "mapping(uint=>mapping(uint=>bytes4)) public multiple_map;\n" "}\n"; + ASTPointer source; ContractDefinition const* contract; ETH_TEST_CHECK_NO_THROW(source = parseTextAndResolveNames(text), "Parsing and Resolving names failed"); diff --git a/test/SolidityTypes.cpp b/test/SolidityTypes.cpp index 4133ce7b7..6b6306479 100644 --- a/test/SolidityTypes.cpp +++ b/test/SolidityTypes.cpp @@ -86,7 +86,6 @@ BOOST_AUTO_TEST_CASE(storage_layout_arrays) BOOST_CHECK(ArrayType(ArrayType::Location::Storage, make_shared(32), 9).getStorageSize() == 9); } - BOOST_AUTO_TEST_SUITE_END() } From f3e8d2b7e9bf54f1f0f6a7895924ecdeddd4cd9d Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Wed, 25 Mar 2015 13:58:15 +0100 Subject: [PATCH 09/23] tests for external types --- libsolidity/AST.cpp | 14 ++--- libsolidity/AST.h | 4 +- libsolidity/ExpressionCompiler.cpp | 2 +- libsolidity/Types.cpp | 7 ++- libsolidity/Types.h | 3 +- test/SolidityNameAndTypeResolution.cpp | 83 ++++++++++++++++---------- 6 files changed, 65 insertions(+), 48 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 9a2c1be07..9e81c2cb2 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -192,7 +192,7 @@ vector, FunctionTypePointer>> const& ContractDefinition::getIn if (functionsSeen.count(f->getName()) == 0 && f->isPartOfExternalInterface()) { functionsSeen.insert(f->getName()); - FixedHash<4> hash(dev::sha3(f->externalSignature())); + FixedHash<4> hash(dev::sha3(f->externalSignature(true))); m_interfaceFunctionList->push_back(make_pair(hash, make_shared(*f, false))); } @@ -202,7 +202,7 @@ vector, FunctionTypePointer>> const& ContractDefinition::getIn FunctionType ftype(*v); solAssert(v->getType().get(), ""); functionsSeen.insert(v->getName()); - FixedHash<4> hash(dev::sha3(ftype.externalSignature(v->getName()))); + FixedHash<4> hash(dev::sha3(ftype.externalSignature(true, v->getName()))); m_interfaceFunctionList->push_back(make_pair(hash, make_shared(*v))); } } @@ -309,7 +309,7 @@ void FunctionDefinition::checkTypeRequirements() { if (!var->getType()->canLiveOutsideStorage()) BOOST_THROW_EXCEPTION(var->createTypeError("Type is required to live outside storage.")); - if (!var->getType()->externalType() && getVisibility() >= Visibility::Public) + if (!(var->getType()->externalType()) && getVisibility() >= Visibility::Public) BOOST_THROW_EXCEPTION(var->createTypeError("Internal type is not allowed for function with external visibility")); } for (ASTPointer const& modifier: m_functionModifiers) @@ -320,9 +320,9 @@ void FunctionDefinition::checkTypeRequirements() m_body->checkTypeRequirements(); } -string FunctionDefinition::externalSignature() const +string FunctionDefinition::externalSignature(bool isExternalCall) const { - return FunctionType(*this).externalSignature(getName()); + return FunctionType(*this).externalSignature(isExternalCall, getName()); } bool VariableDeclaration::isLValue() const @@ -663,10 +663,6 @@ void MemberAccess::checkTypeRequirements() if (!m_type) BOOST_THROW_EXCEPTION(createTypeError("Member \"" + *m_memberName + "\" not found or not " "visible in " + type.toString())); -// if (auto f = dynamic_cast(m_expression->getType().get())) -// if (f->getLocation() == FunctionType::Location::External && !m_type->externalType()) -// BOOST_THROW_EXCEPTION(createTypeError(*m_memberName + " member has an internal type.")); - // This should probably move somewhere else. if (type.getCategory() == Type::Category::Struct) m_isLValue = true; diff --git a/libsolidity/AST.h b/libsolidity/AST.h index 937c2ceab..0d3ef857a 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -422,9 +422,9 @@ public: void checkTypeRequirements(); /// @returns the external signature of the function - /// That consists of the name of the function followed by the external types of the + /// That consists of the name of the function followed by the types(external types if isExternalCall) of the /// arguments separated by commas all enclosed in parentheses without any spaces. - std::string externalSignature() const; + std::string externalSignature(bool isExternalCall = false) const; private: bool m_isConstructor; diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index 90568767b..ddc347e68 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -544,7 +544,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) } if (!event.isAnonymous()) { - m_context << u256(h256::Arith(dev::sha3(function.externalSignature(event.getName())))); + m_context << u256(h256::Arith(dev::sha3(function.externalSignature(false, event.getName())))); ++numIndexed; } solAssert(numIndexed <= 4, "Too many indexed arguments."); diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 6344d128d..1776413a0 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -1127,7 +1127,7 @@ MemberList const& FunctionType::getMembers() const } } -string FunctionType::externalSignature(std::string const& _name) const +string FunctionType::externalSignature(bool isExternalCall, std::string const& _name) const { std::string funcName = _name; if (_name == "") @@ -1139,8 +1139,9 @@ string FunctionType::externalSignature(std::string const& _name) const for (auto it = m_parameterTypes.cbegin(); it != m_parameterTypes.cend(); ++it) { - solAssert(!!(*it)->externalType(), "Parameter should have external type"); - ret += (*it)->externalType()->toString() + (it + 1 == m_parameterTypes.cend() ? "" : ","); + if (isExternalCall) + solAssert(!!(*it)->externalType(), "Parameter should have external type"); + ret += (isExternalCall ? (*it)->externalType()->toString() : (*it)->toString()) + (it + 1 == m_parameterTypes.cend() ? "" : ","); } return ret + ")"; } diff --git a/libsolidity/Types.h b/libsolidity/Types.h index e00e6b983..c075ff07f 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -553,7 +553,8 @@ public: /// @returns the external signature of this function type given the function name /// If @a _name is not provided (empty string) then the @c m_declaration member of the /// function type is used - std::string externalSignature(std::string const& _name = "") const; + /// @a isExternalCall shows if it is external function call + std::string externalSignature(bool isExternalCall = false, std::string const& _name = "") const; Declaration const& getDeclaration() const { solAssert(m_declaration, "Requested declaration from a FunctionType that has none"); diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index a6d1920a1..75a912244 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include "TestHelper.h" using namespace std; @@ -48,16 +49,28 @@ ASTPointer parseTextAndResolveNames(std::string const& _source) ASTPointer sourceUnit = parser.parse(std::make_shared(CharStream(_source))); NameAndTypeResolver resolver({}); resolver.registerDeclarations(*sourceUnit); + std::shared_ptr globalContext = make_shared(); + for (ASTPointer const& node: sourceUnit->getNodes()) if (ContractDefinition* contract = dynamic_cast(node.get())) + { + globalContext->setCurrentContract(*contract); + resolver.updateDeclaration(*globalContext->getCurrentThis()); + resolver.updateDeclaration(*globalContext->getCurrentSuper()); resolver.resolveNamesAndTypes(*contract); + } for (ASTPointer const& node: sourceUnit->getNodes()) if (ContractDefinition* contract = dynamic_cast(node.get())) + { + globalContext->setCurrentContract(*contract); + resolver.updateDeclaration(*globalContext->getCurrentThis()); resolver.checkTypeRequirements(*contract); + } return sourceUnit; } + static ContractDefinition const* retrieveContract(ASTPointer _source, unsigned index) { ContractDefinition* contract; @@ -376,6 +389,8 @@ BOOST_AUTO_TEST_CASE(function_canonical_signature_type_aliases) if (ContractDefinition* contract = dynamic_cast(node.get())) { auto functions = contract->getDefinedFunctions(); + if (functions.empty()) + continue; BOOST_CHECK_EQUAL("boo(uint256,bytes32,address)", functions[0]->externalSignature()); } } @@ -384,8 +399,11 @@ BOOST_AUTO_TEST_CASE(function_external_types) { ASTPointer sourceUnit; char const* text = R"( + contract C { + uint a; + } contract Test { - function boo(uint arg2, bool arg3, bytes8 arg4, bool[2] pairs) public returns (uint ret) { + function boo(uint arg2, bool arg3, bytes8 arg4, bool[2] pairs, uint[] dynamic, C carg) external returns (uint ret) { ret = 5; } })"; @@ -394,40 +412,41 @@ BOOST_AUTO_TEST_CASE(function_external_types) if (ContractDefinition* contract = dynamic_cast(node.get())) { auto functions = contract->getDefinedFunctions(); - BOOST_CHECK_EQUAL("boo(uint256,bool,bytes8)", functions[0]->externalSignature()); + if (functions.empty()) + continue; + BOOST_CHECK_EQUAL("boo(uint256,bool,bytes8,bool[2],uint256[],address)", functions[0]->externalSignature(true)); } } -//BOOST_AUTO_TEST_CASE(function_external_types_throw) -//{ -// ASTPointer sourceUnit; -// char const* text = R"( -// contract ArrayContract { -// bool[2][] m_pairsOfFlags; -// function setAllFlagPairs(bool[2][] newPairs) { -// // assignment to array replaces the complete array -// m_pairsOfFlags = newPairs; -// } -// })"; -// ETH_TEST_REQUIRE_NO_THROW(sourceUnit = parseTextAndResolveNames(text), "Parsing and name Resolving failed"); -// for (ASTPointer const& node: sourceUnit->getNodes()) -// if (ContractDefinition* contract = dynamic_cast(node.get())) -// { -// auto functions = contract->getDefinedFunctions(); -// BOOST_CHECK_EQUAL("boo(uint256,bool,bytes8)", functions[0]->externalSigniture()); -// } -//todo should check arrays and contract. also event -//BOOST_AUTO_TEST_CASE(function_external_types_throw) -//{ -// ASTPointer sourceUnit; -// char const* text = R"( -// contract Test { -// function boo(uint32[] arg5) returns (uint ret) { -// ret = 5; -// } -// })"; -// BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); -//} +BOOST_AUTO_TEST_CASE(function_external_call_conversion) +{ + char const* sourceCode = R"( + contract C {} + contract Test { + function externalCall() { + address arg; + this.g(arg); + } + function g (C c) external {} + })"; + BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError); +} + +BOOST_AUTO_TEST_CASE(function_internal_call_conversion) +{ + char const* text = R"( + contract C { + uint a; + } + contract Test { + address a; + function g (C c) {} + function internalCall() { + g(a); + } + })"; + BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); +} BOOST_AUTO_TEST_CASE(hash_collision_in_interface) { From 8e46d8379052eb40478c330f6f2575e6ff879652 Mon Sep 17 00:00:00 2001 From: Marek Kotewicz Date: Wed, 25 Mar 2015 20:46:30 +0100 Subject: [PATCH 10/23] fixed minimum jsonrpccpp required version --- cmake/EthDependencies.cmake | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmake/EthDependencies.cmake b/cmake/EthDependencies.cmake index c91f93ef0..f27104c14 100644 --- a/cmake/EthDependencies.cmake +++ b/cmake/EthDependencies.cmake @@ -49,8 +49,9 @@ message(" - Jsoncpp header: ${JSONCPP_INCLUDE_DIRS}") message(" - Jsoncpp lib : ${JSONCPP_LIBRARIES}") # TODO get rid of -DETH_JSONRPC +# TODO add EXACT once we commit ourselves to cmake 3.x if (JSONRPC) - find_package (json_rpc_cpp 0.4 EXACT REQUIRED) + find_package (json_rpc_cpp 0.4 REQUIRED) message (" - json-rpc-cpp header: ${JSON_RPC_CPP_INCLUDE_DIRS}") message (" - json-rpc-cpp lib : ${JSON_RPC_CPP_LIBRARIES}") add_definitions(-DETH_JSONRPC) From 8b14b4f4d19ac5137efadc747c17f6d941517cc1 Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Thu, 26 Mar 2015 14:11:24 +0100 Subject: [PATCH 11/23] two more tests style fixes --- libsolidity/AST.cpp | 2 +- libsolidity/AST.h | 2 +- test/SolidityNameAndTypeResolution.cpp | 38 +++++++++++++++++++++++--- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 9e81c2cb2..461c3d0ca 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -431,7 +431,7 @@ void EventDefinition::checkTypeRequirements() if (!var->getType()->canLiveOutsideStorage()) BOOST_THROW_EXCEPTION(var->createTypeError("Type is required to live outside storage.")); if (!var->getType()->externalType()) - BOOST_THROW_EXCEPTION(var->createTypeError("Internal type is not allowed for Events")); + BOOST_THROW_EXCEPTION(var->createTypeError("Internal type is not allowed as event parameter type.")); } if (numIndexed > 3) BOOST_THROW_EXCEPTION(createTypeError("More than 3 indexed arguments for event.")); diff --git a/libsolidity/AST.h b/libsolidity/AST.h index 0d3ef857a..abcae83c5 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -422,7 +422,7 @@ public: void checkTypeRequirements(); /// @returns the external signature of the function - /// That consists of the name of the function followed by the types(external types if isExternalCall) of the + /// That consists of the name of the function followed by the types (external types if isExternalCall) of the /// arguments separated by commas all enclosed in parentheses without any spaces. std::string externalSignature(bool isExternalCall = false) const; diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index 75a912244..6b9916549 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -418,9 +418,23 @@ BOOST_AUTO_TEST_CASE(function_external_types) } } -BOOST_AUTO_TEST_CASE(function_external_call_conversion) +BOOST_AUTO_TEST_CASE(function_external_call_allowed_conversion) { - char const* sourceCode = R"( + char const* text = R"( + contract C {} + contract Test { + function externalCall() { + C arg; + this.g(arg); + } + function g (C c) external {} + })"; + BOOST_CHECK_NO_THROW(parseTextAndResolveNames(text)); +} + +BOOST_AUTO_TEST_CASE(function_external_call_not_allowed_conversion) +{ + char const* text = R"( contract C {} contract Test { function externalCall() { @@ -429,10 +443,26 @@ BOOST_AUTO_TEST_CASE(function_external_call_conversion) } function g (C c) external {} })"; - BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError); + BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); +} + +BOOST_AUTO_TEST_CASE(function_internal_allowed_conversion) +{ + char const* text = R"( + contract C { + uint a; + } + contract Test { + C a; + function g (C c) {} + function internalCall() { + g(a); + } + })"; + BOOST_CHECK_NO_THROW(parseTextAndResolveNames(text)); } -BOOST_AUTO_TEST_CASE(function_internal_call_conversion) +BOOST_AUTO_TEST_CASE(function_internal_not_allowed_conversion) { char const* text = R"( contract C { From e1ad2cb579488720499211b6e9c0881b279b93fb Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 23 Mar 2015 16:53:28 +0100 Subject: [PATCH 12/23] Comments and renames. --- libevmcore/Instruction.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libevmcore/Instruction.h b/libevmcore/Instruction.h index 07c7b52fd..c869e761d 100644 --- a/libevmcore/Instruction.h +++ b/libevmcore/Instruction.h @@ -237,7 +237,7 @@ enum Tier struct InstructionInfo { std::string name; ///< The name of the instruction. - int additional; ///< Additional items required in memory for this instructions (only for PUSH). + int additional; //a/< Additional items required in memory for this instructions (only for PUSH). int args; ///< Number of items required on the stack for this instruction (and, for the purposes of ret, the number taken from the stack). int ret; ///< Number of items placed (back) on the stack by this instruction, assuming args items were removed. bool sideEffects; ///< false if the only effect on the execution environment (apart from gas usage) is a change to a topmost segment of the stack From ea6d5c2c98dcdb6babd04f0715a58992fcf6e9f6 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 20 Mar 2015 18:18:25 +0100 Subject: [PATCH 13/23] Refactoring: Extract equivalence class container. --- libevmcore/CommonSubexpressionEliminator.cpp | 154 +++++-------------- libevmcore/CommonSubexpressionEliminator.h | 52 +++---- libevmcore/ExpressionClasses.cpp | 110 +++++++++++++ libevmcore/ExpressionClasses.h | 70 +++++++++ 4 files changed, 240 insertions(+), 146 deletions(-) create mode 100644 libevmcore/ExpressionClasses.cpp create mode 100644 libevmcore/ExpressionClasses.h diff --git a/libevmcore/CommonSubexpressionEliminator.cpp b/libevmcore/CommonSubexpressionEliminator.cpp index aa532b2ff..8102721cc 100644 --- a/libevmcore/CommonSubexpressionEliminator.cpp +++ b/libevmcore/CommonSubexpressionEliminator.cpp @@ -32,17 +32,17 @@ using namespace dev::eth; vector CommonSubexpressionEliminator::getOptimizedItems() { - map initialStackContents; - map targetStackContents; + map initialStackContents; + map targetStackContents; int minHeight = m_stackHeight + 1; if (!m_stackElements.empty()) minHeight = min(minHeight, m_stackElements.begin()->first.first); for (int height = minHeight; height <= max(0, m_stackHeight); ++height) { // make sure it is created - EquivalenceClassId c = getStackElement(height); + ExpressionClasses::Id c = getStackElement(height); if (height <= 0) - initialStackContents[height] = getClass(AssemblyItem(dupInstruction(1 - height))); + initialStackContents[height] = m_expressionClasses.find(AssemblyItem(dupInstruction(1 - height))); if (height <= m_stackHeight) targetStackContents[height] = c; } @@ -50,21 +50,21 @@ vector CommonSubexpressionEliminator::getOptimizedItems() // Debug info: //stream(cout, currentStackContents, targetStackContents); - return CSECodeGenerator().generateCode(initialStackContents, targetStackContents, m_equivalenceClasses); + return CSECodeGenerator(m_expressionClasses).generateCode(initialStackContents, targetStackContents); } ostream& CommonSubexpressionEliminator::stream( ostream& _out, - map _currentStack, - map _targetStack + map _currentStack, + map _targetStack ) const { - auto streamEquivalenceClass = [this](ostream& _out, EquivalenceClassId _id) + auto streamExpressionClass = [this](ostream& _out, ExpressionClasses::Id _id) { - auto const& eqClass = m_equivalenceClasses.at(_id); - _out << " " << _id << ": " << *eqClass.first; + auto const& expr = m_expressionClasses.representative(_id); + _out << " " << _id << ": " << *expr.item; _out << "("; - for (EquivalenceClassId arg: eqClass.second) + for (ExpressionClasses::Id arg: expr.arguments) _out << dec << arg << ","; _out << ")" << endl; }; @@ -75,23 +75,23 @@ ostream& CommonSubexpressionEliminator::stream( for (auto const& it: m_stackElements) { _out << " " << dec << it.first.first << "(" << it.first.second << ") = "; - streamEquivalenceClass(_out, it.second); + streamExpressionClass(_out, it.second); } _out << "Equivalence classes: " << endl; - for (EquivalenceClassId eqClass = 0; eqClass < m_equivalenceClasses.size(); ++eqClass) - streamEquivalenceClass(_out, eqClass); + for (ExpressionClasses::Id eqClass = 0; eqClass < m_expressionClasses.size(); ++eqClass) + streamExpressionClass(_out, eqClass); _out << "Current stack: " << endl; for (auto const& it: _currentStack) { _out << " " << dec << it.first << ": "; - streamEquivalenceClass(_out, it.second); + streamExpressionClass(_out, it.second); } _out << "Target stack: " << endl; for (auto const& it: _targetStack) { _out << " " << dec << it.first << ": "; - streamEquivalenceClass(_out, it.second); + streamExpressionClass(_out, it.second); } return _out; @@ -103,7 +103,7 @@ void CommonSubexpressionEliminator::feedItem(AssemblyItem const& _item) { if (_item.deposit() != 1) BOOST_THROW_EXCEPTION(InvalidDeposit()); - setStackElement(++m_stackHeight, getClass(_item, {})); + setStackElement(++m_stackHeight, m_expressionClasses.find(_item, {})); } else { @@ -121,16 +121,16 @@ void CommonSubexpressionEliminator::feedItem(AssemblyItem const& _item) ); else if (instruction != Instruction::POP) { - vector arguments(info.args); + vector arguments(info.args); for (int i = 0; i < info.args; ++i) arguments[i] = getStackElement(m_stackHeight - i); - setStackElement(m_stackHeight + _item.deposit(), getClass(_item, arguments)); + setStackElement(m_stackHeight + _item.deposit(), m_expressionClasses.find(_item, arguments)); } m_stackHeight += _item.deposit(); } } -void CommonSubexpressionEliminator::setStackElement(int _stackHeight, EquivalenceClassId _class) +void CommonSubexpressionEliminator::setStackElement(int _stackHeight, ExpressionClasses::Id _class) { unsigned nextSequence = getNextStackElementSequence(_stackHeight); m_stackElements[make_pair(_stackHeight, nextSequence)] = _class; @@ -140,8 +140,8 @@ void CommonSubexpressionEliminator::swapStackElements(int _stackHeightA, int _st { if (_stackHeightA == _stackHeightB) BOOST_THROW_EXCEPTION(OptimizerException() << errinfo_comment("Swap on same stack elements.")); - EquivalenceClassId classA = getStackElement(_stackHeightA); - EquivalenceClassId classB = getStackElement(_stackHeightB); + ExpressionClasses::Id classA = getStackElement(_stackHeightA); + ExpressionClasses::Id classB = getStackElement(_stackHeightB); unsigned nextSequenceA = getNextStackElementSequence(_stackHeightA); unsigned nextSequenceB = getNextStackElementSequence(_stackHeightB); @@ -149,7 +149,7 @@ void CommonSubexpressionEliminator::swapStackElements(int _stackHeightA, int _st m_stackElements[make_pair(_stackHeightB, nextSequenceB)] = classA; } -EquivalenceClassId CommonSubexpressionEliminator::getStackElement(int _stackHeight) +ExpressionClasses::Id CommonSubexpressionEliminator::getStackElement(int _stackHeight) { // retrieve class by last sequence number unsigned nextSequence = getNextStackElementSequence(_stackHeight); @@ -162,86 +162,8 @@ EquivalenceClassId CommonSubexpressionEliminator::getStackElement(int _stackHeig if (_stackHeight <= -16) BOOST_THROW_EXCEPTION(OptimizerException() << errinfo_comment("Stack too deep.")); // This is a special assembly item that refers to elements pre-existing on the initial stack. - m_spareAssemblyItem.push_back(make_shared(dupInstruction(1 - _stackHeight))); - m_equivalenceClasses.push_back(make_pair(m_spareAssemblyItem.back().get(), EquivalenceClassIds())); - return m_stackElements[make_pair(_stackHeight, nextSequence)] = EquivalenceClassId(m_equivalenceClasses.size() - 1); -} - -EquivalenceClassId CommonSubexpressionEliminator::getClass( - const AssemblyItem& _item, - EquivalenceClassIds const& _arguments -) -{ - // TODO: do a clever search, i.e. - // - check for the presence of constants in the argument classes and do arithmetic - // - check whether the two items are equal for a SUB instruction - // - check whether 0 or 1 is in one of the classes for a MUL - - EquivalenceClassIds args = _arguments; - if (SemanticInformation::isCommutativeOperation(_item)) - sort(args.begin(), args.end()); - - //@todo use a better data structure for search here - for (EquivalenceClassId c = 0; c < m_equivalenceClasses.size(); ++c) - { - AssemblyItem const& classItem = *m_equivalenceClasses.at(c).first; - if (classItem != _item) - continue; - - assertThrow( - args.size() == m_equivalenceClasses.at(c).second.size(), - OptimizerException, - "Equal assembly items with different number of arguments." - ); - if (equal(args.begin(), args.end(), m_equivalenceClasses.at(c).second.begin())) - return c; - } - // constant folding - if (_item.type() == Operation && args.size() == 2 && all_of( - args.begin(), - args.end(), - [this](EquivalenceClassId eqc) { return m_equivalenceClasses.at(eqc).first->match(Push); })) - { - auto signextend = [](u256 const& _a, u256 const& _b) -> u256 - { - if (_a >= 31) - return _b; - unsigned testBit = unsigned(_a) * 8 + 7; - u256 mask = (u256(1) << testBit) - 1; - return boost::multiprecision::bit_test(_b, testBit) ? _b | ~mask : _b & mask; - }; - map> const arithmetics = - { - { Instruction::SUB, [](u256 const& _a, u256 const& _b) -> u256 {return _a - _b; } }, - { Instruction::DIV, [](u256 const& _a, u256 const& _b) -> u256 {return _b == 0 ? 0 : _a / _b; } }, - { Instruction::SDIV, [](u256 const& _a, u256 const& _b) -> u256 { return _b == 0 ? 0 : s2u(u2s(_a) / u2s(_b)); } }, - { Instruction::MOD, [](u256 const& _a, u256 const& _b) -> u256 { return _b == 0 ? 0 : _a % _b; } }, - { Instruction::SMOD, [](u256 const& _a, u256 const& _b) -> u256 { return _b == 0 ? 0 : s2u(u2s(_a) % u2s(_b)); } }, - { Instruction::EXP, [](u256 const& _a, u256 const& _b) -> u256 { return (u256)boost::multiprecision::powm(bigint(_a), bigint(_b), bigint(1) << 256); } }, - { Instruction::SIGNEXTEND, signextend }, - { Instruction::LT, [](u256 const& _a, u256 const& _b) -> u256 { return _a < _b ? 1 : 0; } }, - { Instruction::GT, [](u256 const& _a, u256 const& _b) -> u256 { return _a > _b ? 1 : 0; } }, - { Instruction::SLT, [](u256 const& _a, u256 const& _b) -> u256 { return u2s(_a) < u2s(_b) ? 1 : 0; } }, - { Instruction::SGT, [](u256 const& _a, u256 const& _b) -> u256 { return u2s(_a) > u2s(_b) ? 1 : 0; } }, - { Instruction::EQ, [](u256 const& _a, u256 const& _b) -> u256 { return _a == _b ? 1 : 0; } }, - { Instruction::ADD, [](u256 const& _a, u256 const& _b) -> u256 { return _a + _b; } }, - { Instruction::MUL, [](u256 const& _a, u256 const& _b) -> u256 { return _a * _b; } }, - { Instruction::AND, [](u256 const& _a, u256 const& _b) -> u256 { return _a & _b; } }, - { Instruction::OR, [](u256 const& _a, u256 const& _b) -> u256 { return _a | _b; } }, - { Instruction::XOR, [](u256 const& _a, u256 const& _b) -> u256 { return _a ^ _b; } }, - }; - if (arithmetics.count(_item.instruction())) - { - u256 result = arithmetics.at(_item.instruction())( - m_equivalenceClasses.at(args[0]).first->data(), - m_equivalenceClasses.at(args[1]).first->data() - ); - m_spareAssemblyItem.push_back(make_shared(result)); - return getClass(*m_spareAssemblyItem.back()); - } - } - m_equivalenceClasses.push_back(make_pair(&_item, args)); - return m_equivalenceClasses.size() - 1; + return m_stackElements[make_pair(_stackHeight, nextSequence)] = + m_expressionClasses.find(AssemblyItem(dupInstruction(1 - _stackHeight))); } unsigned CommonSubexpressionEliminator::getNextStackElementSequence(int _stackHeight) @@ -318,15 +240,11 @@ bool SemanticInformation::isSwapInstruction(AssemblyItem const& _item) } AssemblyItems CSECodeGenerator::generateCode( - map const& _initialStack, - map const& _targetStackContents, - vector> const& _equivalenceClasses + map const& _initialStack, + map const& _targetStackContents ) { - // reset - *this = move(CSECodeGenerator()); m_stack = _initialStack; - m_equivalenceClasses = _equivalenceClasses; for (auto const& item: m_stack) if (!m_classPositions.count(item.second)) m_classPositions[item.second] = item.first; @@ -377,18 +295,18 @@ AssemblyItems CSECodeGenerator::generateCode( return m_generatedItems; } -void CSECodeGenerator::addDependencies(EquivalenceClassId _c) +void CSECodeGenerator::addDependencies(ExpressionClasses::Id _c) { if (m_neededBy.count(_c)) return; - for (EquivalenceClassId argument: m_equivalenceClasses.at(_c).second) + for (ExpressionClasses::Id argument: m_expressionClasses.representative(_c).arguments) { addDependencies(argument); m_neededBy.insert(make_pair(argument, _c)); } } -int CSECodeGenerator::generateClassElement(EquivalenceClassId _c) +int CSECodeGenerator::generateClassElement(ExpressionClasses::Id _c) { if (m_classPositions.count(_c)) { @@ -399,8 +317,8 @@ int CSECodeGenerator::generateClassElement(EquivalenceClassId _c) ); return m_classPositions[_c]; } - EquivalenceClassIds const& arguments = m_equivalenceClasses.at(_c).second; - for (EquivalenceClassId arg: boost::adaptors::reverse(arguments)) + ExpressionClasses::Ids const& arguments = m_expressionClasses.representative(_c).arguments; + for (ExpressionClasses::Id arg: boost::adaptors::reverse(arguments)) generateClassElement(arg); // The arguments are somewhere on the stack now, so it remains to move them at the correct place. @@ -458,7 +376,7 @@ int CSECodeGenerator::generateClassElement(EquivalenceClassId _c) for (size_t i = 0; i < arguments.size(); ++i) assertThrow(m_stack[m_stackHeight - i] == arguments[i], OptimizerException, "Expected arguments not present." ); - AssemblyItem const& item = *m_equivalenceClasses.at(_c).first; + AssemblyItem const& item = *m_expressionClasses.representative(_c).item; while (SemanticInformation::isCommutativeOperation(item) && !m_generatedItems.empty() && m_generatedItems.back() == AssemblyItem(Instruction::SWAP1)) @@ -469,12 +387,12 @@ int CSECodeGenerator::generateClassElement(EquivalenceClassId _c) m_classPositions[arg] = c_invalidPosition; for (size_t i = 0; i < arguments.size(); ++i) m_stack.erase(m_stackHeight - i); - appendItem(*m_equivalenceClasses.at(_c).first); + appendItem(*m_expressionClasses.representative(_c).item); m_stack[m_stackHeight] = _c; return m_classPositions[_c] = m_stackHeight; } -bool CSECodeGenerator::canBeRemoved(EquivalenceClassId _element, EquivalenceClassId _result) +bool CSECodeGenerator::canBeRemoved(ExpressionClasses::Id _element, ExpressionClasses::Id _result) { // Returns false if _element is finally needed or is needed by a class that has not been // computed yet. Note that m_classPositions also includes classes that were deleted in the meantime. @@ -493,7 +411,7 @@ bool CSECodeGenerator::removeStackTopIfPossible() if (m_stack.empty()) return false; assertThrow(m_stack.count(m_stackHeight), OptimizerException, ""); - EquivalenceClassId top = m_stack[m_stackHeight]; + ExpressionClasses::Id top = m_stack[m_stackHeight]; if (!canBeRemoved(top)) return false; m_generatedItems.push_back(AssemblyItem(Instruction::POP)); diff --git a/libevmcore/CommonSubexpressionEliminator.h b/libevmcore/CommonSubexpressionEliminator.h index edf1e3f38..2a49d888d 100644 --- a/libevmcore/CommonSubexpressionEliminator.h +++ b/libevmcore/CommonSubexpressionEliminator.h @@ -28,6 +28,7 @@ #include #include #include +#include namespace dev { @@ -37,9 +38,6 @@ namespace eth class AssemblyItem; using AssemblyItems = std::vector; -using EquivalenceClassId = unsigned; -using EquivalenceClassIds = std::vector; - /** * Optimizer step that performs common subexpression elimination and stack reorganisation, * i.e. it tries to infer equality among expressions and compute the values of two expressions @@ -67,24 +65,22 @@ public: /// Streams debugging information to @a _out. std::ostream& stream( std::ostream& _out, - std::map _currentStack = std::map(), - std::map _targetStack = std::map() + std::map _currentStack = std::map(), + std::map _targetStack = std::map() ) const; private: /// Feeds the item into the system for analysis. void feedItem(AssemblyItem const& _item); + /// Simplifies the given item using /// Assigns a new equivalence class to the next sequence number of the given stack element. - void setStackElement(int _stackHeight, EquivalenceClassId _class); + void setStackElement(int _stackHeight, ExpressionClasses::Id _class); /// Swaps the given stack elements in their next sequence number. void swapStackElements(int _stackHeightA, int _stackHeightB); /// Retrieves the current equivalence class fo the given stack element (or generates a new /// one if it does not exist yet). - EquivalenceClassId getStackElement(int _stackHeight); - /// Retrieves the equivalence class resulting from the given item applied to the given classes, - /// might also create a new one. - EquivalenceClassId getClass(AssemblyItem const& _item, EquivalenceClassIds const& _arguments = {}); + ExpressionClasses::Id getStackElement(int _stackHeight); /// @returns the next sequence number of the given stack element. unsigned getNextStackElementSequence(int _stackHeight); @@ -92,12 +88,9 @@ private: /// Current stack height, can be negative. int m_stackHeight = 0; /// Mapping (stack height, sequence number) -> equivalence class - std::map, EquivalenceClassId> m_stackElements; - /// Vector of equivalence class representatives - we only store one item of an equivalence - /// class and the index is used as identifier. - std::vector> m_equivalenceClasses; - /// List of items generated during analysis. - std::vector> m_spareAssemblyItem; + std::map, ExpressionClasses::Id> m_stackElements; + /// Structure containing the classes of equivalent expressions. + ExpressionClasses m_expressionClasses; }; /** @@ -121,27 +114,30 @@ struct SemanticInformation class CSECodeGenerator { public: + CSECodeGenerator(ExpressionClasses const& _expressionClasses): + m_expressionClasses(_expressionClasses) + {} + /// @returns the assembly items generated from the given requirements /// @param _initialStack current contents of the stack (up to stack height of zero) /// @param _targetStackContents final contents of the stack, by stack height relative to initial /// @param _equivalenceClasses equivalence classes as expressions of how to compute them - /// @note resuts the state of the object for each call. + /// @note should only be called once on each object. AssemblyItems generateCode( - std::map const& _initialStack, - std::map const& _targetStackContents, - std::vector> const& _equivalenceClasses + std::map const& _initialStack, + std::map const& _targetStackContents ); private: /// Recursively discovers all dependencies to @a m_requests. - void addDependencies(EquivalenceClassId _c); + void addDependencies(ExpressionClasses::Id _c); /// Produce code that generates the given element if it is not yet present. /// @returns the stack position of the element. - int generateClassElement(EquivalenceClassId _c); + int generateClassElement(ExpressionClasses::Id _c); /// @returns true if @a _element can be removed - in general or, if given, while computing @a _result. - bool canBeRemoved(EquivalenceClassId _element, EquivalenceClassId _result = EquivalenceClassId(-1)); + bool canBeRemoved(ExpressionClasses::Id _element, ExpressionClasses::Id _result = ExpressionClasses::Id(-1)); /// Appends code to remove the topmost stack element if it can be removed. bool removeStackTopIfPossible(); @@ -160,16 +156,16 @@ private: /// Current height of the stack relative to the start. int m_stackHeight = 0; /// If (b, a) is in m_requests then b is needed to compute a. - std::multimap m_neededBy; + std::multimap m_neededBy; /// Current content of the stack. - std::map m_stack; + std::map m_stack; /// Current positions of equivalence classes, equal to c_invalidPosition if already deleted. - std::map m_classPositions; + std::map m_classPositions; /// The actual eqivalence class items and how to compute them. - std::vector> m_equivalenceClasses; + ExpressionClasses const& m_expressionClasses; /// The set of equivalence classes that should be present on the stack at the end. - std::set m_finalClasses; + std::set m_finalClasses; }; template diff --git a/libevmcore/ExpressionClasses.cpp b/libevmcore/ExpressionClasses.cpp new file mode 100644 index 000000000..5d7b3e11b --- /dev/null +++ b/libevmcore/ExpressionClasses.cpp @@ -0,0 +1,110 @@ +/* + This file is part of cpp-ethereum. + + cpp-ethereum is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + cpp-ethereum is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with cpp-ethereum. If not, see . +*/ +/** + * @file ExpressionClasses.cpp + * @author Christian + * @date 2015 + * Container for equivalence classes of expressions for use in common subexpression elimination. + */ + +#include +#include +#include +#include + +using namespace std; +using namespace dev; +using namespace dev::eth; + + +ExpressionClasses::Id ExpressionClasses::find(AssemblyItem const& _item, Ids const& _arguments) +{ + // TODO: do a clever search, i.e. + // - check for the presence of constants in the argument classes and do arithmetic + // - check whether the two items are equal for a SUB instruction + // - check whether 0 or 1 is in one of the classes for a MUL + + Expression exp; + exp.item = &_item; + exp.arguments = _arguments; + if (SemanticInformation::isCommutativeOperation(_item)) + sort(exp.arguments.begin(), exp.arguments.end()); + + //@todo use a data structure that allows better searches + for (Expression const& e: m_representatives) + if (std::tie(*e.item, e.arguments) == std::tie(*exp.item, exp.arguments)) + return e.id; + + if (SemanticInformation::isDupInstruction(_item)) + { + // Special item that refers to values pre-existing on the stack + m_spareAssemblyItem.push_back(make_shared(_item)); + exp.item = m_spareAssemblyItem.back().get(); + } + else if (_item.type() == Operation) + { + //@todo try to avoid having to do this multiple times by storing not only one representative of + // an equivalence class + + // constant folding + auto isConstant = [this](Id eqc) { return representative(eqc).item->match(Push); }; + if (exp.arguments.size() == 2 && all_of(exp.arguments.begin(), exp.arguments.end(), isConstant)) + { + auto signextend = [](u256 a, u256 b) -> u256 + { + if (a >= 31) + return b; + unsigned testBit = unsigned(a) * 8 + 7; + u256 mask = (u256(1) << testBit) - 1; + return boost::multiprecision::bit_test(b, testBit) ? b | ~mask : b & mask; + }; + map> const arithmetics = + { + { Instruction::SUB, [](u256 a, u256 b) -> u256 {return a - b; } }, + { Instruction::DIV, [](u256 a, u256 b) -> u256 {return b == 0 ? 0 : a / b; } }, + { Instruction::SDIV, [](u256 a, u256 b) -> u256 { return b == 0 ? 0 : s2u(u2s(a) / u2s(b)); } }, + { Instruction::MOD, [](u256 a, u256 b) -> u256 { return b == 0 ? 0 : a % b; } }, + { Instruction::SMOD, [](u256 a, u256 b) -> u256 { return b == 0 ? 0 : s2u(u2s(a) % u2s(b)); } }, + { Instruction::EXP, [](u256 a, u256 b) -> u256 { return (u256)boost::multiprecision::powm(bigint(a), bigint(b), bigint(1) << 256); } }, + { Instruction::SIGNEXTEND, signextend }, + { Instruction::LT, [](u256 a, u256 b) -> u256 { return a < b ? 1 : 0; } }, + { Instruction::GT, [](u256 a, u256 b) -> u256 { return a > b ? 1 : 0; } }, + { Instruction::SLT, [](u256 a, u256 b) -> u256 { return u2s(a) < u2s(b) ? 1 : 0; } }, + { Instruction::SGT, [](u256 a, u256 b) -> u256 { return u2s(a) > u2s(b) ? 1 : 0; } }, + { Instruction::EQ, [](u256 a, u256 b) -> u256 { return a == b ? 1 : 0; } }, + { Instruction::ADD, [](u256 a, u256 b) -> u256 { return a + b; } }, + { Instruction::MUL, [](u256 a, u256 b) -> u256 { return a * b; } }, + { Instruction::AND, [](u256 a, u256 b) -> u256 { return a & b; } }, + { Instruction::OR, [](u256 a, u256 b) -> u256 { return a | b; } }, + { Instruction::XOR, [](u256 a, u256 b) -> u256 { return a ^ b; } }, + }; + if (arithmetics.count(_item.instruction())) + { + u256 result = arithmetics.at(_item.instruction())( + representative(exp.arguments[0]).item->data(), + representative(exp.arguments[1]).item->data() + ); + m_spareAssemblyItem.push_back(make_shared(result)); + return find(*m_spareAssemblyItem.back()); + } + } + } + exp.id = m_representatives.size(); + m_representatives.push_back(exp); + return exp.id; +} + diff --git a/libevmcore/ExpressionClasses.h b/libevmcore/ExpressionClasses.h new file mode 100644 index 000000000..89485214c --- /dev/null +++ b/libevmcore/ExpressionClasses.h @@ -0,0 +1,70 @@ +/* + This file is part of cpp-ethereum. + + cpp-ethereum is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + cpp-ethereum is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with cpp-ethereum. If not, see . +*/ +/** + * @file ExpressionClasses.h + * @author Christian + * @date 2015 + * Container for equivalence classes of expressions for use in common subexpression elimination. + */ + +#pragma once + +#include +#include +#include + +namespace dev +{ +namespace eth +{ + +class AssemblyItem; + +/** + * Collection of classes of equivalent expressions that can also determine the class of an expression. + * Identifiers are contiguously assigned to new classes starting from zero. + */ +class ExpressionClasses +{ +public: + using Id = unsigned; + using Ids = std::vector; + + struct Expression + { + Id id; + AssemblyItem const* item; + Ids arguments; + }; + + /// Retrieves the id of the expression equivalence class resulting from the given item applied to the + /// given classes, might also create a new one. + Id find(AssemblyItem const& _item, Ids const& _arguments = {}); + /// @returns the canonical representative of an expression class. + Expression const& representative(Id _id) const { return m_representatives.at(_id); } + /// @returns the number of classes. + Id size() const { return m_representatives.size(); } + +private: + + /// Expression equivalence class representatives - we only store one item of an equivalence. + std::vector m_representatives; + std::vector> m_spareAssemblyItem; +}; + +} +} From fd961605b4ded133633f76885dacea8cb4ba9123 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 20 Mar 2015 22:39:42 +0100 Subject: [PATCH 14/23] Optimizing various single operations. --- libevmcore/Assembly.cpp | 39 ++-- libevmcore/CommonSubexpressionEliminator.cpp | 12 +- libevmcore/Exceptions.h | 1 + libevmcore/ExpressionClasses.cpp | 226 +++++++++++++++---- libevmcore/ExpressionClasses.h | 7 +- test/SolidityOptimizer.cpp | 78 ++++--- 6 files changed, 247 insertions(+), 116 deletions(-) diff --git a/libevmcore/Assembly.cpp b/libevmcore/Assembly.cpp index ab9d76911..bd504dc22 100644 --- a/libevmcore/Assembly.cpp +++ b/libevmcore/Assembly.cpp @@ -288,18 +288,6 @@ inline bool matches(AssemblyItemsConstRef _a, AssemblyItemsConstRef _b) return true; } -//@todo this has to move to a special optimizer class soon -template -unsigned bytesRequiredBySlice(Iterator _begin, Iterator _end) -{ - // this is only used in the optimizer, so we can provide a guess for the address length - unsigned addressLength = 4; - unsigned size = 0; - for (; _begin != _end; ++_begin) - size += _begin->bytesRequired(addressLength); - return size; -} - struct OptimiserChannel: public LogChannel { static const char* name() { return "OPT"; } static const int verbosity = 12; }; #define copt dev::LogOutputStream() @@ -315,8 +303,6 @@ Assembly& Assembly::optimise(bool _enable) { Instruction::OR, [](u256 a, u256 b)->u256{return a | b;} }, { Instruction::XOR, [](u256 a, u256 b)->u256{return a ^ b;} }, }; - std::vector> const c_identities = - { { Instruction::ADD, 0}, { Instruction::MUL, 1}, { Instruction::MOD, 0}, { Instruction::OR, 0}, { Instruction::XOR, 0} }; std::vector>> rules = { { { Push, Instruction::POP }, [](AssemblyItemsConstRef) -> AssemblyItems { return {}; } }, @@ -334,17 +320,13 @@ Assembly& Assembly::optimise(bool _enable) rules.push_back({ { Push, Push, i.first }, [&](AssemblyItemsConstRef m) -> AssemblyItems { return { i.second(m[1].data(), m[0].data()) }; } }); rules.push_back({ { Push, i.first, Push, i.first }, [&](AssemblyItemsConstRef m) -> AssemblyItems { return { i.second(m[2].data(), m[0].data()), i.first }; } }); } - for (auto const& i: c_identities) - rules.push_back({{Push, i.first}, [&](AssemblyItemsConstRef m) -> AssemblyItems - { return m[0].data() == i.second ? AssemblyItems() : m.toVector(); }}); // jump to next instruction rules.push_back({ { PushTag, Instruction::JUMP, Tag }, [](AssemblyItemsConstRef m) -> AssemblyItems { if (m[0].m_data == m[2].m_data) return {m[2]}; else return m.toVector(); }}); - copt << *this; - unsigned total = 0; for (unsigned count = 1; count > 0; total += count) { + copt << *this; count = 0; copt << "Performing common subexpression elimination..."; @@ -353,11 +335,22 @@ Assembly& Assembly::optimise(bool _enable) CommonSubexpressionEliminator eliminator; auto orig = iter; iter = eliminator.feedItems(iter, m_items.end()); - AssemblyItems optItems = eliminator.getOptimizedItems(); - copt << "Old size: " << (iter - orig) << ", new size: " << optItems.size(); - if (optItems.size() < size_t(iter - orig)) + AssemblyItems optItems; + bool shouldReplace = false; + try + { + optItems = eliminator.getOptimizedItems(); + shouldReplace = (optItems.size() < size_t(iter - orig)); + } + catch (StackTooDeepException const&) + { + // This might happen if the opcode reconstruction is not as efficient + // as the hand-crafted code. + } + + if (shouldReplace) { - // replace items + copt << "Old size: " << (iter - orig) << ", new size: " << optItems.size(); count++; for (auto moveIter = optItems.begin(); moveIter != optItems.end(); ++orig, ++moveIter) *orig = move(*moveIter); diff --git a/libevmcore/CommonSubexpressionEliminator.cpp b/libevmcore/CommonSubexpressionEliminator.cpp index 8102721cc..65aac74f1 100644 --- a/libevmcore/CommonSubexpressionEliminator.cpp +++ b/libevmcore/CommonSubexpressionEliminator.cpp @@ -157,10 +157,8 @@ ExpressionClasses::Id CommonSubexpressionEliminator::getStackElement(int _stackH return m_stackElements[make_pair(_stackHeight, nextSequence - 1)]; // Stack element not found (not assigned yet), create new equivalence class. - if (_stackHeight > 0) - BOOST_THROW_EXCEPTION(OptimizerException() << errinfo_comment("Stack element accessed before assignment.")); - if (_stackHeight <= -16) - BOOST_THROW_EXCEPTION(OptimizerException() << errinfo_comment("Stack too deep.")); + assertThrow(_stackHeight <= 0, OptimizerException, "Stack element accessed before assignment."); + assertThrow(_stackHeight > -16, StackTooDeepException, ""); // This is a special assembly item that refers to elements pre-existing on the initial stack. return m_stackElements[make_pair(_stackHeight, nextSequence)] = m_expressionClasses.find(AssemblyItem(dupInstruction(1 - _stackHeight))); @@ -423,7 +421,8 @@ bool CSECodeGenerator::removeStackTopIfPossible() void CSECodeGenerator::appendDup(int _fromPosition) { int nr = 1 + m_stackHeight - _fromPosition; - assertThrow(1 <= nr && nr <= 16, OptimizerException, "Stack too deep."); + assertThrow(nr <= 16, StackTooDeepException, "Stack too deep."); + assertThrow(1 <= nr, OptimizerException, "Invalid stack access."); m_generatedItems.push_back(AssemblyItem(dupInstruction(nr))); m_stackHeight++; m_stack[m_stackHeight] = m_stack[_fromPosition]; @@ -434,7 +433,8 @@ void CSECodeGenerator::appendSwapOrRemove(int _fromPosition) if (_fromPosition == m_stackHeight) return; int nr = m_stackHeight - _fromPosition; - assertThrow(1 <= nr && nr <= 16, OptimizerException, "Stack too deep."); + assertThrow(nr <= 16, StackTooDeepException, "Stack too deep."); + assertThrow(1 <= nr, OptimizerException, "Invalid stack access."); m_generatedItems.push_back(AssemblyItem(swapInstruction(nr))); // The value of a class can be present in multiple locations on the stack. We only update the // "canonical" one that is tracked by m_classPositions diff --git a/libevmcore/Exceptions.h b/libevmcore/Exceptions.h index fad972179..fa3c19f13 100644 --- a/libevmcore/Exceptions.h +++ b/libevmcore/Exceptions.h @@ -32,6 +32,7 @@ struct AssemblyException: virtual Exception {}; struct InvalidDeposit: virtual AssemblyException {}; struct InvalidOpcode: virtual AssemblyException {}; struct OptimizerException: virtual AssemblyException {}; +struct StackTooDeepException: virtual OptimizerException {}; } } diff --git a/libevmcore/ExpressionClasses.cpp b/libevmcore/ExpressionClasses.cpp index 5d7b3e11b..eebc98f66 100644 --- a/libevmcore/ExpressionClasses.cpp +++ b/libevmcore/ExpressionClasses.cpp @@ -22,6 +22,9 @@ */ #include +#include +#include +#include #include #include #include @@ -31,22 +34,26 @@ using namespace dev; using namespace dev::eth; -ExpressionClasses::Id ExpressionClasses::find(AssemblyItem const& _item, Ids const& _arguments) +bool ExpressionClasses::Expression::operator<(const ExpressionClasses::Expression& _other) const { - // TODO: do a clever search, i.e. - // - check for the presence of constants in the argument classes and do arithmetic - // - check whether the two items are equal for a SUB instruction - // - check whether 0 or 1 is in one of the classes for a MUL + auto type = item->type(); + auto otherType = _other.item->type(); + return std::tie(type, item->data(), arguments) < + std::tie(otherType, _other.item->data(), _other.arguments); +} +ExpressionClasses::Id ExpressionClasses::find(AssemblyItem const& _item, Ids const& _arguments) +{ Expression exp; exp.item = &_item; exp.arguments = _arguments; + if (SemanticInformation::isCommutativeOperation(_item)) sort(exp.arguments.begin(), exp.arguments.end()); - //@todo use a data structure that allows better searches + //@todo store all class members (not only the representatives) in an efficient data structure to search here for (Expression const& e: m_representatives) - if (std::tie(*e.item, e.arguments) == std::tie(*exp.item, exp.arguments)) + if (!(e < exp || exp < e)) return e.id; if (SemanticInformation::isDupInstruction(_item)) @@ -55,56 +62,175 @@ ExpressionClasses::Id ExpressionClasses::find(AssemblyItem const& _item, Ids con m_spareAssemblyItem.push_back(make_shared(_item)); exp.item = m_spareAssemblyItem.back().get(); } - else if (_item.type() == Operation) + + ExpressionClasses::Id id = tryToSimplify(exp); + if (id < m_representatives.size()) + return id; + + exp.id = m_representatives.size(); + m_representatives.push_back(exp); + return exp.id; +} + +ExpressionClasses::Id ExpressionClasses::tryToSimplify(Expression const& _expr, bool _secondRun) +{ + if (_expr.item->type() != Operation) + return -1; + + // @todo: + // ISZERO ISZERO + // associative operations (as done in Assembly.cpp) + // 2 * x == x + x + + Id arg1; + Id arg2; + Id arg3; + u256 data1; + u256 data2; + u256 data3; + switch (_expr.arguments.size()) { - //@todo try to avoid having to do this multiple times by storing not only one representative of - // an equivalence class + default: + arg3 = _expr.arguments.at(2); + data3 = representative(arg3).item->data(); + case 2: + arg2 = _expr.arguments.at(1); + data2 = representative(arg2).item->data(); + case 1: + arg1 = _expr.arguments.at(0); + data1 = representative(arg1).item->data(); + case 0: + break; + } - // constant folding - auto isConstant = [this](Id eqc) { return representative(eqc).item->match(Push); }; - if (exp.arguments.size() == 2 && all_of(exp.arguments.begin(), exp.arguments.end(), isConstant)) + /** + * Simplification rule. If _strict is false, Push or a constant matches any constant, + * otherwise Push matches "0" and a constant matches itself. + * "UndefinedItem" matches any expression, but all of them must be equal inside one rule. + */ + struct Rule + { + Rule(AssemblyItems const& _pattern, bool _strict, function const& _action): + pattern(_pattern), + assemblyItemAction(_action), + strict(_strict) + {} + Rule(AssemblyItems const& _pattern, function _action): + Rule(_pattern, false, _action) + {} + Rule(AssemblyItems const& _pattern, bool _strict, function const& _action): + pattern(_pattern), + idAction(_action), + strict(_strict) + {} + Rule(AssemblyItems const& _pattern, function _action): + Rule(_pattern, false, _action) + {} + bool matches(ExpressionClasses const& _classes, Expression const& _expr) const { - auto signextend = [](u256 a, u256 b) -> u256 - { - if (a >= 31) - return b; - unsigned testBit = unsigned(a) * 8 + 7; - u256 mask = (u256(1) << testBit) - 1; - return boost::multiprecision::bit_test(b, testBit) ? b | ~mask : b & mask; - }; - map> const arithmetics = + if (!_expr.item->match(pattern.front())) + return false; + assertThrow(_expr.arguments.size() == pattern.size() - 1, OptimizerException, ""); + Id argRequiredToBeEqual(-1); + for (size_t i = 1; i < pattern.size(); ++i) { - { Instruction::SUB, [](u256 a, u256 b) -> u256 {return a - b; } }, - { Instruction::DIV, [](u256 a, u256 b) -> u256 {return b == 0 ? 0 : a / b; } }, - { Instruction::SDIV, [](u256 a, u256 b) -> u256 { return b == 0 ? 0 : s2u(u2s(a) / u2s(b)); } }, - { Instruction::MOD, [](u256 a, u256 b) -> u256 { return b == 0 ? 0 : a % b; } }, - { Instruction::SMOD, [](u256 a, u256 b) -> u256 { return b == 0 ? 0 : s2u(u2s(a) % u2s(b)); } }, - { Instruction::EXP, [](u256 a, u256 b) -> u256 { return (u256)boost::multiprecision::powm(bigint(a), bigint(b), bigint(1) << 256); } }, - { Instruction::SIGNEXTEND, signextend }, - { Instruction::LT, [](u256 a, u256 b) -> u256 { return a < b ? 1 : 0; } }, - { Instruction::GT, [](u256 a, u256 b) -> u256 { return a > b ? 1 : 0; } }, - { Instruction::SLT, [](u256 a, u256 b) -> u256 { return u2s(a) < u2s(b) ? 1 : 0; } }, - { Instruction::SGT, [](u256 a, u256 b) -> u256 { return u2s(a) > u2s(b) ? 1 : 0; } }, - { Instruction::EQ, [](u256 a, u256 b) -> u256 { return a == b ? 1 : 0; } }, - { Instruction::ADD, [](u256 a, u256 b) -> u256 { return a + b; } }, - { Instruction::MUL, [](u256 a, u256 b) -> u256 { return a * b; } }, - { Instruction::AND, [](u256 a, u256 b) -> u256 { return a & b; } }, - { Instruction::OR, [](u256 a, u256 b) -> u256 { return a | b; } }, - { Instruction::XOR, [](u256 a, u256 b) -> u256 { return a ^ b; } }, - }; - if (arithmetics.count(_item.instruction())) + Id arg = _expr.arguments[i - 1]; + if (pattern[i].type() == UndefinedItem) + { + if (argRequiredToBeEqual == Id(-1)) + argRequiredToBeEqual = arg; + else if (argRequiredToBeEqual != arg) + return false; + } + else + { + AssemblyItem const& argItem = *_classes.representative(arg).item; + if (strict && argItem != pattern[i]) + return false; + else if (!strict && !argItem.match(pattern[i])) + return false; + } + } + return true; + } + + AssemblyItems pattern; + function assemblyItemAction; + function idAction; + bool strict; + }; + + vector c_singleLevel{ + // arithmetics on constants involving only stack variables + {{Instruction::ADD, Push, Push}, [&]{ return data1 + data2; }}, + {{Instruction::MUL, Push, Push}, [&]{ return data1 * data2; }}, + {{Instruction::SUB, Push, Push}, [&]{ return data1 - data2; }}, + {{Instruction::DIV, Push, Push}, [&]{ return data2 == 0 ? 0 : data1 / data2; }}, + {{Instruction::SDIV, Push, Push}, [&]{ return data2 == 0 ? 0 : s2u(u2s(data1) / u2s(data2)); }}, + {{Instruction::MOD, Push, Push}, [&]{ return data2 == 0 ? 0 : data1 % data2; }}, + {{Instruction::SMOD, Push, Push}, [&]{ return data2 == 0 ? 0 : s2u(u2s(data1) % u2s(data2)); }}, + {{Instruction::EXP, Push, Push}, [&]{ return u256(boost::multiprecision::powm(bigint(data1), bigint(data2), bigint(1) << 256)); }}, + {{Instruction::NOT, Push}, [&]{ return ~data1; }}, + {{Instruction::LT, Push, Push}, [&]() -> u256 { return data1 < data2 ? 1 : 0; }}, + {{Instruction::GT, Push, Push}, [&]() -> u256 { return data1 > data2 ? 1 : 0; }}, + {{Instruction::SLT, Push, Push}, [&]() -> u256 { return u2s(data1) < u2s( data2) ? 1 : 0; }}, + {{Instruction::SGT, Push, Push}, [&]() -> u256 { return u2s(data1) > u2s( data2) ? 1 : 0; }}, + {{Instruction::EQ, Push, Push}, [&]() -> u256 { return data1 == data2 ? 1 : 0; }}, + {{Instruction::ISZERO, Push}, [&]() -> u256 { return data1 == 0 ? 1 : 0; }}, + {{Instruction::AND, Push, Push}, [&]{ return data1 & data2; }}, + {{Instruction::OR, Push, Push}, [&]{ return data1 | data2; }}, + {{Instruction::XOR, Push, Push}, [&]{ return data1 ^ data2; }}, + {{Instruction::BYTE, Push, Push}, [&]{ return data1 >= 32 ? 0 : (data2 >> unsigned(8 * (31 - data1))) & 0xff; }}, + {{Instruction::ADDMOD, Push, Push, Push}, [&]{ return data3 == 0 ? 0 : u256((bigint(data1) + bigint(data2)) % data3); }}, + {{Instruction::MULMOD, Push, Push, Push}, [&]{ return data3 == 0 ? 0 : u256((bigint(data1) * bigint(data2)) % data3); }}, + {{Instruction::MULMOD, Push, Push, Push}, [&]{ return data1 * data2; }}, + {{Instruction::SIGNEXTEND, Push, Push}, [&]{ + if (data1 >= 31) + return data2; + unsigned testBit = unsigned(data1) * 8 + 7; + u256 mask = (u256(1) << testBit) - 1; + return u256(boost::multiprecision::bit_test(data2, testBit) ? data2 | ~mask : data2 & mask); + }}, + {{Instruction::ADD, UndefinedItem, u256(0)}, true, [&]{ return arg1; }}, + {{Instruction::MUL, UndefinedItem, u256(1)}, true, [&]{ return arg1; }}, + {{Instruction::OR, UndefinedItem, u256(0)}, true, [&]{ return arg1; }}, + {{Instruction::XOR, UndefinedItem, u256(0)}, true, [&]{ return arg1; }}, + {{Instruction::AND, UndefinedItem, ~u256(0)}, true, [&]{ return arg1; }}, + {{Instruction::MUL, UndefinedItem, u256(0)}, true, [&]{ return u256(0); }}, + {{Instruction::DIV, UndefinedItem, u256(0)}, true, [&]{ return u256(0); }}, + {{Instruction::MOD, UndefinedItem, u256(0)}, true, [&]{ return u256(0); }}, + {{Instruction::MOD, u256(0), UndefinedItem}, true, [&]{ return u256(0); }}, + {{Instruction::AND, UndefinedItem, u256(0)}, true, [&]{ return u256(0); }}, + {{Instruction::OR, UndefinedItem, ~u256(0)}, true, [&]{ return ~u256(0); }}, + {{Instruction::AND, UndefinedItem, UndefinedItem}, true, [&]{ return arg1; }}, + {{Instruction::OR, UndefinedItem, UndefinedItem}, true, [&]{ return arg1; }}, + {{Instruction::SUB, UndefinedItem, UndefinedItem}, true, [&]{ return u256(0); }}, + {{Instruction::EQ, UndefinedItem, UndefinedItem}, true, [&]{ return u256(1); }}, + {{Instruction::LT, UndefinedItem, UndefinedItem}, true, [&]{ return u256(0); }}, + {{Instruction::SLT, UndefinedItem, UndefinedItem}, true, [&]{ return u256(0); }}, + {{Instruction::GT, UndefinedItem, UndefinedItem}, true, [&]{ return u256(0); }}, + {{Instruction::SGT, UndefinedItem, UndefinedItem}, true, [&]{ return u256(0); }}, + {{Instruction::MOD, UndefinedItem, UndefinedItem}, true, [&]{ return u256(0); }}, + }; + + for (auto const& rule: c_singleLevel) + if (rule.matches(*this, _expr)) + { + if (rule.idAction) + return rule.idAction(); + else { - u256 result = arithmetics.at(_item.instruction())( - representative(exp.arguments[0]).item->data(), - representative(exp.arguments[1]).item->data() - ); - m_spareAssemblyItem.push_back(make_shared(result)); + m_spareAssemblyItem.push_back(make_shared(rule.assemblyItemAction())); return find(*m_spareAssemblyItem.back()); } } + + if (!_secondRun && _expr.arguments.size() == 2 && SemanticInformation::isCommutativeOperation(*_expr.item)) + { + Expression expr = _expr; + swap(expr.arguments[0], expr.arguments[1]); + return tryToSimplify(expr, true); } - exp.id = m_representatives.size(); - m_representatives.push_back(exp); - return exp.id; -} + return -1; +} diff --git a/libevmcore/ExpressionClasses.h b/libevmcore/ExpressionClasses.h index 89485214c..71fc96109 100644 --- a/libevmcore/ExpressionClasses.h +++ b/libevmcore/ExpressionClasses.h @@ -24,7 +24,7 @@ #pragma once #include -#include +#include #include namespace dev @@ -49,6 +49,7 @@ public: Id id; AssemblyItem const* item; Ids arguments; + bool operator<(Expression const& _other) const; }; /// Retrieves the id of the expression equivalence class resulting from the given item applied to the @@ -60,6 +61,10 @@ public: Id size() const { return m_representatives.size(); } private: + /// Tries to simplify the given expression. + /// @returns its class if it possible or Id(-1) otherwise. + /// @param _secondRun is set to true for the second run where arguments of commutative expressions are reversed + Id tryToSimplify(Expression const& _expr, bool _secondRun = false); /// Expression equivalence class representatives - we only store one item of an equivalence. std::vector m_representatives; diff --git a/test/SolidityOptimizer.cpp b/test/SolidityOptimizer.cpp index 9c6a4e361..2ced97430 100644 --- a/test/SolidityOptimizer.cpp +++ b/test/SolidityOptimizer.cpp @@ -74,6 +74,14 @@ public: "\nOptimized: " + toHex(optimizedOutput)); } + void checkCSE(AssemblyItems const& _input, AssemblyItems const& _expectation) + { + eth::CommonSubexpressionEliminator cse; + BOOST_REQUIRE(cse.feedItems(_input.begin(), _input.end()) == _input.end()); + AssemblyItems output = cse.getOptimizedItems(); + BOOST_CHECK_EQUAL_COLLECTIONS(_expectation.begin(), _expectation.end(), output.begin(), output.end()); + } + protected: Address m_optimizedContract; Address m_nonOptimizedContract; @@ -199,61 +207,59 @@ BOOST_AUTO_TEST_CASE(cse_intermediate_swap) BOOST_AUTO_TEST_CASE(cse_negative_stack_access) { - eth::CommonSubexpressionEliminator cse; - AssemblyItems input{AssemblyItem(Instruction::DUP2), AssemblyItem(u256(0))}; - BOOST_REQUIRE(cse.feedItems(input.begin(), input.end()) == input.end()); - AssemblyItems output = cse.getOptimizedItems(); - BOOST_CHECK_EQUAL_COLLECTIONS(input.begin(), input.end(), output.begin(), output.end()); + AssemblyItems input{Instruction::DUP2, u256(0)}; + checkCSE(input, input); } BOOST_AUTO_TEST_CASE(cse_negative_stack_end) { - eth::CommonSubexpressionEliminator cse; - AssemblyItems input{ - AssemblyItem(Instruction::ADD) - }; - BOOST_REQUIRE(cse.feedItems(input.begin(), input.end()) == input.end()); - AssemblyItems output = cse.getOptimizedItems(); - BOOST_CHECK_EQUAL_COLLECTIONS(input.begin(), input.end(), output.begin(), output.end()); + AssemblyItems input{Instruction::ADD}; + checkCSE(input, input); } BOOST_AUTO_TEST_CASE(cse_intermediate_negative_stack) { - eth::CommonSubexpressionEliminator cse; - AssemblyItems input{ - AssemblyItem(Instruction::ADD), - AssemblyItem(u256(1)), - AssemblyItem(Instruction::DUP2) - }; - BOOST_REQUIRE(cse.feedItems(input.begin(), input.end()) == input.end()); - AssemblyItems output = cse.getOptimizedItems(); - BOOST_CHECK_EQUAL_COLLECTIONS(input.begin(), input.end(), output.begin(), output.end()); + AssemblyItems input{Instruction::ADD, u256(1), Instruction::DUP1}; + checkCSE(input, input); } BOOST_AUTO_TEST_CASE(cse_pop) { - eth::CommonSubexpressionEliminator cse; + checkCSE({Instruction::POP}, {Instruction::POP}); +} + +BOOST_AUTO_TEST_CASE(cse_unneeded_items) +{ AssemblyItems input{ - AssemblyItem(Instruction::POP) + Instruction::ADD, + Instruction::SWAP1, + Instruction::POP, + u256(7), + u256(8), }; - BOOST_REQUIRE(cse.feedItems(input.begin(), input.end()) == input.end()); - AssemblyItems output = cse.getOptimizedItems(); - BOOST_CHECK_EQUAL_COLLECTIONS(input.begin(), input.end(), output.begin(), output.end()); + checkCSE(input, input); } -BOOST_AUTO_TEST_CASE(cse_unneeded_items) +BOOST_AUTO_TEST_CASE(cse_invariants) { - eth::CommonSubexpressionEliminator cse; AssemblyItems input{ - AssemblyItem(Instruction::ADD), - AssemblyItem(Instruction::SWAP1), - AssemblyItem(Instruction::POP), - AssemblyItem(u256(7)), - AssemblyItem(u256(8)), + Instruction::DUP1, + Instruction::DUP1, + u256(0), + Instruction::OR, + Instruction::OR }; - BOOST_REQUIRE(cse.feedItems(input.begin(), input.end()) == input.end()); - AssemblyItems output = cse.getOptimizedItems(); - BOOST_CHECK_EQUAL_COLLECTIONS(input.begin(), input.end(), output.begin(), output.end()); + checkCSE(input, {Instruction::DUP1}); +} + +BOOST_AUTO_TEST_CASE(cse_subself) +{ + checkCSE({Instruction::DUP1, Instruction::SUB}, {Instruction::POP, u256(0)}); +} + +BOOST_AUTO_TEST_CASE(cse_subother) +{ + checkCSE({Instruction::SUB}, {Instruction::SUB}); } BOOST_AUTO_TEST_SUITE_END() From 442a34c9b0e079bf76aa7bb7bdce2515b0688ca4 Mon Sep 17 00:00:00 2001 From: chriseth Date: Sat, 21 Mar 2015 12:34:59 +0100 Subject: [PATCH 15/23] Remove stack sequence id. --- libevmcore/CommonSubexpressionEliminator.cpp | 63 +++++++------------- libevmcore/CommonSubexpressionEliminator.h | 12 ++-- 2 files changed, 28 insertions(+), 47 deletions(-) diff --git a/libevmcore/CommonSubexpressionEliminator.cpp b/libevmcore/CommonSubexpressionEliminator.cpp index 65aac74f1..47251e5c8 100644 --- a/libevmcore/CommonSubexpressionEliminator.cpp +++ b/libevmcore/CommonSubexpressionEliminator.cpp @@ -36,16 +36,11 @@ vector CommonSubexpressionEliminator::getOptimizedItems() map targetStackContents; int minHeight = m_stackHeight + 1; if (!m_stackElements.empty()) - minHeight = min(minHeight, m_stackElements.begin()->first.first); - for (int height = minHeight; height <= max(0, m_stackHeight); ++height) - { - // make sure it is created - ExpressionClasses::Id c = getStackElement(height); - if (height <= 0) - initialStackContents[height] = m_expressionClasses.find(AssemblyItem(dupInstruction(1 - height))); - if (height <= m_stackHeight) - targetStackContents[height] = c; - } + minHeight = min(minHeight, m_stackElements.begin()->first); + for (int height = minHeight; height <= 0; ++height) + initialStackContents[height] = initialStackElement(height); + for (int height = minHeight; height <= m_stackHeight; ++height) + targetStackContents[height] = stackElement(height); // Debug info: //stream(cout, currentStackContents, targetStackContents); @@ -74,7 +69,7 @@ ostream& CommonSubexpressionEliminator::stream( _out << "Stack elements: " << endl; for (auto const& it: m_stackElements) { - _out << " " << dec << it.first.first << "(" << it.first.second << ") = "; + _out << " " << dec << it.first << " = "; streamExpressionClass(_out, it.second); } _out << "Equivalence classes: " << endl; @@ -112,7 +107,7 @@ void CommonSubexpressionEliminator::feedItem(AssemblyItem const& _item) if (SemanticInformation::isDupInstruction(_item)) setStackElement( m_stackHeight + 1, - getStackElement(m_stackHeight - int(instruction) + int(Instruction::DUP1)) + stackElement(m_stackHeight - int(instruction) + int(Instruction::DUP1)) ); else if (SemanticInformation::isSwapInstruction(_item)) swapStackElements( @@ -123,7 +118,7 @@ void CommonSubexpressionEliminator::feedItem(AssemblyItem const& _item) { vector arguments(info.args); for (int i = 0; i < info.args; ++i) - arguments[i] = getStackElement(m_stackHeight - i); + arguments[i] = stackElement(m_stackHeight - i); setStackElement(m_stackHeight + _item.deposit(), m_expressionClasses.find(_item, arguments)); } m_stackHeight += _item.deposit(); @@ -132,48 +127,34 @@ void CommonSubexpressionEliminator::feedItem(AssemblyItem const& _item) void CommonSubexpressionEliminator::setStackElement(int _stackHeight, ExpressionClasses::Id _class) { - unsigned nextSequence = getNextStackElementSequence(_stackHeight); - m_stackElements[make_pair(_stackHeight, nextSequence)] = _class; + m_stackElements[_stackHeight] = _class; } void CommonSubexpressionEliminator::swapStackElements(int _stackHeightA, int _stackHeightB) { if (_stackHeightA == _stackHeightB) BOOST_THROW_EXCEPTION(OptimizerException() << errinfo_comment("Swap on same stack elements.")); - ExpressionClasses::Id classA = getStackElement(_stackHeightA); - ExpressionClasses::Id classB = getStackElement(_stackHeightB); + // ensure they are created + stackElement(_stackHeightA); + stackElement(_stackHeightB); - unsigned nextSequenceA = getNextStackElementSequence(_stackHeightA); - unsigned nextSequenceB = getNextStackElementSequence(_stackHeightB); - m_stackElements[make_pair(_stackHeightA, nextSequenceA)] = classB; - m_stackElements[make_pair(_stackHeightB, nextSequenceB)] = classA; + swap(m_stackElements[_stackHeightA], m_stackElements[_stackHeightB]); } -ExpressionClasses::Id CommonSubexpressionEliminator::getStackElement(int _stackHeight) +ExpressionClasses::Id CommonSubexpressionEliminator::stackElement(int _stackHeight) { - // retrieve class by last sequence number - unsigned nextSequence = getNextStackElementSequence(_stackHeight); - if (nextSequence > 0) - return m_stackElements[make_pair(_stackHeight, nextSequence - 1)]; - + if (m_stackElements.count(_stackHeight)) + return m_stackElements.at(_stackHeight); // Stack element not found (not assigned yet), create new equivalence class. - assertThrow(_stackHeight <= 0, OptimizerException, "Stack element accessed before assignment."); - assertThrow(_stackHeight > -16, StackTooDeepException, ""); - // This is a special assembly item that refers to elements pre-existing on the initial stack. - return m_stackElements[make_pair(_stackHeight, nextSequence)] = - m_expressionClasses.find(AssemblyItem(dupInstruction(1 - _stackHeight))); + return m_stackElements[_stackHeight] = initialStackElement(_stackHeight); } -unsigned CommonSubexpressionEliminator::getNextStackElementSequence(int _stackHeight) +ExpressionClasses::Id CommonSubexpressionEliminator::initialStackElement(int _stackHeight) { - auto it = m_stackElements.upper_bound(make_pair(_stackHeight, unsigned(-1))); - if (it == m_stackElements.begin()) - return 0; - --it; - if (it->first.first == _stackHeight) - return it->first.second + 1; - else - return 0; + assertThrow(_stackHeight <= 0, OptimizerException, "Initial stack element of positive height requested."); + assertThrow(_stackHeight > -16, StackTooDeepException, ""); + // This is a special assembly item that refers to elements pre-existing on the initial stack. + return m_expressionClasses.find(AssemblyItem(dupInstruction(1 - _stackHeight))); } bool SemanticInformation::breaksBasicBlock(AssemblyItem const& _item) diff --git a/libevmcore/CommonSubexpressionEliminator.h b/libevmcore/CommonSubexpressionEliminator.h index 2a49d888d..331a7642d 100644 --- a/libevmcore/CommonSubexpressionEliminator.h +++ b/libevmcore/CommonSubexpressionEliminator.h @@ -80,15 +80,15 @@ private: void swapStackElements(int _stackHeightA, int _stackHeightB); /// Retrieves the current equivalence class fo the given stack element (or generates a new /// one if it does not exist yet). - ExpressionClasses::Id getStackElement(int _stackHeight); - - /// @returns the next sequence number of the given stack element. - unsigned getNextStackElementSequence(int _stackHeight); + ExpressionClasses::Id stackElement(int _stackHeight); + /// @returns the equivalence class id of the special initial stack element at the given height + /// (must not be positive). + ExpressionClasses::Id initialStackElement(int _stackHeight); /// Current stack height, can be negative. int m_stackHeight = 0; - /// Mapping (stack height, sequence number) -> equivalence class - std::map, ExpressionClasses::Id> m_stackElements; + /// Current stack layout, mapping stack height -> equivalence class + std::map m_stackElements; /// Structure containing the classes of equivalent expressions. ExpressionClasses m_expressionClasses; }; From 49712025fd85045c57a69cd2800a86f01520487e Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 23 Mar 2015 13:09:25 +0100 Subject: [PATCH 16/23] Pattern matching for expression simplification. --- libevmcore/Assembly.cpp | 145 +------ libevmcore/Assembly.h | 55 +-- libevmcore/AssemblyItem.cpp | 135 ++++++ libevmcore/AssemblyItem.h | 92 ++++ libevmcore/CommonSubexpressionEliminator.cpp | 2 +- libevmcore/ExpressionClasses.cpp | 418 ++++++++++++------- libevmcore/ExpressionClasses.h | 77 +++- test/SolidityOptimizer.cpp | 41 ++ 8 files changed, 635 insertions(+), 330 deletions(-) create mode 100644 libevmcore/AssemblyItem.cpp create mode 100644 libevmcore/AssemblyItem.h diff --git a/libevmcore/Assembly.cpp b/libevmcore/Assembly.cpp index bd504dc22..abe932282 100644 --- a/libevmcore/Assembly.cpp +++ b/libevmcore/Assembly.cpp @@ -28,122 +28,6 @@ using namespace std; using namespace dev; using namespace dev::eth; -unsigned AssemblyItem::bytesRequired(unsigned _addressLength) const -{ - switch (m_type) - { - case Operation: - case Tag: // 1 byte for the JUMPDEST - return 1; - case PushString: - return 33; - case Push: - return 1 + max(1, dev::bytesRequired(m_data)); - case PushSubSize: - case PushProgramSize: - return 4; // worst case: a 16MB program - case PushTag: - case PushData: - case PushSub: - return 1 + _addressLength; - default: - break; - } - BOOST_THROW_EXCEPTION(InvalidOpcode()); -} - -int AssemblyItem::deposit() const -{ - switch (m_type) - { - case Operation: - return instructionInfo(instruction()).ret - instructionInfo(instruction()).args; - case Push: - case PushString: - case PushTag: - case PushData: - case PushSub: - case PushSubSize: - case PushProgramSize: - return 1; - case Tag: - return 0; - default:; - } - return 0; -} - -string AssemblyItem::getJumpTypeAsString() const -{ - switch (m_jumpType) - { - case JumpType::IntoFunction: - return "[in]"; - case JumpType::OutOfFunction: - return "[out]"; - case JumpType::Ordinary: - default: - return ""; - } -} - -ostream& dev::eth::operator<<(ostream& _out, AssemblyItem const& _item) -{ - switch (_item.type()) - { - case Operation: - _out << " " << instructionInfo(_item.instruction()).name; - if (_item.instruction() == eth::Instruction::JUMP || _item.instruction() == eth::Instruction::JUMPI) - _out << "\t" << _item.getJumpTypeAsString(); - break; - case Push: - _out << " PUSH " << hex << _item.data(); - break; - case PushString: - _out << " PushString" << hex << (unsigned)_item.data(); - break; - case PushTag: - _out << " PushTag " << _item.data(); - break; - case Tag: - _out << " Tag " << _item.data(); - break; - case PushData: - _out << " PushData " << hex << (unsigned)_item.data(); - break; - case PushSub: - _out << " PushSub " << hex << h256(_item.data()).abridged(); - break; - case PushSubSize: - _out << " PushSubSize " << hex << h256(_item.data()).abridged(); - break; - case PushProgramSize: - _out << " PushProgramSize"; - break; - case UndefinedItem: - _out << " ???"; - break; - default: - BOOST_THROW_EXCEPTION(InvalidOpcode()); - } - return _out; -} - -unsigned Assembly::bytesRequired() const -{ - for (unsigned br = 1;; ++br) - { - unsigned ret = 1; - for (auto const& i: m_data) - ret += i.second.size(); - - for (AssemblyItem const& i: m_items) - ret += i.bytesRequired(br); - if (dev::bytesRequired(ret) <= br) - return ret; - } -} - void Assembly::append(Assembly const& _a) { auto newDeposit = m_deposit + _a.deposit(); @@ -180,11 +64,19 @@ void Assembly::append(Assembly const& _a, int _deposit) } } -ostream& dev::eth::operator<<(ostream& _out, AssemblyItemsConstRef _i) +unsigned Assembly::bytesRequired() const { - for (AssemblyItem const& i: _i) - _out << i; - return _out; + for (unsigned br = 1;; ++br) + { + unsigned ret = 1; + for (auto const& i: m_data) + ret += i.second.size(); + + for (AssemblyItem const& i: m_items) + ret += i.bytesRequired(br); + if (dev::bytesRequired(ret) <= br) + return ret; + } } string Assembly::getLocationFromSources(StringMap const& _sourceCodes, SourceLocation const& _location) const @@ -295,14 +187,6 @@ Assembly& Assembly::optimise(bool _enable) { if (!_enable) return *this; - map> const c_associative = - { - { Instruction::ADD, [](u256 a, u256 b)->u256{return a + b;} }, - { Instruction::MUL, [](u256 a, u256 b)->u256{return a * b;} }, - { Instruction::AND, [](u256 a, u256 b)->u256{return a & b;} }, - { Instruction::OR, [](u256 a, u256 b)->u256{return a | b;} }, - { Instruction::XOR, [](u256 a, u256 b)->u256{return a ^ b;} }, - }; std::vector>> rules = { { { Push, Instruction::POP }, [](AssemblyItemsConstRef) -> AssemblyItems { return {}; } }, @@ -315,11 +199,6 @@ Assembly& Assembly::optimise(bool _enable) { { Instruction::ISZERO, Instruction::ISZERO }, [](AssemblyItemsConstRef) -> AssemblyItems { return {}; } }, }; - for (auto const& i: c_associative) - { - rules.push_back({ { Push, Push, i.first }, [&](AssemblyItemsConstRef m) -> AssemblyItems { return { i.second(m[1].data(), m[0].data()) }; } }); - rules.push_back({ { Push, i.first, Push, i.first }, [&](AssemblyItemsConstRef m) -> AssemblyItems { return { i.second(m[2].data(), m[0].data()), i.first }; } }); - } // jump to next instruction rules.push_back({ { PushTag, Instruction::JUMP, Tag }, [](AssemblyItemsConstRef m) -> AssemblyItems { if (m[0].m_data == m[2].m_data) return {m[2]}; else return m.toVector(); }}); diff --git a/libevmcore/Assembly.h b/libevmcore/Assembly.h index 280a2e81c..315e6aaed 100644 --- a/libevmcore/Assembly.h +++ b/libevmcore/Assembly.h @@ -27,6 +27,7 @@ #include #include #include +#include #include "Exceptions.h" namespace dev @@ -34,60 +35,6 @@ namespace dev namespace eth { -enum AssemblyItemType { UndefinedItem, Operation, Push, PushString, PushTag, PushSub, PushSubSize, PushProgramSize, Tag, PushData }; - -class Assembly; - -class AssemblyItem -{ - friend class Assembly; - -public: - enum class JumpType { Ordinary, IntoFunction, OutOfFunction }; - - AssemblyItem(u256 _push): m_type(Push), m_data(_push) {} - AssemblyItem(Instruction _i): m_type(Operation), m_data((byte)_i) {} - AssemblyItem(AssemblyItemType _type, u256 _data = 0): m_type(_type), m_data(_data) {} - - AssemblyItem tag() const { assertThrow(m_type == PushTag || m_type == Tag, Exception, ""); return AssemblyItem(Tag, m_data); } - AssemblyItem pushTag() const { assertThrow(m_type == PushTag || m_type == Tag, Exception, ""); return AssemblyItem(PushTag, m_data); } - - AssemblyItemType type() const { return m_type; } - u256 const& data() const { return m_data; } - /// @returns the instruction of this item (only valid if type() == Operation) - Instruction instruction() const { return Instruction(byte(m_data)); } - - /// @returns true iff the type and data of the items are equal. - bool operator==(AssemblyItem const& _other) const { return m_type == _other.m_type && m_data == _other.m_data; } - bool operator!=(AssemblyItem const& _other) const { return !operator==(_other); } - - /// @returns an upper bound for the number of bytes required by this item, assuming that - /// the value of a jump tag takes @a _addressLength bytes. - unsigned bytesRequired(unsigned _addressLength) const; - int deposit() const; - - bool match(AssemblyItem const& _i) const { return _i.m_type == UndefinedItem || (m_type == _i.m_type && (m_type != Operation || m_data == _i.m_data)); } - void setLocation(SourceLocation const& _location) { m_location = _location; } - SourceLocation const& getLocation() const { return m_location; } - - void setJumpType(JumpType _jumpType) { m_jumpType = _jumpType; } - JumpType getJumpType() const { return m_jumpType; } - std::string getJumpTypeAsString() const; - -private: - AssemblyItemType m_type; - u256 m_data; - SourceLocation m_location; - JumpType m_jumpType = JumpType::Ordinary; -}; - -using AssemblyItems = std::vector; -using AssemblyItemsConstRef = vector_ref; - -std::ostream& operator<<(std::ostream& _out, AssemblyItem const& _item); -std::ostream& operator<<(std::ostream& _out, AssemblyItemsConstRef _i); -inline std::ostream& operator<<(std::ostream& _out, AssemblyItems const& _i) { return operator<<(_out, AssemblyItemsConstRef(&_i)); } - class Assembly { public: diff --git a/libevmcore/AssemblyItem.cpp b/libevmcore/AssemblyItem.cpp new file mode 100644 index 000000000..a4485a144 --- /dev/null +++ b/libevmcore/AssemblyItem.cpp @@ -0,0 +1,135 @@ +/* + This file is part of cpp-ethereum. + + cpp-ethereum is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + cpp-ethereum is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with cpp-ethereum. If not, see . +*/ +/** @file Assembly.cpp + * @author Gav Wood + * @date 2014 + */ + +#include "AssemblyItem.h" +#include + +using namespace std; +using namespace dev; +using namespace dev::eth; + +unsigned AssemblyItem::bytesRequired(unsigned _addressLength) const +{ + switch (m_type) + { + case Operation: + case Tag: // 1 byte for the JUMPDEST + return 1; + case PushString: + return 33; + case Push: + return 1 + max(1, dev::bytesRequired(m_data)); + case PushSubSize: + case PushProgramSize: + return 4; // worst case: a 16MB program + case PushTag: + case PushData: + case PushSub: + return 1 + _addressLength; + default: + break; + } + BOOST_THROW_EXCEPTION(InvalidOpcode()); +} + +int AssemblyItem::deposit() const +{ + switch (m_type) + { + case Operation: + return instructionInfo(instruction()).ret - instructionInfo(instruction()).args; + case Push: + case PushString: + case PushTag: + case PushData: + case PushSub: + case PushSubSize: + case PushProgramSize: + return 1; + case Tag: + return 0; + default:; + } + return 0; +} + +string AssemblyItem::getJumpTypeAsString() const +{ + switch (m_jumpType) + { + case JumpType::IntoFunction: + return "[in]"; + case JumpType::OutOfFunction: + return "[out]"; + case JumpType::Ordinary: + default: + return ""; + } +} + +ostream& dev::eth::operator<<(ostream& _out, AssemblyItem const& _item) +{ + switch (_item.type()) + { + case Operation: + _out << " " << instructionInfo(_item.instruction()).name; + if (_item.instruction() == eth::Instruction::JUMP || _item.instruction() == eth::Instruction::JUMPI) + _out << "\t" << _item.getJumpTypeAsString(); + break; + case Push: + _out << " PUSH " << hex << _item.data(); + break; + case PushString: + _out << " PushString" << hex << (unsigned)_item.data(); + break; + case PushTag: + _out << " PushTag " << _item.data(); + break; + case Tag: + _out << " Tag " << _item.data(); + break; + case PushData: + _out << " PushData " << hex << (unsigned)_item.data(); + break; + case PushSub: + _out << " PushSub " << hex << h256(_item.data()).abridged(); + break; + case PushSubSize: + _out << " PushSubSize " << hex << h256(_item.data()).abridged(); + break; + case PushProgramSize: + _out << " PushProgramSize"; + break; + case UndefinedItem: + _out << " ???"; + break; + default: + BOOST_THROW_EXCEPTION(InvalidOpcode()); + } + return _out; +} + +ostream& dev::eth::operator<<(ostream& _out, AssemblyItemsConstRef _i) +{ + for (AssemblyItem const& i: _i) + _out << i; + return _out; +} diff --git a/libevmcore/AssemblyItem.h b/libevmcore/AssemblyItem.h new file mode 100644 index 000000000..e7beced39 --- /dev/null +++ b/libevmcore/AssemblyItem.h @@ -0,0 +1,92 @@ +/* + This file is part of cpp-ethereum. + + cpp-ethereum is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + cpp-ethereum is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with cpp-ethereum. If not, see . +*/ +/** @file Assembly.h + * @author Gav Wood + * @date 2014 + */ + +#pragma once + +#include +#include +#include +#include +#include +#include +#include "Exceptions.h" + +namespace dev +{ +namespace eth +{ + +enum AssemblyItemType { UndefinedItem, Operation, Push, PushString, PushTag, PushSub, PushSubSize, PushProgramSize, Tag, PushData }; + +class Assembly; + +class AssemblyItem +{ + friend class Assembly; + +public: + enum class JumpType { Ordinary, IntoFunction, OutOfFunction }; + + AssemblyItem(u256 _push): m_type(Push), m_data(_push) {} + AssemblyItem(Instruction _i): m_type(Operation), m_data((byte)_i) {} + AssemblyItem(AssemblyItemType _type, u256 _data = 0): m_type(_type), m_data(_data) {} + + AssemblyItem tag() const { assertThrow(m_type == PushTag || m_type == Tag, Exception, ""); return AssemblyItem(Tag, m_data); } + AssemblyItem pushTag() const { assertThrow(m_type == PushTag || m_type == Tag, Exception, ""); return AssemblyItem(PushTag, m_data); } + + AssemblyItemType type() const { return m_type; } + u256 const& data() const { return m_data; } + /// @returns the instruction of this item (only valid if type() == Operation) + Instruction instruction() const { return Instruction(byte(m_data)); } + + /// @returns true iff the type and data of the items are equal. + bool operator==(AssemblyItem const& _other) const { return m_type == _other.m_type && m_data == _other.m_data; } + bool operator!=(AssemblyItem const& _other) const { return !operator==(_other); } + + /// @returns an upper bound for the number of bytes required by this item, assuming that + /// the value of a jump tag takes @a _addressLength bytes. + unsigned bytesRequired(unsigned _addressLength) const; + int deposit() const; + + bool match(AssemblyItem const& _i) const { return _i.m_type == UndefinedItem || (m_type == _i.m_type && (m_type != Operation || m_data == _i.m_data)); } + void setLocation(SourceLocation const& _location) { m_location = _location; } + SourceLocation const& getLocation() const { return m_location; } + + void setJumpType(JumpType _jumpType) { m_jumpType = _jumpType; } + JumpType getJumpType() const { return m_jumpType; } + std::string getJumpTypeAsString() const; + +private: + AssemblyItemType m_type; + u256 m_data; + SourceLocation m_location; + JumpType m_jumpType = JumpType::Ordinary; +}; + +using AssemblyItems = std::vector; +using AssemblyItemsConstRef = vector_ref; + +std::ostream& operator<<(std::ostream& _out, AssemblyItem const& _item); +std::ostream& operator<<(std::ostream& _out, AssemblyItemsConstRef _i); +inline std::ostream& operator<<(std::ostream& _out, AssemblyItems const& _i) { return operator<<(_out, AssemblyItemsConstRef(&_i)); } + +} +} diff --git a/libevmcore/CommonSubexpressionEliminator.cpp b/libevmcore/CommonSubexpressionEliminator.cpp index 47251e5c8..7fed03b4e 100644 --- a/libevmcore/CommonSubexpressionEliminator.cpp +++ b/libevmcore/CommonSubexpressionEliminator.cpp @@ -43,7 +43,7 @@ vector CommonSubexpressionEliminator::getOptimizedItems() targetStackContents[height] = stackElement(height); // Debug info: - //stream(cout, currentStackContents, targetStackContents); + //stream(cout, initialStackContents, targetStackContents); return CSECodeGenerator(m_expressionClasses).generateCode(initialStackContents, targetStackContents); } diff --git a/libevmcore/ExpressionClasses.cpp b/libevmcore/ExpressionClasses.cpp index eebc98f66..49fd72c1b 100644 --- a/libevmcore/ExpressionClasses.cpp +++ b/libevmcore/ExpressionClasses.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -45,6 +46,7 @@ bool ExpressionClasses::Expression::operator<(const ExpressionClasses::Expressio ExpressionClasses::Id ExpressionClasses::find(AssemblyItem const& _item, Ids const& _arguments) { Expression exp; + exp.id = Id(-1); exp.item = &_item; exp.arguments = _arguments; @@ -72,158 +74,168 @@ ExpressionClasses::Id ExpressionClasses::find(AssemblyItem const& _item, Ids con return exp.id; } -ExpressionClasses::Id ExpressionClasses::tryToSimplify(Expression const& _expr, bool _secondRun) +string ExpressionClasses::fullDAGToString(ExpressionClasses::Id _id) { - if (_expr.item->type() != Operation) - return -1; + Expression const& expr = representative(_id); + stringstream str; + str << dec << expr.id << ":" << *expr.item << "("; + for (Id arg: expr.arguments) + str << fullDAGToString(arg) << ","; + str << ")"; + return str.str(); +} - // @todo: - // ISZERO ISZERO - // associative operations (as done in Assembly.cpp) - // 2 * x == x + x - - Id arg1; - Id arg2; - Id arg3; - u256 data1; - u256 data2; - u256 data3; - switch (_expr.arguments.size()) - { - default: - arg3 = _expr.arguments.at(2); - data3 = representative(arg3).item->data(); - case 2: - arg2 = _expr.arguments.at(1); - data2 = representative(arg2).item->data(); - case 1: - arg1 = _expr.arguments.at(0); - data1 = representative(arg1).item->data(); - case 0: - break; - } +class Rules: public boost::noncopyable +{ +public: + Rules(); + void resetMatchGroups() { m_matchGroups.clear(); } + vector>> rules() const { return m_rules; } - /** - * Simplification rule. If _strict is false, Push or a constant matches any constant, - * otherwise Push matches "0" and a constant matches itself. - * "UndefinedItem" matches any expression, but all of them must be equal inside one rule. - */ - struct Rule - { - Rule(AssemblyItems const& _pattern, bool _strict, function const& _action): - pattern(_pattern), - assemblyItemAction(_action), - strict(_strict) - {} - Rule(AssemblyItems const& _pattern, function _action): - Rule(_pattern, false, _action) - {} - Rule(AssemblyItems const& _pattern, bool _strict, function const& _action): - pattern(_pattern), - idAction(_action), - strict(_strict) - {} - Rule(AssemblyItems const& _pattern, function _action): - Rule(_pattern, false, _action) - {} - bool matches(ExpressionClasses const& _classes, Expression const& _expr) const - { - if (!_expr.item->match(pattern.front())) - return false; - assertThrow(_expr.arguments.size() == pattern.size() - 1, OptimizerException, ""); - Id argRequiredToBeEqual(-1); - for (size_t i = 1; i < pattern.size(); ++i) - { - Id arg = _expr.arguments[i - 1]; - if (pattern[i].type() == UndefinedItem) - { - if (argRequiredToBeEqual == Id(-1)) - argRequiredToBeEqual = arg; - else if (argRequiredToBeEqual != arg) - return false; - } - else - { - AssemblyItem const& argItem = *_classes.representative(arg).item; - if (strict && argItem != pattern[i]) - return false; - else if (!strict && !argItem.match(pattern[i])) - return false; - } - } - return true; - } +private: + using Expression = ExpressionClasses::Expression; + map m_matchGroups; + vector>> m_rules; +}; - AssemblyItems pattern; - function assemblyItemAction; - function idAction; - bool strict; - }; +Rules::Rules() +{ + // Multiple occurences of one of these inside one rule must match the same equivalence class. + // Constants. + Pattern A(Push); + Pattern B(Push); + Pattern C(Push); + // Anything. + Pattern X; + Pattern Y; + Pattern Z; + A.setMatchGroup(1, m_matchGroups); + B.setMatchGroup(2, m_matchGroups); + C.setMatchGroup(3, m_matchGroups); + X.setMatchGroup(4, m_matchGroups); + Y.setMatchGroup(5, m_matchGroups); + Z.setMatchGroup(6, m_matchGroups); - vector c_singleLevel{ - // arithmetics on constants involving only stack variables - {{Instruction::ADD, Push, Push}, [&]{ return data1 + data2; }}, - {{Instruction::MUL, Push, Push}, [&]{ return data1 * data2; }}, - {{Instruction::SUB, Push, Push}, [&]{ return data1 - data2; }}, - {{Instruction::DIV, Push, Push}, [&]{ return data2 == 0 ? 0 : data1 / data2; }}, - {{Instruction::SDIV, Push, Push}, [&]{ return data2 == 0 ? 0 : s2u(u2s(data1) / u2s(data2)); }}, - {{Instruction::MOD, Push, Push}, [&]{ return data2 == 0 ? 0 : data1 % data2; }}, - {{Instruction::SMOD, Push, Push}, [&]{ return data2 == 0 ? 0 : s2u(u2s(data1) % u2s(data2)); }}, - {{Instruction::EXP, Push, Push}, [&]{ return u256(boost::multiprecision::powm(bigint(data1), bigint(data2), bigint(1) << 256)); }}, - {{Instruction::NOT, Push}, [&]{ return ~data1; }}, - {{Instruction::LT, Push, Push}, [&]() -> u256 { return data1 < data2 ? 1 : 0; }}, - {{Instruction::GT, Push, Push}, [&]() -> u256 { return data1 > data2 ? 1 : 0; }}, - {{Instruction::SLT, Push, Push}, [&]() -> u256 { return u2s(data1) < u2s( data2) ? 1 : 0; }}, - {{Instruction::SGT, Push, Push}, [&]() -> u256 { return u2s(data1) > u2s( data2) ? 1 : 0; }}, - {{Instruction::EQ, Push, Push}, [&]() -> u256 { return data1 == data2 ? 1 : 0; }}, - {{Instruction::ISZERO, Push}, [&]() -> u256 { return data1 == 0 ? 1 : 0; }}, - {{Instruction::AND, Push, Push}, [&]{ return data1 & data2; }}, - {{Instruction::OR, Push, Push}, [&]{ return data1 | data2; }}, - {{Instruction::XOR, Push, Push}, [&]{ return data1 ^ data2; }}, - {{Instruction::BYTE, Push, Push}, [&]{ return data1 >= 32 ? 0 : (data2 >> unsigned(8 * (31 - data1))) & 0xff; }}, - {{Instruction::ADDMOD, Push, Push, Push}, [&]{ return data3 == 0 ? 0 : u256((bigint(data1) + bigint(data2)) % data3); }}, - {{Instruction::MULMOD, Push, Push, Push}, [&]{ return data3 == 0 ? 0 : u256((bigint(data1) * bigint(data2)) % data3); }}, - {{Instruction::MULMOD, Push, Push, Push}, [&]{ return data1 * data2; }}, - {{Instruction::SIGNEXTEND, Push, Push}, [&]{ - if (data1 >= 31) - return data2; - unsigned testBit = unsigned(data1) * 8 + 7; + m_rules = vector>>{ + // arithmetics on constants + {{Instruction::ADD, {A, B}}, [=]{ return A.d() + B.d(); }}, + {{Instruction::MUL, {A, B}}, [=]{ return A.d() * B.d(); }}, + {{Instruction::SUB, {A, B}}, [=]{ return A.d() - B.d(); }}, + {{Instruction::DIV, {A, B}}, [=]{ return B.d() == 0 ? 0 : A.d() / B.d(); }}, + {{Instruction::SDIV, {A, B}}, [=]{ return B.d() == 0 ? 0 : s2u(u2s(A.d()) / u2s(B.d())); }}, + {{Instruction::MOD, {A, B}}, [=]{ return B.d() == 0 ? 0 : A.d() % B.d(); }}, + {{Instruction::SMOD, {A, B}}, [=]{ return B.d() == 0 ? 0 : s2u(u2s(A.d()) % u2s(B.d())); }}, + {{Instruction::EXP, {A, B}}, [=]{ return u256(boost::multiprecision::powm(bigint(A.d()), bigint(B.d()), bigint(1) << 256)); }}, + {{Instruction::NOT, {A}}, [=]{ return ~A.d(); }}, + {{Instruction::LT, {A, B}}, [=]() { return A.d() < B.d() ? u256(1) : 0; }}, + {{Instruction::GT, {A, B}}, [=]() -> u256 { return A.d() > B.d() ? 1 : 0; }}, + {{Instruction::SLT, {A, B}}, [=]() -> u256 { return u2s(A.d()) < u2s(B.d()) ? 1 : 0; }}, + {{Instruction::SGT, {A, B}}, [=]() -> u256 { return u2s(A.d()) > u2s(B.d()) ? 1 : 0; }}, + {{Instruction::EQ, {A, B}}, [=]() -> u256 { return A.d() == B.d() ? 1 : 0; }}, + {{Instruction::ISZERO, {A}}, [=]() -> u256 { return A.d() == 0 ? 1 : 0; }}, + {{Instruction::AND, {A, B}}, [=]{ return A.d() & B.d(); }}, + {{Instruction::OR, {A, B}}, [=]{ return A.d() | B.d(); }}, + {{Instruction::XOR, {A, B}}, [=]{ return A.d() ^ B.d(); }}, + {{Instruction::BYTE, {A, B}}, [=]{ return A.d() >= 32 ? 0 : (B.d() >> unsigned(8 * (31 - A.d()))) & 0xff; }}, + {{Instruction::ADDMOD, {A, B, C}}, [=]{ return C.d() == 0 ? 0 : u256((bigint(A.d()) + bigint(B.d())) % C.d()); }}, + {{Instruction::MULMOD, {A, B, C}}, [=]{ return C.d() == 0 ? 0 : u256((bigint(A.d()) * bigint(B.d())) % C.d()); }}, + {{Instruction::MULMOD, {A, B, C}}, [=]{ return A.d() * B.d(); }}, + {{Instruction::SIGNEXTEND, {A, B}}, [=]() -> u256 { + if (A.d() >= 31) + return B.d(); + unsigned testBit = unsigned(A.d()) * 8 + 7; u256 mask = (u256(1) << testBit) - 1; - return u256(boost::multiprecision::bit_test(data2, testBit) ? data2 | ~mask : data2 & mask); + return u256(boost::multiprecision::bit_test(B.d(), testBit) ? B.d() | ~mask : B.d() & mask); }}, - {{Instruction::ADD, UndefinedItem, u256(0)}, true, [&]{ return arg1; }}, - {{Instruction::MUL, UndefinedItem, u256(1)}, true, [&]{ return arg1; }}, - {{Instruction::OR, UndefinedItem, u256(0)}, true, [&]{ return arg1; }}, - {{Instruction::XOR, UndefinedItem, u256(0)}, true, [&]{ return arg1; }}, - {{Instruction::AND, UndefinedItem, ~u256(0)}, true, [&]{ return arg1; }}, - {{Instruction::MUL, UndefinedItem, u256(0)}, true, [&]{ return u256(0); }}, - {{Instruction::DIV, UndefinedItem, u256(0)}, true, [&]{ return u256(0); }}, - {{Instruction::MOD, UndefinedItem, u256(0)}, true, [&]{ return u256(0); }}, - {{Instruction::MOD, u256(0), UndefinedItem}, true, [&]{ return u256(0); }}, - {{Instruction::AND, UndefinedItem, u256(0)}, true, [&]{ return u256(0); }}, - {{Instruction::OR, UndefinedItem, ~u256(0)}, true, [&]{ return ~u256(0); }}, - {{Instruction::AND, UndefinedItem, UndefinedItem}, true, [&]{ return arg1; }}, - {{Instruction::OR, UndefinedItem, UndefinedItem}, true, [&]{ return arg1; }}, - {{Instruction::SUB, UndefinedItem, UndefinedItem}, true, [&]{ return u256(0); }}, - {{Instruction::EQ, UndefinedItem, UndefinedItem}, true, [&]{ return u256(1); }}, - {{Instruction::LT, UndefinedItem, UndefinedItem}, true, [&]{ return u256(0); }}, - {{Instruction::SLT, UndefinedItem, UndefinedItem}, true, [&]{ return u256(0); }}, - {{Instruction::GT, UndefinedItem, UndefinedItem}, true, [&]{ return u256(0); }}, - {{Instruction::SGT, UndefinedItem, UndefinedItem}, true, [&]{ return u256(0); }}, - {{Instruction::MOD, UndefinedItem, UndefinedItem}, true, [&]{ return u256(0); }}, + + // invariants involving known constants + {{Instruction::ADD, {X, 0}}, [=]{ return X; }}, + {{Instruction::MUL, {X, 1}}, [=]{ return X; }}, + {{Instruction::DIV, {X, 1}}, [=]{ return X; }}, + {{Instruction::SDIV, {X, 1}}, [=]{ return X; }}, + {{Instruction::OR, {X, 0}}, [=]{ return X; }}, + {{Instruction::XOR, {X, 0}}, [=]{ return X; }}, + {{Instruction::AND, {X, ~u256(0)}}, [=]{ return X; }}, + {{Instruction::MUL, {X, 0}}, [=]{ return u256(0); }}, + {{Instruction::DIV, {X, 0}}, [=]{ return u256(0); }}, + {{Instruction::MOD, {X, 0}}, [=]{ return u256(0); }}, + {{Instruction::MOD, {0, X}}, [=]{ return u256(0); }}, + {{Instruction::AND, {X, 0}}, [=]{ return u256(0); }}, + {{Instruction::OR, {X, ~u256(0)}}, [=]{ return ~u256(0); }}, + // operations involving an expression and itself + {{Instruction::AND, {X, X}}, [=]{ return X; }}, + {{Instruction::OR, {X, X}}, [=]{ return X; }}, + {{Instruction::SUB, {X, X}}, [=]{ return u256(0); }}, + {{Instruction::EQ, {X, X}}, [=]{ return u256(1); }}, + {{Instruction::EQ, {X, X}}, [=]{ return u256(1); }}, + {{Instruction::LT, {X, X}}, [=]{ return u256(0); }}, + {{Instruction::SLT, {X, X}}, [=]{ return u256(0); }}, + {{Instruction::GT, {X, X}}, [=]{ return u256(0); }}, + {{Instruction::SGT, {X, X}}, [=]{ return u256(0); }}, + {{Instruction::MOD, {X, X}}, [=]{ return u256(0); }}, + + {{Instruction::NOT, {{Instruction::NOT, {X}}}}, [=]{ return X; }}, + }; + // Associative operations + for (auto const& opFun: vector>>{ + {Instruction::ADD, plus()}, + {Instruction::MUL, multiplies()}, + {Instruction::AND, bit_and()}, + {Instruction::OR, bit_or()}, + {Instruction::XOR, bit_xor()} + }) + { + auto op = opFun.first; + auto fun = opFun.second; + // Moving constants to the outside, order matters here! + // we need actions that return expressions (or patterns?) here, and we need also reversed rules + // (X+A)+B -> X+(A+B) + m_rules.push_back({ + {op, {{op, {X, A}}, B}}, + [=]() -> Pattern { return {op, {X, fun(A.d(), B.d())}}; } + }); + // X+(Y+A) -> (X+Y)+A + m_rules.push_back({ + {op, {{op, {X, A}}, Y}}, + [=]() -> Pattern { return {op, {{op, {X, Y}}, A}}; } + }); + // For now, we still need explicit commutativity for the inner pattern + m_rules.push_back({ + {op, {{op, {A, X}}, B}}, + [=]() -> Pattern { return {op, {X, fun(A.d(), B.d())}}; } + }); + m_rules.push_back({ + {op, {{op, {A, X}}, Y}}, + [=]() -> Pattern { return {op, {{op, {X, Y}}, A}}; } + }); }; - for (auto const& rule: c_singleLevel) - if (rule.matches(*this, _expr)) + //@todo: (x+8)-3 and other things +} + +ExpressionClasses::Id ExpressionClasses::tryToSimplify(Expression const& _expr, bool _secondRun) +{ + static Rules rules; + + if (_expr.item->type() != Operation) + return -1; + + for (auto const& rule: rules.rules()) + { + rules.resetMatchGroups(); + if (rule.first.matches(_expr, *this)) { - if (rule.idAction) - return rule.idAction(); - else - { - m_spareAssemblyItem.push_back(make_shared(rule.assemblyItemAction())); - return find(*m_spareAssemblyItem.back()); - } + // Debug info + //cout << "Simplifying " << *_expr.item << "("; + //for (Id arg: _expr.arguments) + // cout << fullDAGToString(arg) << ", "; + //cout << ")" << endl; + //cout << "with rule " << rule.first.toString() << endl; + //ExpressionTemplate t(rule.second()); + //cout << "to" << rule.second().toString() << endl; + return rebuildExpression(ExpressionTemplate(rule.second())); } + } if (!_secondRun && _expr.arguments.size() == 2 && SemanticInformation::isCommutativeOperation(*_expr.item)) { @@ -234,3 +246,127 @@ ExpressionClasses::Id ExpressionClasses::tryToSimplify(Expression const& _expr, return -1; } + +ExpressionClasses::Id ExpressionClasses::rebuildExpression(ExpressionTemplate const& _template) +{ + if (_template.hasId) + return _template.id; + + Ids arguments; + for (ExpressionTemplate const& t: _template.arguments) + arguments.push_back(rebuildExpression(t)); + m_spareAssemblyItem.push_back(make_shared(_template.item)); + return find(*m_spareAssemblyItem.back(), arguments); +} + + +Pattern::Pattern(Instruction _instruction, std::vector const& _arguments): + m_type(Operation), + m_requireDataMatch(true), + m_data(_instruction), + m_arguments(_arguments) +{ +} + +void Pattern::setMatchGroup(unsigned _group, map& _matchGroups) +{ + m_matchGroup = _group; + m_matchGroups = &_matchGroups; +} + +bool Pattern::matches(Expression const& _expr, ExpressionClasses const& _classes) const +{ + if (!matchesBaseItem(*_expr.item)) + return false; + if (m_matchGroup) + { + if (!m_matchGroups->count(m_matchGroup)) + (*m_matchGroups)[m_matchGroup] = &_expr; + else if ((*m_matchGroups)[m_matchGroup]->id != _expr.id) + return false; + } + assertThrow(m_arguments.size() == 0 || _expr.arguments.size() == m_arguments.size(), OptimizerException, ""); + for (size_t i = 0; i < m_arguments.size(); ++i) + if (!m_arguments[i].matches(_classes.representative(_expr.arguments[i]), _classes)) + return false; + return true; +} + +string Pattern::toString() const +{ + stringstream s; + switch (m_type) + { + case Operation: + s << instructionInfo(Instruction(unsigned(m_data))).name; + break; + case Push: + s << "PUSH " << hex << m_data; + break; + case UndefinedItem: + s << "ANY"; + break; + default: + s << "t=" << dec << m_type << " d=" << hex << m_data; + break; + } + if (!m_requireDataMatch) + s << " ~"; + if (m_matchGroup) + s << "[" << dec << m_matchGroup << "]"; + s << "("; + for (Pattern const& p: m_arguments) + s << p.toString() << ", "; + s << ")"; + return s.str(); +} + +bool Pattern::matchesBaseItem(AssemblyItem const& _item) const +{ + if (m_type == UndefinedItem) + return true; + if (m_type != _item.type()) + return false; + if (m_requireDataMatch && m_data != _item.data()) + return false; + return true; +} + +Pattern::Expression const& Pattern::matchGroupValue() const +{ + assertThrow(m_matchGroup > 0, OptimizerException, ""); + assertThrow(!!m_matchGroups, OptimizerException, ""); + assertThrow((*m_matchGroups)[m_matchGroup], OptimizerException, ""); + return *(*m_matchGroups)[m_matchGroup]; +} + + +ExpressionTemplate::ExpressionTemplate(Pattern const& _pattern) +{ + if (_pattern.matchGroup()) + { + hasId = true; + id = _pattern.id(); + } + else + { + hasId = false; + item = _pattern.toAssemblyItem(); + } + for (auto const& arg: _pattern.arguments()) + arguments.push_back(ExpressionTemplate(arg)); +} + +string ExpressionTemplate::toString() const +{ + stringstream s; + if (hasId) + s << id; + else + s << item; + s << "("; + for (auto const& arg: arguments) + s << arg.toString(); + s << ")"; + return s.str(); +} diff --git a/libevmcore/ExpressionClasses.h b/libevmcore/ExpressionClasses.h index 71fc96109..ecd9c9250 100644 --- a/libevmcore/ExpressionClasses.h +++ b/libevmcore/ExpressionClasses.h @@ -26,13 +26,16 @@ #include #include #include +#include +#include namespace dev { namespace eth { -class AssemblyItem; +class Pattern; +struct ExpressionTemplate; /** * Collection of classes of equivalent expressions that can also determine the class of an expression. @@ -60,16 +63,88 @@ public: /// @returns the number of classes. Id size() const { return m_representatives.size(); } + std::string fullDAGToString(Id _id); + private: /// Tries to simplify the given expression. /// @returns its class if it possible or Id(-1) otherwise. /// @param _secondRun is set to true for the second run where arguments of commutative expressions are reversed Id tryToSimplify(Expression const& _expr, bool _secondRun = false); + /// Rebuilds an expression from a (matched) pattern. + Id rebuildExpression(ExpressionTemplate const& _template); + + std::vector>> createRules() const; + /// Expression equivalence class representatives - we only store one item of an equivalence. std::vector m_representatives; std::vector> m_spareAssemblyItem; }; +/** + * Pattern to match against an expression. + * Also stores matched expressions to retrieve them later, for constructing new expressions using + * ExpressionTemplate. + */ +class Pattern +{ +public: + using Expression = ExpressionClasses::Expression; + using Id = ExpressionClasses::Id; + + // Matches a specific constant value. + Pattern(unsigned _value): Pattern(u256(_value)) {} + // Matches a specific constant value. + Pattern(u256 const& _value): m_type(Push), m_requireDataMatch(true), m_data(_value) {} + // Matches a specific assembly item type or anything if not given. + Pattern(AssemblyItemType _type = UndefinedItem): m_type(_type) {} + // Matches a given instruction with given arguments + Pattern(Instruction _instruction, std::vector const& _arguments = {}); + /// Sets this pattern to be part of the match group with the identifier @a _group. + /// Inside one rule, all patterns in the same match group have to match expressions from the + /// same expression equivalence class. + void setMatchGroup(unsigned _group, std::map& _matchGroups); + unsigned matchGroup() const { return m_matchGroup; } + bool matches(Expression const& _expr, ExpressionClasses const& _classes) const; + + AssemblyItem toAssemblyItem() const { return AssemblyItem(m_type, m_data); } + std::vector arguments() const { return m_arguments; } + + /// @returns the id of the matched expression if this pattern is part of a match group. + Id id() const { return matchGroupValue().id; } + /// @returns the data of the matched expression if this pattern is part of a match group. + u256 d() const { return matchGroupValue().item->data(); } + + std::string toString() const; + +private: + bool matchesBaseItem(AssemblyItem const& _item) const; + Expression const& matchGroupValue() const; + + AssemblyItemType m_type; + bool m_requireDataMatch = false; + u256 m_data = 0; + std::vector m_arguments; + unsigned m_matchGroup = 0; + std::map* m_matchGroups = nullptr; +}; + +/** + * Template for a new expression that can be built from matched patterns. + */ +struct ExpressionTemplate +{ + using Expression = ExpressionClasses::Expression; + using Id = ExpressionClasses::Id; + explicit ExpressionTemplate(Pattern const& _pattern); + std::string toString() const; + bool hasId = false; + /// Id of the matched expression, if available. + Id id = Id(-1); + // Otherwise, assembly item. + AssemblyItem item = UndefinedItem; + std::vector arguments; +}; + } } diff --git a/test/SolidityOptimizer.cpp b/test/SolidityOptimizer.cpp index 2ced97430..2d5cff7ac 100644 --- a/test/SolidityOptimizer.cpp +++ b/test/SolidityOptimizer.cpp @@ -240,6 +240,12 @@ BOOST_AUTO_TEST_CASE(cse_unneeded_items) checkCSE(input, input); } +BOOST_AUTO_TEST_CASE(cse_constant_addition) +{ + AssemblyItems input{u256(7), u256(8), Instruction::ADD}; + checkCSE(input, {u256(7 + 8)}); +} + BOOST_AUTO_TEST_CASE(cse_invariants) { AssemblyItems input{ @@ -262,6 +268,41 @@ BOOST_AUTO_TEST_CASE(cse_subother) checkCSE({Instruction::SUB}, {Instruction::SUB}); } +BOOST_AUTO_TEST_CASE(cse_double_negation) +{ + checkCSE({Instruction::DUP5, Instruction::NOT, Instruction::NOT}, {Instruction::DUP5}); +} + +BOOST_AUTO_TEST_CASE(cse_associativity) +{ + AssemblyItems input{ + Instruction::DUP1, + Instruction::DUP1, + u256(0), + Instruction::OR, + Instruction::OR + }; + checkCSE(input, {Instruction::DUP1}); +} + +BOOST_AUTO_TEST_CASE(cse_associativity2) +{ + AssemblyItems input{ + u256(0), + Instruction::DUP2, + u256(2), + u256(1), + Instruction::DUP6, + Instruction::ADD, + u256(2), + Instruction::ADD, + Instruction::ADD, + Instruction::ADD, + Instruction::ADD + }; + checkCSE(input, {Instruction::DUP2, Instruction::DUP2, Instruction::ADD, u256(5), Instruction::ADD}); +} + BOOST_AUTO_TEST_SUITE_END() } From 653eece9e140f1cfea554041dd290fdb46fb6339 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 27 Mar 2015 09:19:02 +0100 Subject: [PATCH 17/23] Windows build fix. --- abi/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/abi/main.cpp b/abi/main.cpp index 7a5fbc584..223ff8283 100644 --- a/abi/main.cpp +++ b/abi/main.cpp @@ -349,7 +349,7 @@ struct ABIMethod if (j == -1) { ret += aligned(_params[pi].first, ABIType(), Format::Decimal, 32); - arity *= fromBigEndian(_params[pi].first); + arity *= fromBigEndian(_params[pi].first); pi++; } else From c2392fd81af9fbf45e7b44034a955673db62a75e Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 27 Mar 2015 09:57:00 +0100 Subject: [PATCH 18/23] Gaa. Get them out of that namespace! --- abi/main.cpp | 55 ++++++++++++++++++++++++++++++++++++++++------- libethereum/ABI.h | 8 +++---- 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/abi/main.cpp b/abi/main.cpp index 223ff8283..22233f1f0 100644 --- a/abi/main.cpp +++ b/abi/main.cpp @@ -162,6 +162,8 @@ struct ABIType { if (base == Base::Fixed) size = ssize = 16; + else if (base == Base::Address || base == Base::Bytes) + size = 0; else size = 32; return; @@ -188,7 +190,7 @@ struct ABIType string ret; switch (base) { - case Base::Bytes: ret = "bytes" + toString(size); break; + case Base::Bytes: ret = "bytes" + (size > 0 ? toString(size) : ""); break; case Base::Address: ret = "address"; break; case Base::Int: ret = "int" + toString(size); break; case Base::Uint: ret = "uint" + toString(size); break; @@ -361,15 +363,30 @@ struct ABIMethod { for (unsigned j = 0; j < inArity[ii]; ++j) { - ret += aligned(_params[pi].first, i, _params[pi].second, (i.base == Base::Bytes && i.size == 1) ? 1 : 32); + if (i.base == Base::Bytes && !i.size) + { + ret += _params[pi].first; + while (ret.size() % 32 != 0) + ret.push_back(0); + } + else + ret += aligned(_params[pi].first, i, _params[pi].second, 32); ++pi; } ++ii; - while (ret.size() % 32 != 0) - ret.push_back(0); } return ret; } + string decode(bytes const& _data, int _index = -1) + { + stringstream out; + if (_index == -1) + out << "["; + (void)_data; + if (_index == -1) + out << "]"; + return out.str(); + } }; string canonSig(string const& _name, vector const& _args) @@ -517,10 +534,7 @@ int main(int argc, char** argv) } string abiData; - if (abiFile == "--") - for (int i = cin.get(); i != -1; i = cin.get()) - abiData.push_back((char)i); - else if (!abiFile.empty()) + if (abiFile.empty()) abiData = contentsString(abiFile); if (mode == Mode::Encode) @@ -546,6 +560,31 @@ int main(int argc, char** argv) } else if (mode == Mode::Decode) { + if (abiData.empty()) + { + cerr << "Please specify an ABI file." << endl; + exit(-1); + } + else + { + ABI abi(abiData); + ABIMethod m; + if (verbose) + cerr << "ABI:" << endl << abi; + try { + m = abi.method(method, args); + } + catch(...) + { + cerr << "Unknown method in ABI." << endl; + exit(-1); + } + string encoded; + for (int i = cin.get(); i != -1; i = cin.get()) + encoded.push_back((char)i); + cout << m.decode(fromHex(encoded)); + } + // TODO: read abi to determine output format. (void)encoding; (void)clearZeroes; diff --git a/libethereum/ABI.h b/libethereum/ABI.h index fabda4dc2..f5cab10b2 100644 --- a/libethereum/ABI.h +++ b/libethereum/ABI.h @@ -21,15 +21,15 @@ #pragma once +#include +#include +#include + namespace dev { namespace eth { -#include -#include -#include - template struct ABISerialiser {}; template struct ABISerialiser> { static bytes serialise(FixedHash const& _t) { static_assert(N <= 32, "Cannot serialise hash > 32 bytes."); static_assert(N > 0, "Cannot serialise zero-length hash."); return bytes(32 - N, 0) + _t.asBytes(); } }; template <> struct ABISerialiser { static bytes serialise(u256 const& _t) { return h256(_t).asBytes(); } }; From d3211d033dab8762e18d74342db6b32fbb4500fa Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 27 Mar 2015 09:57:41 +0100 Subject: [PATCH 19/23] And the rest. --- libethereum/ABI.h | 1 + 1 file changed, 1 insertion(+) diff --git a/libethereum/ABI.h b/libethereum/ABI.h index f5cab10b2..04c623ac5 100644 --- a/libethereum/ABI.h +++ b/libethereum/ABI.h @@ -24,6 +24,7 @@ #include #include #include +#include namespace dev { From 22195f9ca1a134b0e13d5a657697ed7433a0d9cf Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 27 Mar 2015 09:46:01 +0100 Subject: [PATCH 20/23] Fixed includes. --- libethereum/ABI.h | 9 +++++---- libnatspec/NatspecExpressionEvaluator.h | 1 - 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/libethereum/ABI.h b/libethereum/ABI.h index fabda4dc2..04c623ac5 100644 --- a/libethereum/ABI.h +++ b/libethereum/ABI.h @@ -21,15 +21,16 @@ #pragma once +#include +#include +#include +#include + namespace dev { namespace eth { -#include -#include -#include - template struct ABISerialiser {}; template struct ABISerialiser> { static bytes serialise(FixedHash const& _t) { static_assert(N <= 32, "Cannot serialise hash > 32 bytes."); static_assert(N > 0, "Cannot serialise zero-length hash."); return bytes(32 - N, 0) + _t.asBytes(); } }; template <> struct ABISerialiser { static bytes serialise(u256 const& _t) { return h256(_t).asBytes(); } }; diff --git a/libnatspec/NatspecExpressionEvaluator.h b/libnatspec/NatspecExpressionEvaluator.h index 4aca090d6..f7984f6e8 100644 --- a/libnatspec/NatspecExpressionEvaluator.h +++ b/libnatspec/NatspecExpressionEvaluator.h @@ -21,7 +21,6 @@ #pragma once -#include #include #include From c941c3d9c4f7108087f2ac22545093e113a9482e Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 27 Mar 2015 13:17:52 +0100 Subject: [PATCH 21/23] Small review fixes. --- libevmcore/ExpressionClasses.cpp | 4 ++-- libevmcore/ExpressionClasses.h | 2 +- libevmcore/Instruction.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libevmcore/ExpressionClasses.cpp b/libevmcore/ExpressionClasses.cpp index 49fd72c1b..f9ed7d6fc 100644 --- a/libevmcore/ExpressionClasses.cpp +++ b/libevmcore/ExpressionClasses.cpp @@ -35,7 +35,7 @@ using namespace dev; using namespace dev::eth; -bool ExpressionClasses::Expression::operator<(const ExpressionClasses::Expression& _other) const +bool ExpressionClasses::Expression::operator<(ExpressionClasses::Expression const& _other) const { auto type = item->type(); auto otherType = _other.item->type(); @@ -74,7 +74,7 @@ ExpressionClasses::Id ExpressionClasses::find(AssemblyItem const& _item, Ids con return exp.id; } -string ExpressionClasses::fullDAGToString(ExpressionClasses::Id _id) +string ExpressionClasses::fullDAGToString(ExpressionClasses::Id _id) const { Expression const& expr = representative(_id); stringstream str; diff --git a/libevmcore/ExpressionClasses.h b/libevmcore/ExpressionClasses.h index ecd9c9250..eda568e23 100644 --- a/libevmcore/ExpressionClasses.h +++ b/libevmcore/ExpressionClasses.h @@ -63,7 +63,7 @@ public: /// @returns the number of classes. Id size() const { return m_representatives.size(); } - std::string fullDAGToString(Id _id); + std::string fullDAGToString(Id _id) const; private: /// Tries to simplify the given expression. diff --git a/libevmcore/Instruction.h b/libevmcore/Instruction.h index c869e761d..07c7b52fd 100644 --- a/libevmcore/Instruction.h +++ b/libevmcore/Instruction.h @@ -237,7 +237,7 @@ enum Tier struct InstructionInfo { std::string name; ///< The name of the instruction. - int additional; //a/< Additional items required in memory for this instructions (only for PUSH). + int additional; ///< Additional items required in memory for this instructions (only for PUSH). int args; ///< Number of items required on the stack for this instruction (and, for the purposes of ret, the number taken from the stack). int ret; ///< Number of items placed (back) on the stack by this instruction, assuming args items were removed. bool sideEffects; ///< false if the only effect on the execution environment (apart from gas usage) is a change to a topmost segment of the stack From dd15c53ae41b36bafdba0bb0500de58f8cee296f Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Fri, 27 Mar 2015 13:28:32 +0100 Subject: [PATCH 22/23] added externalTypes function to functionType removed flag for externalSigniture --- libsolidity/AST.cpp | 8 ++++---- libsolidity/AST.h | 4 ++-- libsolidity/ExpressionCompiler.cpp | 2 +- libsolidity/Types.cpp | 24 +++++++++++++++++++----- libsolidity/Types.h | 6 ++++-- test/SolidityNameAndTypeResolution.cpp | 2 +- 6 files changed, 31 insertions(+), 15 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 461c3d0ca..1736469eb 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -192,7 +192,7 @@ vector, FunctionTypePointer>> const& ContractDefinition::getIn if (functionsSeen.count(f->getName()) == 0 && f->isPartOfExternalInterface()) { functionsSeen.insert(f->getName()); - FixedHash<4> hash(dev::sha3(f->externalSignature(true))); + FixedHash<4> hash(dev::sha3(f->externalSignature())); m_interfaceFunctionList->push_back(make_pair(hash, make_shared(*f, false))); } @@ -202,7 +202,7 @@ vector, FunctionTypePointer>> const& ContractDefinition::getIn FunctionType ftype(*v); solAssert(v->getType().get(), ""); functionsSeen.insert(v->getName()); - FixedHash<4> hash(dev::sha3(ftype.externalSignature(true, v->getName()))); + FixedHash<4> hash(dev::sha3(ftype.externalSignature(v->getName()))); m_interfaceFunctionList->push_back(make_pair(hash, make_shared(*v))); } } @@ -320,9 +320,9 @@ void FunctionDefinition::checkTypeRequirements() m_body->checkTypeRequirements(); } -string FunctionDefinition::externalSignature(bool isExternalCall) const +string FunctionDefinition::externalSignature() const { - return FunctionType(*this).externalSignature(isExternalCall, getName()); + return FunctionType(*this).externalSignature(getName()); } bool VariableDeclaration::isLValue() const diff --git a/libsolidity/AST.h b/libsolidity/AST.h index abcae83c5..00ad33073 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -422,9 +422,9 @@ public: void checkTypeRequirements(); /// @returns the external signature of the function - /// That consists of the name of the function followed by the types (external types if isExternalCall) of the + /// That consists of the name of the function followed by the types of the /// arguments separated by commas all enclosed in parentheses without any spaces. - std::string externalSignature(bool isExternalCall = false) const; + std::string externalSignature() const; private: bool m_isConstructor; diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index ddc347e68..90568767b 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -544,7 +544,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) } if (!event.isAnonymous()) { - m_context << u256(h256::Arith(dev::sha3(function.externalSignature(false, event.getName())))); + m_context << u256(h256::Arith(dev::sha3(function.externalSignature(event.getName())))); ++numIndexed; } solAssert(numIndexed <= 4, "Too many indexed arguments."); diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 1776413a0..5fd7d24a6 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -1098,6 +1098,19 @@ unsigned FunctionType::getSizeOnStack() const return size; } +TypePointer FunctionType::externalType() const +{ + TypePointers paramTypes; + TypePointers retParamTypes; + + for (auto it = m_parameterTypes.cbegin(); it != m_parameterTypes.cend(); ++it) + paramTypes.push_back((*it)->externalType()); + for (auto it = m_returnParameterTypes.cbegin(); it != m_returnParameterTypes.cend(); ++it) + retParamTypes.push_back((*it)->externalType()); + + return make_shared(paramTypes, retParamTypes, m_location, m_arbitraryParameters); +} + MemberList const& FunctionType::getMembers() const { switch (m_location) @@ -1127,7 +1140,7 @@ MemberList const& FunctionType::getMembers() const } } -string FunctionType::externalSignature(bool isExternalCall, std::string const& _name) const +string FunctionType::externalSignature(std::string const& _name) const { std::string funcName = _name; if (_name == "") @@ -1137,12 +1150,13 @@ string FunctionType::externalSignature(bool isExternalCall, std::string const& _ } string ret = funcName + "("; - for (auto it = m_parameterTypes.cbegin(); it != m_parameterTypes.cend(); ++it) + TypePointers externalParameterTypes = dynamic_cast(*externalType()).getParameterTypes(); + for (auto it = externalParameterTypes.cbegin(); it != externalParameterTypes.cend(); ++it) { - if (isExternalCall) - solAssert(!!(*it)->externalType(), "Parameter should have external type"); - ret += (isExternalCall ? (*it)->externalType()->toString() : (*it)->toString()) + (it + 1 == m_parameterTypes.cend() ? "" : ","); + solAssert(!!(*it), "Parameter should have external type"); + ret += (*it)->toString() + (it + 1 == externalParameterTypes.cend() ? "" : ","); } + return ret + ")"; } diff --git a/libsolidity/Types.h b/libsolidity/Types.h index c075ff07f..99fd878f0 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -511,6 +511,9 @@ public: Bare }; virtual Category getCategory() const override { return Category::Function; } + + virtual TypePointer externalType() const override; + explicit FunctionType(FunctionDefinition const& _function, bool _isInternal = true); explicit FunctionType(VariableDeclaration const& _varDecl); explicit FunctionType(EventDefinition const& _event); @@ -553,8 +556,7 @@ public: /// @returns the external signature of this function type given the function name /// If @a _name is not provided (empty string) then the @c m_declaration member of the /// function type is used - /// @a isExternalCall shows if it is external function call - std::string externalSignature(bool isExternalCall = false, std::string const& _name = "") const; + std::string externalSignature(std::string const& _name = "") const; Declaration const& getDeclaration() const { solAssert(m_declaration, "Requested declaration from a FunctionType that has none"); diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index 6b9916549..3bec70a2b 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -414,7 +414,7 @@ BOOST_AUTO_TEST_CASE(function_external_types) auto functions = contract->getDefinedFunctions(); if (functions.empty()) continue; - BOOST_CHECK_EQUAL("boo(uint256,bool,bytes8,bool[2],uint256[],address)", functions[0]->externalSignature(true)); + BOOST_CHECK_EQUAL("boo(uint256,bool,bytes8,bool[2],uint256[],address)", functions[0]->externalSignature()); } } From ff5dfa2e8f16287a29724d7d3fce90da97a75a65 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 27 Mar 2015 14:56:31 +0100 Subject: [PATCH 23/23] Removed duplicate line. --- libevmcore/ExpressionClasses.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libevmcore/ExpressionClasses.cpp b/libevmcore/ExpressionClasses.cpp index f9ed7d6fc..b43b54113 100644 --- a/libevmcore/ExpressionClasses.cpp +++ b/libevmcore/ExpressionClasses.cpp @@ -167,7 +167,6 @@ Rules::Rules() {{Instruction::OR, {X, X}}, [=]{ return X; }}, {{Instruction::SUB, {X, X}}, [=]{ return u256(0); }}, {{Instruction::EQ, {X, X}}, [=]{ return u256(1); }}, - {{Instruction::EQ, {X, X}}, [=]{ return u256(1); }}, {{Instruction::LT, {X, X}}, [=]{ return u256(0); }}, {{Instruction::SLT, {X, X}}, [=]{ return u256(0); }}, {{Instruction::GT, {X, X}}, [=]{ return u256(0); }},