From a66824471037f7a9a0fd52676f559695c3004a5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Wed, 26 Oct 2022 15:28:35 +0200 Subject: [PATCH 1/3] baseline: Simplify static_assert condition Reported by clang-tidy. --- lib/evmone/baseline.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/evmone/baseline.cpp b/lib/evmone/baseline.cpp index 404ede694c..4318c6feb7 100644 --- a/lib/evmone/baseline.cpp +++ b/lib/evmone/baseline.cpp @@ -104,7 +104,7 @@ inline evmc_status_code check_requirements( const CostTable& cost_table, int64_t& gas_left, ptrdiff_t stack_size) noexcept { static_assert( - !(instr::has_const_gas_cost(Op) && instr::gas_costs[EVMC_FRONTIER][Op] == instr::undefined), + !instr::has_const_gas_cost(Op) || instr::gas_costs[EVMC_FRONTIER][Op] != instr::undefined, "undefined instructions must not be handled by check_requirements()"); auto gas_cost = instr::gas_costs[EVMC_FRONTIER][Op]; // Init assuming const cost. From 7fad7aa01750b64ae74996ee87b2e8f885e74ac9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Wed, 26 Oct 2022 14:03:21 +0200 Subject: [PATCH 2/3] baseline: Move stack size computation to check_requirements() --- lib/evmone/baseline.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/evmone/baseline.cpp b/lib/evmone/baseline.cpp index 4318c6feb7..a56256e72d 100644 --- a/lib/evmone/baseline.cpp +++ b/lib/evmone/baseline.cpp @@ -100,8 +100,8 @@ namespace /// @return Status code with information which check has failed /// or EVMC_SUCCESS if everything is fine. template -inline evmc_status_code check_requirements( - const CostTable& cost_table, int64_t& gas_left, ptrdiff_t stack_size) noexcept +inline evmc_status_code check_requirements(const CostTable& cost_table, int64_t& gas_left, + const uint256* stack_top, const uint256* stack_bottom) noexcept { static_assert( !instr::has_const_gas_cost(Op) || instr::gas_costs[EVMC_FRONTIER][Op] != instr::undefined, @@ -118,6 +118,8 @@ inline evmc_status_code check_requirements( return EVMC_UNDEFINED_INSTRUCTION; } + const auto stack_size = stack_top - stack_bottom; + // Check stack requirements first. This is order is not required, // but it is nicer because complete gas check may need to inspect operands. if constexpr (instr::traits[Op].stack_height_change > 0) @@ -202,8 +204,8 @@ template [[release_inline]] inline Position invoke(const CostTable& cost_table, const uint256* stack_bottom, Position pos, ExecutionState& state) noexcept { - const auto stack_size = pos.stack_top - stack_bottom; - if (const auto status = check_requirements(cost_table, state.gas_left, stack_size); + if (const auto status = + check_requirements(cost_table, state.gas_left, pos.stack_top, stack_bottom); status != EVMC_SUCCESS) { state.status = status; From f18d1f73bdffa42a849b117b06f0cd13f7624b18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Wed, 26 Oct 2022 14:35:02 +0200 Subject: [PATCH 3/3] baseline: Better stack overflow/underflow checks Change stack overflow/underflow checks to stack pointers comparisons. This gives better compiler optimizations. --- lib/evmone/baseline.cpp | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/lib/evmone/baseline.cpp b/lib/evmone/baseline.cpp index a56256e72d..89f2d8e73e 100644 --- a/lib/evmone/baseline.cpp +++ b/lib/evmone/baseline.cpp @@ -93,10 +93,12 @@ namespace /// - if stack height requirements are fulfilled (stack overflow, stack underflow) /// - charges the instruction base gas cost and checks is there is any gas left. /// -/// @tparam Op Instruction opcode. -/// @param cost_table Table of base gas costs. -/// @param [in,out] gas_left Gas left. -/// @param stack_size Current stack height. +/// @tparam Op Instruction opcode. +/// @param cost_table Table of base gas costs. +/// @param [in,out] gas_left Gas left. +/// @param stack_top Pointer to the stack top item. +/// @param stack_bottom Pointer to the stack bottom. +/// The stack height is stack_top - stack_bottom. /// @return Status code with information which check has failed /// or EVMC_SUCCESS if everything is fine. template @@ -118,19 +120,20 @@ inline evmc_status_code check_requirements(const CostTable& cost_table, int64_t& return EVMC_UNDEFINED_INSTRUCTION; } - const auto stack_size = stack_top - stack_bottom; - // Check stack requirements first. This is order is not required, // but it is nicer because complete gas check may need to inspect operands. if constexpr (instr::traits[Op].stack_height_change > 0) { - static_assert(instr::traits[Op].stack_height_change == 1); - if (INTX_UNLIKELY(stack_size == StackSpace::limit)) + static_assert(instr::traits[Op].stack_height_change == 1, + "unexpected instruction with multiple results"); + if (INTX_UNLIKELY(stack_top == stack_bottom + StackSpace::limit)) return EVMC_STACK_OVERFLOW; } if constexpr (instr::traits[Op].stack_height_required > 0) { - if (INTX_UNLIKELY(stack_size < instr::traits[Op].stack_height_required)) + // Check stack underflow using pointer comparison <= (better optimization). + static constexpr auto min_offset = instr::traits[Op].stack_height_required - 1; + if (INTX_UNLIKELY(stack_top <= stack_bottom + min_offset)) return EVMC_STACK_UNDERFLOW; }