fix(evm): do not report a null-pointer dereference as an EVM memory fault - #604
Open
abmcar wants to merge 1 commit into
Open
fix(evm): do not report a null-pointer dereference as an EVM memory fault#604abmcar wants to merge 1 commit into
abmcar wants to merge 1 commit into
Conversation
…ault The JIT trap handler mapped every SIGSEGV to ErrorCode::OutOfBoundsMemory and EVMC_INVALID_MEMORY_ACCESS. That is right for a real out-of-bounds EVM memory access, and wrong for a fault at a null pointer: EVM memory is a heap buffer and is never mapped at page zero, so a fault there is a defect in compiled code, not an EVM condition. The misclassification is not cosmetic. EVMC_INVALID_MEMORY_ACCESS is a consensus status, so an engine crash was handed to the embedder as a plausible exceptional halt; execution continued and the block finished with the wrong gas (mainnet block 25818502: 27730760 used against an expected 27644811) instead of failing. Finding the real cause needed a patched build printing the faulting address. Classify a fault in the first page as EVMNullPointerDereference / EVMC_INTERNAL_ERROR, which is not a consensus status and which an embedder must reject. On the two blocks above the replay now stops with "nested DTVM returned backend/internal status -1" rather than producing a wrong gas number. This is a tripwire, not a fix: the branch should be unreachable. The dereference behind those two blocks is still open, and is JIT-only - both blocks replay correctly under mode=interpreter with database access counts matching the reference exactly (3296 and 5153). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0187PTseLY2NbzxKrnstDWwS
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts EVM JIT trap handling so that faults at (or near) a null pointer are reported as an internal engine error rather than being misclassified as an EVM out-of-bounds memory access, preventing an engine defect from being surfaced as a consensus-visible halt condition.
Changes:
- Add a dedicated
EVMNullPointerDereferenceerror code for JIT-only null-pointer faults. - Update
Runtime::callEVMInJITModesignal-trap mapping to classify low-address SIGSEGV/SIGBUS asEVMC_INTERNAL_ERRORinstead ofEVMC_INVALID_MEMORY_ACCESS.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/runtime/runtime.cpp | Adds low-address (null) fault detection in the EVM JIT trap handler and maps it to an internal EVMC status. |
| src/common/errors.def | Introduces EVMNullPointerDereference to represent JIT null-pointer dereference defects distinctly from EVM execution faults. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+933
to
+936
| /// Size of the unmapped low address range. A faulting address below this is a | ||
| /// null pointer plus a small offset, never a real allocation. | ||
| static constexpr uintptr_t NullPointerPageSize = 4096; | ||
|
|
⚡ Performance Regression Check Results✅ Performance Check Passed (interpreter)Performance Benchmark Results (threshold: 25%)
Summary: 194 benchmarks, 0 regressions ✅ Performance Check Passed (multipass)Performance Benchmark Results (threshold: 25%)
Summary: 194 benchmarks, 0 regressions |
This was referenced Aug 29, 2026
Contributor
Author
|
Updated section 3 of the description: when this PR was opened, the underlying null dereference was still unlocated. It has since been found and fixed in #607 (the multipass JIT reloaded its cached memory size after a runtime helper but never the cached base). Noting the edit here rather than leaving it silent. This PR is unaffected in substance. The relationship between the three, for anyone reviewing them together:
Each stands alone, which is why they are three PRs and not one. |
16 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):
2. What is the scope of this PR (e.g. component or file name):
src/runtime/runtime.cppJIT trap handling for EVM, plus one new error code insrc/common/errors.def.3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
The JIT trap handler mapped every SIGSEGV to
ErrorCode::OutOfBoundsMemoryandEVMC_INVALID_MEMORY_ACCESS. That is correct for a real out-of-bounds EVM memoryaccess and wrong for a fault at a null pointer: EVM memory is a heap buffer and
is never mapped at page zero, so a fault there is a defect in compiled code, not
an EVM condition.
The misclassification is not cosmetic.
EVMC_INVALID_MEMORY_ACCESSis aconsensus status, so an engine crash was handed to the embedder as a plausible
exceptional halt. Execution continued and the block finished with the wrong gas
instead of failing: on mainnet block 25818502 the replay reported 27730760 gas
used against an expected 27644811. Locating the real cause required a patched
build that printed the faulting address.
This classifies a fault in the first page as
EVMNullPointerDereference/EVMC_INTERNAL_ERROR, which is not a consensus status and which an embedder mustreject. On the affected blocks the replay now stops with an internal-status error
rather than producing a wrong gas number.
This is a tripwire, not a fix. The branch should be unreachable, and this PR
does not make it so — it only ensures that if it is reached, the embedder is told
something it must reject rather than a consensus status it may accept.
Update since this PR was opened: the specific dereference behind those two
blocks has since been found and fixed in #607 — the multipass JIT reloaded its
cached memory size after a runtime helper but never the cached base, so a frame
whose first memory growth happened inside a helper kept an entry-time null base.
That does not make this PR redundant; it is the reason the defect survived as
long as it did. Without this reclassification a null dereference is reported as
EVMC_INVALID_MEMORY_ACCESS— a consensus status — so the caller treats anengine crash as a legitimate exceptional halt, execution continues, and the
block's gas comes out wrong instead of the run failing loudly. #607 removes that
cause; this PR makes any remaining member of the class audible. The two are
independent and separately mergeable.
4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):
An embedder that today sees
EVMC_INVALID_MEMORY_ACCESSfor a null-pointer faultin JIT'd code will now see
EVMC_INTERNAL_ERROR. This is deliberate and is thepoint of the change: the old status is a consensus status that an embedder may
legitimately accept as an exceptional halt, and accepting it silently converts an
engine defect into a wrong execution result. No behaviour changes for a genuine
out-of-bounds EVM memory access, which still maps to
EVMC_INVALID_MEMORY_ACCESS.5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:
No automated test is added: the branch is only reachable through a defect in
generated code, and there is no supported way to construct that fault from an EVM
program. Suggestions for a test hook are welcome.
Validation on a GCC 12 / LLVM 15 Release multipass build, configured to match the
existing EVM configuration (
ZEN_ENABLE_EVM,ZEN_ENABLE_MULTIPASS_JIT,ZEN_ENABLE_VIRTUAL_STACK,ZEN_ENABLE_CPU_EXCEPTIONall ON):./tools/format.sh check: PASS338d123: PASStools/easm2bytecode.sh): 209/209ctest: 11/12 test binaries passThe one failure,
solidityContractTests(7 cases), issolc not foundin thisenvironment. It fails identically on a pristine
338d123build configured thesame way, which was built and run side by side for exactly this comparison — so
it is environmental, not a regression from this change. Every other binary
passes, including
evmDifferentialTests,evmJitFrontendTests,evmStateTests,evmInterpTests,evmModuleCacheTests,evmFallbackExecutionTestsandevmProfileGuidedJITTests.Behavioural check: mainnet witness replay of blocks 25818502 and 25818530 through
the EVMC interface. Before, the run completed with an incorrect gas figure; after,
it stops with a backend/internal status. Access counts at the fault are unchanged
(152 and 45), i.e. the underlying dereference is not affected by this change.
6. Release note