Skip to content

Commit 1853e06

Browse files
chfastchfast
committed
Refactor the analysis loop
1 parent ab68bd3 commit 1853e06

File tree

2 files changed

+61
-72
lines changed

2 files changed

+61
-72
lines changed

‎lib/evmone/analysis.cpp‎

Lines changed: 44 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -32,38 +32,20 @@ code_analysis analyze(evmc_revision rev, const uint8_t* code, size_t code_size)
3232
block_info* block = nullptr;
3333

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

3742
const auto code_end = code + code_size;
38-
for (auto code_pos = code; code_pos < code_end; ++instr_index)
43+
auto code_pos = code;
44+
45+
while (code_pos != code_end)
3946
{
40-
// TODO: Loop in reverse order for easier GAS analysis.
4147
const auto opcode = *code_pos++;
4248

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

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

128-
case ANY_DUP:
129-
// TODO: This is not needed, but we keep it
130-
// otherwise compiler will not use the jumptable for switch implementation.
131-
instr.arg.number = opcode - OP_DUP1;
132-
break;
133-
134-
case ANY_SWAP:
135-
// TODO: This is not needed, but we keep it
136-
// otherwise compiler will not use the jumptable for switch implementation.
137-
instr.arg.number = opcode - OP_SWAP1 + 1;
138-
break;
139-
140133
case OP_GAS:
141134
case OP_CALL:
142135
case OP_CALLCODE:
@@ -150,31 +143,22 @@ code_analysis analyze(evmc_revision rev, const uint8_t* code, size_t code_size)
150143
case OP_PC:
151144
instr.arg.number = static_cast<int>(code_pos - code - 1);
152145
break;
146+
}
153147

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

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

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

‎test/unittests/analysis_test.cpp‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ TEST(analysis, jump1)
9090
const auto code = jump(add(4, 2)) + OP_JUMPDEST + mstore(0, 3) + ret(0, 0x20) + jump(6);
9191
const auto analysis = analyze(rev, &code[0], code.size());
9292

93-
ASSERT_EQ(analysis.blocks.size(), 3);
93+
ASSERT_EQ(analysis.blocks.size(), 4);
9494
ASSERT_EQ(analysis.jumpdest_offsets.size(), 1);
9595
ASSERT_EQ(analysis.jumpdest_targets.size(), 1);
9696
EXPECT_EQ(analysis.jumpdest_offsets[0], 6);
@@ -105,9 +105,10 @@ TEST(analysis, empty)
105105
bytes code;
106106
auto analysis = evmone::analyze(rev, &code[0], code.size());
107107

108-
EXPECT_EQ(analysis.blocks.size(), 0);
109-
ASSERT_EQ(analysis.instrs.size(), 1);
110-
EXPECT_EQ(analysis.instrs[0].fn, op_table[OP_STOP]);
108+
EXPECT_EQ(analysis.blocks.size(), 1);
109+
ASSERT_EQ(analysis.instrs.size(), 2);
110+
EXPECT_EQ(analysis.instrs[0].fn, op_table[OPX_BEGINBLOCK]);
111+
EXPECT_EQ(analysis.instrs[1].fn, op_table[OP_STOP]);
111112
}
112113

113114
TEST(analysis, only_jumpdest)
@@ -127,11 +128,12 @@ TEST(analysis, jumpi_at_the_end)
127128
const auto code = bytecode{OP_JUMPI};
128129
auto analysis = evmone::analyze(rev, &code[0], code.size());
129130

130-
EXPECT_EQ(analysis.blocks.size(), 1);
131-
ASSERT_EQ(analysis.instrs.size(), 3);
131+
EXPECT_EQ(analysis.blocks.size(), 2);
132+
ASSERT_EQ(analysis.instrs.size(), 4);
132133
EXPECT_EQ(analysis.instrs[0].fn, op_table[OPX_BEGINBLOCK]);
133134
EXPECT_EQ(analysis.instrs[1].fn, op_table[OP_JUMPI]);
134-
EXPECT_EQ(analysis.instrs[2].fn, op_table[OP_STOP]);
135+
EXPECT_EQ(analysis.instrs[2].fn, op_table[OPX_BEGINBLOCK]);
136+
EXPECT_EQ(analysis.instrs[3].fn, op_table[OP_STOP]);
135137
}
136138

137139
TEST(analysis, terminated_last_block)
@@ -141,19 +143,21 @@ TEST(analysis, terminated_last_block)
141143
const auto code = ret(0, 0);
142144
auto analysis = evmone::analyze(rev, &code[0], code.size());
143145

144-
EXPECT_EQ(analysis.blocks.size(), 1);
145-
ASSERT_EQ(analysis.instrs.size(), 4);
146+
EXPECT_EQ(analysis.blocks.size(), 2);
147+
ASSERT_EQ(analysis.instrs.size(), 6);
146148
EXPECT_EQ(analysis.instrs[0].fn, op_table[OPX_BEGINBLOCK]);
147149
EXPECT_EQ(analysis.instrs[3].fn, op_table[OP_RETURN]);
150+
EXPECT_EQ(analysis.instrs[4].fn, op_table[OPX_BEGINBLOCK]);
151+
EXPECT_EQ(analysis.instrs[5].fn, op_table[OP_STOP]);
148152
}
149153

150154
TEST(analysis, jumpdests_groups)
151155
{
152156
const auto code = 3 * OP_JUMPDEST + push(1) + 3 * OP_JUMPDEST + push(2) + OP_JUMPI;
153157
auto analysis = evmone::analyze(rev, &code[0], code.size());
154158

155-
EXPECT_EQ(analysis.blocks.size(), 6);
156-
ASSERT_EQ(analysis.instrs.size(), 10);
159+
EXPECT_EQ(analysis.blocks.size(), 7);
160+
ASSERT_EQ(analysis.instrs.size(), 11);
157161
EXPECT_EQ(analysis.instrs[0].fn, op_table[OP_JUMPDEST]);
158162
EXPECT_EQ(analysis.instrs[1].fn, op_table[OP_JUMPDEST]);
159163
EXPECT_EQ(analysis.instrs[2].fn, op_table[OP_JUMPDEST]);
@@ -163,7 +167,8 @@ TEST(analysis, jumpdests_groups)
163167
EXPECT_EQ(analysis.instrs[6].fn, op_table[OP_JUMPDEST]);
164168
EXPECT_EQ(analysis.instrs[7].fn, op_table[OP_PUSH1]);
165169
EXPECT_EQ(analysis.instrs[8].fn, op_table[OP_JUMPI]);
166-
EXPECT_EQ(analysis.instrs[9].fn, op_table[OP_STOP]);
170+
EXPECT_EQ(analysis.instrs[9].fn, op_table[OPX_BEGINBLOCK]);
171+
EXPECT_EQ(analysis.instrs[10].fn, op_table[OP_STOP]);
167172

168173

169174
ASSERT_EQ(analysis.jumpdest_offsets.size(), 6);

0 commit comments

Comments
 (0)