Merge pull request #545 from chriseth/accessFunLabels

Allow access to functions in inline assembly.

6 files changed, 62 insertions, 21 deletions

diff --git a/docs/control-structures.rst b/docs/control-structures.rst
index 249b00b0..1bb0f71a 100644
--- a/docs/control-structures.rst
+++ b/docs/control-structures.rst
@@ -509,8 +509,8 @@ Note that the order of arguments is reversed in functional-style as opposed to t
 way. If you use functional-style, the first argument will end up on the stack top.
 
 
-Access to External Variables
-----------------------------
+Access to External Variables and Functions
+------------------------------------------
 
 Solidity variables and other identifiers can be accessed by simply using their name.
 For storage and memory variables, this will push the address and not the value onto the
@@ -518,6 +518,17 @@ stack. Also note that non-struct and non-array storage variable addresses occupy
 on the stack: One for the address and one for the byte offset inside the storage slot.
 In assignments (see below), we can even use local Solidity variables to assign to.
 
+Functions external to inline assembly can also be accessed: The assembly will
+push their entry label (with virtual function resolution applied). The calling semantics
+in solidity are:
+
+ - the caller pushes return label, arg1, arg2, ..., argn
+ - the call returns with ret1, ret2, ..., retn
+
+This feature is still a bit cumbersome to use, because the stack offset essentially
+changes during the call, and thus references to local variables will be wrong.
+It is planned that the stack height changes can be specified in inline assembly.
+
 .. code::
 
 	contract c {
diff --git a/libsolidity/codegen/Compiler.cpp b/libsolidity/codegen/Compiler.cpp
index 1675f659..ebf61266 100644
--- a/libsolidity/codegen/Compiler.cpp
+++ b/libsolidity/codegen/Compiler.cpp
@@ -502,8 +502,9 @@ bool Compiler::visit(InlineAssembly const& _inlineAssembly)
 {
 	ErrorList errors;
 	assembly::CodeGenerator codeGen(_inlineAssembly.operations(), errors);
-	int startStackHeight = m_context.stackHeight();
-	m_context.appendInlineAssembly(codeGen.assemble(
+	unsigned startStackHeight = m_context.stackHeight();
+	codeGen.assemble(
+		m_context.nonConstAssembly(),
 		[&](assembly::Identifier const& _identifier, eth::Assembly& _assembly, assembly::CodeGenerator::IdentifierContext _context) {
 			auto ref = _inlineAssembly.annotation().externalReferences.find(&_identifier);
 			if (ref == _inlineAssembly.annotation().externalReferences.end())
@@ -513,19 +514,14 @@ bool Compiler::visit(InlineAssembly const& _inlineAssembly)
 			if (_context == assembly::CodeGenerator::IdentifierContext::RValue)
 			{
 				solAssert(!!decl->type(), "Type of declaration required but not yet determined.");
-				if (/*FunctionDefinition const* functionDef = */dynamic_cast<FunctionDefinition const*>(decl))
-				{
-					solAssert(false, "Referencing local functions in inline assembly not yet implemented.");
-					// This does not work directly, because the label does not exist in _assembly
-					// (it is a fresh assembly object).
-					// _assembly.append(m_context.virtualFunctionEntryLabel(*functionDef).pushTag());
-				}
+				if (FunctionDefinition const* functionDef = dynamic_cast<FunctionDefinition const*>(decl))
+					_assembly.append(m_context.virtualFunctionEntryLabel(*functionDef).pushTag());
 				else if (auto variable = dynamic_cast<VariableDeclaration const*>(decl))
 				{
 					solAssert(!variable->isConstant(), "");
 					if (m_context.isLocalVariable(variable))
 					{
-						int stackDiff = _assembly.deposit() + startStackHeight - m_context.baseStackOffsetOfVariable(*variable);
+						int stackDiff = _assembly.deposit() - m_context.baseStackOffsetOfVariable(*variable);
 						if (stackDiff < 1 || stackDiff > 16)
 							BOOST_THROW_EXCEPTION(
 								CompilerError() <<
@@ -565,7 +561,7 @@ bool Compiler::visit(InlineAssembly const& _inlineAssembly)
 					"Can only assign to stack variables in inline assembly."
 				);
 				unsigned size = variable->type()->sizeOnStack();
-				int stackDiff = _assembly.deposit() + startStackHeight - m_context.baseStackOffsetOfVariable(*variable) - size;
+				int stackDiff = _assembly.deposit() - m_context.baseStackOffsetOfVariable(*variable) - size;
 				if (stackDiff > 16 || stackDiff < 1)
 					BOOST_THROW_EXCEPTION(
 						CompilerError() <<
@@ -578,8 +574,9 @@ bool Compiler::visit(InlineAssembly const& _inlineAssembly)
 			}
 			return true;
 		}
-	));
+	);
 	solAssert(errors.empty(), "Code generation for inline assembly with errors requested.");
+	m_context.setStackOffset(startStackHeight);
 	return false;
 }
 
diff --git a/libsolidity/codegen/CompilerContext.h b/libsolidity/codegen/CompilerContext.h
index 5abe59fe..a56335ce 100644
--- a/libsolidity/codegen/CompilerContext.h
+++ b/libsolidity/codegen/CompilerContext.h
@@ -113,8 +113,6 @@ public:
 	/// Adds a subroutine to the code (in the data section) and pushes its size (via a tag)
 	/// on the stack. @returns the assembly item corresponding to the pushed subroutine, i.e. its offset.
 	eth::AssemblyItem addSubroutine(eth::Assembly const& _assembly) { return m_asm.appendSubSize(_assembly); }
-	/// Appends the given code (used by inline assembly) ignoring any stack height changes.
-	void appendInlineAssembly(eth::Assembly const& _assembly) { int deposit = m_asm.deposit(); m_asm.append(_assembly); m_asm.setDeposit(deposit); }
 	/// Pushes the size of the final program
 	void appendProgramSize() { return m_asm.appendProgramSize(); }
 	/// Adds data to the data section, pushes a reference to the stack
@@ -140,6 +138,10 @@ public:
 	void optimise(unsigned _runs = 200) { m_asm.optimise(true, true, _runs); }
 
 	eth::Assembly const& assembly() const { return m_asm; }
+	/// @returns non-const reference to the underlying assembly. Should be avoided in favour of
+	/// wrappers in this class.
+	eth::Assembly& nonConstAssembly() { return m_asm; }
+
 	/// @arg _sourceCodes is the map of input files to source code strings
 	/// @arg _inJsonFormat shows whether the out should be in Json format
 	Json::Value streamAssembly(std::ostream& _stream, StringMap const& _sourceCodes = StringMap(), bool _inJsonFormat = false) const
diff --git a/libsolidity/inlineasm/AsmCodeGen.cpp b/libsolidity/inlineasm/AsmCodeGen.cpp
index d571ce0d..53d19b0a 100644
--- a/libsolidity/inlineasm/AsmCodeGen.cpp
+++ b/libsolidity/inlineasm/AsmCodeGen.cpp
@@ -36,7 +36,8 @@ using namespace dev::solidity::assembly;
 
 struct GeneratorState
 {
-	explicit GeneratorState(ErrorList& _errors): errors(_errors) {}
+	GeneratorState(ErrorList& _errors, eth::Assembly& _assembly):
+		errors(_errors), assembly(_assembly) {}
 
 	void addError(Error::Type _type, std::string const& _description, SourceLocation const& _location = SourceLocation())
 	{
@@ -66,10 +67,10 @@ struct GeneratorState
 		return label != labels.end() ? &label->second : nullptr;
 	}
 
-	eth::Assembly assembly;
 	map<string, eth::AssemblyItem> labels;
 	vector<pair<string, int>> variables; ///< name plus stack height
 	ErrorList& errors;
+	eth::Assembly& assembly;
 };
 
 /**
@@ -267,7 +268,8 @@ private:
 bool assembly::CodeGenerator::typeCheck(assembly::CodeGenerator::IdentifierAccess const& _identifierAccess)
 {
 	size_t initialErrorLen = m_errors.size();
-	GeneratorState state(m_errors);
+	eth::Assembly assembly;
+	GeneratorState state(m_errors, assembly);
 	(LabelOrganizer(state))(m_parsedData);
 	(CodeTransform(state, _identifierAccess))(m_parsedData);
 	return m_errors.size() == initialErrorLen;
@@ -275,9 +277,17 @@ bool assembly::CodeGenerator::typeCheck(assembly::CodeGenerator::IdentifierAcces
 
 eth::Assembly assembly::CodeGenerator::assemble(assembly::CodeGenerator::IdentifierAccess const& _identifierAccess)
 {
-	GeneratorState state(m_errors);
+	eth::Assembly assembly;
+	GeneratorState state(m_errors, assembly);
+	(LabelOrganizer(state))(m_parsedData);
+	(CodeTransform(state, _identifierAccess))(m_parsedData);
+	return assembly;
+}
+
+void assembly::CodeGenerator::assemble(eth::Assembly& _assembly, assembly::CodeGenerator::IdentifierAccess const& _identifierAccess)
+{
+	GeneratorState state(m_errors, _assembly);
 	(LabelOrganizer(state))(m_parsedData);
 	(CodeTransform(state, _identifierAccess))(m_parsedData);
-	return state.assembly;
 }
 
diff --git a/libsolidity/inlineasm/AsmCodeGen.h b/libsolidity/inlineasm/AsmCodeGen.h
index aaabda45..b1fafe15 100644
--- a/libsolidity/inlineasm/AsmCodeGen.h
+++ b/libsolidity/inlineasm/AsmCodeGen.h
@@ -55,6 +55,8 @@ public:
 	bool typeCheck(IdentifierAccess const& _identifierAccess = IdentifierAccess());
 	/// Performs code generation and @returns the result.
 	eth::Assembly assemble(IdentifierAccess const& _identifierAccess = IdentifierAccess());
+	/// Performs code generation and appends generated to to _assembly.
+	void assemble(eth::Assembly& _assembly, IdentifierAccess const& _identifierAccess = IdentifierAccess());
 
 private:
 	Block const& m_parsedData;
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 43bdc5a9..8c537346 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -6595,6 +6595,25 @@ BOOST_AUTO_TEST_CASE(inline_assembly_jumps)
 	BOOST_CHECK(callContractFunction("f()", u256(7)) == encodeArgs(u256(34)));
 }
 
+BOOST_AUTO_TEST_CASE(inline_assembly_function_access)
+{
+	char const* sourceCode = R"(
+		contract C {
+			uint public x;
+			function g(uint y) { x = 2 * y; assembly { stop } }
+			function f(uint _x) {
+				assembly {
+					_x
+					jump(g)
+				}
+			}
+		}
+	)";
+	compileAndRun(sourceCode, 0, "C");
+	BOOST_CHECK(callContractFunction("f(uint256)", u256(5)) == encodeArgs());
+	BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(10)));
+}
+
 BOOST_AUTO_TEST_CASE(index_access_with_type_conversion)
 {
 	// Test for a bug where higher order bits cleanup was not done for array index access.