Merge pull request #14295 from hvitved/csharp/lambda-type-flow

C#: Improve lambda dispatch using type flow

Author: hvitved

csharp/ql/lib/ext/System.Collections.Generic.model.yml

@@ -47,6 +47,7 @@ extensions:
       - ["System.Collections.Generic", "List<>", False, "FindAll", "(System.Predicate<T>)", "", "Argument[this].Element", "ReturnValue", "value", "manual"]
       - ["System.Collections.Generic", "List<>", False, "FindLast", "(System.Predicate<T>)", "", "Argument[this].Element", "Argument[0].Parameter[0]", "value", "manual"]
       - ["System.Collections.Generic", "List<>", False, "FindLast", "(System.Predicate<T>)", "", "Argument[this].Element", "ReturnValue", "value", "manual"]
+      - ["System.Collections.Generic", "List<>", False, "ForEach", "(System.Action<T>)", "", "Argument[this].Element", "Argument[0].Parameter[0]", "value", "manual"]
       - ["System.Collections.Generic", "List<>", False, "GetEnumerator", "()", "", "Argument[this].Element", "ReturnValue.Property[System.Collections.Generic.List<>+Enumerator.Current]", "value", "manual"]
       - ["System.Collections.Generic", "List<>", False, "GetRange", "(System.Int32,System.Int32)", "", "Argument[this].Element", "ReturnValue.Element", "value", "manual"]
       - ["System.Collections.Generic", "List<>", False, "InsertRange", "(System.Int32,System.Collections.Generic.IEnumerable<T>)", "", "Argument[1].Element", "Argument[this].Element", "value", "manual"]

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

@@ -4,6 +4,7 @@ import csharp
 private import dotnet
 private import internal.FlowSummaryImpl as Impl
 private import internal.DataFlowDispatch as DataFlowDispatch
+private import Impl::Public::SummaryComponent as SummaryComponentInternal
 
 class ParameterPosition = DataFlowDispatch::ParameterPosition;
 
@@ -18,8 +19,6 @@ class SummaryComponent = Impl::Public::SummaryComponent;
 
 /** Provides predicates for constructing summary components. */
 module SummaryComponent {
-  private import Impl::Public::SummaryComponent as SummaryComponentInternal
-
   predicate content = SummaryComponentInternal::content/1;
 
   /** Gets a summary component for parameter `i`. */
@@ -155,3 +154,45 @@ private class RecordConstructorFlowRequiredSummaryComponentStack extends Require
     )
   }
 }
+
+class Provenance = Impl::Public::Provenance;
+
+private import semmle.code.csharp.frameworks.system.linq.Expressions
+
+private SummaryComponent delegateSelf() {
+  exists(ArgumentPosition pos |
+    result = SummaryComponentInternal::parameter(pos) and
+    pos.isDelegateSelf()
+  )
+}
+
+private predicate mayInvokeCallback(Callable c, int n) {
+  c.getParameter(n).getType() instanceof SystemLinqExpressions::DelegateExtType and
+  not c.fromSource()
+}
+
+private class SummarizedCallableWithCallback extends SummarizedCallable {
+  private int pos;
+
+  SummarizedCallableWithCallback() { mayInvokeCallback(this, pos) }
+
+  override predicate propagatesFlow(
+    SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
+  ) {
+    input = SummaryComponentStack::argument(pos) and
+    output = SummaryComponentStack::push(delegateSelf(), input) and
+    preservesValue = true
+  }
+
+  override predicate hasProvenance(Provenance provenance) { provenance = "hq-generated" }
+}
+
+private class RequiredComponentStackForCallback extends RequiredSummaryComponentStack {
+  override predicate required(SummaryComponent head, SummaryComponentStack tail) {
+    exists(int pos |
+      mayInvokeCallback(_, pos) and
+      head = delegateSelf() and
+      tail = SummaryComponentStack::argument(pos)
+    )
+  }
+}

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

@@ -136,13 +136,15 @@ private module Cached {
   newtype TParameterPosition =
     TPositionalParameterPosition(int i) { i = any(Parameter p).getPosition() } or
     TThisParameterPosition() or
-    TImplicitCapturedParameterPosition(LocalScopeVariable v) { capturedWithFlowIn(v) }
+    TImplicitCapturedParameterPosition(LocalScopeVariable v) { capturedWithFlowIn(v) } or
+    TDelegateSelfParameterPosition()
 
   cached
   newtype TArgumentPosition =
     TPositionalArgumentPosition(int i) { i = any(Parameter p).getPosition() } or
     TQualifierArgumentPosition() or
-    TImplicitCapturedArgumentPosition(LocalScopeVariable v) { capturedWithFlowIn(v) }
+    TImplicitCapturedArgumentPosition(LocalScopeVariable v) { capturedWithFlowIn(v) } or
+    TDelegateSelfArgumentPosition()
 }
 
 import Cached
@@ -480,6 +482,14 @@ class ParameterPosition extends TParameterPosition {
     this = TImplicitCapturedParameterPosition(v)
   }
 
+  /**
+   * Holds if this position represents a reference to a delegate itself.
+   *
+   * Used for tracking flow through captured variables and for improving
+   * delegate dispatch.
+   */
+  predicate isDelegateSelf() { this = TDelegateSelfParameterPosition() }
+
   /** Gets a textual representation of this position. */
   string toString() {
     result = "position " + this.getPosition()
@@ -489,6 +499,9 @@ class ParameterPosition extends TParameterPosition {
     exists(LocalScopeVariable v |
       this.isImplicitCapturedParameterPosition(v) and result = "captured " + v
     )
+    or
+    this.isDelegateSelf() and
+    result = "delegate self"
   }
 }
 
@@ -505,6 +518,14 @@ class ArgumentPosition extends TArgumentPosition {
     this = TImplicitCapturedArgumentPosition(v)
   }
 
+  /**
+   * Holds if this position represents a reference to a delegate itself.
+   *
+   * Used for tracking flow through captured variables and for improving
+   * delegate dispatch.
+   */
+  predicate isDelegateSelf() { this = TDelegateSelfArgumentPosition() }
+
   /** Gets a textual representation of this position. */
   string toString() {
     result = "position " + this.getPosition()
@@ -514,6 +535,9 @@ class ArgumentPosition extends TArgumentPosition {
     exists(LocalScopeVariable v |
       this.isImplicitCapturedArgumentPosition(v) and result = "captured " + v
     )
+    or
+    this.isDelegateSelf() and
+    result = "delegate self"
   }
 }
 
@@ -527,4 +551,6 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
     ppos.isImplicitCapturedParameterPosition(v) and
     apos.isImplicitCapturedArgumentPosition(v)
   )
+  or
+  ppos.isDelegateSelf() and apos.isDelegateSelf()
 }