fix #63092: stack overflow in isReachableFlowNodeWorker with deeply nested flow graphs#63595
Closed
Amilliox wants to merge 2 commits into
Closed
fix #63092: stack overflow in isReachableFlowNodeWorker with deeply nested flow graphs#63595Amilliox wants to merge 2 commits into
Amilliox wants to merge 2 commits into
Conversation
… deeply nested flow graphs
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a compiler crash in control-flow reachability analysis by removing recursion in isReachableFlowNodeWorker for deeply nested FlowFlags.BranchLabel graphs, replacing it with an explicit worklist to prevent stack overflows.
Changes:
- Replaced recursive
some()traversal forBranchLabelreachability with an iterative worklist approach. - Inlined nested
BranchLabelantecedents into the worklist to avoid unbounded call stack growth.
Comment on lines
+28902
to
+28907
| const node = worklist.pop()!; | ||
| if (node.flags & FlowFlags.BranchLabel) { | ||
| const ant = (node as FlowLabel).antecedent; | ||
| if (ant) worklist.push(...ant); | ||
| } | ||
| else if (isReachableFlowNodeWorker(node, /*noCacheCheck*/ false)) { |
Comment on lines
+28898
to
+28901
| // Use explicit worklist to avoid stack overflow on deeply nested flow graphs. | ||
| const antecedents = (flow as FlowLabel).antecedent; | ||
| const worklist = antecedents ? [...antecedents] : []; | ||
| while (worklist.length > 0) { |
Amilliox
force-pushed
the
fix-63092-flow-stack-overflow
branch
from
e977562 to
4348d8b
Compare
Author
|
Thanks for the review. Addressed all three points:
|
… deeply nested flow graphs Replaces recursive some() for BranchLabel with an explicit worklist. Adds cache lookup for Shared nodes and a regression test.
Amilliox
force-pushed
the
fix-63092-flow-stack-overflow
branch
from
4348d8b to
95e4e40
Compare
|
It looks like you've sent a pull request to update some generated declaration files related to the DOM. These files aren't meant to be edited by hand, as they are synchronized with files in the TypeScript-DOM-lib-generator repository. You can read more here. For house-keeping purposes, this pull request will be closed. |
|
You are not allowed to delete the mandatory files in this repo. Total execution time: 6.72 seconds |
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.
Fixes #63092
Root cause:
isReachableFlowNodeWorkeruses recursion viasome()forBranchLabelflow nodes. Deeply nested flow graphs (triggered by syntactically invalid code likeas ;after a for-loop) cause stack overflow.Fix: Replace recursive
some()with an explicit worklist. When a worklist element is itself aBranchLabel, its antecedents are inlined into the worklist instead of being recursed into. This prevents unbounded stack growth.Verification:
RangeError: Maximum call stack size exceededTS2304: Cannot find nameas``