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

Put instruction immediate values in the instruction table #154

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Put instruction immediate values in the instruction table #154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
99a8c27
Put small push data in the program table
chfast Aug 22, 2019
c7f283e
Put large push data pointer in the program table
chfast Aug 22, 2019
c955152
Put block info in the program table
chfast Sep 9, 2019
daac250
Put immediate values of all other instructions in the program table
chfast Aug 22, 2019
d3e21b4
Change instr_info into union/variant type
chfast Sep 9, 2019
276dabe
Convert instr_info into POD type
chfast Sep 12, 2019
4403727
Adjust program size estimation
chfast Sep 12, 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
32 changes: 15 additions & 17 deletions lib/evmone/analysis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,16 @@ code_analysis analyze(evmc_revision rev, const uint8_t* code, size_t code_size)

code_analysis analysis;

const auto max_instrs_size = code_size + 1;
analysis.instrs.reserve(max_instrs_size);
analysis.instrs.reserve(2 * (code_size + 1));
Copy link
Member

Choose a reason for hiding this comment

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

comment why 2x ?


// This is 2x more than needed but using (code_size / 2 + 1) increases page-faults 1000x.
const auto max_args_storage_size = code_size + 1;
analysis.push_values.reserve(max_args_storage_size);

// Create first block.
analysis.instrs.emplace_back(opx_beginblock_fn);
auto block = block_analysis{0};
analysis.instrs.emplace_back().fn = opx_beginblock_fn;
analysis.instrs.emplace_back(); // Placeholder for the block info data.
auto block = block_analysis{analysis.instrs.size() - 1};

const auto code_end = code + code_size;
auto code_pos = code;
Expand All @@ -63,12 +63,10 @@ code_analysis analyze(evmc_revision rev, const uint8_t* code, size_t code_size)
// We don't have to insert anything to the instruction table.
analysis.jumpdest_offsets.emplace_back(static_cast<int16_t>(code_pos - code - 1));
analysis.jumpdest_targets.emplace_back(
static_cast<int16_t>(analysis.instrs.size() - 1));
static_cast<int16_t>(analysis.instrs.size() - 2));
}
else
analysis.instrs.emplace_back(opcode_info.fn);

auto& instr = analysis.instrs.back();
analysis.instrs.emplace_back().fn = opcode_info.fn;

bool is_terminator = false; // A flag whenever this is a block terminating instruction.
switch (opcode)
Expand All @@ -94,7 +92,7 @@ code_analysis analyze(evmc_revision rev, const uint8_t* code, size_t code_size)
value |= uint64_t{*code_pos++} << insert_bit_pos;
insert_bit_pos -= 8;
}
instr.arg.small_push_value = value;
analysis.instrs.emplace_back().small_push_value = value;
break;
}

Expand All @@ -120,7 +118,7 @@ code_analysis analyze(evmc_revision rev, const uint8_t* code, size_t code_size)
while (code_pos < push_end && code_pos < code_end)
*insert_pos-- = *code_pos++;

instr.arg.push_value = &push_value;
analysis.instrs.emplace_back().push_value = &push_value;
break;
}

Expand All @@ -131,11 +129,11 @@ code_analysis analyze(evmc_revision rev, const uint8_t* code, size_t code_size)
case OP_STATICCALL:
case OP_CREATE:
case OP_CREATE2:
instr.arg.number = block.gas_cost;
analysis.instrs.emplace_back().number = static_cast<int>(block.gas_cost);
break;

case OP_PC:
instr.arg.number = static_cast<int>(code_pos - code - 1);
analysis.instrs.emplace_back().number = static_cast<int>(code_pos - code - 1);
break;
}

