From 9fd0b74d94c18d9aa9689273e4dc16e65ed71e03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Tue, 10 Sep 2019 15:22:08 +0200 Subject: [PATCH 1/2] Drop op_table parameter in analysis() --- lib/evmone/analysis.cpp | 4 +- lib/evmone/analysis.hpp | 4 +- lib/evmone/execution.cpp | 5 +- lib/evmone/instructions.cpp | 9 ++- test/bench/bench.cpp | 4 +- test/unittests/analysis_test.cpp | 99 +++++++++++++++----------------- 6 files changed, 59 insertions(+), 66 deletions(-) diff --git a/lib/evmone/analysis.cpp b/lib/evmone/analysis.cpp index f6c6b8597d..0cd6712a20 100644 --- a/lib/evmone/analysis.cpp +++ b/lib/evmone/analysis.cpp @@ -15,9 +15,9 @@ inline constexpr uint64_t load64be(const unsigned char* data) noexcept (uint64_t{data[1]} << 48) | (uint64_t{data[0]} << 56); } -code_analysis analyze( - const exec_fn_table& fns, evmc_revision rev, const uint8_t* code, size_t code_size) noexcept +code_analysis analyze(evmc_revision rev, const uint8_t* code, size_t code_size) noexcept { + const auto& fns = get_op_table(rev); code_analysis analysis; const auto max_instrs_size = code_size + 1; diff --git a/lib/evmone/analysis.hpp b/lib/evmone/analysis.hpp index 91961bf6e8..a5de961a02 100644 --- a/lib/evmone/analysis.hpp +++ b/lib/evmone/analysis.hpp @@ -212,6 +212,8 @@ inline int find_jumpdest(const code_analysis& analysis, int offset) noexcept } EVMC_EXPORT code_analysis analyze( - const exec_fn_table& fns, evmc_revision rev, const uint8_t* code, size_t code_size) noexcept; + evmc_revision rev, const uint8_t* code, size_t code_size) noexcept; + +EVMC_EXPORT const exec_fn_table& get_op_table(evmc_revision rev) noexcept; } // namespace evmone diff --git a/lib/evmone/execution.cpp b/lib/evmone/execution.cpp index 0b440dc05a..b57ea84126 100644 --- a/lib/evmone/execution.cpp +++ b/lib/evmone/execution.cpp @@ -4,17 +4,14 @@ #include "execution.hpp" #include "analysis.hpp" - #include namespace evmone { -extern const exec_fn_table op_table[]; - evmc_result execute(evmc_instance*, evmc_context* ctx, evmc_revision rev, const evmc_message* msg, const uint8_t* code, size_t code_size) noexcept { - auto analysis = analyze(op_table[rev], rev, code, code_size); + auto analysis = analyze(rev, code, code_size); auto state = std::make_unique(); state->analysis = &analysis; diff --git a/lib/evmone/instructions.cpp b/lib/evmone/instructions.cpp index 04a26e88a2..8740cfb9e9 100644 --- a/lib/evmone/instructions.cpp +++ b/lib/evmone/instructions.cpp @@ -1367,9 +1367,8 @@ constexpr exec_fn_table create_op_table_istanbul() noexcept auto table = create_op_table_constantinople(); return table; } -} // namespace -extern const exec_fn_table op_table[] = { +constexpr exec_fn_table op_tables[] = { create_op_table_frontier(), // Frontier create_op_table_homestead(), // Homestead create_op_table_homestead(), // Tangerine Whistle @@ -1379,4 +1378,10 @@ extern const exec_fn_table op_table[] = { create_op_table_constantinople(), // Petersburg create_op_table_istanbul(), // Istanbul }; +} // namespace + +EVMC_EXPORT const exec_fn_table& get_op_table(evmc_revision rev) noexcept +{ + return op_tables[rev]; +} } // namespace evmone diff --git a/test/bench/bench.cpp b/test/bench/bench.cpp index e026f9188e..c12a555a2d 100644 --- a/test/bench/bench.cpp +++ b/test/bench/bench.cpp @@ -59,14 +59,12 @@ void execute(State& state, bytes_view code, bytes_view input) noexcept state.counters["gas_rate"] = Counter(static_cast(total_gas_used), Counter::kIsRate); } -constexpr auto fn_table = evmone::exec_fn_table{}; - void analyse(State& state, bytes_view code) noexcept { auto bytes_analysed = uint64_t{0}; for (auto _ : state) { - auto r = evmone::analyze(fn_table, EVMC_PETERSBURG, code.data(), code.size()); + auto r = evmone::analyze(EVMC_PETERSBURG, code.data(), code.size()); DoNotOptimize(r); bytes_analysed += code.size(); } diff --git a/test/unittests/analysis_test.cpp b/test/unittests/analysis_test.cpp index 1ded47036d..8d0cbe1516 100644 --- a/test/unittests/analysis_test.cpp +++ b/test/unittests/analysis_test.cpp @@ -11,33 +11,24 @@ using namespace evmone; constexpr auto rev = EVMC_BYZANTIUM; - -const auto fake_fn_table = []() noexcept -{ - evmone::exec_fn_table fns; - for (size_t i = 0; i < fns.size(); ++i) - fns[i] = (evmone::exec_fn)i; - return fns; -} -(); - +const auto& op_table = get_op_table(rev); TEST(analysis, example1) { const auto code = push(0x2a) + push(0x1e) + OP_MSTORE8 + OP_MSIZE + push(0) + OP_SSTORE; - const auto analysis = analyze(fake_fn_table, rev, &code[0], code.size()); + const auto analysis = analyze(rev, &code[0], code.size()); ASSERT_EQ(analysis.instrs.size(), 8); - EXPECT_EQ(analysis.instrs[0].fn, fake_fn_table[OPX_BEGINBLOCK]); + EXPECT_EQ(analysis.instrs[0].fn, op_table[OPX_BEGINBLOCK]); EXPECT_EQ(analysis.instrs[0].arg.number, 0); - EXPECT_EQ(analysis.instrs[1].fn, fake_fn_table[OP_PUSH1]); - EXPECT_EQ(analysis.instrs[2].fn, fake_fn_table[OP_PUSH1]); - EXPECT_EQ(analysis.instrs[3].fn, fake_fn_table[OP_MSTORE8]); - EXPECT_EQ(analysis.instrs[4].fn, fake_fn_table[OP_MSIZE]); - EXPECT_EQ(analysis.instrs[5].fn, fake_fn_table[OP_PUSH1]); - EXPECT_EQ(analysis.instrs[6].fn, fake_fn_table[OP_SSTORE]); - EXPECT_EQ(analysis.instrs[7].fn, fake_fn_table[OP_STOP]); + EXPECT_EQ(analysis.instrs[1].fn, op_table[OP_PUSH1]); + EXPECT_EQ(analysis.instrs[2].fn, op_table[OP_PUSH1]); + EXPECT_EQ(analysis.instrs[3].fn, op_table[OP_MSTORE8]); + EXPECT_EQ(analysis.instrs[4].fn, op_table[OP_MSIZE]); + EXPECT_EQ(analysis.instrs[5].fn, op_table[OP_PUSH1]); + EXPECT_EQ(analysis.instrs[6].fn, op_table[OP_SSTORE]); + EXPECT_EQ(analysis.instrs[7].fn, op_table[OP_STOP]); ASSERT_EQ(analysis.blocks.size(), 1); EXPECT_EQ(analysis.blocks[0].gas_cost, 14); @@ -48,15 +39,15 @@ TEST(analysis, example1) TEST(analysis, stack_up_and_down) { const auto code = OP_DUP2 + 6 * OP_DUP1 + 10 * OP_POP + push(0); - const auto analysis = analyze(fake_fn_table, rev, &code[0], code.size()); + const auto analysis = analyze(rev, &code[0], code.size()); ASSERT_EQ(analysis.instrs.size(), 20); - EXPECT_EQ(analysis.instrs[0].fn, fake_fn_table[OPX_BEGINBLOCK]); + EXPECT_EQ(analysis.instrs[0].fn, op_table[OPX_BEGINBLOCK]); EXPECT_EQ(analysis.instrs[0].arg.number, 0); - EXPECT_EQ(analysis.instrs[1].fn, fake_fn_table[OP_DUP2]); - EXPECT_EQ(analysis.instrs[2].fn, fake_fn_table[OP_DUP1]); - EXPECT_EQ(analysis.instrs[8].fn, fake_fn_table[OP_POP]); - EXPECT_EQ(analysis.instrs[18].fn, fake_fn_table[OP_PUSH1]); + EXPECT_EQ(analysis.instrs[1].fn, op_table[OP_DUP2]); + EXPECT_EQ(analysis.instrs[2].fn, op_table[OP_DUP1]); + EXPECT_EQ(analysis.instrs[8].fn, op_table[OP_POP]); + EXPECT_EQ(analysis.instrs[18].fn, op_table[OP_PUSH1]); ASSERT_EQ(analysis.blocks.size(), 1); EXPECT_EQ(analysis.blocks[0].gas_cost, 7 * 3 + 10 * 2 + 3); @@ -68,11 +59,11 @@ TEST(analysis, push) { constexpr auto push_value = 0x8807060504030201; const auto code = push(push_value) + "7f00ee"; - const auto analysis = analyze(fake_fn_table, rev, &code[0], code.size()); + const auto analysis = analyze(rev, &code[0], code.size()); ASSERT_EQ(analysis.instrs.size(), 4); ASSERT_EQ(analysis.push_values.size(), 1); - EXPECT_EQ(analysis.instrs[0].fn, fake_fn_table[OPX_BEGINBLOCK]); + EXPECT_EQ(analysis.instrs[0].fn, op_table[OPX_BEGINBLOCK]); EXPECT_EQ(analysis.instrs[1].arg.small_push_value, push_value); EXPECT_EQ(analysis.instrs[2].arg.push_value, &analysis.push_values[0]); EXPECT_EQ(analysis.push_values[0], intx::uint256{0xee} << 240); @@ -84,20 +75,20 @@ TEST(analysis, jumpdest_skip) // and no new block should be created in this place. const auto code = bytecode{} + OP_STOP + OP_JUMPDEST; - auto analysis = evmone::analyze(fake_fn_table, rev, &code[0], code.size()); + auto analysis = evmone::analyze(rev, &code[0], code.size()); EXPECT_EQ(analysis.blocks.size(), 2); ASSERT_EQ(analysis.instrs.size(), 4); - EXPECT_EQ(analysis.instrs[0].fn, fake_fn_table[OPX_BEGINBLOCK]); - EXPECT_EQ(analysis.instrs[1].fn, fake_fn_table[OP_STOP]); - EXPECT_EQ(analysis.instrs[2].fn, fake_fn_table[OP_JUMPDEST]); - EXPECT_EQ(analysis.instrs[3].fn, fake_fn_table[OP_STOP]); + EXPECT_EQ(analysis.instrs[0].fn, op_table[OPX_BEGINBLOCK]); + EXPECT_EQ(analysis.instrs[1].fn, op_table[OP_STOP]); + EXPECT_EQ(analysis.instrs[2].fn, op_table[OP_JUMPDEST]); + EXPECT_EQ(analysis.instrs[3].fn, op_table[OP_STOP]); } TEST(analysis, jump1) { const auto code = jump(add(4, 2)) + OP_JUMPDEST + mstore(0, 3) + ret(0, 0x20) + jump(6); - const auto analysis = analyze(fake_fn_table, rev, &code[0], code.size()); + const auto analysis = analyze(rev, &code[0], code.size()); ASSERT_EQ(analysis.blocks.size(), 3); ASSERT_EQ(analysis.jumpdest_offsets.size(), 1); @@ -112,17 +103,17 @@ TEST(analysis, jump1) TEST(analysis, empty) { bytes code; - auto analysis = evmone::analyze(fake_fn_table, rev, &code[0], code.size()); + auto analysis = evmone::analyze(rev, &code[0], code.size()); EXPECT_EQ(analysis.blocks.size(), 0); ASSERT_EQ(analysis.instrs.size(), 1); - EXPECT_EQ(analysis.instrs[0].fn, fake_fn_table[OP_STOP]); + EXPECT_EQ(analysis.instrs[0].fn, op_table[OP_STOP]); } TEST(analysis, only_jumpdest) { const auto code = bytecode{OP_JUMPDEST}; - auto analysis = evmone::analyze(fake_fn_table, rev, &code[0], code.size()); + auto analysis = evmone::analyze(rev, &code[0], code.size()); ASSERT_EQ(analysis.blocks.size(), 1); ASSERT_EQ(analysis.jumpdest_offsets.size(), 1); @@ -134,13 +125,13 @@ TEST(analysis, only_jumpdest) TEST(analysis, jumpi_at_the_end) { const auto code = bytecode{OP_JUMPI}; - auto analysis = evmone::analyze(fake_fn_table, rev, &code[0], code.size()); + auto analysis = evmone::analyze(rev, &code[0], code.size()); EXPECT_EQ(analysis.blocks.size(), 1); ASSERT_EQ(analysis.instrs.size(), 3); - EXPECT_EQ(analysis.instrs[0].fn, fake_fn_table[OPX_BEGINBLOCK]); - EXPECT_EQ(analysis.instrs[1].fn, fake_fn_table[OP_JUMPI]); - EXPECT_EQ(analysis.instrs[2].fn, fake_fn_table[OP_STOP]); + EXPECT_EQ(analysis.instrs[0].fn, op_table[OPX_BEGINBLOCK]); + EXPECT_EQ(analysis.instrs[1].fn, op_table[OP_JUMPI]); + EXPECT_EQ(analysis.instrs[2].fn, op_table[OP_STOP]); } TEST(analysis, terminated_last_block) @@ -148,31 +139,31 @@ TEST(analysis, terminated_last_block) // TODO: Even if the last basic block is properly terminated an additional artificial block // is going to be created with only STOP instruction. const auto code = ret(0, 0); - auto analysis = evmone::analyze(fake_fn_table, rev, &code[0], code.size()); + auto analysis = evmone::analyze(rev, &code[0], code.size()); EXPECT_EQ(analysis.blocks.size(), 1); ASSERT_EQ(analysis.instrs.size(), 4); - EXPECT_EQ(analysis.instrs[0].fn, fake_fn_table[OPX_BEGINBLOCK]); - EXPECT_EQ(analysis.instrs[3].fn, fake_fn_table[OP_RETURN]); + EXPECT_EQ(analysis.instrs[0].fn, op_table[OPX_BEGINBLOCK]); + EXPECT_EQ(analysis.instrs[3].fn, op_table[OP_RETURN]); } TEST(analysis, jumpdests_groups) { const auto code = 3 * OP_JUMPDEST + push(1) + 3 * OP_JUMPDEST + push(2) + OP_JUMPI; - auto analysis = evmone::analyze(fake_fn_table, rev, &code[0], code.size()); + auto analysis = evmone::analyze(rev, &code[0], code.size()); EXPECT_EQ(analysis.blocks.size(), 6); ASSERT_EQ(analysis.instrs.size(), 10); - EXPECT_EQ(analysis.instrs[0].fn, fake_fn_table[OP_JUMPDEST]); - EXPECT_EQ(analysis.instrs[1].fn, fake_fn_table[OP_JUMPDEST]); - EXPECT_EQ(analysis.instrs[2].fn, fake_fn_table[OP_JUMPDEST]); - EXPECT_EQ(analysis.instrs[3].fn, fake_fn_table[OP_PUSH1]); - EXPECT_EQ(analysis.instrs[4].fn, fake_fn_table[OP_JUMPDEST]); - EXPECT_EQ(analysis.instrs[5].fn, fake_fn_table[OP_JUMPDEST]); - EXPECT_EQ(analysis.instrs[6].fn, fake_fn_table[OP_JUMPDEST]); - EXPECT_EQ(analysis.instrs[7].fn, fake_fn_table[OP_PUSH1]); - EXPECT_EQ(analysis.instrs[8].fn, fake_fn_table[OP_JUMPI]); - EXPECT_EQ(analysis.instrs[9].fn, fake_fn_table[OP_STOP]); + EXPECT_EQ(analysis.instrs[0].fn, op_table[OP_JUMPDEST]); + EXPECT_EQ(analysis.instrs[1].fn, op_table[OP_JUMPDEST]); + EXPECT_EQ(analysis.instrs[2].fn, op_table[OP_JUMPDEST]); + EXPECT_EQ(analysis.instrs[3].fn, op_table[OP_PUSH1]); + EXPECT_EQ(analysis.instrs[4].fn, op_table[OP_JUMPDEST]); + EXPECT_EQ(analysis.instrs[5].fn, op_table[OP_JUMPDEST]); + EXPECT_EQ(analysis.instrs[6].fn, op_table[OP_JUMPDEST]); + EXPECT_EQ(analysis.instrs[7].fn, op_table[OP_PUSH1]); + EXPECT_EQ(analysis.instrs[8].fn, op_table[OP_JUMPI]); + EXPECT_EQ(analysis.instrs[9].fn, op_table[OP_STOP]); ASSERT_EQ(analysis.jumpdest_offsets.size(), 6); From fb086331f903ed3507346108ef191724a442499f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Tue, 10 Sep 2019 15:47:35 +0200 Subject: [PATCH 2/2] Check number of entries in op_tables --- lib/evmone/instructions.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/evmone/instructions.cpp b/lib/evmone/instructions.cpp index 8740cfb9e9..0d22510518 100644 --- a/lib/evmone/instructions.cpp +++ b/lib/evmone/instructions.cpp @@ -1378,6 +1378,8 @@ constexpr exec_fn_table op_tables[] = { create_op_table_constantinople(), // Petersburg create_op_table_istanbul(), // Istanbul }; +static_assert(sizeof(op_tables) / sizeof(op_tables[0]) > EVMC_MAX_REVISION, + "op table entry missing for an EVMC revision"); } // namespace EVMC_EXPORT const exec_fn_table& get_op_table(evmc_revision rev) noexcept