Merge pull request #1411 from ian-semmle/qlcfg3

C++: QL CFG: Use synthetic_destructor_call table rather than SyntheticDestructorCalls

Author: nickrolfe

cpp/ql/src/semmle/code/cpp/controlflow/internal/CFG.qll

@@ -80,7 +80,6 @@
  */
 
 private import cpp
-private import semmle.code.cpp.controlflow.internal.SyntheticDestructorCalls
 
 /**
  * A control-flow node. This class exists to provide a shorter name than
@@ -115,8 +114,7 @@ private class Node extends ControlFlowNodeBase {
  */
 private predicate excludeNodeAndNodesBelow(Expr e) {
   not exists(e.getParent()) and
-  not e instanceof DestructorCall and
-  not e instanceof SyntheticDestructorCall // Workaround for CPP-320
+  not e instanceof DestructorCall
   or
   // Constructor init lists should be evaluated, and we can change this in
   // the future, but it would mean that a `Function` entry point is not
@@ -983,36 +981,65 @@ private predicate subEdgeIncludingDestructors(Pos p1, Node n1, Node n2, Pos p2)
   // called, connect the "before destructors" node directly to the "after
   // destructors" node. For performance, only do this when the nodes exist.
   exists(Pos afterDtors | afterDtors.isAfterDestructors() | subEdge(afterDtors, n1, _, _)) and
-  not exists(getDestructorCallAfterNode(n1, 0)) and
+  not exists(getSynthesisedDestructorCallAfterNode(n1, 0)) and
   p1.nodeBeforeDestructors(n1, n1) and
   p2.nodeAfterDestructors(n2, n1)
   or
   exists(Node n |
-    // before destructors -> access(0)
-    p1.nodeBeforeDestructors(n1, n) and
-    p2.nodeAt(n2, getDestructorCallAfterNode(n, 0).getAccess())
-    or
-    // access(i) -> call(i)
-    exists(int i |
-      p1.nodeAt(n1, getDestructorCallAfterNode(n, i).getAccess()) and
-      p2.nodeAt(n2, getDestructorCallAfterNode(n, i))
-    )
+    // before destructors -> access(max)
+    exists(int maxCallIndex |
+      maxCallIndex = max(int i | exists(getSynthesisedDestructorCallAfterNode(n, i))) and
+      p1.nodeBeforeDestructors(n1, n) and
+      p2.nodeAt(n2, getSynthesisedDestructorCallAfterNode(n, maxCallIndex).getQualifier()))
     or
-    // call(i) -> access(i+1)
+    // call(i+1) -> access(i)
     exists(int i |
-      p1.nodeAt(n1, getDestructorCallAfterNode(n, i)) and
-      p2.nodeAt(n2, getDestructorCallAfterNode(n, i + 1).getAccess())
+      p1.nodeAt(n1, getSynthesisedDestructorCallAfterNode(n, i + 1)) and
+      p2.nodeAt(n2, getSynthesisedDestructorCallAfterNode(n, i).getQualifier())
     )
     or
-    // call(max) -> after destructors end
-    exists(int maxCallIndex |
-      maxCallIndex = max(int i | exists(getDestructorCallAfterNode(n, i))) and
-      p1.nodeAt(n1, getDestructorCallAfterNode(n, maxCallIndex)) and
-      p2.nodeAfterDestructors(n2, n)
-    )
+    // call(0) -> after destructors end
+    p1.nodeAt(n1, getSynthesisedDestructorCallAfterNode(n, 0)) and
+    p2.nodeAfterDestructors(n2, n)
   )
 }
 
+/**
+ * Gets the synthetic constructor call for the `index`'th entity that
+ * should be destructed following `node`. Note that entities should be
+ * destructed in reverse construction order, so these should be called
+ * from highest to lowest index.
+ *
+ * The exact placement of that call in the CFG depends on the type of
+ * `node` as follows:
+ *
+ * - `Block`: after ordinary control flow falls off the end of the block
+ *   without jumps or exceptions.
+ * - `ReturnStmt`: After the statement itself or after its operand (if
+ *   present).
+ * - `ThrowExpr`: After the `throw` expression or after its operand (if
+ *   present).
+ * - `JumpStmt` (`BreakStmt`, `ContinueStmt`, `GotoStmt`): after the statement.
+ * - A `ForStmt`, `WhileStmt`, `SwitchStmt`, or `IfStmt`: after control flow
+ *   falls off the end of the statement without jumping. Destruction can occur
+ *   here for `for`-loops that have an initializer (`for (C x = a; ...; ...)`)
+ *   and for statements whose condition is a `ConditionDeclExpr`
+ *   (`if (C x = a)`).
+ * - The `getUpdate()` of a `ForStmt`: after the `getUpdate()` expression. This
+ *   can happen when the condition is a `ConditionDeclExpr`
+ * - `Handler`: On the edge out of the `Handler` for the case where the
+ *   exception was not matched and is propagated to the next handler or
+ *   function exit point.
+ * - `MicrosoftTryExceptStmt`: After the false-edge out of the `e` in
+ *   `__except(e)`, before propagating the exception up to the next handler or
+ *   function exit point.
+ * - `MicrosoftTryFinallyStmt`: On the edge following the `__finally` block for
+ *   the case where an exception was thrown and needs to be propagated.
+ */
+DestructorCall getSynthesisedDestructorCallAfterNode(Node n, int i) {
+  synthetic_destructor_call(n, i, result)
+}
+
 /**
  * An expression whose outgoing true/false sub-edges may come from different
  * sub-nodes.

cpp/ql/src/semmle/code/cpp/controlflow/internal/SyntheticDestructorCalls.qll

@@ -181,17 +181,24 @@ tokens(
     int endColumn : int ref
 );
 
-/*
- * Mapping of header files to packages
+/**
+ * Information about packages that provide code used during compilation.
+ * The `id` is just a unique identifier.
+ * The `namespace` is typically the name of the package manager that
+ * provided the package (e.g. "dpkg" or "yum").
+ * The `package_name` is the name of the package, and `version` is its
+ * version (as a string).
  */
-
 external_packages(
     unique int id: @external_package,
-    string namespace : string ref, // "dpkg", "yum", ...
+    string namespace : string ref,
     string package_name : string ref,
     string version : string ref
 );
 
+/**
+ * Holds if File `fileid` was provided by package `package`.
+ */
 header_to_external_package(
     int fileid : @file ref,
     int package : @external_package ref
@@ -1041,6 +1048,17 @@ exprconv(
 
 compgenerated(unique int id: @element ref);
 
+/**
+ * `destructor_call` destructs the `i`'th entity that should be
+ * destructed following `element`. Note that entities should be
+ * destructed in reverse construction order, so for a given `element`
+ * these should be called from highest to lowest `i`.
+ */
+synthetic_destructor_call(
+    int element: @element ref,
+    int i: int ref,
+    unique int destructor_call: @routineexpr ref
+);
 
 namespaces(
     unique int id: @namespace,