Merge pull request #598 from ethereum/memory_grow

Rework memory grow/check helpers - pass gas counter by value

lib/evmone/instructions.hpp

@@ -58,33 +58,34 @@ inline constexpr int64_t num_words(uint64_t size_in_bytes) noexcept
     return static_cast<int64_t>((size_in_bytes + (word_size - 1)) / word_size);
 }
 
-// Grows EVM memory and checks its cost.
-//
-// This function should not be inlined because this may affect other inlining decisions:
-// - making check_memory() too costly to inline,
-// - making mload()/mstore()/mstore8() too costly to inline.
-//
-// TODO: This function should be moved to Memory class.
-[[gnu::noinline]] inline bool grow_memory(ExecutionState& state, uint64_t new_size) noexcept
+/// Grows EVM memory and checks its cost.
+///
+/// This function should not be inlined because this may affect other inlining decisions:
+/// - making check_memory() too costly to inline,
+/// - making mload()/mstore()/mstore8() too costly to inline.
+///
+/// TODO: This function should be moved to Memory class.
+[[gnu::noinline]] inline int64_t grow_memory(
+    int64_t gas_left, Memory& memory, uint64_t new_size) noexcept
 {
     // This implementation recomputes memory.size(). This value is already known to the caller
     // and can be passed as a parameter, but this make no difference to the performance.
 
     const auto new_words = num_words(new_size);
-    const auto current_words = static_cast<int64_t>(state.memory.size() / word_size);
+    const auto current_words = static_cast<int64_t>(memory.size() / word_size);
     const auto new_cost = 3 * new_words + new_words * new_words / 512;
     const auto current_cost = 3 * current_words + current_words * current_words / 512;
     const auto cost = new_cost - current_cost;
 
-    if ((state.gas_left -= cost) < 0)
-        return false;
-
-    state.memory.grow(static_cast<size_t>(new_words * word_size));
-    return true;
+    gas_left -= cost;
+    if (gas_left >= 0) [[likely]]
+        memory.grow(static_cast<size_t>(new_words * word_size));
+    return gas_left;
 }
 
