Merge pull request #1539 from taus-semmle/python-controlflownode-getchild-performance-hotfix

Python: Fix bad join ordering in `ControlFlowNode::getAChild()`.

Author: aschackmull

python/ql/src/analysis/DefinitionTracking.qll

@@ -229,14 +229,18 @@ private predicate method_callsite_defn(MethodCallsiteRefinement def, Definition
 pragma [noinline]
 private predicate module_and_name_for_import_star(ModuleObject mod, string name, ImportStarRefinement def) {
     exists(ImportStarNode im_star |
-        im_star = def.getDefiningNode() |
-        name = def.getSourceVariable().getName() and
-        im_star.getModule().refersTo(mod) and
+        module_and_name_for_import_star_helper(mod, name, im_star, def) and
         mod.exports(name)
     )
 }
 
-/** Holds if `def` is technically a defn of `var`, but the `from ... import *` does not in fact define `var` */
+pragma [noinline]
+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
+    name = def.getSourceVariable().getName()
+}
+        /** Holds if `def` is technically a defn of `var`, but the `from ... import *` does not in fact define `var` */
 pragma [noinline]
 private predicate variable_not_redefined_by_import_star(EssaVariable var, ImportStarRefinement def) {
     var = def.getInput() and

python/ql/src/semmle/dataflow/SsaCompute.qll

@@ -267,6 +267,12 @@ private cached module SsaComputeImpl {
 
     cached module SsaDefinitionsImpl {
 
+        pragma [noinline]
+        private predicate reachesEndOfBlockRec(SsaSourceVariable v, BasicBlock defbb, int defindex, BasicBlock b) {
+            exists(BasicBlock idom | reachesEndOfBlock(v, defbb, defindex, idom) |
+                idom = b.getImmediateDominator()
+            )
+        }
         /**
          * The SSA definition of `v` at `def` reaches the end of a basic block `b`, at
          * which point it is still live, without crossing another SSA definition of `v`.
@@ -277,13 +283,10 @@ private cached module SsaComputeImpl {
           (
             defbb = b and SsaComputeImpl::ssaDefReachesRank(v, defbb, defindex, SsaComputeImpl::lastRank(v, b)) 
             or
-            exists(BasicBlock idom |
-              idom = b.getImmediateDominator() and 
-              // It is sufficient to traverse the dominator graph, cf. discussion above.
-              reachesEndOfBlock(v, defbb, defindex, idom) and
-              not SsaComputeImpl::ssaDef(v, b)
+            // It is sufficient to traverse the dominator graph, cf. discussion above.
+            reachesEndOfBlockRec(v, defbb, defindex, b) and
+            not SsaComputeImpl::ssaDef(v, b)
             )
-          )
         }
 
         /**

python/ql/src/semmle/python/Exprs.qll

@@ -399,7 +399,7 @@ class FloatLiteral extends Num {
 
     FloatLiteral() {
         not this instanceof ImaginaryLiteral and
-        exists(string n | n = this.getN() | n.charAt(_) = "." or n.charAt(_) = "e" or n.charAt(_) = "E")
+        this.getN().regexpMatch(".*[.eE].*")
     }
 
     float getValue() {
@@ -427,15 +427,15 @@ class FloatLiteral extends Num {
 
 /** An imaginary numeric constant, such as `3j` */
 class ImaginaryLiteral extends Num {
+    private float value;
 
     ImaginaryLiteral() {
-        exists(string n | n = this.getN() | n.charAt(_) = "j")
+        value = this.getN().regexpCapture("(.+)j.*", 1).toFloat()
     }
 
     /** Gets the value of this constant as a floating point value */
     float getValue() {
-        exists(string s, int j | s = this.getN() and s.charAt(j) = "j" |
-                       result = s.prefix(j).toFloat())
+        result = value
     }
 
     override string toString() {

python/ql/src/semmle/python/Flow.qll

@@ -402,14 +402,14 @@ class ControlFlowNode extends @py_flow_node {
     }
 
     ControlFlowNode getAChild() {
-        result = this.getExprChild() and
-        result.getBasicBlock().dominates(this.getBasicBlock())
+        result = this.getExprChild(this.getBasicBlock())
     }
 
     /* join-ordering helper for `getAChild() */
     pragma [noinline]
-    private ControlFlowNode getExprChild() {
+    private ControlFlowNode getExprChild(BasicBlock dom) {
         this.getNode().(Expr).getAChildNode() = result.getNode() and
+        result.getBasicBlock().dominates(dom) and
         not this instanceof UnaryExprNode
     }
 

python/ql/src/semmle/python/dataflow/SsaDefinitions.qll

@@ -146,8 +146,13 @@ class NonLocalVariable extends PythonSsaSourceVariable {
         this.(LocalVariable).getScope().getEntryNode() = result
     }
 
+    pragma [noinline]
+    Scope scope_as_local_variable() {
+        result = this.(LocalVariable).getScope()
+    }
+
     override CallNode redefinedAtCallSite() {
-        result.getScope().getScope*() = this.(LocalVariable).getScope()
+        result.getScope().getScope*() = this.scope_as_local_variable()
     }
 
 }
@@ -173,7 +178,7 @@ class ClassLocalVariable extends PythonSsaSourceVariable {
 class BuiltinVariable extends PythonSsaSourceVariable {
 
     BuiltinVariable() {
-        this instanceof GlobalVariable and 
+        this instanceof GlobalVariable and
         not exists(this.(Variable).getAStore()) and
         not this.(Variable).getId() = "__name__" and
         not this.(Variable).getId() = "__package__" and
@@ -207,13 +212,21 @@ class ModuleVariable extends PythonSsaSourceVariable {
         )
     }
 
-    override ControlFlowNode getAnImplicitUse() {
+    pragma [noinline]
+    CallNode global_variable_callnode() {
+        result.getScope() = this.(GlobalVariable).getScope()
+    }
+
+    pragma[noinline]
+    ImportMemberNode global_variable_import() {
         result.getScope() = this.(GlobalVariable).getScope() and
-        (
-            result instanceof CallNode
-            or
-            import_from_dot_in_init(result.(ImportMemberNode).getModule(this.getName()))
-        )
+        import_from_dot_in_init(result.(ImportMemberNode).getModule(this.getName()))
+    }
+
+    override ControlFlowNode getAnImplicitUse() {
+        result = global_variable_callnode()
+        or
+        result = global_variable_import()
         or
         exists(ImportTimeScope scope |
             scope.entryEdge(result, _) |
@@ -292,8 +305,13 @@ class EscapingGlobalVariable extends ModuleVariable {
         result = this.innerScope().getEntryNode()
     }
 
+    pragma [noinline]
+    Scope scope_as_global_variable() {
+        result = this.(GlobalVariable).getScope()
+    }
+
     override CallNode redefinedAtCallSite() {
-        result.(CallNode).getScope().getScope*() = this.(GlobalVariable).getScope()
+        result.(CallNode).getScope().getScope*() = this.scope_as_global_variable()
     }
 
 }
@@ -324,8 +342,13 @@ class SpecialSsaSourceVariable extends PythonSsaSourceVariable {
         this.getScope().getEntryNode() = result
     }
 
+    pragma [noinline]
+    Scope scope_as_global_variable() {
+        result = this.(GlobalVariable).getScope()
+    }
+
     override CallNode redefinedAtCallSite() {
-        result.(CallNode).getScope().getScope*() = this.(GlobalVariable).getScope()
+        result.(CallNode).getScope().getScope*() = this.scope_as_global_variable()
     }
 
 }

python/ql/src/semmle/python/pointsto/PointsTo.qll

@@ -500,12 +500,17 @@ cached module PointsToInternal {
         pointsTo(def.getValue(), context, value, origin)
     }
 
+    pragma [nomagic]
+    private predicate sequence_index_points_to(ControlFlowNode f, PointsToContext context, SequenceObjectInternal sequence, ObjectInternal value, int index) {
+        pointsTo(f, context, sequence, _) and
+        value = sequence.getItem(index)
+    }
+
     pragma [noinline]
     private predicate multi_assignment_points_to(MultiAssignmentDefinition def, PointsToContext context, ObjectInternal value, ControlFlowNode origin) {
         exists(int index, ControlFlowNode rhs, SequenceObjectInternal sequence |
             def.indexOf(index, rhs) and
-            pointsTo(rhs, context, sequence, _) and
-            value = sequence.getItem(index) and
+            sequence_index_points_to(rhs, context, sequence, value, index) and
             origin = def.getDefiningNode()
         )
     }
@@ -662,12 +667,18 @@ private module InterModulePointsTo {
     pragma [noinline]
     private EssaVariable ssa_variable_for_module_attribute(ImportMemberNode f, PointsToContext context) {
         exists(string name, ModuleObjectInternal mod, Module m |
-            mod.getSourceModule() = m and m = f.getEnclosingModule() and m = result.getScope() and
+            mod.getSourceModule() = m and m = result.getScope() and
             PointsToInternal::pointsTo(f.getModule(name), context, mod, _) and
-            result.getSourceVariable().getName() = name and result.getAUse() = f
+            result = ssa_variable_for_module_attribute_helper(f, name, m)
         )
     }
 
+    pragma [noinline]
+    private EssaVariable ssa_variable_for_module_attribute_helper(ImportMemberNode f, string name, Module m) {
+        result.getSourceVariable().getName() = name and result.getAUse() = f
+        and m = f.getEnclosingModule()
+    }
+
     /* Helper for implicit_submodule_points_to */
     private ModuleObjectInternal getModule(ImplicitSubModuleDefinition def) {
        exists(PackageObjectInternal package |

python/ql/src/semmle/python/types/Extensions.qll

@@ -142,7 +142,12 @@ class ReModulePointToExtension extends PointsToExtension {
             sre_constants.attribute("SRE_FLAG_" + name, value, orig) and
             origin = orig.asCfgNodeOrHere(this)
         )
-        and context.appliesTo(this)
+        and pointsTo_helper(context)
+    }
+
+    pragma [noinline]
+    private predicate pointsTo_helper(Context context) {
+        context.appliesTo(this)
     }
 
 }