Merge pull request #500 from chriseth/sol_jumptable

Replace function selector jump table by more resilient linear time check.

libsolidity/Compiler.cpp

@@ -81,36 +81,34 @@ void Compiler::appendFunctionSelector(vector<ASTPointer<FunctionDefinition>> con
 	if (publicFunctions.size() > 255)
 		BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("More than 255 public functions for contract."));
 
-	//@todo check for calldatasize?
-	// retrieve the first byte of the call data
-	m_context << u256(0) << eth::Instruction::CALLDATALOAD << u256(0) << eth::Instruction::BYTE;
-	// check that it is not too large
-	m_context << eth::Instruction::DUP1 << u256(publicFunctions.size() - 1) << eth::Instruction::LT;
-	eth::AssemblyItem returnTag = m_context.appendConditionalJump();
-
-	// otherwise, jump inside jump table (each entry of the table has size 4)
-	m_context << u256(4) << eth::Instruction::MUL;
-	eth::AssemblyItem jumpTableStart = m_context.pushNewTag();
-	m_context << eth::Instruction::ADD << eth::Instruction::JUMP;
-
-	// jump table, tell the optimizer not to remove the JUMPDESTs
-	m_context << eth::AssemblyItem(eth::NoOptimizeBegin) << jumpTableStart;
+	// retrieve the first byte of the call data, which determines the called function
+	// @todo This code had a jump table in a previous version which was more efficient but also
+	// error prone (due to the optimizer and variable length tag addresses)
+	m_context << u256(1) << u256(0) // some constants
+			  << eth::dupInstruction(1) << eth::Instruction::CALLDATALOAD
+			  << eth::dupInstruction(2) << eth::Instruction::BYTE
+			  << eth::dupInstruction(2);
+
+	// stack here: 1 0 <funid> 0, stack top will be counted up until it matches funid
 	for (pair<string, pair<FunctionDefinition const*, eth::AssemblyItem>> const& f: publicFunctions)
-		m_context.appendJumpTo(f.second.second) << eth::Instruction::JUMPDEST;
-	m_context << eth::AssemblyItem(eth::NoOptimizeEnd);
-
-	m_context << returnTag << eth::Instruction::STOP;
+	{
+		eth::AssemblyItem const& callDataUnpackerEntry = f.second.second;
+		m_context << eth::dupInstruction(2) << eth::dupInstruction(2) << eth::Instruction::EQ;
+		m_context.appendConditionalJumpTo(callDataUnpackerEntry);
+		m_context << eth::dupInstruction(4) << eth::Instruction::ADD;
+		//@todo avoid the last ADD (or remove it in the optimizer)
+	}
+	m_context << eth::Instruction::STOP; // function not found
 
 	for (pair<string, pair<FunctionDefinition const*, eth::AssemblyItem>> const& f: publicFunctions)
 	{
 		FunctionDefinition const& function = *f.second.first;
-		m_context << f.second.second;
-
+		eth::AssemblyItem const& callDataUnpackerEntry = f.second.second;
+		m_context << callDataUnpackerEntry;
 		eth::AssemblyItem returnTag = m_context.pushNewTag();
 		appendCalldataUnpacker(function);
 		m_context.appendJumpTo(m_context.getFunctionEntryLabel(function));
 		m_context << returnTag;
-
 		appendReturnValuePacker(function);
 	}
 }

test/solidityCompiler.cpp

@@ -80,7 +80,7 @@ BOOST_AUTO_TEST_CASE(smoke_test)
 							 "}\n";
 	bytes code = compileContract(sourceCode);
 
-	unsigned boilerplateSize = 51;
+	unsigned boilerplateSize = 42;
 	bytes expectation({byte(Instruction::JUMPDEST),
 					   byte(Instruction::PUSH1), 0x0, // initialize local variable x
 					   byte(Instruction::PUSH1), 0x2,
@@ -100,8 +100,8 @@ BOOST_AUTO_TEST_CASE(different_argument_numbers)
 							 "}\n";
 	bytes code = compileContract(sourceCode);
 
-	unsigned shift = 75;
-	unsigned boilerplateSize = 88;
+	unsigned shift = 70;
+	unsigned boilerplateSize = 83;
 	bytes expectation({byte(Instruction::JUMPDEST),
 					   byte(Instruction::PUSH1), 0x0, // initialize return variable d
 					   byte(Instruction::DUP3),
@@ -153,8 +153,8 @@ BOOST_AUTO_TEST_CASE(ifStatement)
 							 "}\n";
 	bytes code = compileContract(sourceCode);
 
-	unsigned shift = 38;
-	unsigned boilerplateSize = 51;
+	unsigned shift = 29;
+	unsigned boilerplateSize = 42;
 	bytes expectation({byte(Instruction::JUMPDEST),
 					   byte(Instruction::PUSH1), 0x0,
 					   byte(Instruction::DUP1),
@@ -195,8 +195,8 @@ BOOST_AUTO_TEST_CASE(loops)
 							 "}\n";
 	bytes code = compileContract(sourceCode);
 
-	unsigned shift = 38;
-	unsigned boilerplateSize = 51;
+	unsigned shift = 29;
+	unsigned boilerplateSize = 42;
 	bytes expectation({byte(Instruction::JUMPDEST),
 					   byte(Instruction::JUMPDEST),
 					   byte(Instruction::PUSH1), 0x1,

test/solidityEndToEndTest.cpp

@@ -148,6 +148,22 @@ BOOST_AUTO_TEST_CASE(recursive_calls)
 	BOOST_CHECK(testSolidityAgainstCpp(0, recursive_calls_cpp, u256(4)));
 }
 
+BOOST_AUTO_TEST_CASE(multiple_functions)
+{
+	char const* sourceCode = "contract test {\n"
+							 "  function a() returns(uint n) { return 0; }\n"
+							 "  function b() returns(uint n) { return 1; }\n"
+							 "  function c() returns(uint n) { return 2; }\n"
+							 "  function f() returns(uint n) { return 3; }\n"
+							 "}\n";
+	compileAndRun(sourceCode);
+	BOOST_CHECK(callFunction(0, bytes()) == toBigEndian(u256(0)));
+	BOOST_CHECK(callFunction(1, bytes()) == toBigEndian(u256(1)));
+	BOOST_CHECK(callFunction(2, bytes()) == toBigEndian(u256(2)));
+	BOOST_CHECK(callFunction(3, bytes()) == toBigEndian(u256(3)));
+	BOOST_CHECK(callFunction(4, bytes()) == bytes());
+}
+
 BOOST_AUTO_TEST_CASE(while_loop)
 {
 	char const* sourceCode = "contract test {\n"