Skip to content

Commit c23e3bb

Browse files
chfastchfast
committed
Refactor the analysis loop
1 parent 65e5b8a commit c23e3bb

File tree

2 files changed

+123
-68
lines changed

2 files changed

+123
-68
lines changed

‎lib/evmone/analysis.cpp‎

Lines changed: 44 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -54,38 +54,20 @@ code_analysis analyze(
5454
block_info* block = nullptr;
5555

5656
int block_stack_change = 0;
57-
int instr_index = 0;
57+
58+
// Create new block.
59+
block = &analysis.blocks.emplace_back();
60+
block_stack_change = 0;
61+
auto& beginblock_instr = analysis.instrs.emplace_back(fns[OPX_BEGINBLOCK]);
62+
beginblock_instr.arg.p.number = static_cast<int>(analysis.blocks.size() - 1);
5863

5964
const auto code_end = code + code_size;
60-
for (auto code_pos = code; code_pos < code_end; ++instr_index)
65+
auto code_pos = code;
66+
67+
while (code_pos != code_end)
6168
{
62-
// TODO: Loop in reverse order for easier GAS analysis.
6369
const auto opcode = *code_pos++;
6470

65-
const bool jumpdest = opcode == OP_JUMPDEST;
66-
67-
if (!block || jumpdest)
68-
{
69-
// Create new block.
70-
block = &analysis.blocks.emplace_back();
71-
block_stack_change = 0;
72-
73-
// Create BEGINBLOCK instruction which either replaces JUMPDEST or is injected
74-
// in case there is no JUMPDEST.
75-
auto& beginblock_instr = analysis.instrs.emplace_back(fns[OPX_BEGINBLOCK]);
76-
beginblock_instr.arg.p.number = static_cast<int>(analysis.blocks.size() - 1);
77-
78-
if (jumpdest) // Add the jumpdest to the map.
79-
{
80-
analysis.jumpdest_offsets.emplace_back(static_cast<int16_t>(code_pos - code - 1));
81-
analysis.jumpdest_targets.emplace_back(static_cast<int16_t>(instr_index));
82-
}
83-
else // Increase instruction count because additional BEGINBLOCK was injected.
84-
++instr_index;
85-
}
86-
87-
auto& instr = jumpdest ? analysis.instrs.back() : analysis.instrs.emplace_back(fns[opcode]);
88-
8971
const auto metrics = instr_table[opcode];
9072
const auto instr_stack_req = metrics.num_stack_arguments;
9173
const auto instr_stack_change = metrics.num_stack_returned_items - instr_stack_req;
@@ -104,8 +86,31 @@ code_analysis analyze(
10486
if (metrics.gas_cost > 0) // can be -1 for undefined instruction
10587
block->gas_cost += metrics.gas_cost;
10688

89+
if (opcode == OP_JUMPDEST)
90+
{
91+
// The JUMPDEST is always the first instruction in the block.
92+
// We don't have to insert anything to the instruction table.
93+
analysis.jumpdest_offsets.emplace_back(static_cast<int16_t>(code_pos - code - 1));
94+
analysis.jumpdest_targets.emplace_back(
95+
static_cast<int16_t>(analysis.instrs.size() - 1));
96+
}
97+
else
98+
analysis.instrs.emplace_back(fns[opcode]);
99+
100+
auto& instr = analysis.instrs.back();
101+
102+
bool is_terminator = false; // A flag whenever this is a block terminating instruction.
107103
switch (opcode)
108104
{
105+
case OP_JUMP:
106+
case OP_JUMPI:
107+
case OP_STOP:
108+
case OP_RETURN:
109+
case OP_REVERT:
110+
case OP_SELFDESTRUCT:
111+
is_terminator = true;
112+
break;
113+
109114
case ANY_SMALL_PUSH:
110115
{
111116
const auto push_size = size_t(opcode - OP_PUSH1 + 1);
@@ -147,18 +152,6 @@ code_analysis analyze(
147152
break;
148153
}
149154

150-
case ANY_DUP:
151-
// TODO: This is not needed, but we keep it
152-
// otherwise compiler will not use the jumptable for switch implementation.
153-
instr.arg.p.number = opcode - OP_DUP1;
154-
break;
155-
156-
case ANY_SWAP:
157-
// TODO: This is not needed, but we keep it
158-
// otherwise compiler will not use the jumptable for switch implementation.
159-
instr.arg.p.number = opcode - OP_SWAP1 + 1;
160-
break;
161-
162155
case OP_GAS:
163156
instr.arg.p.number = block->gas_cost;
164157
break;
@@ -177,31 +170,22 @@ code_analysis analyze(
177170
case OP_PC:
178171
instr.arg.p.number = static_cast<int>(code_pos - code - 1);
179172
break;
173+
}
180174

181-
case OP_LOG0:
182-
case OP_LOG1:
183-
case OP_LOG2:
184-
case OP_LOG3:
185-
case OP_LOG4:
186-
// TODO: This is not needed, but we keep it
187-
// otherwise compiler will not use the jumptable for switch implementation.
188-
instr.arg.p.number = opcode - OP_LOG0;
189-
break;
190-
191-
case OP_JUMP:
192-
case OP_JUMPI:
193-
case OP_STOP:
194-
case OP_RETURN:
195-
case OP_REVERT:
196-
case OP_SELFDESTRUCT:
197-
block = nullptr;
198-
break;
175+
if (is_terminator || (code_pos != code_end && *code_pos == OP_JUMPDEST))
176+
{
177+
// Create new basic block if
178+
// this is a terminating instruction or the next instruction is a JUMPDEST.
179+
block = &analysis.blocks.emplace_back();
180+
block_stack_change = 0;
181+
analysis.instrs.emplace_back(fns[OPX_BEGINBLOCK]).arg.p.number =
182+
static_cast<int>(analysis.blocks.size() - 1);
199183
}
200184
}
201185

202-
// Not terminated block or empty code.
203-
if (block || code_size == 0 || code[code_size - 1] == OP_JUMPI)
204-
analysis.instrs.emplace_back(fns[OP_STOP]);
186+
// Make sure the last block is terminated.
187+
// TODO: This is not needed if the last instruction is a terminating one.
188+
analysis.instrs.emplace_back(fns[OP_STOP]);
205189

206190
// FIXME: assert(analysis.instrs.size() <= max_instrs_size);
207191

‎test/unittests/analysis_test.cpp‎

Lines changed: 79 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,28 @@ TEST(analysis, push)
7878
EXPECT_EQ(analysis.push_values[0], intx::uint256{0xee} << 240);
7979
}
8080

81+
TEST(analysis, jumpdest_skip)
82+
{
83+
// If the JUMPDEST is the first instruction in a basic block it should be just omitted
84+
// and no new block should be created in this place.
85+
86+
const auto code = bytecode{} + OP_STOP + OP_JUMPDEST;
87+
auto analysis = evmone::analyze(fake_fn_table, rev, &code[0], code.size());
88+
89+
EXPECT_EQ(analysis.blocks.size(), 2);
90+
ASSERT_EQ(analysis.instrs.size(), 4);
91+
EXPECT_EQ(analysis.instrs[0].fn, fake_fn_table[OPX_BEGINBLOCK]);
92+
EXPECT_EQ(analysis.instrs[1].fn, fake_fn_table[OP_STOP]);
93+
EXPECT_EQ(analysis.instrs[2].fn, fake_fn_table[OP_JUMPDEST]);
94+
EXPECT_EQ(analysis.instrs[3].fn, fake_fn_table[OP_STOP]);
95+
}
96+
8197
TEST(analysis, jump1)
8298
{
8399
const auto code = jump(add(4, 2)) + OP_JUMPDEST + mstore(0, 3) + ret(0, 0x20) + jump(6);
84100
const auto analysis = analyze(fake_fn_table, rev, &code[0], code.size());
85101

86-
ASSERT_EQ(analysis.blocks.size(), 3);
102+
ASSERT_EQ(analysis.blocks.size(), 4);
87103
ASSERT_EQ(analysis.jumpdest_offsets.size(), 1);
88104
ASSERT_EQ(analysis.jumpdest_targets.size(), 1);
89105
EXPECT_EQ(analysis.jumpdest_offsets[0], 6);
@@ -98,14 +114,15 @@ TEST(analysis, empty)
98114
bytes code;
99115
auto analysis = evmone::analyze(fake_fn_table, rev, &code[0], code.size());
100116

101-
EXPECT_EQ(analysis.blocks.size(), 0);
102-
EXPECT_EQ(analysis.instrs.size(), 1);
103-
EXPECT_EQ(analysis.instrs.back().fn, fake_fn_table[OP_STOP]);
117+
EXPECT_EQ(analysis.blocks.size(), 1);
118+
EXPECT_EQ(analysis.instrs.size(), 2);
119+
EXPECT_EQ(analysis.instrs[0].fn, fake_fn_table[OPX_BEGINBLOCK]);
120+
EXPECT_EQ(analysis.instrs[1].fn, fake_fn_table[OP_STOP]);
104121
}
105122

106123
TEST(analysis, only_jumpdest)
107124
{
108-
auto code = from_hex("5b");
125+
const auto code = bytecode{OP_JUMPDEST};
109126
auto analysis = evmone::analyze(fake_fn_table, rev, &code[0], code.size());
110127

111128
ASSERT_EQ(analysis.blocks.size(), 1);
@@ -117,9 +134,63 @@ TEST(analysis, only_jumpdest)
117134

118135
TEST(analysis, jumpi_at_the_end)
119136
{
120-
auto code = from_hex("57");
137+
const auto code = bytecode{OP_JUMPI};
121138
auto analysis = evmone::analyze(fake_fn_table, rev, &code[0], code.size());
122139

123-
EXPECT_EQ(analysis.blocks.size(), 1);
124-
EXPECT_EQ(analysis.instrs.back().fn, fake_fn_table[OP_STOP]);
140+
EXPECT_EQ(analysis.blocks.size(), 2);
141+
EXPECT_EQ(analysis.instrs[0].fn, fake_fn_table[OPX_BEGINBLOCK]);
142+
EXPECT_EQ(analysis.instrs[1].fn, fake_fn_table[OP_JUMPI]);
143+
EXPECT_EQ(analysis.instrs[2].fn, fake_fn_table[OPX_BEGINBLOCK]);
144+
EXPECT_EQ(analysis.instrs[3].fn, fake_fn_table[OP_STOP]);
145+
}
146+
147+
TEST(analysis, terminated_last_block)
148+
{
149+
// TODO: Even if the last basic block is properly terminated an additional artificial block
150+
// is going to be created with only STOP instruction.
151+
const auto code = ret(0, 0);
152+
auto analysis = evmone::analyze(fake_fn_table, rev, &code[0], code.size());
153+
154+
EXPECT_EQ(analysis.blocks.size(), 2);
155+
ASSERT_EQ(analysis.instrs.size(), 6);
156+
EXPECT_EQ(analysis.instrs[0].fn, fake_fn_table[OPX_BEGINBLOCK]);
157+
EXPECT_EQ(analysis.instrs[3].fn, fake_fn_table[OP_RETURN]);
158+
EXPECT_EQ(analysis.instrs[4].fn, fake_fn_table[OPX_BEGINBLOCK]);
159+
EXPECT_EQ(analysis.instrs[5].fn, fake_fn_table[OP_STOP]);
160+
}
161+
162+
TEST(analysis, jumpdests_groups)
163+
{
164+
const auto code = 3 * OP_JUMPDEST + push(1) + 3 * OP_JUMPDEST + push(2) + OP_JUMPI;
165+
auto analysis = evmone::analyze(fake_fn_table, rev, &code[0], code.size());
166+
167+
EXPECT_EQ(analysis.blocks.size(), 7);
168+
ASSERT_EQ(analysis.instrs.size(), 11);
169+
EXPECT_EQ(analysis.instrs[0].fn, fake_fn_table[OP_JUMPDEST]);
170+
EXPECT_EQ(analysis.instrs[1].fn, fake_fn_table[OP_JUMPDEST]);
171+
EXPECT_EQ(analysis.instrs[2].fn, fake_fn_table[OP_JUMPDEST]);
172+
EXPECT_EQ(analysis.instrs[3].fn, fake_fn_table[OP_PUSH1]);
173+
EXPECT_EQ(analysis.instrs[4].fn, fake_fn_table[OP_JUMPDEST]);
174+
EXPECT_EQ(analysis.instrs[5].fn, fake_fn_table[OP_JUMPDEST]);
175+
EXPECT_EQ(analysis.instrs[6].fn, fake_fn_table[OP_JUMPDEST]);
176+
EXPECT_EQ(analysis.instrs[7].fn, fake_fn_table[OP_PUSH1]);
177+
EXPECT_EQ(analysis.instrs[8].fn, fake_fn_table[OP_JUMPI]);
178+
EXPECT_EQ(analysis.instrs[9].fn, fake_fn_table[OPX_BEGINBLOCK]);
179+
EXPECT_EQ(analysis.instrs[10].fn, fake_fn_table[OP_STOP]);
180+
181+
182+
ASSERT_EQ(analysis.jumpdest_offsets.size(), 6);
183+
ASSERT_EQ(analysis.jumpdest_targets.size(), 6);
184+
EXPECT_EQ(analysis.jumpdest_offsets[0], 0);
185+
EXPECT_EQ(analysis.jumpdest_targets[0], 0);
186+
EXPECT_EQ(analysis.jumpdest_offsets[1], 1);
187+
EXPECT_EQ(analysis.jumpdest_targets[1], 1);
188+
EXPECT_EQ(analysis.jumpdest_offsets[2], 2);
189+
EXPECT_EQ(analysis.jumpdest_targets[2], 2);
190+
EXPECT_EQ(analysis.jumpdest_offsets[3], 5);
191+
EXPECT_EQ(analysis.jumpdest_targets[3], 4);
192+
EXPECT_EQ(analysis.jumpdest_offsets[4], 6);
193+
EXPECT_EQ(analysis.jumpdest_targets[4], 5);
194+
EXPECT_EQ(analysis.jumpdest_offsets[5], 7);
195+
EXPECT_EQ(analysis.jumpdest_targets[5], 6);
125196
}

0 commit comments

Comments
 (0)