Skip to content

Fix #1082: resolve symbolic-key SLOAD over a ConcreteStore - #1083

Merged
msooseth merged 1 commit into
mainfrom
fix-1082-symbolic-key-sload
Aug 4, 2026
Merged

Fix #1082: resolve symbolic-key SLOAD over a ConcreteStore#1083
msooseth merged 1 commit into
mainfrom
fix-1082-symbolic-key-sload

Conversation

@gustavo-grieco

@gustavo-grieco gustavo-grieco commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Fixes #1082.

Problem

readStorage returns an unresolved abstract SLoad for a symbolic slot over a ConcreteStore, even though the store's contents are fully known. The symbolic interpreter then forks on an effectively-known value. Two symptoms:

  • Non-termination: observed through echidna's verification mode (concrete deployment populates a ConcreteStore, then a symbolic mapping read never resolves: 0 SMT queries, frozen coverage).
  • Blowup on populated stores: standalone hevm test, a symbolic-key read of a never-written mapping over a store holding 600 entries of an unrelated mapping does not complete within 120s. All those entries are dragged into the fork and the SMT encoding even though none of them can alias the read.

Fix

Two pieces. Notably this is not the ITE-over-all-entries fold proposed in the issue: that would inline the whole store into every symbolic read and trades non-termination for OOM on populated stores.

  1. An empty ConcreteStore reads 0 at every slot, including symbolic ones. A ConcreteStore is total (unwritten slots are 0), so with no entries every read is 0. This lives in readStorage' (the total variant used by the interpreter's internal-storage path and by expression simplification), not in readStorage: accessStorage relies on readStorage's Nothing/SLoad results to drive lazy RPC slot fetching for external contracts, whose ConcreteStore is a partial cache rather than the full storage. Folding the rule into readStorage would make every first read of an unfetched slot on a forked contract silently return 0 instead of issuing PleaseFetchSlot. A new unit test (readStorage-keeps-nothing-on-concrete-miss) pins that contract.

  2. At read time (OpSload), filter the ConcreteStore to entries that can belong to the read's mapping, using the keccak preimages recorded during concrete execution: a concrete key whose known preimage identifies a different mapping slot cannot equal the symbolic read slot, by keccak injectivity — the same assumption family the store simplifier already uses (idsDontMatch, Lit < 256 vs Keccak). A read of an unwritten or unrelated mapping filters to empty → Lit 0, before any fork. Keys with unknown preimage are kept (conservative, sound). The filtered store is only used for the one read expression and is never written back.

This incidentally fulfills the existing word-eq-bug test's TODO: This should be simplified to (Lit 0) — its expectation is updated accordingly.

