Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop op_table parameter in analysis() #167

Merged
merged 2 commits into from
Sep 10, 2019
Merged

Drop op_table parameter in analysis() #167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9fd0b74
Drop op_table parameter in analysis()
chfast Sep 10, 2019
fb08633
Check number of entries in op_tables
chfast Sep 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/evmone/analysis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion lib/evmone/analysis.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you make it exported?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used in analysis tests (evmone is usually built as shared library).


} // namespace evmone
5 changes: 1 addition & 4 deletions lib/evmone/execution.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,14 @@

#include "execution.hpp"
#include "analysis.hpp"

#include <memory>

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<execution_state>();
state->analysis = &analysis;
Expand Down
11 changes: 9 additions & 2 deletions lib/evmone/instructions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -1379,4 +1378,12 @@ extern const exec_fn_table op_table[] = {
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
{
return op_tables[rev];
}
} // namespace evmone
4 changes: 1 addition & 3 deletions test/bench/bench.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,12 @@ void execute(State& state, bytes_view code, bytes_view input) noexcept
state.counters["gas_rate"] = Counter(static_cast<double>(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();
}
Expand Down
99 changes: 45 additions & 54 deletions test/unittests/analysis_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Danger of static init fiasco? in case op_tables in instructions.cpp is initialized after this

Copy link
Member Author

@chfast chfast Sep 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem does not apply to constexpr globals, but still I can move it inside the function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, haven't noticed constexpr

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good notice anyway.


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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -134,45 +125,45 @@ 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)
{
// 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);
Expand Down