From 0b22154a75254697d0bde328039c5d6bfedd935f Mon Sep 17 00:00:00 2001 From: Yoichi Hirai Date: Tue, 16 May 2017 18:45:08 +0200 Subject: libevmasm: add RETURNDATACOPY and RETURNDATASIZE --- docs/assembly.rst | 4 ++++ libevmasm/Instruction.cpp | 4 ++++ libevmasm/Instruction.h | 2 ++ test/libsolidity/InlineAssembly.cpp | 10 ++++++++++ 4 files changed, 20 insertions(+) diff --git a/docs/assembly.rst b/docs/assembly.rst index 90e70031..cd3ff4c0 100644 --- a/docs/assembly.rst +++ b/docs/assembly.rst @@ -234,6 +234,10 @@ In the grammar, opcodes are represented as pre-defined identifiers. +-------------------------+------+-----------------------------------------------------------------+ | extcodecopy(a, t, f, s) | `-` | like codecopy(t, f, s) but take code at address a | +-------------------------+------+-----------------------------------------------------------------+ +| returndatasize | | size of the last returndata | ++-------------------------+------+-----------------------------------------------------------------+ +| returndatacopy(t, f, s) | `*` | copy s bytes from returndata at position f to mem at position t | ++-------------------------+------+-----------------------------------------------------------------+ | create(v, p, s) | | create new contract with code mem[p..(p+s)) and send v wei | | | | and return the new address | +-------------------------+------+-----------------------------------------------------------------+ diff --git a/libevmasm/Instruction.cpp b/libevmasm/Instruction.cpp index 25eab60b..af7e9ff9 100644 --- a/libevmasm/Instruction.cpp +++ b/libevmasm/Instruction.cpp @@ -67,6 +67,8 @@ const std::map dev::solidity::c_instructions = { "GASPRICE", Instruction::GASPRICE }, { "EXTCODESIZE", Instruction::EXTCODESIZE }, { "EXTCODECOPY", Instruction::EXTCODECOPY }, + { "RETURNDATASIZE", Instruction::RETURNDATASIZE }, + { "RETURNDATACOPY", Instruction::RETURNDATACOPY }, { "BLOCKHASH", Instruction::BLOCKHASH }, { "COINBASE", Instruction::COINBASE }, { "TIMESTAMP", Instruction::TIMESTAMP }, @@ -203,6 +205,8 @@ static const std::map c_instructionInfo = { Instruction::GASPRICE, { "GASPRICE", 0, 0, 1, false, Tier::Base } }, { Instruction::EXTCODESIZE, { "EXTCODESIZE", 0, 1, 1, false, Tier::ExtCode } }, { Instruction::EXTCODECOPY, { "EXTCODECOPY", 0, 4, 0, true, Tier::ExtCode } }, + { Instruction::RETURNDATASIZE, {"RETURNDATASIZE", 0, 0, 1, false, Tier::Base } }, + { Instruction::RETURNDATACOPY, {"RETURNDATACOPY", 0, 3, 0, true, Tier::VeryLow } }, { Instruction::BLOCKHASH, { "BLOCKHASH", 0, 1, 1, false, Tier::Ext } }, { Instruction::COINBASE, { "COINBASE", 0, 0, 1, false, Tier::Base } }, { Instruction::TIMESTAMP, { "TIMESTAMP", 0, 0, 1, false, Tier::Base } }, diff --git a/libevmasm/Instruction.h b/libevmasm/Instruction.h index 09d1e58b..a8c3bf4a 100644 --- a/libevmasm/Instruction.h +++ b/libevmasm/Instruction.h @@ -77,6 +77,8 @@ enum class Instruction: uint8_t GASPRICE, ///< get price of gas in current environment EXTCODESIZE, ///< get external code size (from another contract) EXTCODECOPY, ///< copy external code (from another contract) + RETURNDATASIZE, ///< get size of the last return data + RETURNDATACOPY, ///< copy last return data to memory BLOCKHASH = 0x40, ///< get hash of most recent complete block COINBASE, ///< get the block's coinbase address diff --git a/test/libsolidity/InlineAssembly.cpp b/test/libsolidity/InlineAssembly.cpp index d2d320db..2f9e1ced 100644 --- a/test/libsolidity/InlineAssembly.cpp +++ b/test/libsolidity/InlineAssembly.cpp @@ -542,6 +542,16 @@ BOOST_AUTO_TEST_CASE(keccak256) BOOST_CHECK(successAssemble("{ pop(sha3(0, 0)) }")); } +BOOST_AUTO_TEST_CASE(returndatasize) +{ + BOOST_CHECK(successAssemble("{ let r := returndatasize }")); +} + +BOOST_AUTO_TEST_CASE(returndatacopy) +{ + BOOST_CHECK(successAssemble("{ returndatacopy(0, 32, 64) }")); +} + BOOST_AUTO_TEST_SUITE_END() BOOST_AUTO_TEST_SUITE_END() -- cgit v1.2.3 From 55737213d1b528ca629511a9944266f99b08cb35 Mon Sep 17 00:00:00 2001 From: Yoichi Hirai Date: Wed, 17 May 2017 13:27:04 +0200 Subject: Add a changelog entry --- Changelog.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Changelog.md b/Changelog.md index b398e014..8f1fe045 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,14 +1,15 @@ ### 0.4.12 (unreleased) Features: - * Assembler: renamed ``SHA3`` to `KECCAK256``. - * AST: export all attributes to Json format + * Assembly: renamed ``SHA3`` to `KECCAK256``. + * Assembly: Add ``RETURNDATASIZE`` and ``RETURNDATACOPY`` (EIP211) instructions. + * AST: export all attributes to JSON format. * Inline Assembly: Present proper error message when not supplying enough arguments to a functional instruction. * Inline Assembly: introduce ``keccak256`` as an opcode. ``sha3`` is still a valid alias. Bugfixes: - * Unused variable warnings no longer issued for variables used inside inline assembly + * Unused variable warnings no longer issued for variables used inside inline assembly. ### 0.4.11 (2017-05-03) -- cgit v1.2.3 From 9ff3064d032cdd732dd186a643285b96d51eea94 Mon Sep 17 00:00:00 2001 From: Yoichi Hirai Date: Wed, 17 May 2017 13:29:42 +0200 Subject: Mention RETURNDATACOPY in GasMeter and SemanticInformation --- libevmasm/GasMeter.cpp | 1 + libevmasm/SemanticInformation.cpp | 3 +++ 2 files changed, 4 insertions(+) diff --git a/libevmasm/GasMeter.cpp b/libevmasm/GasMeter.cpp index 260b7439..31a7d13e 100644 --- a/libevmasm/GasMeter.cpp +++ b/libevmasm/GasMeter.cpp @@ -103,6 +103,7 @@ GasMeter::GasConsumption GasMeter::estimateMax(AssemblyItem const& _item, bool _ break; case Instruction::CALLDATACOPY: case Instruction::CODECOPY: + case Instruction::RETURNDATACOPY: gas += memoryGas(0, -2); gas += wordGas(GasCosts::copyGas, m_state->relativeStackElement(-2)); break; diff --git a/libevmasm/SemanticInformation.cpp b/libevmasm/SemanticInformation.cpp index 61586e7b..86feb1d2 100644 --- a/libevmasm/SemanticInformation.cpp +++ b/libevmasm/SemanticInformation.cpp @@ -143,6 +143,8 @@ bool SemanticInformation::isDeterministic(AssemblyItem const& _item) case Instruction::MSIZE: // depends on previous writes and reads, not only on content case Instruction::BALANCE: // depends on previous calls case Instruction::EXTCODESIZE: + case Instruction::RETURNDATACOPY: // depends on previous calls + case Instruction::RETURNDATASIZE: return false; default: return true; @@ -156,6 +158,7 @@ bool SemanticInformation::invalidatesMemory(Instruction _instruction) case Instruction::CALLDATACOPY: case Instruction::CODECOPY: case Instruction::EXTCODECOPY: + case Instruction::RETURNDATACOPY: case Instruction::MSTORE: case Instruction::MSTORE8: case Instruction::CALL: -- cgit v1.2.3 From 05af6c92556acac917983b1a8e0e9e62d28de573 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Wed, 24 May 2017 21:51:54 +0100 Subject: Warn if returndatasize/returndatacopy is used --- libsolidity/inlineasm/AsmAnalysis.cpp | 20 +++++++++++++++++++- libsolidity/inlineasm/AsmAnalysis.h | 1 + 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/libsolidity/inlineasm/AsmAnalysis.cpp b/libsolidity/inlineasm/AsmAnalysis.cpp index 13852880..36ac0e75 100644 --- a/libsolidity/inlineasm/AsmAnalysis.cpp +++ b/libsolidity/inlineasm/AsmAnalysis.cpp @@ -65,6 +65,7 @@ bool AsmAnalyzer::operator()(assembly::Instruction const& _instruction) auto const& info = instructionInfo(_instruction.instruction); m_stackHeight += info.ret - info.args; m_info.stackHeightInfo[&_instruction] = m_stackHeight; + warnOnFutureInstruction(_instruction.instruction, _instruction.location); return true; } @@ -149,6 +150,7 @@ bool AsmAnalyzer::operator()(FunctionalInstruction const& _instr) if (!(*this)(_instr.instruction)) success = false; m_info.stackHeightInfo[&_instr] = m_stackHeight; + warnOnFutureInstruction(_instr.instruction.instruction, _instr.location); return success; } @@ -431,7 +433,6 @@ Scope& AsmAnalyzer::scope(Block const* _block) solAssert(scopePtr, "Scope requested but not present."); return *scopePtr; } - void AsmAnalyzer::expectValidType(string const& type, SourceLocation const& _location) { if (!m_julia) @@ -443,3 +444,20 @@ void AsmAnalyzer::expectValidType(string const& type, SourceLocation const& _loc "\"" + type + "\" is not a valid type (user defined types are not yet supported)." ); } + +void AsmAnalyzer::warnOnFutureInstruction(solidity::Instruction _instr, SourceLocation const& _location) +{ + switch (_instr) + { + case solidity::Instruction::RETURNDATASIZE: + case solidity::Instruction::RETURNDATACOPY: + m_errorReporter.warning( + _location, + "The RETURNDATASIZE/RETURNDATACOPY instructions are only available after " + "the Metropolis hard fork. Before that they act as an invalid instruction." + ); + break; + default: + break; + } +} diff --git a/libsolidity/inlineasm/AsmAnalysis.h b/libsolidity/inlineasm/AsmAnalysis.h index e7748bcf..55b409ba 100644 --- a/libsolidity/inlineasm/AsmAnalysis.h +++ b/libsolidity/inlineasm/AsmAnalysis.h @@ -97,6 +97,7 @@ private: Scope& scope(assembly::Block const* _block); void expectValidType(std::string const& type, SourceLocation const& _location); + void warnOnFutureInstruction(solidity::Instruction _instr, SourceLocation const& _location); int m_stackHeight = 0; julia::ExternalIdentifierAccess::Resolver m_resolver; -- cgit v1.2.3 From 464dea24597135cbb44ffa24d19dd184315d9085 Mon Sep 17 00:00:00 2001 From: Yoichi Hirai Date: Mon, 29 May 2017 14:46:42 +0200 Subject: test: Add different styles of returndatacopy and returndatasize --- test/libsolidity/InlineAssembly.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/libsolidity/InlineAssembly.cpp b/test/libsolidity/InlineAssembly.cpp index 2f9e1ced..435c3dad 100644 --- a/test/libsolidity/InlineAssembly.cpp +++ b/test/libsolidity/InlineAssembly.cpp @@ -547,7 +547,17 @@ BOOST_AUTO_TEST_CASE(returndatasize) BOOST_CHECK(successAssemble("{ let r := returndatasize }")); } +BOOST_AUTO_TEST_CASE(returndatasize_functional) +{ + BOOST_CHECK(successAssemble("{ let r := returndatasize() }")); +} + BOOST_AUTO_TEST_CASE(returndatacopy) +{ + BOOST_CHECK(successAssemble("{ 64 32 0 returndatacopy }")); +} + +BOOST_AUTO_TEST_CASE(returndatacopy_functional) { BOOST_CHECK(successAssemble("{ returndatacopy(0, 32, 64) }")); } -- cgit v1.2.3 From a0f8c94dadb11b697512af43f119c741aeba7f39 Mon Sep 17 00:00:00 2001 From: Yoichi Hirai Date: Thu, 8 Jun 2017 17:17:43 +0200 Subject: Add a test about checking a warning --- test/libsolidity/SolidityNameAndTypeResolution.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index db5b9bf8..1052d709 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -193,11 +193,16 @@ CHECK_ERROR_OR_WARNING(text, type, substring, false, false) #define CHECK_ERROR_ALLOW_MULTI(text, type, substring) \ CHECK_ERROR_OR_WARNING(text, type, substring, false, true) -// [checkWarning(text, type, substring)] asserts that the compilation down to typechecking -// emits a warning of type [type] and with a message containing [substring]. +// [checkWarning(text, substring)] asserts that the compilation down to typechecking +// emits a warning and with a message containing [substring]. #define CHECK_WARNING(text, substring) \ CHECK_ERROR_OR_WARNING(text, Warning, substring, true, false) +// [checkWarningAllowMulti(text, substring)] aserts that the compilation down to typechecking +// emits a warning and with a message containing [substring]. +#define CHECK_WARNING_ALLOW_MULTI(text, substring) \ +CHECK_ERROR_OR_WARNING(text, Warning, substring, true, true) + // [checkSuccess(text)] asserts that the compilation down to typechecking succeeds. #define CHECK_SUCCESS(text) do { BOOST_CHECK(success((text))); } while(0) @@ -5780,6 +5785,13 @@ BOOST_AUTO_TEST_CASE(no_unused_inline_asm) CHECK_SUCCESS_NO_WARNINGS(text); } +BOOST_AUTO_TEST_CASE(returndatacopy_as_variable) +{ + char const* text = R"( + contract c { function f() { uint returndatasize; assembly { returndatasize }}} + )"; + CHECK_WARNING_ALLOW_MULTI(text, "shadowed by an insturction of the same name"); +} BOOST_AUTO_TEST_SUITE_END() -- cgit v1.2.3 From a7241df4b736bb1743048204d901c1bb1a902d0c Mon Sep 17 00:00:00 2001 From: Yoichi Hirai Date: Tue, 13 Jun 2017 16:25:52 +0200 Subject: Add a failing test as suggested in https://github.com/ethereum/solidity/pull/2275#discussion_r121438333 --- test/libsolidity/SolidityNameAndTypeResolution.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 1052d709..438a2b66 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -5793,6 +5793,15 @@ BOOST_AUTO_TEST_CASE(returndatacopy_as_variable) CHECK_WARNING_ALLOW_MULTI(text, "shadowed by an insturction of the same name"); } +BOOST_AUTO_TEST_CASE(shadowing_warning_can_be_removed) +{ + char const* text = R"( + contract C {function f() {assembly {}}} + )"; + CHECK_SUCCESS_NO_WARNINGS(text); +} + + BOOST_AUTO_TEST_SUITE_END() -- cgit v1.2.3 From 8775e77305f84117827f1e6165c4d3776c51f667 Mon Sep 17 00:00:00 2001 From: Yoichi Hirai Date: Fri, 9 Jun 2017 15:35:27 +0200 Subject: Add a warning about a varialbe of the name of an instruction --- libsolidity/analysis/NameAndTypeResolver.cpp | 22 ++++++++++++++++++++++ libsolidity/analysis/NameAndTypeResolver.h | 3 +++ libsolidity/analysis/ReferencesResolver.cpp | 2 ++ test/libsolidity/SolidityNameAndTypeResolution.cpp | 2 +- 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp index 2742dcf2..aac90311 100644 --- a/libsolidity/analysis/NameAndTypeResolver.cpp +++ b/libsolidity/analysis/NameAndTypeResolver.cpp @@ -26,6 +26,8 @@ #include #include +#include + using namespace std; namespace dev @@ -232,6 +234,26 @@ vector NameAndTypeResolver::cleanedDeclarations( return uniqueFunctions; } +void NameAndTypeResolver::warnVariablesNamedLikeInstructions() +{ + for (auto const& instruction: c_instructions) + { + string const instructionName{boost::algorithm::to_lower_copy(instruction.first)}; + auto declarations = nameFromCurrentScope(instructionName); + for (Declaration const* const declaration: declarations) + { + solAssert(!!declaration, ""); + if (dynamic_cast(declaration)) + // Don't warn the user for what the user did not. + continue; + m_errorReporter.warning( + declaration->location(), + "Variable is shadowed in inline assembly by an instruction of the same name" + ); + } + } +} + bool NameAndTypeResolver::resolveNamesAndTypesInternal(ASTNode& _node, bool _resolveInsideCode) { if (ContractDefinition* contract = dynamic_cast(&_node)) diff --git a/libsolidity/analysis/NameAndTypeResolver.h b/libsolidity/analysis/NameAndTypeResolver.h index 0441867d..84628778 100644 --- a/libsolidity/analysis/NameAndTypeResolver.h +++ b/libsolidity/analysis/NameAndTypeResolver.h @@ -90,6 +90,9 @@ public: std::vector const& _declarations ); + /// Generate and store warnings about variables that are named like instructions. + void warnVariablesNamedLikeInstructions(); + private: /// Internal version of @a resolveNamesAndTypes (called from there) throws exceptions on fatal errors. bool resolveNamesAndTypesInternal(ASTNode& _node, bool _resolveInsideCode = true); diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp index edf2fc02..2a5f27df 100644 --- a/libsolidity/analysis/ReferencesResolver.cpp +++ b/libsolidity/analysis/ReferencesResolver.cpp @@ -162,6 +162,8 @@ void ReferencesResolver::endVisit(ArrayTypeName const& _typeName) bool ReferencesResolver::visit(InlineAssembly const& _inlineAssembly) { + m_resolver.warnVariablesNamedLikeInstructions(); + // Errors created in this stage are completely ignored because we do not yet know // the type and size of external identifiers, which would result in false errors. // The only purpose of this step is to fill the inline assembly annotation with diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 438a2b66..ba2ade66 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -5790,7 +5790,7 @@ BOOST_AUTO_TEST_CASE(returndatacopy_as_variable) char const* text = R"( contract c { function f() { uint returndatasize; assembly { returndatasize }}} )"; - CHECK_WARNING_ALLOW_MULTI(text, "shadowed by an insturction of the same name"); + CHECK_WARNING_ALLOW_MULTI(text, "Variable is shadowed in inline assembly by an instruction of the same name"); } BOOST_AUTO_TEST_CASE(shadowing_warning_can_be_removed) -- cgit v1.2.3