Tests

  • Expr unit tests (test/EVM/Expr/ExprTests.hs): empty-store rule for symbolic slots, preserved Nothing-on-concrete-miss contract of readStorage, write-stripping down to an empty base, and filterStoreByReadSlot drop/keep/ignore cases (using a real keccak' preimage).
  • Foundry tests (test/EVM/Test/FoundryTests.hs):
    • pass/symbolicKeySload.sol: the issue's literal empty-store repro, plus a 600-entry populated mapping with a symbolic read of an untouched mapping. Without the fix the latter does not complete (this is the regression signal); with it, ~1s. Both contracts use bool public constant IS_TEST deliberately — a scalar state var would keep a preimage-less slot in the store and route the read through the solver instead of resolving to 0.
    • fail/symbolicKeySload.sol: soundness guard — a symbolic read of the same mapping that holds concrete entries (10, enough to be representative while keeping the SMT query small) must keep them, so the assert is falsifiable. hevm finds and concretely validates the counterexample. If the filter ever unsoundly dropped same-mapping entries, this test would start "passing".

Measurements (arm64 macOS, z3)

Scenario before after
issue repro prove_symbolic_key (empty store) hangs via echidna verification mode 0.08s
unrelated-mapping read, 60 entries 1.44s 0.07s
unrelated-mapping read, 600 entries >120s (killed) ~1s
same-mapping read (must falsify) cex validated same cex, same time

Notes

  • keccakPreImgs are only recorded during concrete execution (OpSha3), so the filter benefits exactly the flows that hit this bug: concrete deployment/setUp() followed by symbolic execution (hevm test, echidna verification mode). During pure symbolic execution the preimage map is empty and the filter keeps everything (status quo).
  • The filter only applies to a bare ConcreteStore; reads over SStore write-chains are left to the existing simplification/decomposition machinery.
  • Entries at scalar slots (no recorded preimage) are always kept. If that ever shows up as a bottleneck, dropping k < 256 keys for keccak-shaped reads would mirror the existing small-lit-vs-keccak assumption; not needed for the cases here.

🤖 Generated with Claude Code

@gustavo-grieco
gustavo-grieco force-pushed the fix-1082-symbolic-key-sload branch from a9abf66 to b68e625 Compare July 23, 2026 09:03
readStorage returned an unresolved abstract SLoad for a symbolic slot over a
ConcreteStore, so the symbolic interpreter forked on an effectively-known
value and could fail to terminate (observed through echidna's verification
mode: 0 SMT queries, frozen coverage). Standalone, the unresolved read drags
the whole concrete store into the fork: a symbolic read over an unrelated
mapping populated with 600 entries does not complete within minutes.

Two sound, scalable pieces (NOT the ITE-over-all-entries proposed in the
issue, which trades non-termination for OOM on populated stores):

1. An empty ConcreteStore reads 0 at every slot, including symbolic ones.
   This lives in readStorage' (the total variant used by the interpreter's
   internal-storage path and by expression simplification), NOT in
   readStorage: accessStorage relies on readStorage's Nothing/SLoad results
   to drive lazy RPC slot fetching for external contracts, whose
   ConcreteStore is a partial cache rather than the full storage. A new unit
   test pins that contract.

2. For a populated store, at read time (OpSload) filter the ConcreteStore to
   entries in the read's mapping, using recorded keccak preimages: a
   concrete key whose preimage identifies a different mapping slot cannot
   equal the symbolic read slot (keccak injectivity, consistent with the
   existing store-simplification assumptions). A read of an
   unwritten/unrelated mapping filters to empty -> Lit 0, before any fork.
   Keys with unknown preimage are kept (conservative/sound).

Tests:
- Expr unit tests for the empty-store rule, the preserved readStorage
  Nothing contract, and filterStoreByReadSlot (drop/keep/ignore). The
  existing word-eq-bug expectation is updated to Lit 0, fulfilling its
  "should be simplified to (Lit 0)" TODO.
- Foundry tests: the issue's empty-store repro and a 600-entry
  unrelated-mapping read (does not complete without the fix, ~1s with it)
  in pass/; a same-mapping read in fail/ guards filter soundness (entries
  kept -> still falsifiable, counterexample validated concretely).

Verified: the issue's prove_symbolic_key verifies in under a second;
unrelated-mapping reads go from 1.44s (60 entries) / >120s, killed
(600 entries) to 0.07s / ~1s; same-mapping reads still falsify with a
validated counterexample; concrete-slot misses still return Nothing so the
RPC fetch path is unchanged by construction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@gustavo-grieco
gustavo-grieco force-pushed the fix-1082-symbolic-key-sload branch from b68e625 to 11ec8fd Compare July 23, 2026 10:06
@gustavo-grieco

Copy link
Copy Markdown
Collaborator Author

CI fix: the Symbolic-Key-SLoad-Fail foundry test made bitwuzla exit with std::bad_alloc (the same-mapping query carried 60 kept entries plus forge-std state, and with the solver dead the runner reported no cex, flipping the result to (True, False)). The query is semantically identical pre- and post-patch — the filter keeps everything for same-mapping reads — so this was test weight, not a regression. Slimmed the soundness canary to a bare contract with 10 entries; locally it finds and concretely validates the counterexample in ~2s with no warnings.

@msooseth msooseth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@msooseth
msooseth merged commit 576e869 into main Aug 4, 2026
9 checks passed
@msooseth
msooseth deleted the fix-1082-symbolic-key-sload branch August 4, 2026 13:31
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.

Symbolic-key SLOAD over a ConcreteStore is left unresolved, causing non-termination in symbolic execution

2 participants