From f5730edddb632a2c35cd24dbc2b1747f9cbde0ae Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 10 Nov 2014 13:13:40 +0100 Subject: [PATCH] Replace function selector jump table by more resilient linear time check. --- libsolidity/Compiler.cpp | 39 +++++++++++++++++------------------ test/solidityCompiler.cpp | 14 ++++++------- test/solidityEndToEndTest.cpp | 16 ++++++++++++++ 3 files changed, 42 insertions(+), 27 deletions(-) diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index ce87f7bb0..c5e25a120 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -81,35 +81,34 @@ void Compiler::appendFunctionSelector(vector> 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 @todo it could be that the optimizer destroys this - m_context << 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 0, stack top will be counted up until it matches funid for (pair> const& f: publicFunctions) - m_context.appendJumpTo(f.second.second) << eth::Instruction::JUMPDEST; - - 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> 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); } } diff --git a/test/solidityCompiler.cpp b/test/solidityCompiler.cpp index ba2db67e6..054ad3297 100644 --- a/test/solidityCompiler.cpp +++ b/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, diff --git a/test/solidityEndToEndTest.cpp b/test/solidityEndToEndTest.cpp index 796adcb15..788c388e4 100644 --- a/test/solidityEndToEndTest.cpp +++ b/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"