From fdc1573b79525244f90c45c4d859235b52ae63a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Tue, 2 Jul 2019 13:14:44 +0200 Subject: [PATCH 1/3] Do not export and allow inlining of find_jumpdest --- lib/evmone/analysis.cpp | 11 ----------- lib/evmone/analysis.hpp | 14 +++++++++++--- lib/evmone/instructions.cpp | 2 +- test/unittests/analysis_test.cpp | 4 ++-- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/lib/evmone/analysis.cpp b/lib/evmone/analysis.cpp index b7cea2a910..a3047a7567 100644 --- a/lib/evmone/analysis.cpp +++ b/lib/evmone/analysis.cpp @@ -17,17 +17,6 @@ bool is_terminator(uint8_t c) noexcept } } // namespace -int code_analysis::find_jumpdest(int offset) const noexcept -{ - // TODO: Replace with lower_bound(). - for (const auto& d : jumpdest_map) - { - if (d.first == offset) - return d.second; - } - return -1; -} - evmc_call_kind op2call_kind(uint8_t opcode) noexcept { switch (opcode) diff --git a/lib/evmone/analysis.hpp b/lib/evmone/analysis.hpp index ef15eda4bb..9d1bf9dd86 100644 --- a/lib/evmone/analysis.hpp +++ b/lib/evmone/analysis.hpp @@ -153,11 +153,19 @@ struct code_analysis std::deque args_storage; std::vector> jumpdest_map; - - // TODO: Exported for unit tests. Rework unit tests? - EVMC_EXPORT int find_jumpdest(int offset) const noexcept; }; +inline int find_jumpdest(const code_analysis& analysis, int offset) noexcept +{ + // TODO: Replace with lower_bound(). + for (const auto& d : analysis.jumpdest_map) + { + if (d.first == offset) + return d.second; + } + return -1; +} + EVMC_EXPORT code_analysis analyze( const exec_fn_table& fns, evmc_revision rev, const uint8_t* code, size_t code_size) noexcept; diff --git a/lib/evmone/instructions.cpp b/lib/evmone/instructions.cpp index 55df30b0c8..8dbade886f 100644 --- a/lib/evmone/instructions.cpp +++ b/lib/evmone/instructions.cpp @@ -481,7 +481,7 @@ void op_jump(execution_state& state, instr_argument) noexcept const auto dst = state.stack.pop(); auto pc = -1; if (std::numeric_limits::max() < dst || - (pc = state.analysis->find_jumpdest(static_cast(dst))) < 0) + (pc = find_jumpdest(*state.analysis, static_cast(dst))) < 0) return state.exit(EVMC_BAD_JUMP_DESTINATION); state.next_instr = &state.analysis->instrs[static_cast(pc)]; diff --git a/test/unittests/analysis_test.cpp b/test/unittests/analysis_test.cpp index d2baf43b01..4a105438d0 100644 --- a/test/unittests/analysis_test.cpp +++ b/test/unittests/analysis_test.cpp @@ -88,8 +88,8 @@ TEST(analysis, jump1) ASSERT_EQ(analysis.blocks.size(), 3); ASSERT_EQ(analysis.jumpdest_map.size(), 1); EXPECT_EQ(analysis.jumpdest_map[0], std::pair(6, 5)); - EXPECT_EQ(analysis.find_jumpdest(6), 5); - EXPECT_EQ(analysis.find_jumpdest(0), -1); + EXPECT_EQ(find_jumpdest(analysis, 6), 5); + EXPECT_EQ(find_jumpdest(analysis, 0), -1); } TEST(analysis, empty) From f49efcea4c5fd7f2a9787d8c2cafb6c970690ed8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Tue, 2 Jul 2019 14:07:09 +0200 Subject: [PATCH 2/3] Use lower_bound to find jumpdest --- lib/evmone/analysis.hpp | 11 ++++------- test/unittests/analysis_test.cpp | 1 + 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/evmone/analysis.hpp b/lib/evmone/analysis.hpp index 9d1bf9dd86..40be182078 100644 --- a/lib/evmone/analysis.hpp +++ b/lib/evmone/analysis.hpp @@ -157,13 +157,10 @@ struct code_analysis inline int find_jumpdest(const code_analysis& analysis, int offset) noexcept { - // TODO: Replace with lower_bound(). - for (const auto& d : analysis.jumpdest_map) - { - if (d.first == offset) - return d.second; - } - return -1; + const auto& m = analysis.jumpdest_map; + const auto it = std::lower_bound(std::begin(m), std::end(m), offset, + [](std::pair p, int v) noexcept { return p.first < v; }); + return (it != std::end(m) && it->first == offset) ? it->second : -1; } EVMC_EXPORT code_analysis analyze( diff --git a/test/unittests/analysis_test.cpp b/test/unittests/analysis_test.cpp index 4a105438d0..1dd33cc3f4 100644 --- a/test/unittests/analysis_test.cpp +++ b/test/unittests/analysis_test.cpp @@ -90,6 +90,7 @@ TEST(analysis, jump1) EXPECT_EQ(analysis.jumpdest_map[0], std::pair(6, 5)); EXPECT_EQ(find_jumpdest(analysis, 6), 5); EXPECT_EQ(find_jumpdest(analysis, 0), -1); + EXPECT_EQ(find_jumpdest(analysis, 7), -1); } TEST(analysis, empty) From e4bd49780115bf63834ccad23fbbeac813c655da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Fri, 26 Jul 2019 12:35:09 +0200 Subject: [PATCH 3/3] Split jumpdest map into 2 vectors of int16_t --- lib/evmone/analysis.cpp | 5 ++++- lib/evmone/analysis.hpp | 18 +++++++++++++----- test/unittests/analysis_test.cpp | 12 ++++++++---- test/utils/dump.cpp | 10 +++++----- 4 files changed, 30 insertions(+), 15 deletions(-) diff --git a/lib/evmone/analysis.cpp b/lib/evmone/analysis.cpp index a3047a7567..bb360d1904 100644 --- a/lib/evmone/analysis.cpp +++ b/lib/evmone/analysis.cpp @@ -65,7 +65,10 @@ code_analysis analyze( beginblock_instr.arg.p.number = static_cast(analysis.blocks.size() - 1); if (jumpdest) // Add the jumpdest to the map. - analysis.jumpdest_map.emplace_back(static_cast(i), instr_index); + { + analysis.jumpdest_offsets.emplace_back(static_cast(i)); + analysis.jumpdest_targets.emplace_back(static_cast(instr_index)); + } else // Increase instruction count because additional BEGINBLOCK was injected. ++instr_index; } diff --git a/lib/evmone/analysis.hpp b/lib/evmone/analysis.hpp index 40be182078..92c57caf06 100644 --- a/lib/evmone/analysis.hpp +++ b/lib/evmone/analysis.hpp @@ -152,15 +152,23 @@ struct code_analysis /// invalidated when the container grows. std::deque args_storage; - std::vector> jumpdest_map; + /// The offsets of JUMPDESTs in the original code. + /// These are values that JUMP/JUMPI receives as an argument. + /// The elements are sorted. + std::vector jumpdest_offsets; + + /// The indexes of the instructions in the generated instruction table + /// matching the elements from jumdest_offsets. + /// This is value to which the next instruction pointer must be set in JUMP/JUMPI. + std::vector jumpdest_targets; }; inline int find_jumpdest(const code_analysis& analysis, int offset) noexcept { - const auto& m = analysis.jumpdest_map; - const auto it = std::lower_bound(std::begin(m), std::end(m), offset, - [](std::pair p, int v) noexcept { return p.first < v; }); - return (it != std::end(m) && it->first == offset) ? it->second : -1; + const auto begin = std::begin(analysis.jumpdest_offsets); + const auto end = std::end(analysis.jumpdest_offsets); + const auto it = std::lower_bound(begin, end, offset); + return (it != end && *it == offset) ? analysis.jumpdest_targets[it - begin] : -1; } EVMC_EXPORT code_analysis analyze( diff --git a/test/unittests/analysis_test.cpp b/test/unittests/analysis_test.cpp index 1dd33cc3f4..9401bdad26 100644 --- a/test/unittests/analysis_test.cpp +++ b/test/unittests/analysis_test.cpp @@ -86,8 +86,10 @@ TEST(analysis, jump1) const auto analysis = analyze(fake_fn_table, rev, &code[0], code.size()); ASSERT_EQ(analysis.blocks.size(), 3); - ASSERT_EQ(analysis.jumpdest_map.size(), 1); - EXPECT_EQ(analysis.jumpdest_map[0], std::pair(6, 5)); + ASSERT_EQ(analysis.jumpdest_offsets.size(), 1); + ASSERT_EQ(analysis.jumpdest_targets.size(), 1); + EXPECT_EQ(analysis.jumpdest_offsets[0], 6); + EXPECT_EQ(analysis.jumpdest_targets[0], 5); EXPECT_EQ(find_jumpdest(analysis, 6), 5); EXPECT_EQ(find_jumpdest(analysis, 0), -1); EXPECT_EQ(find_jumpdest(analysis, 7), -1); @@ -109,8 +111,10 @@ TEST(analysis, only_jumpdest) auto analysis = evmone::analyze(fake_fn_table, rev, &code[0], code.size()); ASSERT_EQ(analysis.blocks.size(), 1); - ASSERT_EQ(analysis.jumpdest_map.size(), 1); - EXPECT_EQ(analysis.jumpdest_map[0], std::pair(0, 0)); + ASSERT_EQ(analysis.jumpdest_offsets.size(), 1); + ASSERT_EQ(analysis.jumpdest_targets.size(), 1); + EXPECT_EQ(analysis.jumpdest_offsets[0], 0); + EXPECT_EQ(analysis.jumpdest_targets[0], 0); } TEST(analysis, jumpi_at_the_end) diff --git a/test/utils/dump.cpp b/test/utils/dump.cpp index 735019bb9f..ce5113d1bc 100644 --- a/test/utils/dump.cpp +++ b/test/utils/dump.cpp @@ -30,14 +30,14 @@ void dump_analysis(const evmone::code_analysis& analysis) { block = &analysis.blocks[size_t(instr.arg.p.number)]; - auto get_jumpdest_offset = [&analysis](size_t index) noexcept + const auto get_jumpdest_offset = [&analysis](size_t index) noexcept { - for (const auto& d : analysis.jumpdest_map) + for (size_t t = 0; t < analysis.jumpdest_targets.size(); ++t) { - if (d.second == static_cast(index)) - return d.first; + if (t == index) + return analysis.jumpdest_offsets[t]; } - return -1; + return int16_t{-1}; }; std::cout << "┌ ";