Repository navigation
fix(analysis): read local constants from their MemorySSA clobber - #262
Merged
Merged
Conversation
…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.
This was referenced Nov 6, 2026
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.
Summary
tryGetConstFromValueused 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:
(unsigned char)300read as 300.The result fed three things: the "appears unreachable" note on buffer overflows, the
VLA-versus-constant decision, and the local stack size.
FunctionFacts, is asimple store of the same type to the same pointer. That store wrote exactly the bytes the load
reads.
overlap, a call, or the function entry.
ConstantFoldCastOperand.encodeLoadinSmtEncoding.cpp.isStaticallyUnreachableStackAccessnow requires every edge intothe access to be dead. One dead edge used to be enough, even when another edge reached the access.
Reachability,AllocaUsage,StackComputationandDynamicAllocapass aFunctionFacts. The callers that had none build it the first time an operand is not already aconstant.
Diagnostic changes
The analyzer's full output was compared on every fixture, default and smt-z3 passes, against
mainat 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:13and:14.All three are accesses after a loop, where the index is 10, so they run. The old lookup read
i = 0and concluded thati < 10always holds.Existing expectation.
global-array-overflow.clisted that wrong note in its expectation.The fixture passed anyway, because
run_test.pymatches 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:
4096 bytes;
Verification
python3 run_test.py --jobs=4: 2505/2505 pass at 6d7af3e.mainpasses 2467, and thedifference is the new tests. The two test-only commits after it were checked on all 45 fixtures
of
test/bound-storageandtest/vla, in both passes.global-array-overflow.cmatches the note in the 703da23 output,in both passes;
vla-size-later-store-is-not-read.cchecks its detail lines (strict-expectation-details).With 7 bytes in the expectation it fails; without the flag, it passed.
testTryGetConstFromValuechecks 9 cases: exact store, trunc, sext, storeafter the load, narrower load, merge, may-alias store, volatile load, entry value.
multi-edge fixture alone.
Limits
arithmetic (
n = m + 1) stays unknown, as before.A block reached only from dead blocks, or through a
switch, is not recognized.main. Onmain,test/false-positive-repro/stb-like-next-code-uninitialized.cprints its two MultipleStoresToStackBuffer notes in an order that changes between runs. This PR
does not touch that.