wip

csharp/ql/lib/semmle/code/csharp/ExprOrStmtParent.qll

@@ -5,6 +5,7 @@
  */
 
 import csharp
+private import internal.Location
 
 /**
  * INTERNAL: Do not use.
@@ -65,14 +66,6 @@ private ControlFlowElement getBody(Callable c) {
   result = getStatementBody(c)
 }
 
-pragma[nomagic]
-private SourceLocation getASourceLocation(Element e) {
-  result = e.getALocation().(SourceLocation) and
-  not exists(e.getALocation().(SourceLocation).getMappedLocation())
-  or
-  result = e.getALocation().(SourceLocation).getMappedLocation()
-}
-
 pragma[nomagic]
 private predicate hasNoSourceLocation(Element e) { not exists(getASourceLocation(e)) }
 

csharp/ql/lib/semmle/code/csharp/Location.qll

@@ -50,6 +50,27 @@ class Location extends @location {
 
   /** Gets the 1-based column number (inclusive) where this location ends. */
   final int getEndColumn() { this.hasLocationInfo(_, _, _, _, result) }
+
+  /** Holds if this location starts strictly before the specified location. */
+  bindingset[this, other]
+  pragma[inline_late]
+  predicate strictlyBefore(Location other) {
+    this.getFile() = other.getFile() and
+    (
+      this.getStartLine() < other.getStartLine()
+      or
+      this.getStartLine() = other.getStartLine() and this.getStartColumn() < other.getStartColumn()
+    )
+  }
+
+  /** Holds if this location starts before the specified location. */
+  bindingset[this, other]
+  pragma[inline_late]
+  predicate before(Location other) {
+    this.getStartLine() < other.getStartLine()
+    or
+    this.getStartLine() = other.getStartLine() and this.getStartColumn() <= other.getStartColumn()
+  }
 }
 
 /** An empty location. */

csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll

