Merge pull request #22380 from yoff/yoff-fix-shared-cfg-ssa-regressions

Python: fix shared CFG exception-handler reachability

Author: yoff

python/ql/lib/semmle/python/controlflow/internal/AstNodeImpl.qll

@@ -1626,19 +1626,24 @@ private module Input implements InputSig1, InputSig2 {
     n = any(Ast::AssertStmt a).getTest()
   }
 
+  predicate postOrInOrder(Ast::AstNode n) { mayThrow(n) }
+
   private string assertThrowTag() { result = "[assert-throw]" }
 
   /**
    * Holds if the expression node `e` may raise an exception at runtime as part of
    * its normal evaluation (not via an explicit `raise`/`assert`, which are
    * modeled separately).
    *
-   * The set mirrors what the legacy CFG used to flag implicitly: function
+   * `TypeError` is intentionally excluded along with exceptions thrown by dunder
+   * implementations to avoid cluttering the CFG; we expect user code to catch
+   * these only rarely.
+   *
+   * Otherwise, the set mirrors what the legacy CFG used to flag implicitly: function
    * calls (anything can raise), attribute access (`AttributeError`),
-   * subscript access (`IndexError`/`KeyError`/`TypeError`), arithmetic and
-   * comparison operators (`TypeError`/`ZeroDivisionError`), imports
+   * subscript access (`IndexError`/`KeyError`), division (`ZeroDivisionError`), imports
    * (`ImportError`/`ModuleNotFoundError`), and generator/coroutine
-   * suspension points (`await`/`yield`/`yield from`).
+   * suspension points (`await`/`yield`/`yield from` throwing `CancelledError` or `GeneratorExit`).
    *
    * Bare `Name` reads are intentionally excluded — modeling every name
    * read as `mayThrow` would explode CFG edge count for negligible
@@ -1652,11 +1657,7 @@ private module Input implements InputSig1, InputSig2 {
     or
     e instanceof Py::Subscript
     or
-    e instanceof Py::BinaryExpr
-    or
-    e instanceof Py::UnaryExpr
-    or
-    e instanceof Py::Compare
+    e.(Py::BinaryExpr).getOp() instanceof Py::Div
     or
     e instanceof Py::ImportExpr
     or

python/ql/test/library-tests/ControlFlow/shared-cfg-exceptions/ExceptionReachabilityTest.expected

@@ -0,0 +1,29 @@
+/**
+ * Inline-expectations test for exception-handler reachability in the shared CFG.
+ */
+
+import python
+import semmle.python.controlflow.internal.AstNodeImpl as CfgImpl
+import semmle.python.controlflow.internal.Cfg as Cfg
+import utils.test.InlineExpectationsTest
+
+module ExceptionReachabilityTest implements TestSig {
+  string getARelevantTag() { result = "exception-handler" }
+
+  predicate hasActualResult(Location location, string element, string tag, string value) {
+    exists(
+      Expr source, ExceptStmt handler, Cfg::ControlFlowNode sourceCfg,
+      Cfg::ControlFlowNode handlerEntry
+    |
+      sourceCfg.getNode() = source and
+      handlerEntry = sourceCfg.getAnExceptionalSuccessor() and
+      CfgImpl::astNodeToPyNode(handlerEntry.getAstNode()) = handler and
+      location = source.getLocation() and
+      element = source.toString() and
+      tag = "exception-handler" and
+      value = handler.getType().toString()
+    )
+  }
+}
+
+import MakeTest<ExceptionReachabilityTest>

python/ql/test/library-tests/ControlFlow/shared-cfg-exceptions/ExceptionReachabilityTest.ql

@@ -0,0 +1,12 @@
+def generator():
+    try:
+        yield  # $ exception-handler=GeneratorExit
+    except GeneratorExit:
+        return
+
+
+def load_module():
+    try:
+        import unavailable_module  # $ exception-handler=ImportError
+    except ImportError:
+        return None