Fix out-of-bounds crash in Stream affinity analysis for scf.while#24723
Open
pstarkcdpr wants to merge 2 commits into
iree-org:mainiree-org/iree:mainfrom
pstarkcdpr:fix-scf-while-assertpstarkcdpr/iree:fix-scf-while-assertCopy head branch name to clipboard
Open
Fix out-of-bounds crash in Stream affinity analysis for scf.while#24723pstarkcdpr wants to merge 2 commits intoiree-org:mainiree-org/iree:mainfrom pstarkcdpr:fix-scf-while-assertpstarkcdpr/iree:fix-scf-while-assertCopy head branch name to clipboard
pstarkcdpr wants to merge 2 commits into
iree-org:mainiree-org/iree:mainfrom
pstarkcdpr:fix-scf-while-assertpstarkcdpr/iree:fix-scf-while-assertCopy head branch name to clipboard
Conversation
When an scf.while carries more values through its "before" region than the op has results (i.e. some loop-carried values are not forwarded to a result via scf.condition), the affinity analysis mapped an scf.yield operand number directly onto whileOp->getResult(operandNumber). Since the "after" region yield has one operand per init operand (N) while the op may have fewer results (M), this indexed getResult() out of bounds, tripping the getOpResultImpl assertion (or a non-deterministic SIGSEGV in release builds). Two independent sites made this naive mapping: - Affinity.cpp (ValueConsumerAffinityPVS::updateFromUse): drop the whileOp->getResult(operandNumber) propagation in the scf.yield -> scf.while branch and keep only the before-region-argument propagation. While results are already handled by the scf.condition case. - Explorer.cpp (walkTransitiveUses): guard the ReturnLike result-mapping fallback with !isa<RegionBranchTerminatorOpInterface> plus a getNumResults() bounds check. Region branch terminators such as scf.yield are already handled correctly above via the successor-operand mapping. Adds regression test @scf_while_extra_loop_carried, which exercises the N > M shape (two loop-carried values, one result) that previously crashed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Paul Stark <paul.stark@cdprojektred.com>
pstarkcdpr
requested review from
MaheshRavishankar,
benvanik and
hanhanW
as code owners
July 17, 2026 23:46
AGindinson
self-requested a review
July 21, 2026 06:39
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
1 participant
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.
See issue #24722
The issue contains a deeper explanation of the issue and a repro case (which is the same as the lit test here).
When an
scf.whilecarries more values through its "before" region than the op has results (i.e. some loop-carried values are not forwarded to a result viascf.condition), the affinity analysis mapped anscf.yieldoperand number directly ontowhileOp->getResult(operandNumber). Since the "after" region yield has one operand per init operand (N) while the op may have fewerresults (
M), this lead togetResult()being out of bounds.Two independent sites made this naive mapping:
Affinity.cpp: in thescf.yield→scf.whilebranch ofValueConsumerAffinityPVS::updateFromUse, drop thewhileOp->getResult(operandNumber)propagation and keep only the before-region-argument propagation. (Results are already handled by thescf.conditioncase.)Explorer.cpp: guard theReturnLikeresult-mapping fallback inwalkTransitiveUseswith!isa<RegionBranchTerminatorOpInterface>(ownerOp)(region-branch terminators likescf.yieldare already handled correctly above) plus ause.getOperandNumber() < parent->getNumResults()bounds check.Added regression test
@scf_while_extra_loop_carried, which exercises theN > Mshape (two loop-carried values, one result) that previously asserted.Note that Claude made the fix suggestions and, while they look okay to me, I'm not at all familiar with this code.