Expand All @@ -147,12 +145,13 @@ code_analysis analyze(evmc_revision rev, const uint8_t* code, size_t code_size)
static_cast<int16_t>(block.stack_req) :
stack_req_max;
const auto stack_max_growth = static_cast<int16_t>(block.stack_max_growth);
analysis.instrs[block.begin_block_index].arg.block = {
analysis.instrs[block.begin_block_index].block = {
block.gas_cost, stack_req, stack_max_growth};


// Create new block.
analysis.instrs.emplace_back(opx_beginblock_fn);
analysis.instrs.emplace_back().fn = opx_beginblock_fn;
analysis.instrs.emplace_back();
block = block_analysis{analysis.instrs.size() - 1};
}
}
Expand All @@ -161,12 +160,11 @@ code_analysis analyze(evmc_revision rev, const uint8_t* code, size_t code_size)
const auto stack_req =
block.stack_req <= stack_req_max ? static_cast<int16_t>(block.stack_req) : stack_req_max;
const auto stack_max_growth = static_cast<int16_t>(block.stack_max_growth);
analysis.instrs[block.begin_block_index].arg.block = {
block.gas_cost, stack_req, stack_max_growth};
analysis.instrs[block.begin_block_index].block = {block.gas_cost, stack_req, stack_max_growth};

// Make sure the last block is terminated.
// TODO: This is not needed if the last instruction is a terminating one.
analysis.instrs.emplace_back(op_tbl[OP_STOP].fn);
analysis.instrs.emplace_back().fn = op_tbl[OP_STOP].fn;

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

Expand Down
30 changes: 11 additions & 19 deletions lib/evmone/analysis.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,25 +83,25 @@ class evm_memory
void resize(size_t new_size) { m_memory.resize(new_size); }
};

struct instr_info;
union instr_info;

struct block_info
{
/// The total base gas cost of all instructions in the block.
/// This cannot overflow, see the static_assert() below.
int32_t gas_cost = 0;
int32_t gas_cost;
Copy link
Member

Choose a reason for hiding this comment

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

why removing initializer?


static_assert(
max_code_size * max_instruction_base_cost < std::numeric_limits<decltype(gas_cost)>::max(),
"Potential block_info::gas_cost overflow");

/// The stack height required to execute the block.
/// This MAY overflow.
int16_t stack_req = 0;
int16_t stack_req;

/// The maximum stack height growth relative to the stack height at block start.
/// This cannot overflow, see the static_assert() below.
int16_t stack_max_growth = 0;
int16_t stack_max_growth;

static_assert(max_code_size * max_instruction_stack_increase <
std::numeric_limits<decltype(stack_max_growth)>::max(),
Expand Down Expand Up @@ -143,16 +143,6 @@ struct execution_state
}
};

union instr_argument
{
int number;
const uint8_t* data;
Copy link
Member

Choose a reason for hiding this comment

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

was this data member never used?

const intx::uint256* push_value;
uint64_t small_push_value;
block_info block{};
};

static_assert(sizeof(instr_argument) == sizeof(void*), "Incorrect size of instr_argument");

using exec_fn = const instr_info* (*)(const instr_info*, execution_state&);

Expand Down Expand Up @@ -180,13 +170,15 @@ struct op_table_entry

using op_table = std::array<op_table_entry, 256>;

struct instr_info
union instr_info
Copy link
Member

Choose a reason for hiding this comment

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

Maybe some comment for this union?

{
exec_fn fn = nullptr;
instr_argument arg;

explicit constexpr instr_info(exec_fn f) noexcept : fn{f}, arg{} {};
exec_fn fn;
int number;
const intx::uint256* push_value;
uint64_t small_push_value;
block_info block;
};
static_assert(sizeof(instr_info) == sizeof(void*), "Incorrect size of instr_info");

static_assert(sizeof(block_info) == 8);

Expand Down
21 changes: 11 additions & 10 deletions lib/evmone/instructions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ const instr_info* op_jumpi(const instr_info* instr, execution_state& state) noex

const instr_info* op_pc(const instr_info* instr, execution_state& state) noexcept
{
state.stack.push(instr->arg.number);
state.stack.push((++instr)->number);
Copy link
Member

Choose a reason for hiding this comment

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

just a thought, maybe helpers like these could improve clarity

inline const instr_info& arg(const instr_info* instr) { return *(instr+1); }

inline const instr_info* next_instr(const instr_info* instr) { return instr += 2; }

return ++instr;
}

