Merge pull request github#7658 from hvitved/csharp/dataflow/no-negative-positions

C#: Get rid of negative parameter/argument data-flow positions

Author: hvitved

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

@@ -40,7 +40,12 @@ module SummaryComponent {
   predicate return = SummaryComponentInternal::return/1;
 
   /** Gets a summary component that represents a qualifier. */
-  SummaryComponent qualifier() { result = argument(-1) }
+  SummaryComponent qualifier() {
+    exists(ParameterPosition pos |
+      result = SummaryComponentInternal::argument(pos) and
+      pos.isThisParameter()
+    )
+  }
 
   /** Gets a summary component that represents an element in a collection. */
   SummaryComponent element() { result = content(any(DataFlow::ElementContent c)) }
@@ -140,12 +145,17 @@ private class SummarizedCallableDefaultClearsContent extends Impl::Public::Summa
 
   // By default, we assume that all stores into arguments are definite
   override predicate clearsContent(ParameterPosition pos, DataFlow::Content content) {
-    exists(SummaryComponentStack output |
+    exists(SummaryComponentStack output, SummaryComponent target |
       this.propagatesFlow(_, output, _) and
       output.drop(_) =
         SummaryComponentStack::push(SummaryComponent::content(content),
-          SummaryComponentStack::argument(pos.getPosition())) and
+          SummaryComponentStack::singleton(target)) and
       not content instanceof DataFlow::ElementContent
+    |
+      target = SummaryComponent::argument(pos.getPosition())
+      or
+      target = SummaryComponent::qualifier() and
+      pos.isThisParameter()
     )
   }
 }

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

@@ -116,19 +116,23 @@ private module Cached {
   cached
   DataFlowCallable viableCallable(DataFlowCall call) { result = call.getARuntimeTarget() }
 
-  private int parameterPosition() {
-    result =
-      [
-        -1, any(Parameter p).getPosition(),
-        ImplicitCapturedParameterNodeImpl::getParameterPosition(_)
-      ]
+  private predicate capturedWithFlowIn(LocalScopeVariable v) {
+    exists(Ssa::ExplicitDefinition def | def.isCapturedVariableDefinitionFlowIn(_, _, _) |
+      v = def.getSourceVariable().getAssignable()
+    )
   }
 
   cached
-  newtype TParameterPosition = MkParameterPosition(int i) { i = parameterPosition() }
+  newtype TParameterPosition =
+    TPositionalParameterPosition(int i) { i = any(Parameter p).getPosition() } or
+    TThisParameterPosition() or
+    TImplicitCapturedParameterPosition(LocalScopeVariable v) { capturedWithFlowIn(v) }
 
   cached
-  newtype TArgumentPosition = MkArgumentPosition(int i) { i = parameterPosition() }
+  newtype TArgumentPosition =
+    TPositionalArgumentPosition(int i) { i = any(Parameter p).getPosition() } or
+    TQualifierArgumentPosition() or
+    TImplicitCapturedArgumentPosition(LocalScopeVariable v) { capturedWithFlowIn(v) }
 }
 
 import Cached
@@ -268,8 +272,8 @@ abstract class DataFlowCall extends TDataFlowCall {
   /** Gets the underlying expression, if any. */
   final DotNet::Expr getExpr() { result = this.getNode().asExpr() }
 
-  /** Gets the `i`th argument of this call. */
-  final ArgumentNode getArgument(int i) { result.argumentOf(this, i) }
+  /** Gets the argument at position `pos` of this call. */
+  final ArgumentNode getArgument(ArgumentPosition pos) { result.argumentOf(this, pos) }
 
   /** Gets a textual representation of this call. */
   abstract string toString();
@@ -425,36 +429,64 @@ class SummaryCall extends DelegateDataFlowCall, TSummaryCall {
   override Location getLocation() { result = c.getLocation() }
 }
 
-/** A parameter position represented by an integer. */
-class ParameterPosition extends MkParameterPosition {
-  private int i;
+/** A parameter position. */
+class ParameterPosition extends TParameterPosition {
+  /** Gets the underlying integer position, if any. */
+  int getPosition() { this = TPositionalParameterPosition(result) }
 
-  ParameterPosition() { this = MkParameterPosition(i) }
+  /** Holds if this position represents a `this` parameter. */
+  predicate isThisParameter() { this = TThisParameterPosition() }
 
-  /** Gets the underlying integer. */
-  int getPosition() { result = i }
+  /** Holds if this position is used to model flow through captured variables. */
+  predicate isImplicitCapturedParameterPosition(LocalScopeVariable v) {
+    this = TImplicitCapturedParameterPosition(v)
+  }
 
   /** Gets a textual representation of this position. */
-  string toString() { result = i.toString() }
+  string toString() {
+    result = "position " + this.getPosition()
+    or
+    this.isThisParameter() and result = "this"
+    or
+    exists(LocalScopeVariable v |
+      this.isImplicitCapturedParameterPosition(v) and result = "captured " + v
+    )
+  }
 }
 
-/** An argument position represented by an integer. */
-class ArgumentPosition extends MkArgumentPosition {
-  private int i;
+/** An argument position. */
+class ArgumentPosition extends TArgumentPosition {
+  /** Gets the underlying integer position, if any. */
+  int getPosition() { this = TPositionalArgumentPosition(result) }
 
-  ArgumentPosition() { this = MkArgumentPosition(i) }
+  /** Holds if this position represents a qualifier. */
+  predicate isQualifier() { this = TQualifierArgumentPosition() }
 
-  /** Gets the underlying integer. */
-  int getPosition() { result = i }
+  /** Holds if this position is used to model flow through captured variables. */
+  predicate isImplicitCapturedArgumentPosition(LocalScopeVariable v) {
+    this = TImplicitCapturedArgumentPosition(v)
+  }
 
   /** Gets a textual representation of this position. */
-  string toString() { result = i.toString() }
+  string toString() {
+    result = "position " + this.getPosition()
+    or
+    this.isQualifier() and result = "qualifier"
+    or
+    exists(LocalScopeVariable v |
+      this.isImplicitCapturedArgumentPosition(v) and result = "captured " + v
+    )
+  }
 }
 
 /** Holds if arguments at position `apos` match parameters at position `ppos`. */
 predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
-  exists(int i |
-    ppos = MkParameterPosition(i) and
-    apos = MkArgumentPosition(i)
+  ppos.getPosition() = apos.getPosition()
+  or
+  ppos.isThisParameter() and apos.isQualifier()
+  or
+  exists(LocalScopeVariable v |
+    ppos.isImplicitCapturedParameterPosition(v) and
+    apos.isImplicitCapturedArgumentPosition(v)
   )
 }