@@ -9,6 +9,22 @@ import csharp
  */
 module Ssa {
   private import internal.SsaImpl as SsaImpl
+  private import semmle.code.csharp.internal.Location
+
+  pragma[nomagic]
+  private predicate assignableDefinitionLocalVariable(
+    AssignableDefinition ad, LocalScopeVariable v, Callable c
+  ) {
+    ad.getTarget() = v and
+    ad.getEnclosingCallable() = c
+  }
+
+  private predicate localSourceVariable(
+    SourceVariables::LocalScopeSourceVariable sv, LocalScopeVariable v, Callable c
+  ) {
+    sv.getAssignable() = v and
+    sv.getEnclosingCallable() = c
+  }
 
   /**
    * A variable that can be SSA converted.
@@ -34,11 +50,10 @@ module Ssa {
       or
       // Local variable declaration without initializer
       not exists(result.getTargetAccess()) and
-      this =
-        any(SourceVariables::LocalScopeSourceVariable v |
-          result.getTarget() = v.getAssignable() and
-          result.getEnclosingCallable() = v.getEnclosingCallable()
-        )
+      exists(LocalScopeVariable v, Callable c |
+        assignableDefinitionLocalVariable(result, v, c) and
+        localSourceVariable(this, v, c)
+      )
     }
 
     /**
@@ -564,9 +579,15 @@ module Ssa {
     override Location getLocation() { result = this.getCallable().getLocation() }
   }
 
-  bindingset[e1, e2]
-  pragma[inline_late]
-  private predicate inSameFile(Location e1, Location e2) { e1.getFile() = e2.getFile() }
+  private module NearestLocationInput implements NearestLocationInputSig {
+    class C = ImplicitParameterDefinition;
+
+    predicate relevantLocations(ImplicitParameterDefinition def, Location l1, Location l2) {
+      not def.getBasicBlock() instanceof ControlFlow::BasicBlocks::EntryBlock and
+      l1 = def.getParameter().getALocation() and
+      l2 = def.getBasicBlock().getLocation()
+    }
+  }
 
   /**
    * An SSA definition representing the implicit initialization of a parameter
@@ -590,13 +611,11 @@ module Ssa {
     }
 
     override Location getLocation() {
-      // Each parameter has an implicit entry definition. In case of multiple bodies,
-      // we need to match the location of the parameter with the location of the
-      // assigning basic block; requiring them to be in the same file should be
-      // sufficient (multiple method bodies in the same file would be a compilation
-      // error).
-      result = p.getALocation() and
-      inSameFile(result, this.getBasicBlock().getLocation())
+      this.getBasicBlock() instanceof ControlFlow::BasicBlocks::EntryBlock and
+      result = p.getLocation()
+      or
+      // multi-bodied method: use matching parameter location
+      NearestLocation<NearestLocationInput>::nearestLocation(this, result, _)
     }
   }
 

csharp/ql/lib/semmle/code/csharp/dataflow/internal/BaseSSA.qll

@@ -31,7 +31,8 @@ module BaseSsa {
       c = entry.getCallable() and
       // In case `c` has multiple bodies, we want each body to gets its own implicit
       // entry definition. In case `c` doesn't have multiple bodies, the line below
-      // is simply the same as `bb = entry`.
+      // is simply the same as `bb = entry`, because `entry.getFirstNode().getASuccessor()`
+      // will be in the entry block.
       bb = entry.getFirstNode().getASuccessor().getBasicBlock() and
       c = v.getCallable()
     |

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowDispatch.qll

@@ -8,6 +8,7 @@ private import semmle.code.csharp.dispatch.Dispatch
 private import semmle.code.csharp.dispatch.RuntimeCallable
 private import semmle.code.csharp.frameworks.system.Collections
 private import semmle.code.csharp.frameworks.system.collections.Generic
+private import semmle.code.csharp.internal.Location
 
 /**
  * Gets a source declaration of callable `c` that has a body and is
@@ -24,6 +25,21 @@ newtype TReturnKind =
   TOutReturnKind(int i) { i = any(Parameter p | p.isOut()).getPosition() } or
   TRefReturnKind(int i) { i = any(Parameter p | p.isRef()).getPosition() }
 
+private predicate hasMultipleSourceLocations(Callable c) { strictcount(getASourceLocation(c)) > 1 }
+
+private module NearestBodyLocationInput implements NearestLocationInputSig {
+  class C = ControlFlowElement;
+
+  predicate relevantLocations(ControlFlowElement body, Location l1, Location l2) {
+    exists(Callable c |
+      hasMultipleSourceLocations(c) and
+      l1 = getASourceLocation(c) and
+      body = c.getBody() and
+      l2 = body.getLocation()
+    )
+  }
+}
+
 cached
 private module Cached {
   /**
@@ -33,7 +49,18 @@ private module Cached {
    */
   cached
   newtype TDataFlowCallable =
-    TCallable(Callable c) { c.isUnboundDeclaration() } or
+    TCallable(Callable c, Location l) {
+      c.isUnboundDeclaration() and
+      l = [c.getLocation(), getASourceLocation(c)] and
+      (
+        not hasMultipleSourceLocations(c)
+        or
+        // when `c` has multiple source locations, only use those with a body;
+        // for example, `partial` methods may have multiple source locations but
+        // we are only interested in the one with a body
+        NearestLocation<NearestBodyLocationInput>::nearestLocation(_, l, _)
+      )
+    } or
     TSummarizedCallable(FlowSummary::SummarizedCallable sc) or
     TFieldOrPropertyCallable(FieldOrProperty f) or
     TCapturedVariableCallable(LocalScopeVariable v) { v.isCaptured() }
@@ -88,6 +115,7 @@ private module DispatchImpl {
    */
   DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) {
     exists(DispatchCall dc | dc = call.(NonDelegateDataFlowCall).getDispatchCall() |
+      result = call.getARuntimeTarget() and
       result.getUnderlyingCallable() =
         getCallableForDataFlow(dc.getADynamicTargetInCallContext(ctx.(NonDelegateDataFlowCall)
                 .getDispatchCall()).getUnboundDeclaration())
@@ -159,7 +187,51 @@ class RefReturnKind extends OutRefReturnKind, TRefReturnKind {
 /** A callable used for data flow. */
 class DataFlowCallable extends TDataFlowCallable {
   /** Gets the underlying source code callable, if any. */
-  Callable asCallable() { this = TCallable(result) }
+  Callable asCallable(Location l) { this = TCallable(result, l) }
+
+  /** Holds if this callable is multi-bodied. */
+  pragma[nomagic]
+  predicate isMultiBodied() { hasMultipleSourceLocations(this.asCallable(_)) }
+
+  private ControlFlow::Nodes::ElementNode getAMultiBodyEntryNode() {
+    exists(ControlFlowElement body, Location l |
+      body = this.asCallable(l).getBody() and
+      NearestLocation<NearestBodyLocationInput>::nearestLocation(body, l, _) and
+      result = body.getAControlFlowEntryNode()
+    )
+  }
+
+  pragma[nomagic]
+  private ControlFlow::Nodes::ElementNode getAMultiBodyControlFlowNodePred() {
+    result = this.getAMultiBodyEntryNode().getAPredecessor()
+    or
+    result = this.getAMultiBodyControlFlowNodePred().getAPredecessor()
+  }
+
+  pragma[nomagic]
+  private ControlFlow::Nodes::ElementNode getAMultiBodyControlFlowNodeSucc() {
+    result = this.getAMultiBodyEntryNode().getASuccessor()
+    or
+    result = this.getAMultiBodyControlFlowNodeSucc().getASuccessor()
+  }
+
+  pragma[nomagic]
+  private ControlFlow::Nodes::ElementNode getAMultiBodyControlFlowNode() {
+    result =
+      [
+        this.getAMultiBodyEntryNode(), this.getAMultiBodyControlFlowNodePred(),
+        this.getAMultiBodyControlFlowNodeSucc()
+      ]
+  }
+
+  /** Gets a control flow node belonging to this callable. */
+  pragma[inline]
+  ControlFlow::Node getAControlFlowNode() {
+    result = this.getAMultiBodyControlFlowNode()
+    or
+    not this.isMultiBodied() and
+    result.getEnclosingCallable() = this.asCallable(_)
+  }
 
   /** Gets the underlying summarized callable, if any. */
   FlowSummary::SummarizedCallable asSummarizedCallable() { this = TSummarizedCallable(result) }
@@ -171,7 +243,7 @@ class DataFlowCallable extends TDataFlowCallable {
 
   /** Gets the underlying callable. */
   Callable getUnderlyingCallable() {
-    result = this.asCallable() or result = this.asSummarizedCallable()
+    result = this.asCallable(_) or result = this.asSummarizedCallable()
   }
 
   /** Gets a textual representation of this dataflow callable. */
@@ -185,7 +257,9 @@ class DataFlowCallable extends TDataFlowCallable {
 
   /** Get the location of this dataflow callable. */
   Location getLocation() {
-    result = this.getUnderlyingCallable().getLocation()
+    this = TCallable(_, result)
+    or
+    result = this.asSummarizedCallable().getLocation()
     or
     result = this.asFieldOrProperty().getLocation()
     or
@@ -256,6 +330,26 @@ abstract class DataFlowCall extends TDataFlowCall {
   }
 }
 
+private predicate relevantFolder(Folder f) {
+  exists(NonDelegateDataFlowCall call, Location l | f = l.getFile().getParentContainer() |
+    l = call.getLocation() and
+    call.getARuntimeTargetCandidate(_, _).isMultiBodied()
+    or
+    call.getARuntimeTargetCandidate(l, _).isMultiBodied()
+  )
+}
+
+private predicate adjacentFolders(Folder f1, Folder f2) {
+  f1 = f2.getParentContainer()
+  or
+  f2 = f1.getParentContainer()
+}
+
+bindingset[f1, f2]
+pragma[inline_late]
+private predicate folderDist(Folder f1, Folder f2, int i) =
+  shortestDistances(relevantFolder/1, adjacentFolders/2)(f1, f2, i)
+
 /** A non-delegate C# call relevant for data flow. */
 class NonDelegateDataFlowCall extends DataFlowCall, TNonDelegateCall {
   private ControlFlow::Nodes::ElementNode cfn;
@@ -266,23 +360,59 @@ class NonDelegateDataFlowCall extends DataFlowCall, TNonDelegateCall {
   /** Gets the underlying call. */
   DispatchCall getDispatchCall() { result = dc }
 
-  override DataFlowCallable getARuntimeTarget() {
-    result.asCallable() = getCallableForDataFlow(dc.getADynamicTarget())
-    or
+  pragma[nomagic]
+  private predicate hasSourceTarget() { dc.getADynamicTarget().fromSource() }
+
+  pragma[nomagic]
+  private FlowSummary::SummarizedCallable getASummarizedCallableTarget() {
     // Only use summarized callables with generated summaries in case
     // we are not able to dispatch to a source declaration.
-    exists(FlowSummary::SummarizedCallable sc, boolean static |
-      result.asSummarizedCallable() = sc and
-      sc = this.getATarget(static) and
+    exists(boolean static |
+      result = this.getATarget(static) and
       not (
-        sc.applyGeneratedModel() and
-        dc.getADynamicTarget().getUnboundDeclaration().getFile().fromSource()
+        result.applyGeneratedModel() and
+        this.hasSourceTarget()
       )
     |
       static = false
       or
-      static = true and not sc instanceof RuntimeCallable
+      static = true and not result instanceof RuntimeCallable
+    )
+  }
+
+  pragma[nomagic]
+  DataFlowCallable getARuntimeTargetCandidate(Location l, Callable c) {
+    c = result.asCallable(l) and
+    c = getCallableForDataFlow(dc.getADynamicTarget())
+  }
+
+  pragma[nomagic]
+  private DataFlowCallable getAMultiBodiedRuntimeTargetCandidate(Callable c, int dist) {
+    result.isMultiBodied() and
+    exists(Location l | result = this.getARuntimeTargetCandidate(l, c) |
+      inSameFile(l, this.getLocation()) and
+      dist = -1
+      or
+      folderDist(l.getFile().getParentContainer(),
+        this.getLocation().getFile().getParentContainer(), dist)
+    )
+  }
+
+  pragma[nomagic]
+  override DataFlowCallable getARuntimeTarget() {
+    exists(Callable c |
+      result =
+        min(DataFlowCallable cand, int dist |
+          cand = this.getAMultiBodiedRuntimeTargetCandidate(c, dist)
+        |
+          cand order by dist
+        )
     )
+    or
+    result = this.getARuntimeTargetCandidate(_, _) and
+    not result.isMultiBodied()
+    or
+    result.asSummarizedCallable() = this.getASummarizedCallableTarget()
   }
 
   /** Gets a static or dynamic target of this call. */
@@ -296,9 +426,7 @@ class NonDelegateDataFlowCall extends DataFlowCall, TNonDelegateCall {
 
   override DataFlow::ExprNode getNode() { result.getControlFlowNode() = cfn }
 
-  override DataFlowCallable getEnclosingCallable() {
-    result.asCallable() = cfn.getEnclosingCallable()
-  }
+  override DataFlowCallable getEnclosingCallable() { result.getAControlFlowNode() = cfn }
 
   override string toString() { result = cfn.toString() }
 
@@ -326,9 +454,7 @@ class ExplicitDelegateLikeDataFlowCall extends DelegateDataFlowCall, TExplicitDe
 
   override DataFlow::ExprNode getNode() { result.getControlFlowNode() = cfn }
 
-  override DataFlowCallable getEnclosingCallable() {
-    result.asCallable() = cfn.getEnclosingCallable()
-  }
+  override DataFlowCallable getEnclosingCallable() { result.getAControlFlowNode() = cfn }
 
   override string toString() { result = cfn.toString() }