Merge pull request #714 from owen-mc/fix-get-enclosing-callable

Extend DataFlowCallable to include file scopes

ql/lib/change-notes/2022-04-08-fix-global-dataflow.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Fixed a bug where dataflow steps were ignored if both ends were inside the initialiser routine of a file-level variable.

ql/lib/semmle/go/dataflow/internal/DataFlowDispatch.qll

@@ -93,7 +93,7 @@ DataFlowCallable viableCallable(CallExpr ma) {
     else
       if isInterfaceMethodCall(call)
       then result = getRestrictedInterfaceTarget(call)
-      else result = call.getACalleeIncludingExternals()
+      else result.asCallable() = call.getACalleeIncludingExternals()
   )
 }
 

ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll

@@ -26,11 +26,15 @@ private newtype TNode =
 /** Nodes intended for only use inside the data-flow libraries. */
 module Private {
   /** Gets the callable in which this node occurs. */
-  DataFlowCallable nodeGetEnclosingCallable(Node n) { result = n.getEnclosingCallable() }
+  DataFlowCallable nodeGetEnclosingCallable(Node n) {
+    result.asCallable() = n.getEnclosingCallable()
+    or
+    not exists(n.getEnclosingCallable()) and result.asFileScope() = n.getFile()
+  }
 
   /** Holds if `p` is a `ParameterNode` of `c` with position `pos`. */
   predicate isParameterNode(ParameterNode p, DataFlowCallable c, int pos) {
-    p.isParameterOf(c, pos)
+    p.isParameterOf(c.asCallable(), pos)
   }
 
   /** A data flow node that represents returning a value from a function. */
@@ -108,9 +112,11 @@ module Public {
     Callable getEnclosingCallable() {
       result.getFuncDef() = this.getRoot()
       or
-      this = MkSummarizedParameterNode(result, _)
-      or
-      this = MkSummaryInternalNode(result, _)
+      exists(DataFlowCallable dfc | result = dfc.asCallable() |
+        this = MkSummarizedParameterNode(dfc, _)
+        or
+        this = MkSummaryInternalNode(dfc, _)
+      )
     }
 
     /** Gets the type of this node. */
@@ -570,7 +576,9 @@ module Public {
     Callable c;
     int i;
 
-    SummarizedParameterNode() { this = MkSummarizedParameterNode(c, i) }
+    SummarizedParameterNode() {
+      this = MkSummarizedParameterNode(any(DataFlowCallable dfc | c = dfc.asCallable()), i)
+    }
 
     // There are no AST representations of summarized parameter nodes
     override ControlFlow::Root getRoot() { none() }

ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll

@@ -197,7 +197,35 @@ class DataFlowType = Type;
 
 class DataFlowLocation = Location;
 
-class DataFlowCallable = Callable;
+private newtype TDataFlowCallable =
+  TCallable(Callable c) or
+  TFileScope(File f)
+
+class DataFlowCallable extends TDataFlowCallable {
+  Callable asCallable() { this = TCallable(result) }
+
+  File asFileScope() { this = TFileScope(result) }
+
+  FuncDef getFuncDef() { result = this.asCallable().getFuncDef() }
+
+  Function asFunction() { result = this.asCallable().asFunction() }
+
+  FuncLit asFuncLit() { result = this.asCallable().asFuncLit() }
+
+  SignatureType getType() { result = this.asCallable().getType() }
+
+  string toString() {
+    result = this.asCallable().toString() or
+    result = "File scope: " + this.asFileScope().toString()
+  }
+
+  predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    this.asCallable().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) or
+    this.asFileScope().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+  }
+}
 
 /** A function call relevant for data flow. */
 class DataFlowCall extends Expr {
@@ -214,7 +242,11 @@ class DataFlowCall extends Expr {
   ExprNode getNode() { result = call }
 
   /** Gets the enclosing callable of this call. */
-  DataFlowCallable getEnclosingCallable() { result.getFuncDef() = this.getEnclosingFunction() }
+  DataFlowCallable getEnclosingCallable() {
+    result.asCallable().getFuncDef() = this.getEnclosingFunction()
+    or
+    not exists(this.getEnclosingFunction()) and result.asFileScope() = this.getFile()
+  }
 }
 
 /** Holds if `e` is an expression that always has the same Boolean value `val`. */