eof: Change max stack height to stack height increase (#1181)

chfast · web-flow · commit 66dcad5af58e · 2025-04-11T13:42:59.000+02:00
This changes the meaning of the stack height parameter of the code type
from "max stack height" (including code inputs)
to "stack height increase" (excluding code inputs).

The bytecode test unitilty keeps the `max_stack_height` to minimize
changes to tests.
diff --git a/lib/evmone/eof.cpp b/lib/evmone/eof.cpp
@@ -31,7 +31,7 @@ constexpr auto CODE_SECTION_SIZE_SIZE = sizeof(uint16_t);
 constexpr auto CONTAINER_SECTION_SIZE_SIZE = sizeof(uint32_t);
 constexpr auto CODE_SECTION_NUMBER_LIMIT = 1024;
 constexpr auto CONTAINER_SECTION_NUMBER_LIMIT = 256;
-constexpr auto MAX_STACK_HEIGHT = 0x03FF;
+constexpr auto MAX_STACK_INCREASE_LIMIT = 0x03FF;
 constexpr auto OUTPUTS_INPUTS_NUMBER_LIMIT = 0x7F;
 constexpr auto REL_OFFSET_SIZE = sizeof(int16_t);
 constexpr auto STACK_SIZE_LIMIT = 1024;
@@ -237,7 +237,7 @@ EOFValidationError validate_types(bytes_view container, const EOF1Header& header
 {
     for (size_t i = 0; i < header.get_type_count(); ++i)
     {
-        const auto [inputs, outputs, max_stack_height] = header.get_type(container, i);
+        const auto [inputs, outputs, max_stack_increase] = header.get_type(container, i);
 
         // First type should be (0, 0x80)
         if (i == 0 && (inputs != 0 || outputs != NON_RETURNING_FUNCTION))
@@ -247,8 +247,8 @@ EOFValidationError validate_types(bytes_view container, const EOF1Header& header
             inputs > OUTPUTS_INPUTS_NUMBER_LIMIT)
             return EOFValidationError::inputs_outputs_num_above_limit;
 
-        if (max_stack_height > MAX_STACK_HEIGHT)
-            return EOFValidationError::max_stack_height_above_limit;
+        if (max_stack_increase > MAX_STACK_INCREASE_LIMIT)
+            return EOFValidationError::max_stack_increase_above_limit;
     }
 
     return EOFValidationError::success;
@@ -423,8 +423,11 @@ bool validate_rjump_destinations(bytes_view code) noexcept
     return true;
 }
 
+/// Validates stack height of the function.
+///
 /// Requires that the input is validated against truncation.
-std::variant<EOFValidationError, int32_t> validate_max_stack_height(
+/// Returns computed max stack increase or the validation error.
+std::variant<int32_t, EOFValidationError> validate_stack_height(
     bytes_view code, size_t func_index, const EOF1Header& header, bytes_view container)
 {
     // Special value used for detecting errors.
@@ -467,8 +470,7 @@ std::variant<EOFValidationError, int32_t> validate_max_stack_height(
             const auto callee_type = header.get_type(container, fid);
             stack_height_required = callee_type.inputs;
 
-            if (stack_height.max + callee_type.max_stack_height - stack_height_required >
-                STACK_SIZE_LIMIT)
+            if (stack_height.max + callee_type.max_stack_increase > STACK_SIZE_LIMIT)
                 return EOFValidationError::stack_overflow;
 
             // Instruction validation ensures target function is returning
@@ -480,8 +482,7 @@ std::variant<EOFValidationError, int32_t> validate_max_stack_height(
             const auto fid = read_uint16_be(&code[i + 1]);
             const auto callee_type = header.get_type(container, fid);
 
-            if (stack_height.max + callee_type.max_stack_height - callee_type.inputs >
-                STACK_SIZE_LIMIT)
+            if (stack_height.max + callee_type.max_stack_increase > STACK_SIZE_LIMIT)
                 return EOFValidationError::stack_overflow;
 
             if (callee_type.outputs == NON_RETURNING_FUNCTION)
@@ -601,7 +602,8 @@ std::variant<EOFValidationError, int32_t> validate_max_stack_height(
 
     const auto max_stack_height_it = std::ranges::max_element(stack_heights,
         [](StackHeightRange lhs, StackHeightRange rhs) noexcept { return lhs.max < rhs.max; });
-    return max_stack_height_it->max;
+    const auto max_stack_increase = max_stack_height_it->max - type.inputs;
+    return max_stack_increase;
 }
 
 EOFValidationError validate_eof1(
@@ -684,16 +686,16 @@ EOFValidationError validate_eof1(
                 return EOFValidationError::invalid_rjump_destination;
 
             // Validate stack
-            auto msh_or_error = validate_max_stack_height(
+            const auto shi_or_error = validate_stack_height(
                 header.get_code(container, code_idx), code_idx, header, container);
-            if (const auto* error = std::get_if<EOFValidationError>(&msh_or_error))
+            if (const auto* error = std::get_if<EOFValidationError>(&shi_or_error))
                 return *error;
             // TODO(clang-tidy): Too restrictive, see
             //   https://github.com/llvm/llvm-project/issues/120867.
             // NOLINTNEXTLINE(modernize-use-integer-sign-comparison)
-            if (std::get<int32_t>(msh_or_error) !=
-                header.get_type(container, code_idx).max_stack_height)
-                return EOFValidationError::invalid_max_stack_height;
+            if (std::get<int32_t>(shi_or_error) !=
+                header.get_type(container, code_idx).max_stack_increase)
+                return EOFValidationError::invalid_max_stack_increase;
         }
 
         if (std::ranges::find(visited_code_sections, false) != visited_code_sections.end())
@@ -975,10 +977,10 @@ std::string_view get_error_message(EOFValidationError err) noexcept
         return "invalid_type_section_size";
     case EOFValidationError::invalid_first_section_type:
         return "invalid_first_section_type";
-    case EOFValidationError::invalid_max_stack_height:
-        return "invalid_max_stack_height";
-    case EOFValidationError::max_stack_height_above_limit:
-        return "max_stack_height_above_limit";
+    case EOFValidationError::invalid_max_stack_increase:
+        return "invalid_max_stack_increase";
+    case EOFValidationError::max_stack_increase_above_limit:
+        return "max_stack_increase_above_limit";
     case EOFValidationError::inputs_outputs_num_above_limit:
         return "inputs_outputs_num_above_limit";
     case EOFValidationError::no_terminating_instruction:
diff --git a/lib/evmone/eof.hpp b/lib/evmone/eof.hpp
@@ -58,12 +58,12 @@ static constexpr auto EOF_CODE_HASH_SENTINEL =
 
 struct EOFCodeType
 {
-    uint8_t inputs;             ///< Number of code inputs.
-    uint8_t outputs;            ///< Number of code outputs.
-    uint16_t max_stack_height;  ///< Maximum stack height reached in the code.
+    uint8_t inputs;               ///< Number of code inputs.
+    uint8_t outputs;              ///< Number of code outputs.
+    uint16_t max_stack_increase;  ///< Maximum stack height above the inputs reached in the code.
 
-    EOFCodeType(uint8_t inputs_, uint8_t outputs_, uint16_t max_stack_height_)
-      : inputs{inputs_}, outputs{outputs_}, max_stack_height{max_stack_height_}
+    EOFCodeType(uint8_t inputs_, uint8_t outputs_, uint16_t max_stack_increase_)
+      : inputs{inputs_}, outputs{outputs_}, max_stack_increase{max_stack_increase_}
     {}
 };
 
@@ -177,11 +177,11 @@ enum class EOFValidationError
     too_many_code_sections,
     invalid_type_section_size,
     invalid_first_section_type,
-    invalid_max_stack_height,
+    invalid_max_stack_increase,
     no_terminating_instruction,
     stack_height_mismatch,
     stack_higher_than_outputs_required,
-    max_stack_height_above_limit,
+    max_stack_increase_above_limit,
     inputs_outputs_num_above_limit,
     unreachable_instructions,
     stack_underflow,
diff --git a/lib/evmone/instructions.hpp b/lib/evmone/instructions.hpp
@@ -1105,8 +1105,7 @@ inline code_iterator callf(StackTop stack, ExecutionState& state, code_iterator
     const auto& header = state.analysis.baseline->eof_header();
     const auto stack_size = &stack.top() - state.stack_space.bottom();
     const auto callee_type = header.get_type(state.original_code, index);
-    const auto callee_required_stack_size = callee_type.max_stack_height - callee_type.inputs;
-    if (stack_size + callee_required_stack_size > StackSpace::limit)
+    if (stack_size + callee_type.max_stack_increase > StackSpace::limit)
     {
         state.status = EVMC_STACK_OVERFLOW;
         return nullptr;
@@ -1137,8 +1136,7 @@ inline code_iterator jumpf(StackTop stack, ExecutionState& state, code_iterator
     const auto& header = state.analysis.baseline->eof_header();
     const auto stack_size = &stack.top() - state.stack_space.bottom();
     const auto callee_type = header.get_type(state.original_code, index);
-    const auto callee_required_stack_size = callee_type.max_stack_height - callee_type.inputs;
-    if (stack_size + callee_required_stack_size > StackSpace::limit)
+    if (stack_size + callee_type.max_stack_increase > StackSpace::limit)
     {
         state.status = EVMC_STACK_OVERFLOW;
         return nullptr;
diff --git a/test/unittests/eof_example_test.cpp b/test/unittests/eof_example_test.cpp
@@ -98,16 +98,16 @@ TEST_F(state_transition, eof_examples_callf)
         //               |                                                  |              |
         //    version    |                         Header terminator        |              |
         //    |          |________________         |                        |_____________ |
-        "EF00 01 01 0008 02 0002 0006 0001 FF 0000 00 00 80 0001 01 01 0001 602A E30001 00 E4"
+        "EF00 01 01 0008 02 0002 0006 0001 FF 0000 00 00 80 0001 01 01 0000 602A E30001 00 E4"
         //       |‾‾‾‾‾‾                   |‾‾‾‾‾‾    |‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾
         //       |                         Header: data section 0 bytes long
         //       |                                    |
         //       Header: types section 8 bytes long   |
         //                                            |
         //                                            Types section: first code section 0 inputs,
-        //                                            non-returning, max stack height 1;
+        //                                            non-returning, max stack increase 1;
         //                                            second code section 1 input,
-        //                                            1 output, max stack height 1
+        //                                            1 output, max stack increase 0
     );
 
     // Tests the code is valid EOF.
diff --git a/test/unittests/eof_validation.cpp b/test/unittests/eof_validation.cpp
@@ -53,10 +53,10 @@ std::string_view get_tests_error_message(EOFValidationError err) noexcept
         return "EOF_InvalidTypeSectionSize";
     case EOFValidationError::invalid_first_section_type:
         return "EOF_InvalidFirstSectionType";
-    case EOFValidationError::invalid_max_stack_height:
-        return "EOF_InvalidMaxStackHeight";
-    case EOFValidationError::max_stack_height_above_limit:
-        return "EOF_MaxStackHeightExceeded";
+    case EOFValidationError::invalid_max_stack_increase:
+        return "EOFException.INVALID_MAX_STACK_INCREASE";
+    case EOFValidationError::max_stack_increase_above_limit:
+        return "EOFException.MAX_STACK_INCREASE_ABOVE_LIMIT";
     case EOFValidationError::inputs_outputs_num_above_limit:
         return "EOF_InputsOutputsNumAboveLimit";
     case EOFValidationError::no_terminating_instruction:
diff --git a/test/unittests/eof_validation_stack_test.cpp b/test/unittests/eof_validation_stack_test.cpp
@@ -76,7 +76,7 @@ TEST_F(eof_validation, stack_range_maximally_broad)
     add_test_case(eof_bytecode(code, 1023), EOFValidationError::success, "valid_1023_rjumpis");
 
     code = rjumpi(offset, OP_PUSH0) + OP_PUSH0 + code;
-    add_test_case(eof_bytecode(code, 1023), EOFValidationError::invalid_max_stack_height,
+    add_test_case(eof_bytecode(code, 1023), EOFValidationError::invalid_max_stack_increase,
         "invalid_1024_rjumpis");
 }
 
@@ -882,7 +882,7 @@ TEST_F(eof_validation, underflow_variable_stack)
     // JUMPF to returning function - neither min nor max enough for 5 inputs
     add_test_case(eof_bytecode(callf(1) + OP_STOP, 3)
                       .code(varstack + jumpf(2), 0, 3, 3)
-                      .code(bytecode{OP_POP} + OP_POP + OP_RETF, 5, 3, 3),
+                      .code(bytecode{OP_POP} + OP_POP + OP_RETF, 5, 3, 5),
         EOFValidationError::stack_underflow);
 
     // JUMPF to non-returning function - [1, 3] stack - neither min nor max enough for 5 inputs
@@ -1181,7 +1181,7 @@ TEST_F(eof_validation, jumpf_to_returning_variable_stack)
     // JUMPF from [1, 3] stack to function with 5 inputs
     add_test_case(eof_bytecode(callf(1) + OP_STOP, 3)
                       .code(varstack + jumpf(2), 0, 3, 3)
-                      .code(push0() + OP_RETF, 5, 3, 3),
+                      .code(push0() + OP_RETF, 5, 3, 5),
         EOFValidationError::stack_underflow);
 
     // JUMPF from [1, 3] stack to function with 3 inputs
diff --git a/test/unittests/eof_validation_test.cpp b/test/unittests/eof_validation_test.cpp
@@ -678,19 +678,19 @@ TEST_F(eof_validation, max_stack_height)
         EOFValidationError::success);
 
     add_test_case(eof_bytecode(1024 * push(1) + OP_STOP, 1024),
-        EOFValidationError::max_stack_height_above_limit);
+        EOFValidationError::max_stack_increase_above_limit);
 
     add_test_case(eof_bytecode(0x400 * push(1) + callf(1) + 0x400 * OP_POP + OP_STOP, 1024)
                       .code(OP_RETF, 0, 0, 0),
-        EOFValidationError::max_stack_height_above_limit);
+        EOFValidationError::max_stack_increase_above_limit);
 
     add_test_case(eof_bytecode(callf(1) + OP_STOP, 0)
                       .code(0x400 * push(1) + 0x400 * OP_POP + OP_RETF, 0, 0, 1023),
-        EOFValidationError::invalid_max_stack_height);
+        EOFValidationError::invalid_max_stack_increase);
 
     add_test_case(eof_bytecode(1024 * push(1) + callf(1) + 1024 * OP_POP + OP_STOP, 1023)
                       .code(OP_RETF, 0, 0, 0),
-        EOFValidationError::invalid_max_stack_height);
+        EOFValidationError::invalid_max_stack_increase);
 
     add_test_case(eof_bytecode(rjumpi(2, 0) + 1 + OP_STOP, 1), EOFValidationError::success);
 
diff --git a/test/unittests/evm_eof_function_test.cpp b/test/unittests/evm_eof_function_test.cpp
@@ -30,7 +30,7 @@ TEST_P(evm, eof_function_example2)
 
     rev = EVMC_OSAKA;
     const auto code =
-        "ef0001 01000c 020003 003b 0017 001d FF0000 00 00800004 01010003 01010004"
+        "ef0001 01000c 020003 003b 0017 001d FF0000 00 00800004 01010002 01010003"
         "60043560003560e01c63c76652678114e1001c63c6c2ea178114e100065050600080fd50e30002600052602060"
         "00f350e3000160005260206000f3"
         "60018111e10004506001e460018103e3000181029050e4"
diff --git a/test/utils/bytecode.hpp b/test/utils/bytecode.hpp
@@ -157,7 +157,7 @@ struct eof_bytecode
 
         // types section
         for (const auto& type : m_types)
-            out += bytes{type.inputs, type.outputs} + big_endian(type.max_stack_height);
+            out += bytes{type.inputs, type.outputs} + big_endian(type.max_stack_increase);
         return out;
     }
 
@@ -169,7 +169,7 @@ struct eof_bytecode
     auto& code(bytecode c, uint8_t inputs, uint8_t outputs, uint16_t max_stack_height)
     {
         m_codes.emplace_back(std::move(c));
-        m_types.emplace_back(inputs, outputs, max_stack_height);
+        m_types.emplace_back(inputs, outputs, max_stack_height - inputs);
         return *this;
     }
 

Original file line number	Diff line number	Diff line change
`@@ -157,7 +157,7 @@ struct eof_bytecode`
`157`	`157`
`158`	`158`	`// types section`
`159`	`159`	`for (const auto& type : m_types)`
`160`		`- out += bytes{type.inputs, type.outputs} + big_endian(type.max_stack_height);`
	`160`	`+ out += bytes{type.inputs, type.outputs} + big_endian(type.max_stack_increase);`
`161`	`161`	`return out;`
`162`	`162`	`}`
`163`	`163`
`@@ -169,7 +169,7 @@ struct eof_bytecode`
`169`	`169`	`auto& code(bytecode c, uint8_t inputs, uint8_t outputs, uint16_t max_stack_height)`
`170`	`170`	`{`
`171`	`171`	`m_codes.emplace_back(std::move(c));`
`172`		`- m_types.emplace_back(inputs, outputs, max_stack_height);`
	`172`	`+ m_types.emplace_back(inputs, outputs, max_stack_height - inputs);`
`173`	`173`	`return *this;`
`174`	`174`	`}`
`175`	`175`