Skip to content

Commit d9ee8b0

Browse files
chfastchfast
committed
Refactor the analysis loop
1 parent b7be3e5 commit d9ee8b0

File tree

2 files changed

+123
-68
lines changed

2 files changed

+123
-68
lines changed

‎lib/evmone/analysis.cpp

+44-60
Original file line numberDiff line numberDiff line change
@@ -33,38 +33,20 @@ code_analysis analyze(
3333
block_info* block = nullptr;
3434

3535
int block_stack_change = 0;
36-
int instr_index = 0;
36+
37+
// Create new block.
38+
block = &analysis.blocks.emplace_back();
39+
block_stack_change = 0;
40+
auto& beginblock_instr = analysis.instrs.emplace_back(fns[OPX_BEGINBLOCK]);
41+
beginblock_instr.arg.number = static_cast<int>(analysis.blocks.size() - 1);
3742

3843
const auto code_end = code + code_size;
39-
for (auto code_pos = code; code_pos < code_end; ++instr_index)
44+
auto code_pos = code;
45+
46+
while (code_pos != code_end)
4047
{
41-
// TODO: Loop in reverse order for easier GAS analysis.
4248
const auto opcode = *code_pos++;
4349

44-
const bool jumpdest = opcode == OP_JUMPDEST;
45-
46-
if (!block || jumpdest)
47-
{
48-
// Create new block.
49-
block = &analysis.blocks.emplace_back();
50-
block_stack_change = 0;
51-
52-
// Create BEGINBLOCK instruction which either replaces JUMPDEST or is injected
53-
// in case there is no JUMPDEST.
54-
auto& beginblock_instr = analysis.instrs.emplace_back(fns[OPX_BEGINBLOCK]);
55-
beginblock_instr.arg.number = static_cast<int>(analysis.blocks.size() - 1);
56-
57-
if (jumpdest) // Add the jumpdest to the map.
58-
{
59-
analysis.jumpdest_offsets.emplace_back(static_cast<int16_t>(code_pos - code - 1));
60-
analysis.jumpdest_targets.emplace_back(static_cast<int16_t>(instr_index));
61-
}
62-
else // Increase instruction count because additional BEGINBLOCK was injected.
63-
++instr_index;
64-
}
65-
66-
auto& instr = jumpdest ? analysis.instrs.back() : analysis.instrs.emplace_back(fns[opcode]);
67-
6850
const auto metrics = instr_table[opcode];
6951
const auto instr_stack_req = metrics.num_stack_arguments;
7052
const auto instr_stack_change = metrics.num_stack_returned_items - instr_stack_req;
@@ -83,8 +65,31 @@ code_analysis analyze(
8365
if (metrics.gas_cost > 0) // can be -1 for undefined instruction
8466
block->gas_cost += metrics.gas_cost;
8567

68+
if (opcode == OP_JUMPDEST)
69+
{
70+
// The JUMPDEST is always the first instruction in the block.
71+
// We don't have to insert anything to the instruction table.
72+
analysis.jumpdest_offsets.emplace_back(static_cast<int16_t>(code_pos - code - 1));
73+
analysis.jumpdest_targets.emplace_back(
74+
static_cast<int16_t>(analysis.instrs.size() - 1));
75+
}
76+
else
77+
analysis.instrs.emplace_back(fns[opcode]);
78+
79+
auto& instr = analysis.instrs.back();
80+
81+
bool is_terminator = false; // A flag whenever this is a block terminating instruction.
8682
switch (opcode)
8783
{
84+
case OP_JUMP:
85+
case OP_JUMPI:
86+
case OP_STOP:
87+
case OP_RETURN:
88+
case OP_REVERT:
89+
case OP_SELFDESTRUCT:
90+
is_terminator = true;
91+
break;
92+
8893
case ANY_SMALL_PUSH:
8994
{
9095
const auto push_size = size_t(opcode - OP_PUSH1 + 1);
@@ -126,18 +131,6 @@ code_analysis analyze(
126131
break;
127132
}
128133

129-
case ANY_DUP:
130-
// TODO: This is not needed, but we keep it
131-
// otherwise compiler will not use the jumptable for switch implementation.
132-
instr.arg.number = opcode - OP_DUP1;
133-
break;
134-
135-
case ANY_SWAP:
136-
// TODO: This is not needed, but we keep it
137-
// otherwise compiler will not use the jumptable for switch implementation.
138-
instr.arg.number = opcode - OP_SWAP1 + 1;
139-
break;
140-
141134
case OP_GAS:
142135
case OP_CALL:
143136
case OP_CALLCODE:
@@ -151,31 +144,22 @@ code_analysis analyze(
151144
case OP_PC:
152145
instr.arg.number = static_cast<int>(code_pos - code - 1);
153146
break;
147+
}
154148

155-
case OP_LOG0:
156-
case OP_LOG1:
157-
case OP_LOG2:
158-
case OP_LOG3:
159-
case OP_LOG4:
160-
// TODO: This is not needed, but we keep it
161-
// otherwise compiler will not use the jumptable for switch implementation.
162-
instr.arg.number = opcode - OP_LOG0;
163-
break;
164-
165-
case OP_JUMP:
166-
case OP_JUMPI:
167-
case OP_STOP:
168-
case OP_RETURN:
169-
case OP_REVERT:
170-
case OP_SELFDESTRUCT:
171-
block = nullptr;
172-
break;
149+
if (is_terminator || (code_pos != code_end && *code_pos == OP_JUMPDEST))
150+
{
151+
// Create new basic block if
152+
// this is a terminating instruction or the next instruction is a JUMPDEST.
153+
block = &analysis.blocks.emplace_back();
154+
block_stack_change = 0;
155+
analysis.instrs.emplace_back(fns[OPX_BEGINBLOCK]).arg.number =
156+
static_cast<int>(analysis.blocks.size() - 1);
173157
}
174158
}
175159

176-
// Not terminated block or empty code.
177-
if (block || code_size == 0 || code[code_size - 1] == OP_JUMPI)
178-
analysis.instrs.emplace_back(fns[OP_STOP]);
160+
// Make sure the last block is terminated.
161+
// TODO: This is not needed if the last instruction is a terminating one.
162+
analysis.instrs.emplace_back(fns[OP_STOP]);
179163

180164
// FIXME: assert(analysis.instrs.size() <= max_instrs_size);
181165

‎test/unittests/analysis_test.cpp

+79-8
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)