Python: Remove points-to to from ControlFlowNode

Moves the existing points-to predicates to the newly added class
`ControlFlowNodeWithPointsTo` which resides in the `LegacyPointsTo`
module.

(Existing code that uses these predicates should import this module, and
references to `ControlFlowNode` should be changed to
`ControlFlowNodeWithPointsTo`.)

Also updates all existing points-to based code to do just this.

Author: tausbn

python/ql/examples/snippets/store_none.ql

@@ -10,9 +10,10 @@
  */
 
 import python
+private import LegacyPointsTo
 
 from SubscriptNode store
 where
   store.isStore() and
-  store.getIndex().pointsTo(Value::named("None"))
+  store.getIndex().(ControlFlowNodeWithPointsTo).pointsTo(Value::named("None"))
 select store

python/ql/lib/LegacyPointsTo.qll

@@ -0,0 +1,121 @@
+/**
+ * DEPRECATED: Using the methods in this module may lead to a degradation of performance. Use at
+ * your own peril.
+ *
+ * This module contains legacy points-to predicates and methods for various classes in the
+ * points-to analysis.
+ *
+ * Existing code that depends on, say, points-to predicates on `ControlFlowNode` should be modified
+ * to use `ControlFlowNodeWithPointsTo` instead. In particular, if inside a method call chain such
+ * as
+ *
+ * `someCallNode.getFunction().pointsTo(...)`
+ *
+ *  an explicit cast should be added as follows
+ *
+ * `someCallNode.getFunction().(ControlFlowNodeWithPointsTo).pointsTo(...)`
+ *
+ * Similarly, if a bound variable has type `ControlFlowNode`, and a points-to method is called on
+ * it, the type should be changed to `ControlFlowNodeWithPointsTo`.
+ */
+
+private import python
+private import semmle.python.pointsto.PointsTo
+
+/**
+ * An extension of `ControlFlowNode` that provides points-to predicates.
+ */
+class ControlFlowNodeWithPointsTo extends ControlFlowNode {
+  /** Gets the value that this ControlFlowNode points-to. */
+  predicate pointsTo(Value value) { this.pointsTo(_, value, _) }
+
+  /** Gets the value that this ControlFlowNode points-to. */
+  Value pointsTo() { this.pointsTo(_, result, _) }
+
+  /** Gets a value that this ControlFlowNode may points-to. */
+  Value inferredValue() { this.pointsTo(_, result, _) }
+
+  /** Gets the value and origin that this ControlFlowNode points-to. */
+  predicate pointsTo(Value value, ControlFlowNode origin) { this.pointsTo(_, value, origin) }
+
+  /** Gets the value and origin that this ControlFlowNode points-to, given the context. */
+  predicate pointsTo(Context context, Value value, ControlFlowNode origin) {
+    PointsTo::pointsTo(this, context, value, origin)
+  }
+
+  /**
+   * Gets what this flow node might "refer-to". Performs a combination of localized (intra-procedural) points-to
+   * analysis and global module-level analysis. This points-to analysis favours precision over recall. It is highly
+   * precise, but may not provide information for a significant number of flow-nodes.
+   * If the class is unimportant then use `refersTo(value)` or `refersTo(value, origin)` instead.
+   */
+  pragma[nomagic]
+  predicate refersTo(Object obj, ClassObject cls, ControlFlowNode origin) {
+    this.refersTo(_, obj, cls, origin)
+  }
+
+  /** Gets what this expression might "refer-to" in the given `context`. */
+  pragma[nomagic]
+  predicate refersTo(Context context, Object obj, ClassObject cls, ControlFlowNode origin) {
+    not obj = unknownValue() and
+    not cls = theUnknownType() and
+    PointsTo::points_to(this, context, obj, cls, origin)
+  }
+
+  /**
+   * Whether this flow node might "refer-to" to `value` which is from `origin`
+   * Unlike `this.refersTo(value, _, origin)` this predicate includes results
+   * where the class cannot be inferred.
+   */
+  pragma[nomagic]
+  predicate refersTo(Object obj, ControlFlowNode origin) {
+    not obj = unknownValue() and
+    PointsTo::points_to(this, _, obj, _, origin)
+  }
+
+  /** Equivalent to `this.refersTo(value, _)` */
+  predicate refersTo(Object obj) { this.refersTo(obj, _) }
+
+  /**
+   * Check whether this control-flow node has complete points-to information.
+   * This would mean that the analysis managed to infer an over approximation
+   * of possible values at runtime.
+   */
+  predicate hasCompletePointsToSet() {
+    // If the tracking failed, then `this` will be its own "origin". In that
+    // case, we want to exclude nodes for which there is also a different
+    // origin, as that would indicate that some paths failed and some did not.
+    this.refersTo(_, _, this) and
+    not exists(ControlFlowNode other | other != this and this.refersTo(_, _, other))
+    or
+    // If `this` is a use of a variable, then we must have complete points-to
+    // for that variable.
+    exists(SsaVariable v | v.getAUse() = this | varHasCompletePointsToSet(v))
+  }
+}
+
+/**
+ * Check whether a SSA variable has complete points-to information.
+ * This would mean that the analysis managed to infer an overapproximation
+ * of possible values at runtime.
+ */
+private predicate varHasCompletePointsToSet(SsaVariable var) {
+  // Global variables may be modified non-locally or concurrently.
+  not var.getVariable() instanceof GlobalVariable and
+  (
+    // If we have complete points-to information on the definition of
+    // this variable, then the variable has complete information.
+    var.getDefinition()
+        .(DefinitionNode)
+        .getValue()
+        .(ControlFlowNodeWithPointsTo)
+        .hasCompletePointsToSet()
+    or
+    // If this variable is a phi output, then we have complete
+    // points-to information about it if all phi inputs had complete
+    // information.
+    forex(SsaVariable phiInput | phiInput = var.getAPhiInput() |
+      varHasCompletePointsToSet(phiInput)
+    )
+  )
+}

