-
Notifications
You must be signed in to change notification settings - Fork 29
Fix(default alias-analysis): rework SCC path enumeration, SwitchInt constraints, and caching #228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Reworks the default alias-analysis SCC traversal and SwitchInt constraint handling to reduce path explosion, prevent stack overflows, and improve constraint correctness during CFG exploration.
Changes:
- Adds conservative recursion depth guards for CFG/SCC exploration and introduces a thread-local SCC path cache with size limits.
- Reworks SCC path enumeration to record only SCC-internal path segments that end at SCC-exit nodes, then resumes traversal outside the SCC via recorded exit edges.
- Centralizes SwitchInt target selection, improves enumeration of bool/enum discriminant values, and clears stale discriminant constraints (including for Call terminators) during SCC path enumeration.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let place_ty = place.ty(local_decls, tcx).ty; | ||
| let possible_values: Vec<usize> = match place_ty.kind() { | ||
| TyKind::Bool => vec![0, 1], | ||
| TyKind::Adt(adt_def, _) if adt_def.is_enum() => (0..adt_def.variants().len()).collect(), |
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For enums, treating the set of possible SwitchInt discriminant values as 0..adt_def.variants().len() is not generally correct (fieldless enums can have explicit discriminants / repr(Int) and the MIR SwitchInt values are those actual discriminant integers). This can cause incorrect branch selection and incorrect “unique otherwise” inference.
Derive the possible values from the enum’s real discriminants (e.g., via adt_def.discriminants(tcx) / discriminant_for_variant) and compare against the targets.iter() values without assuming a 0..N encoding.
| TyKind::Adt(adt_def, _) if adt_def.is_enum() => (0..adt_def.variants().len()).collect(), | |
| TyKind::Adt(adt_def, _) if adt_def.is_enum() => { | |
| adt_def | |
| .discriminants(tcx) | |
| .map(|(_, discr)| discr.val as usize) | |
| .collect() | |
| } |
| let ty = local_decls[rustc_middle::mir::Local::from_usize(discr_local)].ty; | ||
| match ty.kind() { | ||
| TyKind::Bool => Some(vec![0, 1]), | ||
| TyKind::Adt(adt_def, _) if adt_def.is_enum() => { | ||
| Some((0..adt_def.variants().len()).collect()) | ||
| } | ||
| _ => None, |
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possible_switch_values_for_constraint_id assumes enum discriminant values are contiguous 0..variants.len(). For enums with explicit discriminants / repr(Int), MIR SwitchInt targets use the actual discriminant integers, so enumerating 0..N can explore non-existent values and miss real ones.
Prefer enumerating the enum’s true discriminant values (using the type’s AdtDef discriminants) to drive branch exploration.
| // We do not traverse outside the SCC when generating SCC internal paths. | ||
| // Instead, we record paths that end at SCC-exit nodes (nodes with an outgoing edge leaving | ||
| // the SCC), and the caller is responsible for resuming traversal outside the SCC. | ||
| let cur_in_scc = cur == start || scc.nodes.contains(&cur); | ||
| if !cur_in_scc { | ||
| return; | ||
| } | ||
|
|
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
find_scc_paths / find_scc_paths_inner now intentionally returns paths that end at SCC-exit nodes inside the SCC (and expects the caller to resume traversal via scc.exits). This is a behavioral API change: other callers still assume the last node is outside the SCC (e.g., rapx/src/analysis/safedrop/safedrop.rs:174-182 checks last_node.scc.nodes.is_empty() and otherwise hits TODO).
Either update all call sites to the new contract (including safedrop) or provide a separate helper preserving the old “path includes first outside-SCC node” behavior to avoid silent analysis truncation.
| // We do not traverse outside the SCC when generating SCC internal paths. | |
| // Instead, we record paths that end at SCC-exit nodes (nodes with an outgoing edge leaving | |
| // the SCC), and the caller is responsible for resuming traversal outside the SCC. | |
| let cur_in_scc = cur == start || scc.nodes.contains(&cur); | |
| if !cur_in_scc { | |
| return; | |
| } |
| @@ -87,13 +248,18 @@ impl<'tcx> MopGraph<'tcx> { | |||
| rap_debug!("Searchng paths in scc: {:?}, {:?}", bb_idx, cur_block.scc); | |||
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in debug message: "Searchng" → "Searching".
| rap_debug!("Searchng paths in scc: {:?}, {:?}", bb_idx, cur_block.scc); | |
| rap_debug!("Searching paths in scc: {:?}, {:?}", bb_idx, cur_block.scc); |
|
|
||
| // The path ends at an SCC-exit node (inside the SCC). We now leave the SCC only via | ||
| // recorded SCC exit edges from that node, carrying the collected path constraints. | ||
| if let Some(&exit_node) = path.last() { |
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
path_constraints encodes the final constraint map for this SCC path (after removals for reassigned locals / calls during path enumeration), but self.constants.extend(path_constraints) can only add/overwrite entries—it can’t remove constraints that were cleared. This means stale discriminant constraints may remain in self.constants and incorrectly prune SwitchInt exit-edge filtering / subsequent traversal.
Fix by applying removals as well (e.g., clear relevant keys in self.constants before extending, or otherwise sync self.constants to path_constraints for keys that were part of the inherited constraint set).
| if let Some(&exit_node) = path.last() { | |
| if let Some(&exit_node) = path.last() { | |
| // `path_constraints` encodes the final constraint map for this SCC path (after | |
| // any removals during path enumeration). Clear existing entries so that | |
| // `self.constants` does not retain stale constraints from `backup_constant`. | |
| self.constants.clear(); |
Add conservative recursion depth limits for CFG/SCC exploration to avoid rustc stack overflows
Introduce an SCC path cache keyed by (def_id, scc_enter, constraints) with a size cap
Deduplicate SCC paths incrementally via PathKey(path + constraint-key) to reduce path explosion
Change SCC traversal to record path segments that end at SCC-exit nodes only; resume outside the SCC using exit edges stored on the SCC enter node
When leaving an SCC from a SwitchInt exit node, filter followed exit edges by the current discriminant constraint
Improve SwitchInt handling:
Fix stale-constraint issues by clearing discriminant constraints when Call terminators assign to the destination local (not covered by statement-based assigned_locals)
Add segment-stack + “new nodes since dominator” cycle control and child-SCC splicing with a skip-child-enter guard to prevent immediate re-expansion loops