Skip to content

Expr: sound disjointness rule for readWord/WriteWord with bounded offset#1065

Open
msooseth wants to merge 3 commits into
mainfrom
write-disjoint-readword
Open

Expr: sound disjointness rule for readWord/WriteWord with bounded offset#1065
msooseth wants to merge 3 commits into
mainfrom
write-disjoint-readword

Conversation

@msooseth
Copy link
Copy Markdown
Collaborator

Summary

Adds a new readWord clause that elides a WriteWord whose offset is of the form Add (Lit c) X, when X has a derivable static upper bound and the write region is provably disjoint from the read in EVM (mod 2^256) arithmetic.

  • New helper upperBound :: Expr EWord -> Maybe W256 — conservative static upper bound handling Lit, And-with-Lit, Mod-by-Lit, Div-by-Lit, Mul/Add with one Lit operand, and SHR by a Lit shift.
  • New helper writeDisjointFromReadWord :: W256 -> Expr EWord -> Bool — uses Integer arithmetic to detect wrap on both the read and write byte ranges before concluding the write can be skipped, so the rule does not assume non-wrapping addition.
  • Hook in readWord at the WriteWord case.

Split out from #1063 so the disjointness rule is reviewable on its own; #1063 will be rebased on top of this.

Tests

5 new unit tests in test/EVM/Expr/ExprTests.hs:

  • positive: WriteWord at Add(Lit, And-bounded) skipped
  • positive: WriteWord at Add(Lit, Mul-And-bounded) skipped
  • negative: unbounded X — not skipped
  • negative: wrap-risk near maxBound — not skipped
  • negative: read region overlaps write min offset — not skipped

Checklist

  • tested locally
  • added automated tests
  • updated the docs — N/A, internal simplifier rule
  • updated the changelog

🤖 Generated with Claude Code

Adds a new readWord clause that elides a WriteWord whose offset is of
the form `Add (Lit c) X`, when X has a derivable static upper bound and
the write region is provably disjoint from the read in EVM (mod 2^256)
arithmetic.  Backed by a new conservative `upperBound :: Expr EWord ->
Maybe W256` helper that handles Lit, And-with-Lit, Mod-by-Lit, Div-by-
Lit, Mul/Add with one Lit operand, and SHR by a Lit shift.

The disjointness check uses Integer arithmetic to detect wrap on both
the read and write byte ranges before concluding the write can be
skipped, so the rule does not assume non-wrapping addition.

Tests (5 local Expr unit tests in memoryTests):
  * positive: WriteWord at Add(Lit, And-bounded) skipped
  * positive: WriteWord at Add(Lit, Mul-And-bounded) skipped
  * negative: unbounded X — not skipped
  * negative: wrap-risk near maxBound — not skipped
  * negative: read region overlaps write min offset — not skipped

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@blishko blishko left a comment

Choose a reason for hiding this comment

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

Looks good!

I am suggesting one small change that I think makes the condition more readable.

Comment thread src/EVM/Expr.hs
readMaxI = iI + 31
in readMaxI <= maxI
&& writeMaxI <= maxI
&& (cI >= iI + 32 || writeMaxI < iI)
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.

Suggested change
&& (cI >= iI + 32 || writeMaxI < iI)
&& (cI > readMaxI || writeMaxI < iI)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Wait, doesn't that modify the code?

readMaxI  = iI + 31

But you also changed >= to > ?

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.

Yes.
readMaxI = iI + 31 means you can rewrite cI > readMaxI as cI > iL + 31and that's equivalent to cI >= iL + 32, which is you original expression, no?

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