python/ql/lib/analysis/DefinitionTracking.qll

@@ -3,6 +3,7 @@
  */
 
 import python
+private import LegacyPointsTo
 import semmle.python.pointsto.PointsTo
 import IDEContextual
 
@@ -36,22 +37,22 @@ private predicate jump_to_defn(ControlFlowNode use, Definition defn) {
   )
   or
   exists(PythonModuleObject mod |
-    use.(ImportExprNode).refersTo(mod) and
+    use.(ImportExprNode).(ControlFlowNodeWithPointsTo).refersTo(mod) and
     defn.getAstNode() = mod.getModule()
   )
   or
   exists(PythonModuleObject mod, string name |
-    use.(ImportMemberNode).getModule(name).refersTo(mod) and
+    use.(ImportMemberNode).getModule(name).(ControlFlowNodeWithPointsTo).refersTo(mod) and
     scope_jump_to_defn_attribute(mod.getModule(), name, defn)
   )
   or
   exists(PackageObject package |
-    use.(ImportExprNode).refersTo(package) and
+    use.(ImportExprNode).(ControlFlowNodeWithPointsTo).refersTo(package) and
     defn.getAstNode() = package.getInitModule().getModule()
   )
   or
   exists(PackageObject package, string name |
-    use.(ImportMemberNode).getModule(name).refersTo(package) and
+    use.(ImportMemberNode).getModule(name).(ControlFlowNodeWithPointsTo).refersTo(package) and
     scope_jump_to_defn_attribute(package.getInitModule().getModule(), name, defn)
   )
   or
@@ -230,7 +231,7 @@ private predicate module_and_name_for_import_star_helper(
   ModuleObject mod, string name, ImportStarNode im_star, ImportStarRefinement def
 ) {
   im_star = def.getDefiningNode() and
-  im_star.getModule().refersTo(mod) and
+  im_star.getModule().(ControlFlowNodeWithPointsTo).refersTo(mod) and
   name = def.getSourceVariable().getName()
 }
 
@@ -239,7 +240,7 @@ pragma[noinline]
 private predicate variable_not_redefined_by_import_star(EssaVariable var, ImportStarRefinement def) {
   var = def.getInput() and
   exists(ModuleObject mod |
-    def.getDefiningNode().(ImportStarNode).getModule().refersTo(mod) and
+    def.getDefiningNode().(ImportStarNode).getModule().(ControlFlowNodeWithPointsTo).refersTo(mod) and
     not mod.exports(var.getSourceVariable().getName())
   )
 }
@@ -352,7 +353,9 @@ private predicate scope_jump_to_defn_attribute(ImportTimeScope s, string name, D
   )
 }
 