-// Check memory requirements of a reasonable size.
-inline bool check_memory(ExecutionState& state, const uint256& offset, uint64_t size) noexcept
+/// Check memory requirements of a reasonable size.
+inline bool check_memory(
+    int64_t& gas_left, Memory& memory, const uint256& offset, uint64_t size) noexcept
 {
     // TODO: This should be done in intx.
     // There is "branchless" variant of this using | instead of ||, but benchmarks difference
@@ -93,14 +94,15 @@ inline bool check_memory(ExecutionState& state, const uint256& offset, uint64_t
         return false;
 
     const auto new_size = static_cast<uint64_t>(offset) + size;
-    if (new_size > state.memory.size())
-        return grow_memory(state, new_size);
+    if (new_size > memory.size())
+        gas_left = grow_memory(gas_left, memory, new_size);
 
-    return true;
+    return gas_left >= 0;  // Always true for no-grow case.
 }
 
-// Check memory requirements for "copy" instructions.
-inline bool check_memory(ExecutionState& state, const uint256& offset, const uint256& size) noexcept
+/// Check memory requirements for "copy" instructions.
+inline bool check_memory(
+    int64_t& gas_left, Memory& memory, const uint256& offset, const uint256& size) noexcept
 {
     if (size == 0)  // Copy of size 0 is always valid (even if offset is huge).
         return true;
@@ -111,7 +113,7 @@ inline bool check_memory(ExecutionState& state, const uint256& offset, const uin
     if (((size[3] | size[2] | size[1]) != 0) || (size[0] > max_buffer_size))
         return false;
 
-    return check_memory(state, offset, static_cast<uint64_t>(size));
+    return check_memory(gas_left, memory, offset, static_cast<uint64_t>(size));
 }
 
 namespace instr::core
@@ -337,7 +339,7 @@ inline evmc_status_code keccak256(StackTop stack, ExecutionState& state) noexcep
     const auto& index = stack.pop();
     auto& size = stack.top();
 
-    if (!check_memory(state, index, size))
+    if (!check_memory(state.gas_left, state.memory, index, size))
         return EVMC_OUT_OF_GAS;
 
     const auto i = static_cast<size_t>(index);
@@ -418,7 +420,7 @@ inline evmc_status_code calldatacopy(StackTop stack, ExecutionState& state) noex
     const auto& input_index = stack.pop();
     const auto& size = stack.pop();
 
-    if (!check_memory(state, mem_index, size))
+    if (!check_memory(state.gas_left, state.memory, mem_index, size))
         return EVMC_OUT_OF_GAS;
 
     auto dst = static_cast<size_t>(mem_index);
@@ -453,7 +455,7 @@ inline evmc_status_code codecopy(StackTop stack, ExecutionState& state) noexcept
     const auto& input_index = stack.pop();
     const auto& size = stack.pop();
 
-    if (!check_memory(state, mem_index, size))
+    if (!check_memory(state.gas_left, state.memory, mem_index, size))
         return EVMC_OUT_OF_GAS;
 
     const auto code_size = state.original_code.size();
@@ -509,7 +511,7 @@ inline evmc_status_code extcodecopy(StackTop stack, ExecutionState& state) noexc
     const auto& input_index = stack.pop();
     const auto& size = stack.pop();
 
-    if (!check_memory(state, mem_index, size))
+    if (!check_memory(state.gas_left, state.memory, mem_index, size))
         return EVMC_OUT_OF_GAS;
 
     const auto s = static_cast<size_t>(size);
@@ -547,7 +549,7 @@ inline evmc_status_code returndatacopy(StackTop stack, ExecutionState& state) no
     const auto& input_index = stack.pop();
     const auto& size = stack.pop();
 
-    if (!check_memory(state, mem_index, size))
+    if (!check_memory(state.gas_left, state.memory, mem_index, size))
         return EVMC_OUT_OF_GAS;
 
     auto dst = static_cast<size_t>(mem_index);
@@ -640,7 +642,7 @@ inline evmc_status_code mload(StackTop stack, ExecutionState& state) noexcept
 {
     auto& index = stack.top();
 
-    if (!check_memory(state, index, 32))
+    if (!check_memory(state.gas_left, state.memory, index, 32))
         return EVMC_OUT_OF_GAS;
 
     index = intx::be::unsafe::load<uint256>(&state.memory[static_cast<size_t>(index)]);
@@ -652,7 +654,7 @@ inline evmc_status_code mstore(StackTop stack, ExecutionState& state) noexcept
     const auto& index = stack.pop();
     const auto& value = stack.pop();
 
-    if (!check_memory(state, index, 32))
+    if (!check_memory(state.gas_left, state.memory, index, 32))
         return EVMC_OUT_OF_GAS;
 
     intx::be::unsafe::store(&state.memory[static_cast<size_t>(index)], value);
@@ -664,7 +666,7 @@ inline evmc_status_code mstore8(StackTop stack, ExecutionState& state) noexcept
     const auto& index = stack.pop();
     const auto& value = stack.pop();
 
-    if (!check_memory(state, index, 1))
+    if (!check_memory(state.gas_left, state.memory, index, 1))
         return EVMC_OUT_OF_GAS;
 
     state.memory[static_cast<size_t>(index)] = static_cast<uint8_t>(value);
@@ -901,7 +903,7 @@ inline evmc_status_code log(StackTop stack, ExecutionState& state) noexcept
     const auto& offset = stack.pop();
     const auto& size = stack.pop();
 
-    if (!check_memory(state, offset, size))
+    if (!check_memory(state.gas_left, state.memory, offset, size))
         return EVMC_OUT_OF_GAS;
 
     const auto o = static_cast<size_t>(offset);
@@ -970,7 +972,7 @@ inline StopToken return_impl(StackTop stack, ExecutionState& state) noexcept
     const auto& offset = stack[0];
     const auto& size = stack[1];
 
-    if (!check_memory(state, offset, size))
+    if (!check_memory(state.gas_left, state.memory, offset, size))
         return {EVMC_OUT_OF_GAS};
 
     state.output_size = static_cast<size_t>(size);

lib/evmone/instructions_calls.cpp

@@ -30,10 +30,10 @@ evmc_status_code call_impl(StackTop stack, ExecutionState& state) noexcept
             return EVMC_OUT_OF_GAS;
     }
 
-    if (!check_memory(state, input_offset_u256, input_size_u256))
+    if (!check_memory(state.gas_left, state.memory, input_offset_u256, input_size_u256))
         return EVMC_OUT_OF_GAS;
 
-    if (!check_memory(state, output_offset_u256, output_size_u256))
+    if (!check_memory(state.gas_left, state.memory, output_offset_u256, output_size_u256))
         return EVMC_OUT_OF_GAS;
 
     const auto input_offset = static_cast<size_t>(input_offset_u256);
@@ -131,7 +131,7 @@ evmc_status_code create_impl(StackTop stack, ExecutionState& state) noexcept
     stack.push(0);  // Assume failure.
     state.return_data.clear();
 
-    if (!check_memory(state, init_code_offset_u256, init_code_size_u256))
+    if (!check_memory(state.gas_left, state.memory, init_code_offset_u256, init_code_size_u256))
         return EVMC_OUT_OF_GAS;
 
     const auto init_code_offset = static_cast<size_t>(init_code_offset_u256);

test/unittests/evm_test.cpp

@@ -53,7 +53,7 @@ TEST_P(evm, stack_underflow)
 
 TEST_P(evm, add)
 {
-    execute(25, bytecode{"6007600d0160005260206000f3"});
+    execute(25, add(7, 13) + ret_top());
     EXPECT_GAS_USED(EVMC_SUCCESS, 24);
     EXPECT_OUTPUT_INT(20);
 }