Merge pull request #1483 from markshannon/merge-121

Merge rc/1.21 into master

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

@@ -985,6 +985,7 @@ class BasicBlock extends @py_flow_node {
 
     /** Dominance frontier of a node x is the set of all nodes `other` such that `this` dominates a predecessor
      * of `other` but does not strictly dominate `other` */
+    pragma[noinline]
     predicate dominanceFrontier(BasicBlock other) {
         this.dominates(other.getAPredecessor()) and not this.strictlyDominates(other)
     }

python/ql/src/semmle/python/Module.qll

@@ -9,9 +9,13 @@ class Module extends Module_, Scope, AstNode {
     override string toString() {
         result = this.getKind() + " " + this.getName()
         or
-        /* No name is defined, which means that this is not on an import path. So it must be a script */
+        /* No name is defined, which means that this module is not on an import path. So it must be a script */
         not exists(this.getName()) and not this.isPackage() and
         result = "Script " + this.getFile().getShortName()
+        or
+        /* Package missing name, so just use the path instead */
+        not exists(this.getName()) and this.isPackage() and
+        result = "Package at "  + this.getPath().getAbsolutePath()
     }
 
     /** This method will be deprecated in the next release. Please use `getEnclosingScope()` instead.
@@ -67,9 +71,9 @@ class Module extends Module_, Scope, AstNode {
     string getAnExport() {
         py_exports(this, result)
         or
-        exists(ModuleValue mod |
+        exists(ModuleObjectInternal mod |
             mod.getSource() = this.getEntryNode() |
-            mod.exports(result)
+            mod.(ModuleValue).exports(result)
         )
     }
 
@@ -191,6 +195,7 @@ class Module extends Module_, Scope, AstNode {
 
 }
 
+
 bindingset[name]
 private predicate legalDottedName(string name) {
     name.regexpMatch("(\\p{L}|_)(\\p{L}|\\d|_)*(\\.(\\p{L}|_)(\\p{L}|\\d|_)*)*")
@@ -240,3 +245,30 @@ private predicate isStubRoot(Folder f) {
     f.getAbsolutePath().matches("%/data/python/stubs")
 }
 
+
+/** Holds if the Container `c` should be the preferred file or folder for
+ * the given name when performing imports.
+ * Trivially true for any container if it is the only one with its name.
+ * However, if there are several modules with the same name, then
+ * this is the module most likely to be imported under that name.
+ */
+predicate isPreferredModuleForName(Container c, string name) {
+    exists(int p |
+        p = min(int x | x = priorityForName(_, name)) and
+        p = priorityForName(c, name)
+    )
+}
+
+private int priorityForName(Container c, string name) {
+    name = moduleNameFromFile(c) and
+    (
+        // In the source
+        exists(c.getRelativePath()) and result = -1
+        or
+        // On an import path
+        exists(c.getImportRoot(result))
+        or
+        // Otherwise
+        result = 10000
+    )
+}

python/ql/src/semmle/python/objects/Callables.qll

@@ -11,9 +11,6 @@ private import semmle.python.types.Builtins
 
 abstract class CallableObjectInternal extends ObjectInternal {
 
-    /** Gets the name of this callable */
-    abstract string getName();
-
     /** Gets the scope of this callable if it has one */
     abstract Function getScope();
 

python/ql/src/semmle/python/objects/Constants.qll

@@ -73,11 +73,6 @@ abstract class ConstantObjectInternal extends ObjectInternal {
 
 private abstract class BooleanObjectInternal extends ConstantObjectInternal {
 
-    BooleanObjectInternal() {
-        this = TTrue() or this = TFalse()
-    }
-
-
     override ObjectInternal getClass() {
         result = TBuiltinClassObject(Builtin::special("bool"))
     }

python/ql/src/semmle/python/objects/Instances.qll

@@ -440,9 +440,8 @@ class SuperInstance extends TSuperInstance, ObjectInternal {
     pragma [noinline] override predicate descriptorGetInstance(ObjectInternal instance, ObjectInternal value, CfgOrigin origin) { none() }
 
     pragma [noinline] override predicate attribute(string name, ObjectInternal value, CfgOrigin origin) {
-        PointsToInternal::attributeRequired(this, name) and
         exists(ObjectInternal cls_attr, CfgOrigin attr_orig |
-            this.lookup(name, cls_attr, attr_orig)
+            this.attribute_descriptor(name, cls_attr, attr_orig)
             |
             cls_attr.isDescriptor() = false and value = cls_attr and origin = attr_orig
             or
@@ -450,6 +449,12 @@ class SuperInstance extends TSuperInstance, ObjectInternal {
         )
     }
 
+    /* Helper for `attribute` */
+    pragma [noinline] private predicate attribute_descriptor(string name, ObjectInternal cls_attr, CfgOrigin attr_orig) {
+        PointsToInternal::attributeRequired(this, name) and
+        this.lookup(name, cls_attr, attr_orig)
+    }
+
     private predicate lookup(string name, ObjectInternal value, CfgOrigin origin) {
         Types::getMro(this.getSelf().getClass()).startingAt(this.getStartClass()).getTail().lookup(name, value, origin)
     }

python/ql/src/semmle/python/objects/ObjectAPI.qll

@@ -71,13 +71,6 @@ class Value extends TObject {
         this.(ObjectInternal).attribute(name, result, _)
     }
 
-    /** DEPRECATED: For backwards compatibility with old API
-     * Use `Value` instead of `ObjectSource`.
-     */
-    deprecated ObjectSource getSource() {
-        result = this.(ObjectInternal).getSource()
-    }
-
     /** Holds if this value is builtin. Applies to built-in functions and methods,
      * but also integers and strings.
      */
@@ -153,9 +146,9 @@ module Module {
      *
      * Note that the name used to refer to a module is not
      * necessarily its name. For example,
-     * there are modules refered to by the name `os.path`,
+     * there are modules referred to by the name `os.path`,
      * but that are not named `os.path`, for example the module `posixpath`.
-     * Such that the follwing is true:
+     * Such that the following is true:
      * `Module::named("os.path").getName() = "posixpath"
      */
     ModuleValue named(string name) {

python/ql/src/semmle/python/objects/TObject.qll

@@ -47,12 +47,13 @@ cached newtype TObject =
     or
     /* Package objects */
     TPackageObject(Folder f) {
-        exists(moduleNameFromFile(f))
+        isPreferredModuleForName(f, _)
     }
     or
     /* Python module objects */
     TPythonModule(Module m) {
-        not m.isPackage() and not exists(SyntaxError se | se.getFile() = m.getFile())
+        not m.isPackage() and isPreferredModuleForName(m.getFile(), _) and
+        not exists(SyntaxError se | se.getFile() = m.getFile())
     }
     or
     /* `True` */

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

@@ -141,7 +141,7 @@ module PointsTo {
         )
         or
         not f.isParameter() and
-        exists(Value value |
+        exists(ObjectInternal value |
             PointsToInternal::pointsTo(f.(DefinitionNode).getValue(), context, value, origin) and
             cls = value.getClass().getSource() |
             obj = value.getSource() or
@@ -151,7 +151,7 @@ module PointsTo {
 
     deprecated predicate
     ssa_variable_points_to(EssaVariable var, PointsToContext context, Object obj, ClassObject cls, CfgOrigin origin) {
-        exists(Value value |
+        exists(ObjectInternal value |
             PointsToInternal::variablePointsTo(var, context, value, origin) and
             cls = value.getClass().getSource() |
             obj = value.getSource()
@@ -160,8 +160,8 @@ module PointsTo {
 
     deprecated
     CallNode get_a_call(Object func, PointsToContext context) {
-        exists(Value value |
-            result = value.getACall(context) and
+        exists(ObjectInternal value |
+            result = value.(Value).getACall(context) and
             func = value.getSource()
         )
     }
@@ -402,7 +402,7 @@ cached module PointsToInternal {
         or
         scope_entry_points_to(def, context, value, origin)
         or
-        InterModulePointsTo::implicit_submodule_points_to(def, context, value, origin)
+        InterModulePointsTo::implicit_submodule_points_to(def, value, origin) and context.isImport()
         or
         iteration_definition_points_to(def, context, value, origin)
         /*
@@ -647,18 +647,22 @@ private module InterModulePointsTo {
         )
     }
 
+    /* Helper for implicit_submodule_points_to */
+    private ModuleObjectInternal getModule(ImplicitSubModuleDefinition def) {
+       exists(PackageObjectInternal package |
+           package.getSourceModule() = def.getDefiningNode().getScope() and 
+           result =  package.submodule(def.getSourceVariable().getName())
+       )
+    }
+
     /** Implicit "definition" of the names of submodules at the start of an `__init__.py` file.
      *
      * PointsTo isn't exactly how the interpreter works, but is the best approximation we can manage statically.
      */
     pragma [noinline]
-    predicate implicit_submodule_points_to(ImplicitSubModuleDefinition def, PointsToContext context, ModuleObjectInternal value, ControlFlowNode origin) {
-        exists(PackageObjectInternal package |
-            package.getSourceModule() = def.getDefiningNode().getScope() |
-            value = package.submodule(def.getSourceVariable().getName()) and
-            origin = CfgOrigin::fromObject(value).asCfgNodeOrHere(def.getDefiningNode()) and
-            context.isImport()
-        )
+    predicate implicit_submodule_points_to(ImplicitSubModuleDefinition def, ModuleObjectInternal value, ControlFlowNode origin) {
+        value = getModule(def) and
+        origin = CfgOrigin::fromObject(value).asCfgNodeOrHere(def.getDefiningNode())
     }
 
     /** Points-to for `from ... import *`. */
@@ -1869,9 +1873,13 @@ cached module Types {
         result = getInheritedMetaclass(cls, 0)
         or
         // Best guess if base is not a known class
+        hasUnknownBase(cls) and result = ObjectInternal::unknownClass()
+    }
+
+    /* Helper for getInheritedMetaclass */
+    private predicate hasUnknownBase(ClassObjectInternal cls) {
         exists(ObjectInternal base |
-            base = getBase(cls, _) and
-            result = ObjectInternal::unknownClass() |
+            base = getBase(cls, _) |
             base.isClass() = false
             or
             base = ObjectInternal::unknownClass()
@@ -1881,14 +1889,18 @@ cached module Types {
     private ClassObjectInternal getInheritedMetaclass(ClassObjectInternal cls, int n) {
         exists(Class c |
             c = cls.(PythonClassObjectInternal).getScope() and
-            n = count(c.getABase())
+            n = count(c.getABase()) and n != 1
             |
             result = ObjectInternal::type() and major_version() = 3
             or
             result = ObjectInternal::classType() and major_version() = 2
         )
         or
+        base_count(cls) = 1 and n = 0 and
+        result = getBase(cls, 0).getClass()
+        or
         exists(ClassObjectInternal meta1, ClassObjectInternal meta2 |
+            base_count(cls) > 1 and
             meta1 = getBase(cls, n).getClass() and
             meta2 = getInheritedMetaclass(cls, n+1)
             |

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

@@ -182,7 +182,7 @@ class PointsToContext extends TPointsToContext {
         this = TRuntimeContext() and executes_in_runtime_context(s)
         or
         /* Called functions, regardless of their name */
-        exists(CallableObjectInternal callable, ControlFlowNode call, TPointsToContext outerContext |
+        exists(PythonFunctionObjectInternal callable, ControlFlowNode call, TPointsToContext outerContext |
             call = callable.getACall(outerContext) and
             this = TCallContext(call, outerContext, _) |
             s = callable.getScope()

python/ql/src/semmle/python/security/TaintTracking.qll

@@ -144,7 +144,7 @@ abstract class TaintKind extends string {
      * the `result` would be `theStrType()`.
      */
     ClassValue getType() {
-        result.getSource() = this.getClass()
+        result.(ClassObjectInternal).getSource() = this.getClass()
     }
 
     /** Gets the boolean values (may be one, neither, or both) that
@@ -1562,7 +1562,7 @@ class CallContext extends TCallContext {
             f.getFunction() = s and f.getACall() = call
         )
         or
-        exists(ClassValue cls,CallNode call |
+        exists(ClassValue cls, CallNode call |
             this = TCalleeContext(call, _, _) and
             call.getFunction().pointsTo(cls) and
             s = cls.lookup("__init__").(CallableValue).getScope() and

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

@@ -4,6 +4,7 @@ private import semmle.python.objects.Instances
 private import semmle.python.pointsto.PointsTo
 private import semmle.python.pointsto.MRO
 private import semmle.python.types.Builtins
+private import semmle.python.objects.ObjectInternal
 
 
 /** A class whose instances represents Python classes.
@@ -99,7 +100,7 @@ class ClassObject extends Object {
 
     /** Returns an attribute declared on this class (not on a super-class) */
     Object declaredAttribute(string name) {
-        exists(Value val |
+        exists(ObjectInternal val |
             Types::declaredAttribute(theClass(), name, val, _) and
             result = val.getSource()
         )
@@ -113,7 +114,7 @@ class ClassObject extends Object {
     /** Returns an attribute as it would be when looked up at runtime on this class.
       Will include attributes of super-classes */
     Object lookupAttribute(string name) {
-        exists(Value val |
+        exists(ObjectInternal val |
             theClass().lookup(name, val, _) and
             result = val.getSource()
         )
@@ -125,7 +126,7 @@ class ClassObject extends Object {
 
     /** Looks up an attribute by searching this class' MRO starting at `start` */
     Object lookupMro(ClassObject start, string name) {
-        exists(ClassObjectInternal other, ClassObjectInternal decl, Value val |
+        exists(ClassObjectInternal other, ClassObjectInternal decl, ObjectInternal val |
             other.getSource() = start and
             decl = Types::getMro(theClass()).startingAt(other).findDeclaringClass(name) and
             Types::declaredAttribute(decl, name, val, _) and
@@ -140,7 +141,7 @@ class ClassObject extends Object {
 
     /** Whether the named attribute refers to the object, class and origin */
     predicate attributeRefersTo(string name, Object obj, ClassObject cls, ControlFlowNode origin) {
-        exists(Value val, CfgOrigin valorig |
+        exists(ObjectInternal val, CfgOrigin valorig |
             theClass().lookup(name, val, valorig) and
             obj = val.getSource() and
             cls = val.getClass().getSource() and

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

@@ -4,13 +4,14 @@ private import semmle.python.pointsto.PointsTo
 private import semmle.python.objects.Callables
 private import semmle.python.libraries.Zope
 private import semmle.python.pointsto.Base
+private import semmle.python.objects.ObjectInternal
 private import semmle.python.types.Builtins
 
 /** A function object, whether written in Python or builtin */
 abstract class FunctionObject extends Object {
 
     CallableValue theCallable() {
-        result.getSource() = this
+        result.(ObjectInternal).getSource() = this
     }
 
     predicate isOverridingMethod() {

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

@@ -1,6 +1,6 @@
 import python
 private import semmle.python.pointsto.PointsTo
-private import semmle.python.objects.Modules
+private import semmle.python.objects.ObjectInternal
 private import semmle.python.types.ModuleKind
 
 abstract class ModuleObject extends Object {
@@ -32,6 +32,9 @@ abstract class ModuleObject extends Object {
 
     override string toString() {
         result = "Module " + this.getName()
+        or
+        not exists(this.getName()) and
+        result = this.getModule().toString()
     }
 
     /** Gets the named attribute of this module. Using attributeRefersTo() instead
@@ -54,7 +57,7 @@ abstract class ModuleObject extends Object {
     }
 
     predicate attributeRefersTo(string name, Object obj, ControlFlowNode origin) {
-        exists(Value val, CfgOrigin valorig |
+        exists(ObjectInternal val, CfgOrigin valorig |
             theModule().(ModuleObjectInternal).attribute(name, val, valorig) and
             obj = val.getSource() and
             origin = valorig.toCfgNode()
@@ -62,7 +65,7 @@ abstract class ModuleObject extends Object {
     }
 
     predicate attributeRefersTo(string name, Object obj, ClassObject cls, ControlFlowNode origin) {
-        exists(Value val, CfgOrigin valorig |
+        exists(ObjectInternal val, CfgOrigin valorig |
             theModule().(ModuleObjectInternal).attribute(name, val, valorig) and
             obj = val.getSource() and
             cls = val.getClass().getSource() and
@@ -223,7 +226,7 @@ class PackageObject extends ModuleObject {
     }
 
     override Object getAttribute(string name) {
-        exists(Value val |
+        exists(ObjectInternal val |
             theModule().(PackageObjectInternal).attribute(name, val, _) and
             result = val.getSource()
         )

python/ql/test/3/library-tests/PointsTo/attributes/Test.expected

@@ -1,3 +1,4 @@
+| 1 | ControlFlowNode for unicode_literals | ImportMember | 1 |
 | 2 | ControlFlowNode for C | class C | 2 |
 | 2 | ControlFlowNode for ClassExpr | class C | 2 |
 | 2 | ControlFlowNode for object | builtin-class object | 2 |

python/ql/test/library-tests/ControlFlow/PointsToSupport/UseFromDefinition.ql

@@ -1,9 +1,3 @@
-/**
- * @name UseFromDefinition
- * @description Insert description here...
- * @kind table
- * @problem.severity warning
- */
 
 import python
 

python/ql/test/library-tests/PointsTo/general/GlobalPointsTo.expected

@@ -98,6 +98,7 @@
 | Module pointsto_test | 76 | ControlFlowNode for sys | Module sys |
 | Module pointsto_test | 76 | ControlFlowNode for type | builtin-class type |
 | Module pointsto_test | 76 | ControlFlowNode for type() | builtin-class module |
+| Module pointsto_test | 77 | ControlFlowNode for unknown | ImportMember |
 | Module pointsto_test | 78 | ControlFlowNode for type | builtin-class type |
 | Module pointsto_test | 79 | ControlFlowNode for Dict | Dict |
 | Module pointsto_test | 79 | ControlFlowNode for Tuple | Tuple |

python/ql/test/library-tests/PointsTo/general/LocalPointsTo.expected

@@ -106,6 +106,7 @@
 | 76 | ControlFlowNode for sys | Module sys |
 | 76 | ControlFlowNode for type | builtin-class type |
 | 76 | ControlFlowNode for type() | builtin-class module |
+| 77 | ControlFlowNode for unknown | ImportMember |
 | 78 | ControlFlowNode for type | builtin-class type |
 | 79 | ControlFlowNode for Dict | Dict |
 | 79 | ControlFlowNode for Tuple | Tuple |

python/ql/test/library-tests/PointsTo/new/PointsToWithContext.expected

@@ -1144,6 +1144,7 @@ WARNING: Predicate points_to has been deprecated and may be removed in future (P
 | t_type.py:7 | ControlFlowNode for sys | Module sys | builtin-class module | 1 | import |
 | t_type.py:7 | ControlFlowNode for type | builtin-class type | builtin-class type | 7 | import |
 | t_type.py:7 | ControlFlowNode for type() | builtin-class module | builtin-class type | 7 | import |
+| t_type.py:8 | ControlFlowNode for unknown | ImportMember | *UNKNOWN TYPE* | 8 | import |
 | t_type.py:9 | ControlFlowNode for type | builtin-class type | builtin-class type | 9 | import |
 | t_type.py:9 | ControlFlowNode for type() | *UNKNOWN TYPE* | *UNKNOWN TYPE* | 9 | import |
 | t_type.py:10 | ControlFlowNode for Dict | Dict | builtin-class dict | 10 | import |

python/ql/test/library-tests/PointsTo/new/TestEvaluate.expected

@@ -1,5 +1,3 @@
-WARNING: Predicate getSource has been deprecated and may be removed in future (TestEvaluate.ql:14,16-25)
-WARNING: Predicate getSource has been deprecated and may be removed in future (TestEvaluate.ql:16,20-29)
 | b_condition.py:7 | Compare | true | x | NoneType None |
 | b_condition.py:13 | Compare | false | x | NoneType None |
 | b_condition.py:19 | UnaryExpr | true | x | NoneType None |

python/ql/test/library-tests/PointsTo/new/TestEvaluate.ql

@@ -5,13 +5,13 @@ import semmle.python.pointsto.PointsToContext
 import Util
 
 
-from ControlFlowNode test, ControlFlowNode use, Value val, boolean eval, PointsToContext ctx, ControlFlowNode origin, string what
+from ControlFlowNode test, ControlFlowNode use, ObjectInternal val, boolean eval, PointsToContext ctx, ControlFlowNode origin, string what
 where 
 not use instanceof NameConstantNode and
 not use.getNode() instanceof ImmutableLiteral and
 eval = Conditionals::testEvaluates(test, use, ctx, val, origin) and
 (
-    what = val.getSource().toString()
+    what = val.getSource().(Object).toString()
     or
     not exists(val.getSource()) and what = origin.getNode().toString()
 )

python/ql/test/library-tests/modules/duplicate_name/Modules.expected

@@ -0,0 +1,3 @@
+| sqlite3 | 2 | 1 |
+| sqlite3.__init__ | 2 | 1 |
+| sqlite3.dump | 2 | 1 |

python/ql/test/library-tests/modules/duplicate_name/Modules.ql

@@ -0,0 +1,7 @@
+
+import python
+
+from string name, int mcnt
+where mcnt = strictcount(Module m | m.getName() = name) and mcnt > 1
+select name, mcnt, strictcount(ModuleValue val | val.getName() = name)
+

python/ql/test/library-tests/modules/duplicate_name/options

@@ -0,0 +1,2 @@
+semmle-extractor-options: -R .
+optimize: true

python/ql/test/library-tests/modules/duplicate_name/test.py

@@ -0,0 +1 @@
+import sqlite3.dump