Optimize block info access

lib/evmone/CMakeLists.txt

@@ -15,6 +15,7 @@ add_library(evmone
     execution.cpp
     execution.hpp
     instructions.cpp
+    limits.hpp
     opcodes_helpers.h
 )
 target_link_libraries(evmone PUBLIC evmc::evmc PRIVATE intx::intx evmc::instructions ethash::keccak)

lib/evmone/analysis.cpp

@@ -66,14 +66,13 @@ code_analysis analyze(
 
         if (!block || jumpdest)
         {
-            // Create new block.
-            block = &analysis.blocks.emplace_back();
-            block_stack_change = 0;
-
             // Create BEGINBLOCK instruction which either replaces JUMPDEST or is injected
             // in case there is no JUMPDEST.
             auto& beginblock_instr = analysis.instrs.emplace_back(fns[OPX_BEGINBLOCK]);
-            beginblock_instr.arg.p.number = static_cast<int>(analysis.blocks.size() - 1);
+
+            // Start new block.
+            block = &beginblock_instr.arg.block;
+            block_stack_change = 0;
 
             if (jumpdest)  // Add the jumpdest to the map.
             {
@@ -90,9 +89,16 @@ code_analysis analyze(
         const auto instr_stack_req = metrics.num_stack_arguments;
         const auto instr_stack_change = metrics.num_stack_returned_items - instr_stack_req;
 
-        block->stack_req = std::max(block->stack_req, instr_stack_req - block_stack_change);
+        // TODO: Define a block_analysis struct with regular ints for analysis.
+        //       Compress it when block is closed.
+        auto stack_req = instr_stack_req - block_stack_change;
+        if (stack_req > std::numeric_limits<decltype(block->stack_req)>::max())
+            stack_req = std::numeric_limits<decltype(block->stack_req)>::max();
+
+        block->stack_req = std::max(block->stack_req, static_cast<int16_t>(stack_req));
         block_stack_change += instr_stack_change;
-        block->stack_max_growth = std::max(block->stack_max_growth, block_stack_change);
+        block->stack_max_growth =
+            static_cast<int16_t>(std::max(int{block->stack_max_growth}, block_stack_change));
 
         if (metrics.gas_cost > 0)  // can be -1 for undefined instruction
             block->gas_cost += metrics.gas_cost;
@@ -153,7 +159,7 @@ code_analysis analyze(
             break;
 
         case OP_GAS:
-            instr.arg.p.number = static_cast<int>(block->gas_cost);
+            instr.arg.p.number = block->gas_cost;
             break;
 
         case OP_CALL:
@@ -162,7 +168,7 @@ code_analysis analyze(
         case OP_STATICCALL:
         case OP_CREATE:
         case OP_CREATE2:
-            instr.arg.p.number = static_cast<int>(block->gas_cost);
+            instr.arg.p.number = block->gas_cost;
             instr.arg.p.call_kind =
                 op2call_kind(opcode == OP_STATICCALL ? uint8_t{OP_CALL} : opcode);
             break;

lib/evmone/analysis.hpp

@@ -3,6 +3,7 @@
 // Licensed under the Apache License, Version 2.0.
 #pragma once
 
+#include "limits.hpp"
 #include <evmc/evmc.hpp>
 #include <evmc/instructions.h>
 #include <evmc/utils.h>
@@ -84,6 +85,29 @@ class evm_memory
 
 struct 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;
+
+    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;
+
+    /// 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;
+
+    static_assert(max_code_size * max_instruction_stack_increase <
+                      std::numeric_limits<decltype(stack_max_growth)>::max(),
+        "Potential block_info::stack_max_growth overflow");
+};
+
 struct execution_state
 {
     evmc_status_code status = EVMC_SUCCESS;
@@ -99,7 +123,7 @@ struct execution_state
     ///
     /// This is only needed to correctly calculate remaining gas for GAS instruction.
     /// TODO: Maybe this should be precomputed in analysis.
-    int64_t current_block_cost = 0;
+    int32_t current_block_cost = 0;
 
     struct code_analysis* analysis = nullptr;
     bytes return_data;
@@ -129,6 +153,7 @@ union instr_argument
     const uint8_t* data;
     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");
@@ -159,22 +184,11 @@ struct instr_info
     explicit constexpr instr_info(exec_fn f) noexcept : fn{f}, arg{} {};
 };
 
-struct block_info
-{
-    /// The total base gas cost of all instructions in the block.
-    int64_t gas_cost = 0;
-
-    /// The stack height required to execute the block.
-    int stack_req = 0;
-
-    /// The maximum stack height growth relative to the stack height at block start.
-    int stack_max_growth = 0;
-};
+static_assert(sizeof(block_info) == 8);
 
 struct code_analysis
 {
     std::vector<instr_info> instrs;
-    std::vector<block_info> blocks;
 
     /// Storage for large push values.
     std::vector<intx::uint256> push_values;

lib/evmone/instructions.cpp

@@ -1200,7 +1200,7 @@ 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 = state.analysis->blocks[static_cast<size_t>(instr->arg.p.number)];
+    auto& block = instr->arg.block;
 
     if ((state.gas_left -= block.gas_cost) < 0)
         return state.exit(EVMC_OUT_OF_GAS);

lib/evmone/limits.hpp

@@ -0,0 +1,16 @@
+// evmone: Fast Ethereum Virtual Machine implementation
+// Copyright 2019 Pawel Bylica.
+// Licensed under the Apache License, Version 2.0.
+#pragma once
+
+/// The maximum EVM bytecode size allowed by the Ethereum spec.
+constexpr auto max_code_size = 0x6000;
+
+/// The maximum base cost of any EVM instruction.
+/// The value comes from the cost of the SSTORE instruction.
+constexpr auto max_instruction_base_cost = 20000;
+
+/// The maximum stack increase any instruction can cause.
+/// Only instructions taking 0 arguments and pushing an item to the stack
+/// can increase the stack height and only by 1.
+constexpr auto max_instruction_stack_increase = 1;

test/unittests/CMakeLists.txt

@@ -12,6 +12,7 @@ add_library(evm-unittests OBJECT
     evm_test.cpp
 )
 target_link_libraries(evm-unittests PRIVATE testutils evmc::evmc GTest::gtest)
+target_include_directories(evm-unittests PRIVATE ${evmone_private_include_dir})
 
 # The internal evmone unit tests. The generic EVM ones are also built in.
 add_executable(evmone-unittests

test/unittests/analysis_test.cpp

@@ -30,7 +30,6 @@ TEST(analysis, example1)
     ASSERT_EQ(analysis.instrs.size(), 8);
 
     EXPECT_EQ(analysis.instrs[0].fn, fake_fn_table[OPX_BEGINBLOCK]);
-    EXPECT_EQ(analysis.instrs[0].arg.p.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]);
@@ -39,10 +38,9 @@ TEST(analysis, example1)
     EXPECT_EQ(analysis.instrs[6].fn, fake_fn_table[OP_SSTORE]);
     EXPECT_EQ(analysis.instrs[7].fn, fake_fn_table[OP_STOP]);
 
-    ASSERT_EQ(analysis.blocks.size(), 1);
-    EXPECT_EQ(analysis.blocks[0].gas_cost, 14);
-    EXPECT_EQ(analysis.blocks[0].stack_req, 0);
-    EXPECT_EQ(analysis.blocks[0].stack_max_growth, 2);
+    EXPECT_EQ(analysis.instrs[0].arg.block.gas_cost, 14);
+    EXPECT_EQ(analysis.instrs[0].arg.block.stack_req, 0);
+    EXPECT_EQ(analysis.instrs[0].arg.block.stack_max_growth, 2);
 }
 
 TEST(analysis, stack_up_and_down)
@@ -52,16 +50,14 @@ TEST(analysis, stack_up_and_down)
 
     ASSERT_EQ(analysis.instrs.size(), 20);
     EXPECT_EQ(analysis.instrs[0].fn, fake_fn_table[OPX_BEGINBLOCK]);
-    EXPECT_EQ(analysis.instrs[0].arg.p.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]);
 
-    ASSERT_EQ(analysis.blocks.size(), 1);
-    EXPECT_EQ(analysis.blocks[0].gas_cost, 7 * 3 + 10 * 2 + 3);
-    EXPECT_EQ(analysis.blocks[0].stack_req, 3);
-    EXPECT_EQ(analysis.blocks[0].stack_max_growth, 7);
+    EXPECT_EQ(analysis.instrs[0].arg.block.gas_cost, 7 * 3 + 10 * 2 + 3);
+    EXPECT_EQ(analysis.instrs[0].arg.block.stack_req, 3);
+    EXPECT_EQ(analysis.instrs[0].arg.block.stack_max_growth, 7);
 }
 
 TEST(analysis, push)
@@ -82,7 +78,6 @@ 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());
 
-    ASSERT_EQ(analysis.blocks.size(), 3);
     ASSERT_EQ(analysis.jumpdest_offsets.size(), 1);
     ASSERT_EQ(analysis.jumpdest_targets.size(), 1);
     EXPECT_EQ(analysis.jumpdest_offsets[0], 6);
@@ -97,7 +92,6 @@ TEST(analysis, empty)
     bytes code;
     auto analysis = evmone::analyze(fake_fn_table, rev, &code[0], code.size());
 
-    EXPECT_EQ(analysis.blocks.size(), 0);
     EXPECT_EQ(analysis.instrs.size(), 1);
     EXPECT_EQ(analysis.instrs.back().fn, fake_fn_table[OP_STOP]);
 }
@@ -107,7 +101,6 @@ TEST(analysis, only_jumpdest)
     auto code = from_hex("5b");
     auto analysis = evmone::analyze(fake_fn_table, rev, &code[0], code.size());
 
-    ASSERT_EQ(analysis.blocks.size(), 1);
     ASSERT_EQ(analysis.jumpdest_offsets.size(), 1);
     ASSERT_EQ(analysis.jumpdest_targets.size(), 1);
     EXPECT_EQ(analysis.jumpdest_offsets[0], 0);
@@ -119,6 +112,5 @@ TEST(analysis, jumpi_at_the_end)
     auto code = from_hex("57");
     auto analysis = evmone::analyze(fake_fn_table, rev, &code[0], code.size());
 
-    EXPECT_EQ(analysis.blocks.size(), 1);
     EXPECT_EQ(analysis.instrs.back().fn, fake_fn_table[OP_STOP]);
 }

