Skip to content

fix(evm): consume all gas when a frame halts exceptionally - #26

Closed
abmcar wants to merge 1 commit into
feat/evm-persistent-code-cachefrom
fix/evm-halt-consumes-all-gas
Closed

abmcar wants to merge 1 commit into
feat/evm-persistent-code-cachefrom
fix/evm-halt-consumes-all-gas

Conversation

@abmcar

@abmcar abmcar commented Aug 28, 2026 •

Copy link
Copy Markdown
Owner

The three places that finalise an evmc_result all ended with

Result.gas_left = <instance>.getGas();

which unconditionally overwrites whatever the halt paths had already decided.
The instance's gas counter is not authoritative after a halt: the interpreter
zeroes the frame's gas, and the JIT trap path never touches gas at all. So
an exceptional halt handed the caller back gas the EVM had already burned.

evmone states the opposite rule directly, in make_execution_result:

// An exceptional halt consumes all gas; only a success or revert keeps gas_left.
if (state.status != EVMC_SUCCESS && state.status != EVMC_REVERT)
    gas_left = 0;

This routes the three sites through one helper that applies that rule.

How it was found

Mainnet witness replay, blocks 25818502 and 25818530: a nested frame halted with
EVMC_INVALID_MEMORY_ACCESS and returned 339011 of its 440977 gas limit. The
host bridge (abmcar/reth#1) rejects any halted result that retained gas, which is what surfaced
it.

Regression risk

Low, and bounded by construction. The bridge already rejected every result
that was both halted and carrying non-zero gas, so the only executions whose
behaviour changes are ones that were already failing. Under the interpreter the
frame's gas was zeroed anyway; the difference is that the frame's decision now
survives to the caller.

This is necessary but not sufficient for those two blocks

The halt itself is a separate JIT defect. Trap instrumentation shows
faulting=(nil) with memsize=1088 — a null-pointer dereference inside JIT'd
code, far from the 16 MB memory bound, which the trap handler classifies
uniformly as out-of-bounds memory and reports as EVMC_INVALID_MEMORY_ACCESS.

Decisive isolation: same library, same two blocks, with mode=interpreter both
pass, with access counts matching the reference leg exactly (5153/5153 and
3296/3296) and verified post-state roots. So the remaining defect is JIT-only
— not EVM semantics, not the host bridge.

With only this gas fix applied, block 25818502 no longer triggers a host fault
but its block gas used still disagrees (27730760 against an expected 27644811),
and 25818530 times out. The null dereference is still open and being tracked
separately.

Base

Branches from feat/evm-persistent-code-cache at 35d2f13, so the diff is this
change alone. The commit was cherry-picked over 37b36b1 (the benchmark cache
knob, sent as its own PR) to keep the two independent; the patch and message are
byte-identical to the original — git patch-id matches. Note that the validated
library (bbddb301…) was built from the original commit in place; this exact
tree has not itself been compiled.

The three places that finalise an evmc_result all ended with

    Result.gas_left = <instance>.getGas();

which overwrites whatever the halt paths had already decided. The instance's
gas counter is not authoritative after a halt: the interpreter zeroes the
*frame's* gas (interpreter.cpp) and the JIT trap path never touches gas at
all, so an exceptional halt handed the caller back gas the EVM had already
burned. evmone applies the opposite rule in make_execution_result - "an
exceptional halt consumes all gas; only a success or revert keeps gas_left" -
and that is the consensus behaviour.

Route the three sites through one helper that applies that rule. Observed on
mainnet blocks 25818502 and 25818530, where a nested frame halted with
EVMC_INVALID_MEMORY_ACCESS and returned 339011 of its 440977 gas limit.

This is necessary but not sufficient for those two blocks: the halt itself is
a separate JIT defect (a null-pointer dereference inside JIT'd code that the
trap handler reports as out-of-bounds memory). Both blocks replay correctly
under mode=interpreter, so the remaining defect is JIT-only.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0187PTseLY2NbzxKrnstDWwS
@github-actions

Copy link
Copy Markdown

⚠️ Performance Regression Detected (multipass)

No benchmark summary available.

@github-actions

Copy link
Copy Markdown

⚠️ Performance Regression Detected (interpreter)

No benchmark summary available.

@github-actions

Copy link
Copy Markdown

⚡ Performance Regression Check Results

⚠️ Performance Regression Detected (interpreter)

No benchmark summary available.


⚠️ Performance Regression Detected (multipass)

No benchmark summary available.


@abmcar

abmcar commented Aug 29, 2026

Copy link
Copy Markdown
Owner Author

Superseded by DTVMStack#605, which targets upstream main instead of this fork's feat/evm-persistent-code-cache. Same commit, rebased onto 338d123, with one clang-format reflow in src/evm/evm.h required by upstream's format check. Verified there: format check clean, Release build passes, 11/12 ctest binaries pass (the one failure is solc not found, identical on a pristine 338d123 build). Closing in favour of the upstream PR.

@abmcar abmcar closed this Aug 29, 2026
@abmcar

abmcar commented Aug 29, 2026

Copy link
Copy Markdown
Owner Author

Correction to the description above, for anyone reading this closed PR as history.

The paragraph citing faulting=(nil) with memsize=1088 — arguing the fault is "far from the 16 MB memory bound" and therefore not a real out-of-bounds access — should not be relied on. The memsize figure is unusable as evidence for two independent reasons:

  1. offsetof(EVMInstance, MemorySize) is itself 1088 (MemoryBase at 0x438 = 1080, MemorySize at 0x440 = 1088), measured. The value collides exactly with an offset the trap handler also has in scope.
  2. The trap handler is known to attribute the fault to the wrong frame — it names an EVMInstance whose jit-exit has already printed — so the fields it reports may belong to a different instance.

The null faulting address itself comes from the signal, not from the handler's frame guess, so "the fault is at a null pointer" stands; only the corroborating memsize reasoning is withdrawn. Neither upstream PR (DTVMStack#604, DTVMStack#605) cites this number — both were checked.

The JIT defect behind blocks 25818502 and 25818530 remains open and is being investigated with a read that bypasses the handler's frame attribution entirely.

@abmcar

abmcar commented Aug 29, 2026

Copy link
Copy Markdown
Owner Author

Follow-up: reason 2 in my correction above is itself wrong, and the conclusion has changed. Amending rather than editing, to keep the trail.

Reason 2 (frame misattribution) is withdrawn. A read that bypasses the handler entirely — %r14/%r12 out of uc_mcontext — shows tls[0] handling=1 on exactly the instance the faulting instruction was addressing, validated through its Mod field against RIP, with all outer frames handling=0. The innermost frame is the one we land on. The handler does not misattribute.

Reason 1 is now confirmed, and is sufficient on its own. The dump shows %rcx == 0x440 — offsetof(EVMInstance, MemorySize) computed off a null base. That is literally where 1088 came from. It was never a memory size.

So the original conclusion stands on firmer ground than the reasoning I gave for it: memsize=1088 is not evidence of anything, and the si_addr boundary still holds — the null faulting address comes from the signal, so "the fault is at a null pointer" was always safe.

The defect is no longer open. The mechanism is lazy allocation, not nested calls: a frame starts with MemoryBase == nullptr and the buffer is created on that frame's first real growth. When that first growth happens inside one of 15 runtime helpers rather than through expandMemoryIR, the helper allocates and sets the instance's base, the JIT reloads only the size, and the entry-time null survives in the cached base. Depth 0 on a fresh instance qualifies — no nested call needed. Fix is 8403be3 on fix/evm-jit-null-membase; block 25818502 now passes with the correct gasUsed 27644811.

@abmcar

abmcar commented Aug 29, 2026

Copy link
Copy Markdown
Owner Author

Current state of the record — this comment supersedes my two above. They are left in place for the trail, but read this one.

The thread got hard to follow because I corrected twice and overstated once. Consolidated:

On memsize=1088 (the original description's reasoning)

Withdrawn, and the reason is now confirmed rather than circumstantial: the uc_mcontext dump shows %rcx == 0x440, i.e. offsetof(EVMInstance, MemorySize) computed off a null base. 1088 was never a memory size. It is not evidence of anything.

What survives untouched: the faulting address comes from the signal's si_addr and never passes through the handler's own bookkeeping, so "the fault is at a null pointer" was always safe. That is the line — si_addr trustworthy, the handler's printed fields not.

On "the handler misattributes the frame"

I wrote above that this was falsified. That was one notch too strong — the same overreach as 1088 itself. Accurately, in three parts:

  1. The observation stands. An EVMInstance that had already logged jit-exit did appear to be executing.
  2. The proposed mechanism is refuted for this fault. Reading %r14/%r12 out of uc_mcontext puts tls[0] handling=1 on exactly the instance the faulting instruction addressed, validated through Mod against RIP, outer frames handling=0. siglongjmp lands where it should here. This says nothing about whether misattribution can occur in general.
  3. The likely explanation is instance pooling, and is probably benign. getOrCreateInstance returns VM->CacheInsts[depth-1] (or CachedMainInst at depth 0) and resets rather than recreates. A jit-exit log keyed on the instance address will therefore show an already-exited address reappearing as the expected pointer for the next call at that depth. The unstated premise was that an instance address identifies a frame; under pooling it does not.

Separately and still open: RESET-ON-LIVE is real. Keying getOrCreateInstance purely on Msg->depth is safe only while depth uniquely identifies a live frame. Not this trigger, but a genuine latent issue.

On the defect itself

Resolved, and the mechanism is lazy allocation rather than nested calls. A frame starts with MemoryBase == nullptr; the buffer is created on that frame's first real growth. When that growth happens inside one of 15 runtime helpers rather than through expandMemoryIR, the helper allocates and sets the instance's base, the JIT reloads only the size, and the entry-time null survives in the cached base. Depth 0 on a fresh instance qualifies — no nested call required. Fix is 8403be3 on fix/evm-jit-null-membase; block 25818502 passes with the correct gasUsed 27644811.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants