From 3b813ed29569dde02b965c97c9fdd60469876f66 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Fri, 19 May 2017 17:06:26 +0100 Subject: [PATCH 1/3] Support multiple assignment in inline assembly --- Changelog.md | 1 + libjulia/backends/evm/EVMCodeTransform.cpp | 42 ++++++++++++---------- libjulia/backends/evm/EVMCodeTransform.h | 2 +- libsolidity/inlineasm/AsmAnalysis.cpp | 20 +++++++++-- libsolidity/inlineasm/AsmData.h | 6 +++- libsolidity/inlineasm/AsmParser.cpp | 30 +++++++++++++++- libsolidity/inlineasm/AsmPrinter.cpp | 6 +++- test/libjulia/Parser.cpp | 20 +++++++++++ test/libsolidity/InlineAssembly.cpp | 18 ++++++++++ test/libsolidity/SolidityEndToEndTest.cpp | 25 +++++++++++++ 10 files changed, 145 insertions(+), 25 deletions(-) diff --git a/Changelog.md b/Changelog.md index f1b1a19cf..9c623805c 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,7 @@ Features: * Support ``pragma experimental "v0.5.0";`` to turn on upcoming breaking changes. + * Assembly Parser: Support multiple assignment (``x, y := f()``). * Code Generator: Added ``.selector`` member on external function types to retrieve their signature. * Code Generator: Keep a single copy of encoding functions when using the experimental "ABIEncoderV2". * Optimizer: Add new optimization step to remove unused ``JUMPDEST``s. diff --git a/libjulia/backends/evm/EVMCodeTransform.cpp b/libjulia/backends/evm/EVMCodeTransform.cpp index e0b11cf33..e80903d59 100644 --- a/libjulia/backends/evm/EVMCodeTransform.cpp +++ b/libjulia/backends/evm/EVMCodeTransform.cpp @@ -60,16 +60,19 @@ void CodeTransform::operator()(VariableDeclaration const& _varDecl) void CodeTransform::operator()(Assignment const& _assignment) { - visitExpression(*_assignment.value); + int height = m_assembly.stackHeight(); + boost::apply_visitor(*this, *_assignment.value); + expectDeposit(_assignment.variableNames.size(), height); + m_assembly.setSourceLocation(_assignment.location); - generateAssignment(_assignment.variableName); + generateAssignment(_assignment.variableNames); checkStackHeight(&_assignment); } void CodeTransform::operator()(StackAssignment const& _assignment) { m_assembly.setSourceLocation(_assignment.location); - generateAssignment(_assignment.variableName); + generateAssignment({_assignment.variableName}); checkStackHeight(&_assignment); } @@ -469,24 +472,27 @@ void CodeTransform::finalizeBlock(Block const& _block, int blockStartStackHeight checkStackHeight(&_block); } -void CodeTransform::generateAssignment(Identifier const& _variableName) +void CodeTransform::generateAssignment(vector const& _variableNames) { solAssert(m_scope, ""); - auto var = m_scope->lookup(_variableName.name); - if (var) + for (auto const& variableName: _variableNames | boost::adaptors::reversed) { - Scope::Variable const& _var = boost::get(*var); - if (int heightDiff = variableHeightDiff(_var, true)) - m_assembly.appendInstruction(solidity::swapInstruction(heightDiff - 1)); - m_assembly.appendInstruction(solidity::Instruction::POP); - } - else - { - solAssert( - m_identifierAccess.generateCode, - "Identifier not found and no external access available." - ); - m_identifierAccess.generateCode(_variableName, IdentifierContext::LValue, m_assembly); + auto var = m_scope->lookup(variableName.name); + if (var) + { + Scope::Variable const& _var = boost::get(*var); + if (int heightDiff = variableHeightDiff(_var, true)) + m_assembly.appendInstruction(solidity::swapInstruction(heightDiff - 1)); + m_assembly.appendInstruction(solidity::Instruction::POP); + } + else + { + solAssert( + m_identifierAccess.generateCode, + "Identifier not found and no external access available." + ); + m_identifierAccess.generateCode(variableName, IdentifierContext::LValue, m_assembly); + } } } diff --git a/libjulia/backends/evm/EVMCodeTransform.h b/libjulia/backends/evm/EVMCodeTransform.h index 2c0fd10cc..bb2be7864 100644 --- a/libjulia/backends/evm/EVMCodeTransform.h +++ b/libjulia/backends/evm/EVMCodeTransform.h @@ -124,7 +124,7 @@ private: /// to @a _blackStartStackHeight. void finalizeBlock(solidity::assembly::Block const& _block, int _blockStartStackHeight); - void generateAssignment(solidity::assembly::Identifier const& _variableName); + void generateAssignment(std::vector const& _variableNames); /// Determines the stack height difference to the given variables. Throws /// if it is not yet in scope or the height difference is too large. Returns diff --git a/libsolidity/inlineasm/AsmAnalysis.cpp b/libsolidity/inlineasm/AsmAnalysis.cpp index 76b0bbd54..e5bdc90f9 100644 --- a/libsolidity/inlineasm/AsmAnalysis.cpp +++ b/libsolidity/inlineasm/AsmAnalysis.cpp @@ -163,11 +163,25 @@ bool AsmAnalyzer::operator()(assembly::StackAssignment const& _assignment) bool AsmAnalyzer::operator()(assembly::Assignment const& _assignment) { + int const expectedItems = _assignment.variableNames.size(); + solAssert(expectedItems >= 1, ""); int const stackHeight = m_stackHeight; bool success = boost::apply_visitor(*this, *_assignment.value); - solAssert(m_stackHeight >= stackHeight, "Negative value size."); - if (!checkAssignment(_assignment.variableName, m_stackHeight - stackHeight)) - success = false; + if ((m_stackHeight - stackHeight) != expectedItems) + { + m_errorReporter.declarationError( + _assignment.location, + "Variable count does not match number of values (" + + to_string(expectedItems) + + " vs. " + + to_string(m_stackHeight - stackHeight) + + ")" + ); + return false; + } + for (auto const& variableName: _assignment.variableNames) + if (!checkAssignment(variableName, 1)) + success = false; m_info.stackHeightInfo[&_assignment] = m_stackHeight; return success; } diff --git a/libsolidity/inlineasm/AsmData.h b/libsolidity/inlineasm/AsmData.h index db5840bc9..b0dd85ca8 100644 --- a/libsolidity/inlineasm/AsmData.h +++ b/libsolidity/inlineasm/AsmData.h @@ -54,7 +54,11 @@ struct Label { SourceLocation location; std::string name; }; struct StackAssignment { SourceLocation location; Identifier variableName; }; /// Assignment ("x := mload(20:u256)", expects push-1-expression on the right hand /// side and requires x to occupy exactly one stack slot. -struct Assignment { SourceLocation location; Identifier variableName; std::shared_ptr value; }; +/// +/// Multiple assignment ("x, y := f()"), where the left hand side variables each occupy +/// a single stack slot and expects a single expression on the right hand returning +/// the same amount of items as the number of variables. +struct Assignment { SourceLocation location; std::vector variableNames; std::shared_ptr value; }; /// Functional instruction, e.g. "mul(mload(20:u256), add(2:u256, x))" struct FunctionalInstruction { SourceLocation location; Instruction instruction; std::vector arguments; }; struct FunctionCall { SourceLocation location; Identifier functionName; std::vector arguments; }; diff --git a/libsolidity/inlineasm/AsmParser.cpp b/libsolidity/inlineasm/AsmParser.cpp index d84fe999c..3087ad864 100644 --- a/libsolidity/inlineasm/AsmParser.cpp +++ b/libsolidity/inlineasm/AsmParser.cpp @@ -122,6 +122,34 @@ assembly::Statement Parser::parseStatement() { case Token::LParen: return parseCall(std::move(statement)); + case Token::Comma: + { + // if a comma follows, a multiple assignment is assumed + + if (statement.type() != typeid(assembly::Identifier)) + fatalParserError("Label name / variable name must precede \",\" (multiple assignment)."); + assembly::Identifier const& identifier = boost::get(statement); + + Assignment assignment = createWithLocation(identifier.location); + assignment.variableNames.emplace_back(identifier); + + do + { + expectToken(Token::Comma); + statement = parseElementaryOperation(false); + if (statement.type() != typeid(assembly::Identifier)) + fatalParserError("Variable name expected in multiple assignemnt."); + assignment.variableNames.emplace_back(boost::get(statement)); + } + while (currentToken() == Token::Comma); + + expectToken(Token::Colon); + expectToken(Token::Assign); + + assignment.value.reset(new Statement(parseExpression())); + assignment.location.end = locationOf(*assignment.value).end; + return assignment; + } case Token::Colon: { if (statement.type() != typeid(assembly::Identifier)) @@ -136,7 +164,7 @@ assembly::Statement Parser::parseStatement() if (!m_julia && instructions().count(identifier.name)) fatalParserError("Cannot use instruction names for identifier names."); advance(); - assignment.variableName = identifier; + assignment.variableNames.emplace_back(identifier); assignment.value.reset(new Statement(parseExpression())); assignment.location.end = locationOf(*assignment.value).end; return assignment; diff --git a/libsolidity/inlineasm/AsmPrinter.cpp b/libsolidity/inlineasm/AsmPrinter.cpp index 47ede91d4..a52728084 100644 --- a/libsolidity/inlineasm/AsmPrinter.cpp +++ b/libsolidity/inlineasm/AsmPrinter.cpp @@ -116,7 +116,11 @@ string AsmPrinter::operator()(assembly::StackAssignment const& _assignment) string AsmPrinter::operator()(assembly::Assignment const& _assignment) { - return (*this)(_assignment.variableName) + " := " + boost::apply_visitor(*this, *_assignment.value); + solAssert(_assignment.variableNames.size() >= 1, ""); + string variables = (*this)(_assignment.variableNames.front()); + for (size_t i = 1; i < _assignment.variableNames.size(); ++i) + variables += ", " + (*this)(_assignment.variableNames[i]); + return variables + " := " + boost::apply_visitor(*this, *_assignment.value); } string AsmPrinter::operator()(assembly::VariableDeclaration const& _variableDeclaration) diff --git a/test/libjulia/Parser.cpp b/test/libjulia/Parser.cpp index 510703708..f8c1aa4de 100644 --- a/test/libjulia/Parser.cpp +++ b/test/libjulia/Parser.cpp @@ -249,6 +249,26 @@ BOOST_AUTO_TEST_CASE(recursion_depth) CHECK_ERROR(input, ParserError, "recursion"); } +BOOST_AUTO_TEST_CASE(multiple_assignment) +{ + CHECK_ERROR("{ let x:u256 function f() -> a:u256, b:u256 {} 123:u256, x := f() }", ParserError, "Label name / variable name must precede \",\" (multiple assignment)."); + CHECK_ERROR("{ let x:u256 function f() -> a:u256, b:u256 {} x, 123:u256 := f() }", ParserError, "Variable name expected in multiple assignemnt."); + + /// NOTE: Travis hiccups if not having a variable + char const* text = R"( + { + function f(a:u256) -> r1:u256, r2:u256 { + r1 := a + r2 := 7:u256 + } + let x:u256 := 9:u256 + let y:u256 := 2:u256 + x, y := f(x) + } + )"; + BOOST_CHECK(successParse(text)); +} + BOOST_AUTO_TEST_SUITE_END() } diff --git a/test/libsolidity/InlineAssembly.cpp b/test/libsolidity/InlineAssembly.cpp index 0debc66d8..da3522b41 100644 --- a/test/libsolidity/InlineAssembly.cpp +++ b/test/libsolidity/InlineAssembly.cpp @@ -412,7 +412,25 @@ BOOST_AUTO_TEST_CASE(recursion_depth) CHECK_PARSE_ERROR(input, ParserError, "recursion"); } +BOOST_AUTO_TEST_CASE(multiple_assignment) +{ + CHECK_PARSE_ERROR("{ let x function f() -> a, b {} 123, x := f() }", ParserError, "Label name / variable name must precede \",\" (multiple assignment)."); + CHECK_PARSE_ERROR("{ let x function f() -> a, b {} x, 123 := f() }", ParserError, "Variable name expected in multiple assignemnt."); + /// NOTE: Travis hiccups if not having a variable + char const* text = R"( + { + function f(a) -> r1, r2 { + r1 := a + r2 := 7 + } + let x := 9 + let y := 2 + x, y := f(x) + } + )"; + BOOST_CHECK(successParse(text)); +} BOOST_AUTO_TEST_SUITE_END() diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index bdac82783..458b64f40 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -7867,6 +7867,31 @@ BOOST_AUTO_TEST_CASE(inline_assembly_function_call) BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(1), u256(2), u256(7))); } +BOOST_AUTO_TEST_CASE(inline_assembly_function_call_assignment) +{ + char const* sourceCode = R"( + contract C { + function f() { + assembly { + let a1, b1, c1 + function asmfun(a, b, c) -> x, y, z { + x := a + y := b + z := 7 + } + a1, b1, c1 := asmfun(1, 2, 3) + mstore(0x00, a1) + mstore(0x20, b1) + mstore(0x40, c1) + return(0, 0x60) + } + } + } + )"; + compileAndRun(sourceCode, 0, "C"); + BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(1), u256(2), u256(7))); +} + BOOST_AUTO_TEST_CASE(inline_assembly_function_call2) { char const* sourceCode = R"( From 6948758156ba31b22fb74a3cd3e7cec0b925208b Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Tue, 19 Sep 2017 12:03:45 +0100 Subject: [PATCH 2/3] Limit parser recursion depth further (needed by increased assembly data structure size) --- libsolidity/parsing/ParserBase.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libsolidity/parsing/ParserBase.cpp b/libsolidity/parsing/ParserBase.cpp index fe95b0fee..5b83c5bdc 100644 --- a/libsolidity/parsing/ParserBase.cpp +++ b/libsolidity/parsing/ParserBase.cpp @@ -104,7 +104,7 @@ void ParserBase::expectToken(Token::Value _value) void ParserBase::increaseRecursionDepth() { m_recursionDepth++; - if (m_recursionDepth >= 3000) + if (m_recursionDepth >= 2560) fatalParserError("Maximum recursion depth reached during parsing."); } From e14ab959f928c0a058b7b46d6ba4ee30e7ec08b7 Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 20 Sep 2017 11:16:07 +0200 Subject: [PATCH 3/3] Remove unintentional copy in assignment operation. --- libjulia/backends/evm/EVMCodeTransform.cpp | 42 ++++++++++++---------- libjulia/backends/evm/EVMCodeTransform.h | 3 +- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/libjulia/backends/evm/EVMCodeTransform.cpp b/libjulia/backends/evm/EVMCodeTransform.cpp index e80903d59..66f593e80 100644 --- a/libjulia/backends/evm/EVMCodeTransform.cpp +++ b/libjulia/backends/evm/EVMCodeTransform.cpp @@ -65,14 +65,14 @@ void CodeTransform::operator()(Assignment const& _assignment) expectDeposit(_assignment.variableNames.size(), height); m_assembly.setSourceLocation(_assignment.location); - generateAssignment(_assignment.variableNames); + generateMultiAssignment(_assignment.variableNames); checkStackHeight(&_assignment); } void CodeTransform::operator()(StackAssignment const& _assignment) { m_assembly.setSourceLocation(_assignment.location); - generateAssignment({_assignment.variableName}); + generateAssignment(_assignment.variableName); checkStackHeight(&_assignment); } @@ -472,27 +472,31 @@ void CodeTransform::finalizeBlock(Block const& _block, int blockStartStackHeight checkStackHeight(&_block); } -void CodeTransform::generateAssignment(vector const& _variableNames) +void CodeTransform::generateMultiAssignment(vector const& _variableNames) { solAssert(m_scope, ""); for (auto const& variableName: _variableNames | boost::adaptors::reversed) + generateAssignment(variableName); +} + +void CodeTransform::generateAssignment(Identifier const& _variableName) +{ + solAssert(m_scope, ""); + auto var = m_scope->lookup(_variableName.name); + if (var) { - auto var = m_scope->lookup(variableName.name); - if (var) - { - Scope::Variable const& _var = boost::get(*var); - if (int heightDiff = variableHeightDiff(_var, true)) - m_assembly.appendInstruction(solidity::swapInstruction(heightDiff - 1)); - m_assembly.appendInstruction(solidity::Instruction::POP); - } - else - { - solAssert( - m_identifierAccess.generateCode, - "Identifier not found and no external access available." - ); - m_identifierAccess.generateCode(variableName, IdentifierContext::LValue, m_assembly); - } + Scope::Variable const& _var = boost::get(*var); + if (int heightDiff = variableHeightDiff(_var, true)) + m_assembly.appendInstruction(solidity::swapInstruction(heightDiff - 1)); + m_assembly.appendInstruction(solidity::Instruction::POP); + } + else + { + solAssert( + m_identifierAccess.generateCode, + "Identifier not found and no external access available." + ); + m_identifierAccess.generateCode(_variableName, IdentifierContext::LValue, m_assembly); } } diff --git a/libjulia/backends/evm/EVMCodeTransform.h b/libjulia/backends/evm/EVMCodeTransform.h index bb2be7864..951c8a50e 100644 --- a/libjulia/backends/evm/EVMCodeTransform.h +++ b/libjulia/backends/evm/EVMCodeTransform.h @@ -124,7 +124,8 @@ private: /// to @a _blackStartStackHeight. void finalizeBlock(solidity::assembly::Block const& _block, int _blockStartStackHeight); - void generateAssignment(std::vector const& _variableNames); + void generateMultiAssignment(std::vector const& _variableNames); + void generateAssignment(solidity::assembly::Identifier const& _variableName); /// Determines the stack height difference to the given variables. Throws /// if it is not yet in scope or the height difference is too large. Returns