Expand All @@ -558,7 +558,7 @@ const instr_info* op_msize(const instr_info* instr, execution_state& state) noex

const instr_info* op_gas(const instr_info* instr, execution_state& state) noexcept
{
const auto correction = state.current_block_cost - instr->arg.number;
const auto correction = state.current_block_cost - (++instr)->number;
const auto gas = static_cast<uint64_t>(state.gas_left + correction);
state.stack.push(gas);
return ++instr;
Expand Down Expand Up @@ -693,13 +693,13 @@ const instr_info* op_gaslimit(const instr_info* instr, execution_state& state) n

const instr_info* op_push_small(const instr_info* instr, execution_state& state) noexcept
{
state.stack.push(instr->arg.small_push_value);
state.stack.push((++instr)->small_push_value);
return ++instr;
}

const instr_info* op_push_full(const instr_info* instr, execution_state& state) noexcept
{
state.stack.push(*instr->arg.push_value);
state.stack.push(*(++instr)->push_value);
return ++instr;
}

Expand Down Expand Up @@ -794,7 +794,7 @@ const instr_info* op_revert(const instr_info*, execution_state& state) noexcept
template <evmc_call_kind kind>
const instr_info* op_call(const instr_info* instr, execution_state& state) noexcept
{
const auto arg = instr->arg;
const auto arg = *(++instr);
auto gas = state.stack[0];
const auto dst = intx::be::trunc<evmc::address>(state.stack[1]);
auto value = state.stack[2];
Expand Down Expand Up @@ -916,7 +916,7 @@ const instr_info* op_call(const instr_info* instr, execution_state& state) noexc

const instr_info* op_delegatecall(const instr_info* instr, execution_state& state) noexcept
{
const auto arg = instr->arg;
const auto arg = *(++instr);
auto gas = state.stack[0];
const auto dst = intx::be::trunc<evmc::address>(state.stack[1]);
auto input_offset = state.stack[2];
Expand Down Expand Up @@ -985,7 +985,7 @@ const instr_info* op_delegatecall(const instr_info* instr, execution_state& stat

const instr_info* op_staticcall(const instr_info* instr, execution_state& state) noexcept
{
const auto arg = instr->arg;
const auto arg = *(++instr);
auto gas = state.stack[0];
const auto dst = intx::be::trunc<evmc::address>(state.stack[1]);
auto input_offset = state.stack[2];
Expand Down Expand Up @@ -1052,7 +1052,7 @@ const instr_info* op_create(const instr_info* instr, execution_state& state) noe
if (state.msg->flags & EVMC_STATIC)
return state.exit(EVMC_STATIC_MODE_VIOLATION);

const auto arg = instr->arg;
const auto arg = *(++instr);
auto endowment = state.stack[0];
auto init_code_offset = state.stack[1];
auto init_code_size = state.stack[2];
Expand Down Expand Up @@ -1111,7 +1111,7 @@ const instr_info* op_create2(const instr_info* instr, execution_state& state) no
if (state.msg->flags & EVMC_STATIC)
return state.exit(EVMC_STATIC_MODE_VIOLATION);

const auto arg = instr->arg;
const auto arg = *(++instr);
auto endowment = state.stack[0];
auto init_code_offset = state.stack[1];
auto init_code_size = state.stack[2];
Expand Down Expand Up @@ -1202,7 +1202,8 @@ const instr_info* op_selfdestruct(const instr_info*, execution_state& state) noe

const instr_info* opx_beginblock(const instr_info* instr, execution_state& state) noexcept
{
auto& block = instr->arg.block;
// FIXME: Do not use reference, check assembly.
const auto& block = (++instr)->block;

if ((state.gas_left -= block.gas_cost) < 0)
return state.exit(EVMC_OUT_OF_GAS);
Expand Down
Loading