test/unittests/evm_test.cpp

@@ -4,6 +4,7 @@
 
 #include "evm_fixture.hpp"
 #include <evmc/instructions.h>
+#include <evmone/limits.hpp>
 #include <intx/intx.hpp>
 #include <test/utils/bytecode.hpp>
 #include <algorithm>
@@ -12,8 +13,6 @@
 using namespace evmc::literals;
 using namespace intx;
 
-constexpr auto max_code_size = 0x6000;
-
 TEST_F(evm, empty)
 {
     execute(0, "");
@@ -1373,3 +1372,14 @@ TEST_F(evm, memory_access)
         }
     }
 }
+
+TEST_F(evm, block_stack_req_overflow)
+{
+    // This tests constructs a code with single basic block which stack requirement is > int16 max.
+    // Such basic block can cause int16_t overflow during analysis.
+    // The CALL instruction is going to be used because it has -6 stack change.
+
+    const auto code = push(1) + 10 * OP_DUP1 + 5463 * OP_CALL;
+    execute(code);
+    EXPECT_STATUS(EVMC_STACK_UNDERFLOW);
+}

test/utils/dump.cpp

@@ -28,7 +28,7 @@ void dump_analysis(const evmone::code_analysis& analysis)
 
         if (c == OPX_BEGINBLOCK)
         {
-            block = &analysis.blocks[size_t(instr.arg.p.number)];
+            block = &instr.arg.block;
 
             const auto get_jumpdest_offset = [&analysis](size_t index) noexcept
             {