Python: model exception edges for raise-prone expressions inside try/with#21937
Draft
yoff wants to merge 1 commit into
Draft
Python: model exception edges for raise-prone expressions inside try/with#21937yoff wants to merge 1 commit into
yoff wants to merge 1 commit into
Conversation
83c5f33 to
647b976
Compare
…with The new CFG previously only emitted exception edges for explicit `raise` and `assert` statements. As a result, code that became reachable only via the exception path of an arbitrary expression (e.g., the body of an `except` handler following a try-body whose `call()` could raise) was classified as dead, breaking analyses like StackTraceExposure, FileNotAlwaysClosed, ExceptionInfo, UseOfExit, and CatchingBaseException. This commit adds a `mayThrow` predicate over expressions that are known sources of implicit exceptions in Python (calls, attribute access, subscripts, arithmetic/comparison operators, imports, await/yield/yield from) plus `from m import *` at the statement level, and routes them through the shared CFG's `beginAbruptCompletion(_, _, ExceptionSuccessor, always=false)` hook. The set of exception sources is restricted to nodes that are syntactically inside a `try`/`with` statement in the same scope. This mirrors Java's `ControlFlowGraph::mayThrow`, which only emits exception edges where local handling can observe them — outside such contexts, the edges add CFG complexity (weakening BarrierGuard precision and breaking SSA continuity around augmented assignments and subscript stores) without analysis benefit, since exceptions just propagate to the function exit anyway. Net effect on the test suite: ~100 alerts restored across the exception- related query tests (StackTraceExposure +29, ExceptionInfo +17, FileNotAlwaysClosed +52, UseOfExit +1, CatchingBaseException restored) with no precision regressions. Affected `.expected` files and the regression-guard `dead_under_no_raise.py` are updated accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
0b3f28f to
db94cd9
Compare
647b976 to
c34dc45
Compare
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.
Stacked on top of #21925 (the shared-CFG dataflow flip).
Problem
After the flip, the new CFG only modelled exception edges for explicit
raiseandassertstatements. Code reachable only via the implicit exception path of an arbitrary expression (e.g., the body of anexcepthandler following a try-body whosecall()could raise) was classified as dead, breaking several exception-related queries:py/stack-trace-exposure(StackTraceExposure)py/file-not-closed(FileNotAlwaysClosed)py/catch-base-exception(CatchingBaseException)py/empty-except(EmptyExcept)py/use-of-exit(UseOfExit)ExceptionInfotest.Approach
Add a
mayThrowpredicate over expressions known to be implicit exception sources in Python (calls, attribute access, subscripts, arithmetic/comparison operators, imports,await/yield/yield from) plusfrom m import *at the statement level, and route them through the shared CFG'sbeginAbruptCompletion(_, _, ExceptionSuccessor, always=false)hook.Restricted to nodes inside a
try/withstatement in the same scope. This mirrors Java'sControlFlowGraph::mayThrow, which only emits exception edges where local handling can observe them. Outside such contexts the edges would add CFG complexity (weakening BarrierGuard precision, breaking SSA continuity around augmented assignments and subscript stores) without analysis benefit, since exceptions just propagate to the function exit anyway.Effect on the test suite
Improvements:
FileNotAlwaysClosed(lines 20/30/39/58/69/79/91/182/225/252/275) are suppressed because the query can now correctly trace exception flow to the close.ViewAliasInExceptnow found by FindSubclass; CmpTest narrows gap to legacy; NormalDataflowTest fixes 1 missing result — theMISSING:keyword was dropped fromtest.py:847per inline-expectation convention).dead_under_no_raise.pyupdated (its docstring literally predicted "if raise modelling is later added, this file will need to be revisited").Regressions (documented):
FileNotAlwaysClosedre-marks lines 49/141/237 asMISSING:Alert. These were producing alerts on flip and were cleaned up there; with this commit's exception modelling, the query becomes optimistic about close-paths through buggy guards (e.g.,finally: if f.closed: f.close()) because it now sees real exception edges into those guards. The test file comments at lines 52/147/244 already note "We don't precisely consider this condition, so this result is MISSING". These are known query limitations, not new bugs.Zero new dataflow precision regressions — narrowing to in-try/with avoids both the BarrierGuard weakening (
is_safe(...) == True-style patterns) and SSA-continuity issues (+=, subscript stores) that an unconditionalmayThrowwould cause.CPython CFG consistency: all 11 checks pass.
Full Python suite: 973/975 passing (2 pre-existing failures unrelated:
hidden/test.qlandlong_path/LongPath.ql).Stack