fix(zkvm): provide single-threaded __atomic_* builtins for the rv64im guest - #106
Closed
Gabriel-Trintinalia wants to merge 1 commit into
Closed
Conversation
… guest
Any guest code that performs an atomic operation blows the 4 MiB stack on
the first call, and dies as a write below the stack region:
Mem::write_silent() invalid addr=9ffffff8 write section start=a0000000
compiler_rt's __atomic_<op>_N chooses between a spinlock and a native
atomic on `@sizeOf(T) > largest_atomic_size`, and largest_atomic_size is
derived from the pointer width (8 on rv64) rather than from whether the
target has atomic instructions. Our guest subtracts .a/.zaamo/.zalrsc
because ZisK's ISA is rv64im, so a u64 takes the "native" branch, LLVM
cannot lower it, and it emits a libcall straight back into the same
function. The disassembly is an unconditional self-call:
80139c7c <compiler_rt.atomics.__atomic_load_8>:
80139c84: li a1, 0x5
80139c86: auipc ra, 0x0; jalr -0xa(ra) -> 80139c7c
16 bytes of stack per turn exhausts 4 MiB in ~262k frames.
Define the builtins here instead. The linker then resolves them from
zesu.o and never pulls compiler_rt's versions. Plain loads and stores are
correct because the guest is strictly single-threaded — one hart, no
interrupts, no preemption — so nothing can observe a partial operation.
This surfaced as "mainnet blocks crash in the guest since Consensys-Incorporated#97", because
std.heap.ArenaAllocator was the first code to reach an atomic. Consensys-Incorporated#97 is not
at fault and needs no change; every other caller of an atomic would have
hit the same wall, only ever on real blocks and never in the native
suites.
Verified at main (99d2546) with Consensys-Incorporated#97's arenas untouched: the
mainnet_fusaka_24758573 vector reproduces its expected root under
ziskemu 1.1.0-alpha, `zig build test` passes, and the 500-block mainnet
corpus completes with zero errors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Closed in favour of #107 |
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.
Problem
Since
d399be9(#97), every real mainnet block crashes in the ZisK guest:0x9ffffff8is 8 bytes below the floor of the entire writable region —sphas descended the full 4 MiB stack. All 500 blocks of the mainnet corpus fail; the Amsterdam synthetic fixtures and every native suite (zig build test, blockchain-tests, zkevm) pass, which is why it merged. It also blockszesu-zkvmfrom pinningZESU_REFpast65f2ca8.Root cause — not #97
compiler_rt's__atomic_<op>_Npicks its implementation on@sizeOf(T) > largest_atomic_size:largest_atomic_sizeis derived from the pointer width (8 on rv64), not from whether the target actually has atomic instructions. Our guest target doescpu_features_sub = .{ .a, .zaamo, .zalrsc }(ZisK's ISA is rv64im), so au64takes the "native atomic" branch, LLVM cannot lower it, and emits a libcall back into the very same function.The disassembly is an unconditional self-call with no base case:
16 bytes of stack per turn exhausts 4 MiB in ~262k frames. Confirmed from an emulator change-trace of the final steps before the fault.
And the callers:
#97 is not at fault and needs no change.
std.heap.ArenaAllocatorwas simply the first guest code to reach an atomic. Any other caller — a refcount, astdcontainer that grew one internally, a futurestd.once— would have hit the identical wall, only on real blocks and never in the native suites.Change
src/zkvm/atomics.zigdefines the builtins for widths 1/2/4/8 across load / store / exchange / compare_exchange / fetch_{add,sub,and,or,xor,nand}. The linker resolves them fromzesu.oand never pulls compiler_rt's. Plain loads and stores are correct because the guest is strictly single-threaded — one hart, no interrupts, no preemption — so no operation here can be observed partially; the memory-order argument is ignored.The
_16variants are deliberately not defined:16 > largest_atomic_size, so those already take compiler_rt's spinlock path and do not recurse.Verification
All at main (
99d2546) with #97's arenas untouched:mainnet_fusaka_24758573(zesu-zkvm vector) reproduces its expected root under ziskemu 1.1.0-alpha — the same vector panics without this changezig build testpassesTrace cells over the corpus: 12,040,230,859,277 vs 11,960,081,652,573 at
65f2ca8(+0.67%) — that delta is main's accumulated changes since65f2ca8, not a cost of this fix.Repro (~30s)
Follow-up
Unblocks bumping
ZESU_REFinzesu-zkvmto zesu main; it has been pinned to65f2ca8since Aug 31 solely because of this.🤖 Generated with Claude Code