-private predicate jump_to_defn_attribute(ControlFlowNode use, string name, Definition defn) {
+private predicate jump_to_defn_attribute(
+  ControlFlowNodeWithPointsTo use, string name, Definition defn
+) {
   /* Local attribute */
   exists(EssaVariable var |
     use = var.getASourceUse() and
@@ -367,7 +370,7 @@ private predicate jump_to_defn_attribute(ControlFlowNode use, string name, Defin
   /* Super attributes */
   exists(AttrNode f, SuperBoundMethod sbm, Object function |
     use = f.getObject(name) and
-    f.refersTo(sbm) and
+    f.(ControlFlowNodeWithPointsTo).refersTo(sbm) and
     function = sbm.getFunction(_) and
     function.getOrigin() = defn.getAstNode()
   )
@@ -408,7 +411,7 @@ private predicate attribute_assignment_jump_to_defn_attribute(
 private predicate sets_attribute(ArgumentRefinement def, string name) {
   exists(CallNode call |
     call = def.getDefiningNode() and
-    call.getFunction().refersTo(Object::builtin("setattr")) and
+    call.getFunction().(ControlFlowNodeWithPointsTo).refersTo(Object::builtin("setattr")) and
     def.getInput().getAUse() = call.getArg(0) and
     call.getArg(1).getNode().(StringLiteral).getText() = name
   )

python/ql/lib/semmle/python/Exprs.qll

@@ -1,4 +1,5 @@
 import python
+private import LegacyPointsTo
 private import semmle.python.pointsto.PointsTo
 private import semmle.python.objects.ObjectInternal
 private import semmle.python.internal.CachedStages
@@ -71,7 +72,9 @@ class Expr extends Expr_, AstNode {
    * Gets what this expression might "refer-to" in the given `context`.
    */
   predicate refersTo(Context context, Object obj, ClassObject cls, AstNode origin) {
-    this.getAFlowNode().refersTo(context, obj, cls, origin.getAFlowNode())
+    this.getAFlowNode()
+        .(ControlFlowNodeWithPointsTo)
+        .refersTo(context, obj, cls, origin.getAFlowNode())
   }
 
   /**
@@ -82,7 +85,7 @@ class Expr extends Expr_, AstNode {
    */
   pragma[nomagic]
   predicate refersTo(Object obj, AstNode origin) {
-    this.getAFlowNode().refersTo(obj, origin.getAFlowNode())
+    this.getAFlowNode().(ControlFlowNodeWithPointsTo).refersTo(obj, origin.getAFlowNode())
   }
 
   /**
@@ -96,14 +99,16 @@ class Expr extends Expr_, AstNode {
    * in the given `context`.
    */
   predicate pointsTo(Context context, Value value, AstNode origin) {
-    this.getAFlowNode().pointsTo(context, value, origin.getAFlowNode())
+    this.getAFlowNode()
+        .(ControlFlowNodeWithPointsTo)
+        .pointsTo(context, value, origin.getAFlowNode())
   }
 
   /**
    * Holds if this expression might "point-to" to `value` which is from `origin`.
    */
   predicate pointsTo(Value value, AstNode origin) {
-    this.getAFlowNode().pointsTo(value, origin.getAFlowNode())
+    this.getAFlowNode().(ControlFlowNodeWithPointsTo).pointsTo(value, origin.getAFlowNode())
   }
 
   /**

python/ql/lib/semmle/python/Flow.qll

@@ -1,5 +1,4 @@
 import python
-private import semmle.python.pointsto.PointsTo
 private import semmle.python.internal.CachedStages
 private import codeql.controlflow.BasicBlock as BB
 
@@ -144,56 +143,6 @@ class ControlFlowNode extends @py_flow_node {
   /** Whether this flow node is the first in its scope */
   predicate isEntryNode() { py_scope_flow(this, _, -1) }
 
-  /** Gets the value that this ControlFlowNode points-to. */
-  predicate pointsTo(Value value) { this.pointsTo(_, value, _) }
-
-  /** Gets the value that this ControlFlowNode points-to. */
-  Value pointsTo() { this.pointsTo(_, result, _) }
-
-  /** Gets a value that this ControlFlowNode may points-to. */
-  Value inferredValue() { this.pointsTo(_, result, _) }
-
-  /** Gets the value and origin that this ControlFlowNode points-to. */
-  predicate pointsTo(Value value, ControlFlowNode origin) { this.pointsTo(_, value, origin) }
-
-  /** Gets the value and origin that this ControlFlowNode points-to, given the context. */
-  predicate pointsTo(Context context, Value value, ControlFlowNode origin) {
-    PointsTo::pointsTo(this, context, value, origin)
-  }
-
-  /**
-   * Gets what this flow node might "refer-to". Performs a combination of localized (intra-procedural) points-to
-   * analysis and global module-level analysis. This points-to analysis favours precision over recall. It is highly
-   * precise, but may not provide information for a significant number of flow-nodes.
-   * If the class is unimportant then use `refersTo(value)` or `refersTo(value, origin)` instead.
-   */
-  pragma[nomagic]
-  predicate refersTo(Object obj, ClassObject cls, ControlFlowNode origin) {
-    this.refersTo(_, obj, cls, origin)
-  }
-
-  /** Gets what this expression might "refer-to" in the given `context`. */
-  pragma[nomagic]
-  predicate refersTo(Context context, Object obj, ClassObject cls, ControlFlowNode origin) {
-    not obj = unknownValue() and
-    not cls = theUnknownType() and
-    PointsTo::points_to(this, context, obj, cls, origin)
-  }
-
-  /**
-   * Whether this flow node might "refer-to" to `value` which is from `origin`
-   * Unlike `this.refersTo(value, _, origin)` this predicate includes results
-   * where the class cannot be inferred.
-   */
-  pragma[nomagic]
-  predicate refersTo(Object obj, ControlFlowNode origin) {
-    not obj = unknownValue() and
-    PointsTo::points_to(this, _, obj, _, origin)
-  }
-
-  /** Equivalent to `this.refersTo(value, _)` */
-  predicate refersTo(Object obj) { this.refersTo(obj, _) }
-
   /** Gets the basic block containing this flow node */
   BasicBlock getBasicBlock() { result.contains(this) }
 
@@ -259,23 +208,6 @@ class ControlFlowNode extends @py_flow_node {
     )
   }
 
-  /**
-   * Check whether this control-flow node has complete points-to information.
-   * This would mean that the analysis managed to infer an over approximation
-   * of possible values at runtime.
-   */
-  predicate hasCompletePointsToSet() {
-    // If the tracking failed, then `this` will be its own "origin". In that
-    // case, we want to exclude nodes for which there is also a different
-    // origin, as that would indicate that some paths failed and some did not.
-    this.refersTo(_, _, this) and
-    not exists(ControlFlowNode other | other != this and this.refersTo(_, _, other))
-    or
-    // If `this` is a use of a variable, then we must have complete points-to
-    // for that variable.
-    exists(SsaVariable v | v.getAUse() = this | varHasCompletePointsToSet(v))
-  }
-
   /** Whether this strictly dominates other. */
   pragma[inline]
   predicate strictlyDominates(ControlFlowNode other) {
@@ -332,28 +264,6 @@ private class AnyNode extends ControlFlowNode {
   override AstNode getNode() { result = super.getNode() }
 }
 
-/**
- * Check whether a SSA variable has complete points-to information.
- * This would mean that the analysis managed to infer an overapproximation
- * of possible values at runtime.
- */
-private predicate varHasCompletePointsToSet(SsaVariable var) {
-  // Global variables may be modified non-locally or concurrently.
-  not var.getVariable() instanceof GlobalVariable and
-  (
-    // If we have complete points-to information on the definition of
-    // this variable, then the variable has complete information.
-    var.getDefinition().(DefinitionNode).getValue().hasCompletePointsToSet()
-    or
-    // If this variable is a phi output, then we have complete
-    // points-to information about it if all phi inputs had complete
-    // information.
-    forex(SsaVariable phiInput | phiInput = var.getAPhiInput() |
-      varHasCompletePointsToSet(phiInput)
-    )
-  )
-}
-
 /** A control flow node corresponding to a call expression, such as `func(...)` */
 class CallNode extends ControlFlowNode {
   CallNode() { toAst(this) instanceof Call }

python/ql/lib/semmle/python/Metrics.qll

@@ -1,4 +1,5 @@
 import python
+private import LegacyPointsTo
 
 /** The metrics for a function */
 class FunctionMetrics extends Function {
@@ -59,7 +60,7 @@ class FunctionMetrics extends Function {
     not non_coupling_method(result) and
     exists(Call call | call.getScope() = this |
       exists(FunctionObject callee | callee.getFunction() = result |
-        call.getAFlowNode().getFunction().refersTo(callee)
+        call.getAFlowNode().getFunction().(ControlFlowNodeWithPointsTo).refersTo(callee)
       )
       or
       exists(Attribute a | call.getFunc() = a |