From 6540d679684ca667d9551218ac551f686ac992b6 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 6 Mar 2015 16:35:40 +0100 Subject: [PATCH 1/3] Output source lines for sub-assemblies. --- libevmcore/Assembly.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/libevmcore/Assembly.cpp b/libevmcore/Assembly.cpp index 5e9d2fcd3..4fee624b1 100644 --- a/libevmcore/Assembly.cpp +++ b/libevmcore/Assembly.cpp @@ -199,7 +199,6 @@ ostream& Assembly::streamRLP(ostream& _out, string const& _prefix, StringMap con _out << _prefix << ".code:" << endl; for (AssemblyItem const& i: m_items) { - string sourceLine = getLocationFromSources(_sourceCodes, i.getLocation()); _out << _prefix; switch (i.m_type) { @@ -239,7 +238,7 @@ ostream& Assembly::streamRLP(ostream& _out, string const& _prefix, StringMap con default: BOOST_THROW_EXCEPTION(InvalidOpcode()); } - _out << string("\t\t") << sourceLine << endl; + _out << string("\t\t") << getLocationFromSources(_sourceCodes, i.getLocation()) << endl; } if (!m_data.empty() || !m_subs.empty()) @@ -251,7 +250,7 @@ ostream& Assembly::streamRLP(ostream& _out, string const& _prefix, StringMap con for (auto const& i: m_subs) { _out << _prefix << " " << hex << (unsigned)(u256)i.first << ": " << endl; - i.second.streamRLP(_out, _prefix + " "); + i.second.streamRLP(_out, _prefix + " ", _sourceCodes); } } return _out; From cec5b3b60eee71ac087bfa8679ca34e84d557561 Mon Sep 17 00:00:00 2001 From: chriseth Date: Sat, 7 Mar 2015 23:08:39 +0100 Subject: [PATCH 2/3] Fixed: Some instructions did not have source locations. --- libevmcore/Assembly.cpp | 5 +++-- libevmcore/Assembly.h | 11 ++++++--- libsolidity/Compiler.cpp | 32 +++++++++++++------------- libsolidity/CompilerContext.cpp | 36 ++++++------------------------ libsolidity/CompilerContext.h | 19 +++++++++------- libsolidity/ExpressionCompiler.cpp | 18 ++++++++------- 6 files changed, 56 insertions(+), 65 deletions(-) diff --git a/libevmcore/Assembly.cpp b/libevmcore/Assembly.cpp index 5e9d2fcd3..cb47228bb 100644 --- a/libevmcore/Assembly.cpp +++ b/libevmcore/Assembly.cpp @@ -257,11 +257,12 @@ ostream& Assembly::streamRLP(ostream& _out, string const& _prefix, StringMap con return _out; } -AssemblyItem const& Assembly::append(AssemblyItem const& _i, SourceLocation const& _location) +AssemblyItem const& Assembly::append(AssemblyItem const& _i) { m_deposit += _i.deposit(); m_items.push_back(_i); - m_items.back().setLocation(_location); + if (m_items.back().getLocation().isEmpty() && !m_currentSourceLocation.isEmpty()) + m_items.back().setLocation(m_currentSourceLocation); return back(); } diff --git a/libevmcore/Assembly.h b/libevmcore/Assembly.h index 4b818e624..c3bc1bc55 100644 --- a/libevmcore/Assembly.h +++ b/libevmcore/Assembly.h @@ -88,9 +88,9 @@ public: AssemblyItem append() { return append(newTag()); } void append(Assembly const& _a); void append(Assembly const& _a, int _deposit); - AssemblyItem const& append(AssemblyItem const& _i, SourceLocation const& _location = SourceLocation()); - AssemblyItem const& append(std::string const& _data, SourceLocation const& _location = SourceLocation()) { return append(newPushString(_data), _location); } - AssemblyItem const& append(bytes const& _data, SourceLocation const& _location = SourceLocation()) { return append(newData(_data), _location); } + AssemblyItem const& append(AssemblyItem const& _i); + AssemblyItem const& append(std::string const& _data) { return append(newPushString(_data)); } + AssemblyItem const& append(bytes const& _data) { return append(newData(_data)); } AssemblyItem appendSubSize(Assembly const& _a) { auto ret = newSub(_a); append(newPushSubSize(ret.data())); return ret; } /// Pushes the final size of the current assembly itself. Use this when the code is modified /// after compilation and CODESIZE is not an option. @@ -119,6 +119,9 @@ public: void adjustDeposit(int _adjustment) { m_deposit += _adjustment; if (asserts(m_deposit >= 0)) BOOST_THROW_EXCEPTION(InvalidDeposit()); } void setDeposit(int _deposit) { m_deposit = _deposit; if (asserts(m_deposit >= 0)) BOOST_THROW_EXCEPTION(InvalidDeposit()); } + /// Changes the source location used for each appended item. + void setSourceLocation(SourceLocation const& _location) { m_currentSourceLocation = _location; } + bytes assemble() const; Assembly& optimise(bool _enable); std::ostream& streamRLP(std::ostream& _out, std::string const& _prefix = "", const StringMap &_sourceCodes = StringMap()) const; @@ -137,6 +140,8 @@ protected: int m_deposit = 0; int m_baseDeposit = 0; int m_totalDeposit = 0; + + SourceLocation m_currentSourceLocation; }; inline std::ostream& operator<<(std::ostream& _out, Assembly const& _a) diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index 5eeb0c3e2..7ff846bdb 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -132,7 +132,7 @@ void Compiler::packIntoContractCreator(ContractDefinition const& _contract, Comp void Compiler::appendBaseConstructor(FunctionDefinition const& _constructor) { - CompilerContext::LocationSetter locationSetter(m_context, &_constructor); + CompilerContext::LocationSetter locationSetter(m_context, _constructor); FunctionType constructorType(_constructor); if (!constructorType.getParameterTypes().empty()) { @@ -146,7 +146,7 @@ void Compiler::appendBaseConstructor(FunctionDefinition const& _constructor) void Compiler::appendConstructor(FunctionDefinition const& _constructor) { - CompilerContext::LocationSetter locationSetter(m_context, &_constructor); + CompilerContext::LocationSetter locationSetter(m_context, _constructor); // copy constructor arguments from code to memory and then to stack, they are supplied after the actual program unsigned argumentSize = 0; for (ASTPointer const& var: _constructor.getParameters()) @@ -192,10 +192,12 @@ void Compiler::appendFunctionSelector(ContractDefinition const& _contract) for (auto const& it: interfaceFunctions) { FunctionTypePointer const& functionType = it.second; + solAssert(functionType->hasDeclaration(), ""); + CompilerContext::LocationSetter locationSetter(m_context, functionType->getDeclaration()); m_context << callDataUnpackerEntryPoints.at(it.first); eth::AssemblyItem returnTag = m_context.pushNewTag(); appendCalldataUnpacker(functionType->getParameterTypes()); - m_context.appendJumpTo(m_context.getFunctionEntryLabel(it.second->getDeclaration())); + m_context.appendJumpTo(m_context.getFunctionEntryLabel(functionType->getDeclaration())); m_context << returnTag; appendReturnValuePacker(functionType->getReturnParameterTypes()); } @@ -286,7 +288,7 @@ void Compiler::initializeStateVariables(ContractDefinition const& _contract) bool Compiler::visit(VariableDeclaration const& _variableDeclaration) { solAssert(_variableDeclaration.isStateVariable(), "Compiler visit to non-state variable declaration."); - CompilerContext::LocationSetter locationSetter(m_context, &_variableDeclaration); + CompilerContext::LocationSetter locationSetter(m_context, _variableDeclaration); m_context.startFunction(_variableDeclaration); m_breakTags.clear(); @@ -300,7 +302,7 @@ bool Compiler::visit(VariableDeclaration const& _variableDeclaration) bool Compiler::visit(FunctionDefinition const& _function) { - CompilerContext::LocationSetter locationSetter(m_context, &_function); + CompilerContext::LocationSetter locationSetter(m_context, _function); //@todo to simplify this, the calling convention could by changed such that // caller puts: [retarg0] ... [retargm] [return address] [arg0] ... [argn] // although note that this reduces the size of the visible stack @@ -384,7 +386,7 @@ bool Compiler::visit(FunctionDefinition const& _function) bool Compiler::visit(IfStatement const& _ifStatement) { StackHeightChecker checker(m_context); - CompilerContext::LocationSetter locationSetter(m_context, &_ifStatement); + CompilerContext::LocationSetter locationSetter(m_context, _ifStatement); compileExpression(_ifStatement.getCondition()); eth::AssemblyItem trueTag = m_context.appendConditionalJump(); if (_ifStatement.getFalseStatement()) @@ -401,7 +403,7 @@ bool Compiler::visit(IfStatement const& _ifStatement) bool Compiler::visit(WhileStatement const& _whileStatement) { StackHeightChecker checker(m_context); - CompilerContext::LocationSetter locationSetter(m_context, &_whileStatement); + CompilerContext::LocationSetter locationSetter(m_context, _whileStatement); eth::AssemblyItem loopStart = m_context.newTag(); eth::AssemblyItem loopEnd = m_context.newTag(); m_continueTags.push_back(loopStart); @@ -427,7 +429,7 @@ bool Compiler::visit(WhileStatement const& _whileStatement) bool Compiler::visit(ForStatement const& _forStatement) { StackHeightChecker checker(m_context); - CompilerContext::LocationSetter locationSetter(m_context, &_forStatement); + CompilerContext::LocationSetter locationSetter(m_context, _forStatement); eth::AssemblyItem loopStart = m_context.newTag(); eth::AssemblyItem loopEnd = m_context.newTag(); m_continueTags.push_back(loopStart); @@ -464,7 +466,7 @@ bool Compiler::visit(ForStatement const& _forStatement) bool Compiler::visit(Continue const& _continueStatement) { - CompilerContext::LocationSetter locationSetter(m_context, &_continueStatement); + CompilerContext::LocationSetter locationSetter(m_context, _continueStatement); if (!m_continueTags.empty()) m_context.appendJumpTo(m_continueTags.back()); return false; @@ -472,7 +474,7 @@ bool Compiler::visit(Continue const& _continueStatement) bool Compiler::visit(Break const& _breakStatement) { - CompilerContext::LocationSetter locationSetter(m_context, &_breakStatement); + CompilerContext::LocationSetter locationSetter(m_context, _breakStatement); if (!m_breakTags.empty()) m_context.appendJumpTo(m_breakTags.back()); return false; @@ -480,7 +482,7 @@ bool Compiler::visit(Break const& _breakStatement) bool Compiler::visit(Return const& _return) { - CompilerContext::LocationSetter locationSetter(m_context, &_return); + CompilerContext::LocationSetter locationSetter(m_context, _return); //@todo modifications are needed to make this work with functions returning multiple values if (Expression const* expression = _return.getExpression()) { @@ -499,7 +501,7 @@ bool Compiler::visit(Return const& _return) bool Compiler::visit(VariableDeclarationStatement const& _variableDeclarationStatement) { StackHeightChecker checker(m_context); - CompilerContext::LocationSetter locationSetter(m_context, &_variableDeclarationStatement); + CompilerContext::LocationSetter locationSetter(m_context, _variableDeclarationStatement); if (Expression const* expression = _variableDeclarationStatement.getExpression()) { compileExpression(*expression, _variableDeclarationStatement.getDeclaration().getType()); @@ -512,7 +514,7 @@ bool Compiler::visit(VariableDeclarationStatement const& _variableDeclarationSta bool Compiler::visit(ExpressionStatement const& _expressionStatement) { StackHeightChecker checker(m_context); - CompilerContext::LocationSetter locationSetter(m_context, &_expressionStatement); + CompilerContext::LocationSetter locationSetter(m_context, _expressionStatement); Expression const& expression = _expressionStatement.getExpression(); compileExpression(expression); CompilerUtils(m_context).popStackElement(*expression.getType()); @@ -523,7 +525,7 @@ bool Compiler::visit(ExpressionStatement const& _expressionStatement) bool Compiler::visit(PlaceholderStatement const& _placeholderStatement) { StackHeightChecker checker(m_context); - CompilerContext::LocationSetter locationSetter(m_context, &_placeholderStatement); + CompilerContext::LocationSetter locationSetter(m_context, _placeholderStatement); ++m_modifierDepth; appendModifierOrFunctionCode(); --m_modifierDepth; @@ -550,7 +552,7 @@ void Compiler::appendModifierOrFunctionCode() } ModifierDefinition const& modifier = m_context.getFunctionModifier(modifierInvocation->getName()->getName()); - CompilerContext::LocationSetter locationSetter(m_context, &modifier); + CompilerContext::LocationSetter locationSetter(m_context, modifier); solAssert(modifier.getParameters().size() == modifierInvocation->getArguments().size(), ""); for (unsigned i = 0; i < modifier.getParameters().size(); ++i) { diff --git a/libsolidity/CompilerContext.cpp b/libsolidity/CompilerContext.cpp index ee8d3e00c..1dea62e93 100644 --- a/libsolidity/CompilerContext.cpp +++ b/libsolidity/CompilerContext.cpp @@ -69,7 +69,7 @@ void CompilerContext::removeVariable(VariableDeclaration const& _declaration) void CompilerContext::addAndInitializeVariable(VariableDeclaration const& _declaration) { - LocationSetter locationSetter(*this, &_declaration); + LocationSetter locationSetter(*this, _declaration); addVariable(_declaration); int const size = _declaration.getType()->getSizeOnStack(); for (int i = 0; i < size; ++i) @@ -182,34 +182,7 @@ void CompilerContext::resetVisitedNodes(ASTNode const* _node) stack newStack; newStack.push(_node); std::swap(m_visitedNodes, newStack); -} - -CompilerContext& CompilerContext::operator<<(eth::AssemblyItem const& _item) -{ - solAssert(!m_visitedNodes.empty(), "No node on the visited stack"); - m_asm.append(_item, m_visitedNodes.top()->getLocation()); - return *this; -} - -CompilerContext& CompilerContext::operator<<(eth::Instruction _instruction) -{ - solAssert(!m_visitedNodes.empty(), "No node on the visited stack"); - m_asm.append(_instruction, m_visitedNodes.top()->getLocation()); - return *this; -} - -CompilerContext& CompilerContext::operator<<(u256 const& _value) -{ - solAssert(!m_visitedNodes.empty(), "No node on the visited stack"); - m_asm.append(_value, m_visitedNodes.top()->getLocation()); - return *this; -} - -CompilerContext& CompilerContext::operator<<(bytes const& _data) -{ - solAssert(!m_visitedNodes.empty(), "No node on the visited stack"); - m_asm.append(_data, m_visitedNodes.top()->getLocation()); - return *this; + updateSourceLocation(); } vector::const_iterator CompilerContext::getSuperContract(ContractDefinition const& _contract) const @@ -220,5 +193,10 @@ vector::const_iterator CompilerContext::getSuperContr return ++it; } +void CompilerContext::updateSourceLocation() +{ + m_asm.setSourceLocation(m_visitedNodes.empty() ? SourceLocation() : m_visitedNodes.top()->getLocation()); +} + } } diff --git a/libsolidity/CompilerContext.h b/libsolidity/CompilerContext.h index e42e7c76c..3f31a1600 100644 --- a/libsolidity/CompilerContext.h +++ b/libsolidity/CompilerContext.h @@ -108,15 +108,15 @@ public: /// Resets the stack of visited nodes with a new stack having only @c _node void resetVisitedNodes(ASTNode const* _node); /// Pops the stack of visited nodes - void popVisitedNodes() { m_visitedNodes.pop(); } + void popVisitedNodes() { m_visitedNodes.pop(); updateSourceLocation(); } /// Pushes an ASTNode to the stack of visited nodes - void pushVisitedNodes(ASTNode const* _node) { m_visitedNodes.push(_node); } + void pushVisitedNodes(ASTNode const* _node) { m_visitedNodes.push(_node); updateSourceLocation(); } /// Append elements to the current instruction list and adjust @a m_stackOffset. - CompilerContext& operator<<(eth::AssemblyItem const& _item); - CompilerContext& operator<<(eth::Instruction _instruction); - CompilerContext& operator<<(u256 const& _value); - CompilerContext& operator<<(bytes const& _data); + CompilerContext& operator<<(eth::AssemblyItem const& _item) { m_asm.append(_item); return *this; } + CompilerContext& operator<<(eth::Instruction _instruction) { m_asm.append(_instruction); return *this; } + CompilerContext& operator<<(u256 const& _value) { m_asm.append(_value); return *this; } + CompilerContext& operator<<(bytes const& _data) { m_asm.append(_data); return *this; } eth::Assembly const& getAssembly() const { return m_asm; } /// @arg _sourceCodes is the map of input files to source code strings @@ -130,12 +130,15 @@ public: class LocationSetter: public ScopeGuard { public: - LocationSetter(CompilerContext& _compilerContext, ASTNode const* _node): - ScopeGuard(std::bind(&CompilerContext::popVisitedNodes, _compilerContext)) { _compilerContext.pushVisitedNodes(_node); } + LocationSetter(CompilerContext& _compilerContext, ASTNode const& _node): + ScopeGuard([&]{ _compilerContext.popVisitedNodes(); }) + { _compilerContext.pushVisitedNodes(&_node); } }; private: std::vector::const_iterator getSuperContract(const ContractDefinition &_contract) const; + /// Updates source location set in the assembly. + void updateSourceLocation(); eth::Assembly m_asm; /// Magic global variables like msg, tx or this, distinguished by type. diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index 3d7a25311..b02aecf5f 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -48,7 +48,7 @@ void ExpressionCompiler::appendStateVariableInitialization(VariableDeclaration c if (!_varDecl.getValue()) return; solAssert(!!_varDecl.getValue()->getType(), "Type information not available."); - CompilerContext::LocationSetter locationSetter(m_context, &_varDecl); + CompilerContext::LocationSetter locationSetter(m_context, _varDecl); _varDecl.getValue()->accept(*this); appendTypeConversion(*_varDecl.getValue()->getType(), *_varDecl.getType(), true); @@ -57,7 +57,7 @@ void ExpressionCompiler::appendStateVariableInitialization(VariableDeclaration c void ExpressionCompiler::appendStateVariableAccessor(VariableDeclaration const& _varDecl) { - CompilerContext::LocationSetter locationSetter(m_context, &_varDecl); + CompilerContext::LocationSetter locationSetter(m_context, _varDecl); FunctionType accessorType(_varDecl); unsigned length = 0; @@ -204,7 +204,7 @@ void ExpressionCompiler::appendTypeConversion(Type const& _typeOnStack, Type con bool ExpressionCompiler::visit(Assignment const& _assignment) { - CompilerContext::LocationSetter locationSetter(m_context, &_assignment); + CompilerContext::LocationSetter locationSetter(m_context, _assignment); _assignment.getRightHandSide().accept(*this); if (_assignment.getType()->isValueType()) appendTypeConversion(*_assignment.getRightHandSide().getType(), *_assignment.getType()); @@ -237,7 +237,7 @@ bool ExpressionCompiler::visit(Assignment const& _assignment) bool ExpressionCompiler::visit(UnaryOperation const& _unaryOperation) { - CompilerContext::LocationSetter locationSetter(m_context, &_unaryOperation); + CompilerContext::LocationSetter locationSetter(m_context, _unaryOperation); //@todo type checking and creating code for an operator should be in the same place: // the operator should know how to convert itself and to which types it applies, so // put this code together with "Type::acceptsBinary/UnaryOperator" into a class that @@ -307,7 +307,7 @@ bool ExpressionCompiler::visit(UnaryOperation const& _unaryOperation) bool ExpressionCompiler::visit(BinaryOperation const& _binaryOperation) { - CompilerContext::LocationSetter locationSetter(m_context, &_binaryOperation); + CompilerContext::LocationSetter locationSetter(m_context, _binaryOperation); Expression const& leftExpression = _binaryOperation.getLeftExpression(); Expression const& rightExpression = _binaryOperation.getRightExpression(); Type const& commonType = _binaryOperation.getCommonType(); @@ -354,7 +354,7 @@ bool ExpressionCompiler::visit(BinaryOperation const& _binaryOperation) bool ExpressionCompiler::visit(FunctionCall const& _functionCall) { - CompilerContext::LocationSetter locationSetter(m_context, &_functionCall); + CompilerContext::LocationSetter locationSetter(m_context, _functionCall); using Location = FunctionType::Location; if (_functionCall.isTypeConversion()) { @@ -572,7 +572,7 @@ bool ExpressionCompiler::visit(NewExpression const&) void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess) { - CompilerContext::LocationSetter locationSetter(m_context, &_memberAccess); + CompilerContext::LocationSetter locationSetter(m_context, _memberAccess); ASTString const& member = _memberAccess.getMemberName(); switch (_memberAccess.getExpression().getType()->getCategory()) { @@ -707,7 +707,7 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess) bool ExpressionCompiler::visit(IndexAccess const& _indexAccess) { - CompilerContext::LocationSetter locationSetter(m_context, &_indexAccess); + CompilerContext::LocationSetter locationSetter(m_context, _indexAccess); _indexAccess.getBaseExpression().accept(*this); Type const& baseType = *_indexAccess.getBaseExpression().getType(); @@ -821,6 +821,7 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess) void ExpressionCompiler::endVisit(Identifier const& _identifier) { + CompilerContext::LocationSetter locationSetter(m_context, _identifier); Declaration const* declaration = _identifier.getReferencedDeclaration(); if (MagicVariableDeclaration const* magicVar = dynamic_cast(declaration)) { @@ -853,6 +854,7 @@ void ExpressionCompiler::endVisit(Identifier const& _identifier) void ExpressionCompiler::endVisit(Literal const& _literal) { + CompilerContext::LocationSetter locationSetter(m_context, _literal); switch (_literal.getType()->getCategory()) { case Type::Category::IntegerConstant: From fb38209e9120f5ad2e4a4f0c386c888bfd60cf4a Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 9 Mar 2015 13:28:25 +0100 Subject: [PATCH 3/3] Style fixes. --- libsolidity/CompilerContext.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libsolidity/CompilerContext.h b/libsolidity/CompilerContext.h index 3f31a1600..4d63d8ba0 100644 --- a/libsolidity/CompilerContext.h +++ b/libsolidity/CompilerContext.h @@ -131,8 +131,7 @@ public: { public: LocationSetter(CompilerContext& _compilerContext, ASTNode const& _node): - ScopeGuard([&]{ _compilerContext.popVisitedNodes(); }) - { _compilerContext.pushVisitedNodes(&_node); } + ScopeGuard([&]{ _compilerContext.popVisitedNodes(); }) { _compilerContext.pushVisitedNodes(&_node); } }; private: