Fixed: Some instructions did not have source locations.

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();
 }
 

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)

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<VariableDeclaration> 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)
 		{

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<ASTNode const*> 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<ContractDefinition const*>::const_iterator CompilerContext::getSuperContract(ContractDefinition const& _contract) const
@@ -220,5 +193,10 @@ vector<ContractDefinition const*>::const_iterator CompilerContext::getSuperContr
 	return ++it;
 }
 
+void CompilerContext::updateSourceLocation()
+{
+	m_asm.setSourceLocation(m_visitedNodes.empty() ? SourceLocation() : m_visitedNodes.top()->getLocation());
+}
+
 }
 }

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<ContractDefinition const*>::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.

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<MagicVariableDeclaration const*>(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: