Skip to content

fix(analysis): read local constants from their MemorySSA clobber - #262

Merged
SizzleUnrlsd merged 9 commits into
mainfrom
fix/const-from-value-memoryssa
Nov 8, 2026
Merged

SizzleUnrlsd merged 9 commits into
mainfrom
fix/const-from-value-memoryssa

Conversation

@SizzleUnrlsd

@SizzleUnrlsd SizzleUnrlsd commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

tryGetConstFromValue used to return the last constant stored to a slot anywhere in the function,
and it dropped every cast on the way. Three things changed its answer even though the value read
was the same:

  • a store made after the read;
  • a store made on one branch only;
  • a cast, so (unsigned char)300 read as 300.

The result fed three things: the "appears unreachable" note on buffer overflows, the
VLA-versus-constant decision, and the local stack size.

  • MemorySSA. A load now resolves only when its MemorySSA clobber, from FunctionFacts, is a
    simple store of the same type to the same pointer. That store wrote exactly the bytes the load
    reads.
  • Unknown cases. A merge of several stores gives no constant, and so does a store that may only
    overlap, a call, or the function entry.
  • Casts are folded with ConstantFoldCastOperand.
  • Same logic as SMT. This mirrors encodeLoad in SmtEncoding.cpp.
  • Reachability. Separately, isStaticallyUnreachableStackAccess now requires every edge into
    the access to be dead. One dead edge used to be enough, even when another edge reached the access.
  • Callers. Reachability, AllocaUsage, StackComputation and DynamicAlloca pass a
    FunctionFacts. The callers that had none build it the first time an operand is not already a
    constant.

Diagnostic changes

The analyzer's full output was compared on every fixture, default and smt-z3 passes, against
main at 703da23. Diagnostic blocks were compared regardless of their order; see Limits.

  • Existing fixtures. Three "appears unreachable" notes are gone:

    • test/bound-storage/global-array-overflow.c, the write after the loop;
    • test/coretrace-legacy/bound_index.cc:13 and :14.

    All three are accesses after a loop, where the index is 10, so they run. The old lookup read
    i = 0 and concluded that i < 10 always holds.

  • Existing expectation. global-array-overflow.c listed that wrong note in its expectation.
    The fixture passed anyway, because run_test.py matches the location and the headline only.
    That line is removed, and a negative expectation now forbids the note.

  • Warnings and errors. None appears or disappears on any existing fixture.

  • New fixtures (6) cover:

    • a store after the read;
    • a store on one branch;
    • a truncating cast;
    • an access reached by a dead and a live edge;
    • a VLA whose size is stored on one branch, now reported as dynamic, where it was taken as
      4096 bytes;
    • a VLA whose size variable is reassigned after it, now 6 bytes, where it was taken as 100 bytes.

Verification

  • python3 run_test.py --jobs=4: 2505/2505 pass at 6d7af3e. main passes 2467, and the
    difference is the new tests. The two test-only commits after it were checked on all 45 fixtures
    of test/bound-storage and test/vla, in both passes.
  • Each new guard fails when it should:
    • the negative expectation in global-array-overflow.c matches the note in the 703da23 output,
      in both passes;
    • vla-size-later-store-is-not-read.c checks its detail lines (strict-expectation-details).
      With 7 bytes in the expectation it fails; without the flag, it passed.
  • The new unit test testTryGetConstFromValue checks 9 cases: exact store, trunc, sext, store
    after the load, narrower load, merge, may-alias store, volatile load, entry value.
  • The intermediate commit (all edges dead, old lookup) builds, passes the unit tests and fixes the
    multi-edge fixture alone.
  • clang-format 20 reports no violation on the touched files.

Limits

  • Constant values only. Only plain constants and casts of them resolve. A value computed by
    arithmetic (n = m + 1) stays unknown, as before.
  • Reachability scope. Reachability still looks only at the branches directly into the access.
    A block reached only from dead blocks, or through a switch, is not recognized.
  • Run-to-run ordering on main. On main, test/false-positive-repro/stb-like-next-code-uninitialized.c
    prints its two MultipleStoresToStackBuffer notes in an order that changes between runs. This PR
    does not touch that.

…m locals

Each access runs, so none may carry the "appears unreachable" note. The
condition reads a local before a later store, after a store made on one
branch only, and through a cast that truncates 300 to 44. All three get
the note today: tryGetConstFromValue takes the last constant stored to
the slot anywhere in the function and drops every cast.
…e edge

`x > 0 || zero` branches to the access from two blocks. The `zero` edge
is dead and the `x > 0` edge is not, yet one dead edge is enough for the
"appears unreachable" note today.
A size stored on one branch only is not one constant, and a store made
after the VLA does not change its size. Today the first is taken as
4096 bytes and the second as 100 bytes.
…to it is dead

An access was called unreachable as soon as one conditional branch into
its block could not take that edge, even when another edge reached it.
Require every edge into the block to come from a conditional branch whose
comparison folds to the other successor. Operands that fold to constants
of different widths prove nothing. ICmpInst::compare replaces the switch
over the predicates.
…lying casts

tryGetConstFromValue took the last constant stored to the slot anywhere
in the function and dropped every cast on the way. A store after the
read, or on one branch only, changed the value, and a truncation of 300
to i8 read as 300.

A load now resolves only when its MemorySSA clobber, from FunctionFacts,
is a simple store of the same type to the same pointer: that store wrote
exactly the bytes the load reads. Casts are folded with
ConstantFoldCastOperand. A merge of several stores, a store that may
only overlap, a call or the function entry gives no constant.

Reachability, AllocaUsage, StackComputation and DynamicAlloca pass a
FunctionFacts; those that had none build it on the first operand that is
not already a constant. On the fixtures, the "appears unreachable" note
leaves three accesses after a loop, which run (global-array-overflow.c:18,
bound_index.cc:13 and :14). No warning or error changes.
An exact store, a truncation, a sign extension and a store made after the
load resolve. A narrower load, a merge of two stores, a store through a
pointer that may alias, a volatile load and memory on entry do not.
auto* LI = dyn_cast<const LoadInst>(cur);
if (!LI)
return nullptr;
if (const auto* constant = dyn_cast<ConstantInt>(value))
…the loop

The write after the loop in global-array-overflow.c runs with i == 10,
so the "[info] this access appears unreachable" line its expectation
listed was wrong. The fixture passed anyway because expectations match
on the location and headline only. Drop that line and add a negative
expectation so the note cannot come back. The access moves up to line
17, where the expectation already placed it.
The fixture pins "requested stack size: 6 bytes" in a detail line, but
the headline-only fallback ignores detail lines: an expectation of
7 bytes passed too. Turn strict-expectation-details on.
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