Merge pull request #9210 from michaelnebel/dataflow/summarizedcallablerefactor

DataFlow - SummarizedCallable refactor

Author: michaelnebel

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

@@ -515,11 +515,7 @@ Element interpretElement(
 /**
  * Holds if `c` has a `generated` summary.
  */
-predicate hasSummary(Callable c, boolean generated) {
-  exists(DataFlowCallable dc |
-    dc.asSummarizedCallable() = c and summaryElement(dc, _, _, _, generated)
-  )
-}
+predicate hasSummary(Callable c, boolean generated) { summaryElement(c, _, _, _, generated) }
 
 cached
 private module Cached {

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

@@ -114,69 +114,7 @@ module SummaryComponentStack {
   SummaryComponentStack jump(Callable c) { result = singleton(SummaryComponent::jump(c)) }
 }
 
-/**
- * A class for synthesized callables given by a summary.
- */
-abstract class SummarizedCallable extends DotNet::Callable {
-  SummarizedCallable() { this.isUnboundDeclaration() }
-
-  /**
-   * Holds if data may flow from `input` to `output` through this callable.
-   *
-   * `preservesValue` indicates whether this is a value-preserving step
-   * or a taint-step.
-   *
-   * Input specifications are restricted to stacks that end with
-   * `SummaryComponent::argument(_)`, preceded by zero or more
-   * `SummaryComponent::return(_)` or `SummaryComponent::content(_)` components.
-   *
-   * Output specifications are restricted to stacks that end with
-   * `SummaryComponent::return(_)` or `SummaryComponent::argument(_)`.
-   *
-   * Output stacks ending with `SummaryComponent::return(_)` can be preceded by zero
-   * or more `SummaryComponent::content(_)` components.
-   *
-   * Output stacks ending with `SummaryComponent::argument(_)` can be preceded by an
-   * optional `SummaryComponent::parameter(_)` component, which in turn can be preceded
-   * by zero or more `SummaryComponent::content(_)` components.
-   */
-  pragma[nomagic]
-  predicate propagatesFlow(
-    SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
-  ) {
-    none()
-  }
-
-  /**
-   * Holds if values stored inside `content` are cleared on objects passed as
-   * arguments at position `pos` to this callable.
-   */
-  pragma[nomagic]
-  predicate clearsContent(ParameterPosition pos, DataFlow::ContentSet content) { none() }
-
-  /**
-   * Holds if the summary is auto generated.
-   */
-  predicate isAutoGenerated() { none() }
-}
-
-private class SummarizedCallableAdapter extends Impl::Public::SummarizedCallable {
-  private SummarizedCallable sc;
-
-  SummarizedCallableAdapter() { this = DataFlowDispatch::TSummarizedCallable(sc) }
-
-  final override predicate propagatesFlow(
-    SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
-  ) {
-    sc.propagatesFlow(input, output, preservesValue)
-  }
-
-  final override predicate clearsContent(ParameterPosition pos, DataFlow::ContentSet content) {
-    sc.clearsContent(pos, content)
-  }
-
-  final override predicate isAutoGenerated() { sc.isAutoGenerated() }
-}
+class SummarizedCallable = Impl::Public::SummarizedCallable;
 
 private predicate recordConstructorFlow(Constructor c, int i, Property p) {
   c = any(RecordType r).getAMember() and

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

@@ -457,7 +457,7 @@ class SummaryCall extends DelegateDataFlowCall, TSummaryCall {
 
   override DataFlow::Node getNode() { none() }
 
-  override DataFlowCallable getEnclosingCallable() { result = c }
+  override DataFlowCallable getEnclosingCallable() { result.asSummarizedCallable() = c }
 
   override string toString() { result = "[summary] call to " + receiver + " in " + c }
 

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

@@ -976,17 +976,15 @@ private module ParameterNodes {
     SummaryParameterNode() { this = TSummaryParameterNode(sc, pos_) }
 
     override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
-      sc = c and pos = pos_
+      sc = c.asSummarizedCallable() and pos = pos_
     }
 
-    override DataFlowCallable getEnclosingCallableImpl() { result = sc }
+    override DataFlowCallable getEnclosingCallableImpl() { result.asSummarizedCallable() = sc }
 
     override Type getTypeImpl() {
-      exists(int i |
-        pos_.getPosition() = i and result = sc.asSummarizedCallable().getParameter(i).getType()
-      )
+      exists(int i | pos_.getPosition() = i and result = sc.getParameter(i).getType())
       or
-      pos_.isThisParameter() and result = sc.asSummarizedCallable().getDeclaringType()
+      pos_.isThisParameter() and result = sc.getDeclaringType()
     }
 
     override ControlFlow::Node getControlFlowNodeImpl() { none() }
@@ -1464,7 +1462,7 @@ class SummaryNode extends NodeImpl, TSummaryNode {
 
   SummaryNode() { this = TSummaryNode(c, state) }
 
-  override DataFlowCallable getEnclosingCallableImpl() { result = c }
+  override DataFlowCallable getEnclosingCallableImpl() { result.asSummarizedCallable() = c }
 
   override DataFlowType getDataFlowType() {
     result = FlowSummaryImpl::Private::summaryNodeType(this)

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

@@ -195,7 +195,10 @@ module Public {
   }
 
   /** A callable with a flow summary. */
-  abstract class SummarizedCallable extends DataFlowCallable {
+  abstract class SummarizedCallable extends SummarizedCallableBase {
+    bindingset[this]
+    SummarizedCallable() { any() }
+
     /**
      * Holds if data may flow from `input` to `output` through this callable.
      *
@@ -493,7 +496,7 @@ module Private {
       or
       exists(ParameterPosition pos |
         parameterReadState(c, state, pos) and
-        result.(ParamNode).isParameterOf(c, pos)
+        result.(ParamNode).isParameterOf(inject(c), pos)
       )
     )
   }
@@ -621,7 +624,7 @@ module Private {
   predicate summaryPostUpdateNode(Node post, Node pre) {
     exists(SummarizedCallable c, ParameterPosition pos |
       isParameterPostUpdate(post, c, pos) and
-      pre.(ParamNode).isParameterOf(c, pos)
+      pre.(ParamNode).isParameterOf(inject(c), pos)
     )
     or
     exists(SummarizedCallable callable, SummaryComponentStack s |
@@ -644,7 +647,7 @@ module Private {
    * node, and back out to `p`.
    */
   predicate summaryAllowParameterReturnInSelf(ParamNode p) {
-    exists(SummarizedCallable c, ParameterPosition ppos | p.isParameterOf(c, ppos) |
+    exists(SummarizedCallable c, ParameterPosition ppos | p.isParameterOf(inject(c), ppos) |
       exists(SummaryComponentStack inputContents, SummaryComponentStack outputContents |
         summary(c, inputContents, outputContents, _) and
         inputContents.bottom() = pragma[only_bind_into](TArgumentSummaryComponent(ppos)) and
@@ -748,8 +751,11 @@ module Private {
     private predicate viableParam(
       DataFlowCall call, SummarizedCallable sc, ParameterPosition ppos, ParamNode p
     ) {
-      p.isParameterOf(sc, ppos) and
-      sc = viableCallable(call)
+      exists(DataFlowCallable c |
+        c = inject(sc) and
+        p.isParameterOf(c, ppos) and
+        c = viableCallable(call)
+      )
     }
 
     pragma[nomagic]
@@ -1067,16 +1073,18 @@ module Private {
   /** Provides a query predicate for outputting a set of relevant flow summaries. */
   module TestOutput {
     /** A flow summary to include in the `summary/3` query predicate. */
-    abstract class RelevantSummarizedCallable extends SummarizedCallable {
+    abstract class RelevantSummarizedCallable instanceof SummarizedCallable {
       /** Gets the string representation of this callable used by `summary/1`. */
       abstract string getCallableCsv();
 
       /** Holds if flow is propagated between `input` and `output`. */
       predicate relevantSummary(
         SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
       ) {
-        this.propagatesFlow(input, output, preservesValue)
+        super.propagatesFlow(input, output, preservesValue)
       }
+
+      string toString() { result = super.toString() }
     }
 
     /** Render the kind in the format used in flow summaries. */
@@ -1087,7 +1095,7 @@ module Private {
     }
 
     private string renderGenerated(RelevantSummarizedCallable c) {
-      if c.isAutoGenerated() then result = "generated:" else result = ""
+      if c.(SummarizedCallable).isAutoGenerated() then result = "generated:" else result = ""
     }
 
     /**
@@ -1117,19 +1125,21 @@ module Private {
    */
   module RenderSummarizedCallable {
     /** A summarized callable to include in the graph. */
-    abstract class RelevantSummarizedCallable extends SummarizedCallable { }
+    abstract class RelevantSummarizedCallable instanceof SummarizedCallable {
+      string toString() { result = super.toString() }
+    }
 
     private newtype TNodeOrCall =
       MkNode(Node n) {
         exists(RelevantSummarizedCallable c |
           n = summaryNode(c, _)
           or
-          n.(ParamNode).isParameterOf(c, _)
+          n.(ParamNode).isParameterOf(inject(c), _)
         )
       } or
       MkCall(DataFlowCall call) {
         call = summaryDataFlowCall(_) and
-        call.getEnclosingCallable() instanceof RelevantSummarizedCallable
+        call.getEnclosingCallable() = inject(any(RelevantSummarizedCallable c))
       }
 
     private class NodeOrCall extends TNodeOrCall {

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

@@ -15,6 +15,12 @@ private import semmle.code.csharp.Unification
 private import semmle.code.csharp.dataflow.ExternalFlow
 private import semmle.code.csharp.dataflow.FlowSummary as FlowSummary
 
+class SummarizedCallableBase extends Callable {
+  SummarizedCallableBase() { this.isUnboundDeclaration() }
+}
+
+DataFlowCallable inject(SummarizedCallable c) { result.asSummarizedCallable() = c }
+
 /** Gets the parameter position of the instance parameter. */
 ArgumentPosition instanceParameterPosition() { none() } // disables implicit summary flow to `this` for callbacks
 
@@ -55,7 +61,7 @@ private DataFlowType getReturnTypeBase(DotNet::Callable c, ReturnKind rk) {
 /** Gets the return type of kind `rk` for callable `c`. */
 bindingset[c]
 DataFlowType getReturnType(SummarizedCallable c, ReturnKind rk) {
-  result = getReturnTypeBase(c.asSummarizedCallable(), rk)
+  result = getReturnTypeBase(c, rk)
   or
   rk =
     any(JumpReturnKind jrk | result = getReturnTypeBase(jrk.getTarget(), jrk.getTargetReturnKind()))
@@ -85,9 +91,12 @@ DataFlowType getCallbackReturnType(DataFlowType t, ReturnKind rk) {
   )
 }
 
-private predicate summaryElement0(
-  DotNet::Callable c, string input, string output, string kind, boolean generated
-) {
+/**
+ * Holds if an external flow summary exists for `c` with input specification
+ * `input`, output specification `output`, kind `kind`, and a flag `generated`
+ * stating whether the summary is autogenerated.
+ */
+predicate summaryElement(Callable c, string input, string output, string kind, boolean generated) {
   exists(
     string namespace, string type, boolean subtypes, string name, string signature, string ext
   |
@@ -96,21 +105,6 @@ private predicate summaryElement0(
   )
 }
 
-private class SummarizedCallableExternal extends FlowSummary::SummarizedCallable {
-  SummarizedCallableExternal() { summaryElement0(this, _, _, _, _) }
-}
-
-/**
- * Holds if an external flow summary exists for `c` with input specification
- * `input`, output specification `output`, kind `kind`, and a flag `generated`
- * stating whether the summary is autogenerated.
- */
-predicate summaryElement(
-  DataFlowCallable c, string input, string output, string kind, boolean generated
-) {
-  summaryElement0(c.asSummarizedCallable(), input, output, kind, generated)
-}
-
 /**
  * Holds if an external source specification exists for `e` with output specification
  * `output`, kind `kind`, and a flag `generated` stating whether the source specification is

csharp/ql/lib/semmle/code/csharp/frameworks/EntityFramework.qll

@@ -88,7 +88,10 @@ module EntityFramework {
   }
 
   /** A flow summary for EntityFramework. */
-  abstract class EFSummarizedCallable extends SummarizedCallable { }
+  abstract class EFSummarizedCallable extends SummarizedCallable {
+    bindingset[this]
+    EFSummarizedCallable() { any() }
+  }
 
   private class DbSetAddOrUpdateRequiredSummaryComponentStack extends RequiredSummaryComponentStack {
     override predicate required(SummaryComponent head, SummaryComponentStack tail) {

csharp/ql/test/library-tests/dataflow/external-models/steps.ql

@@ -42,9 +42,7 @@ query predicate summarySetterStep(DataFlow::Node arg, DataFlow::Node out, Conten
   FlowSummaryImpl::Private::Steps::summarySetterStep(arg, c, out)
 }
 
-query predicate clearsContent(
-  FlowSummaryImpl::Public::SummarizedCallable c, DataFlow::Content k, ParameterPosition pos
-) {
+query predicate clearsContent(SummarizedCallable c, DataFlow::Content k, ParameterPosition pos) {
   c.clearsContent(pos, k) and
-  c.asSummarizedCallable().fromSource()
+  c.fromSource()
 }

csharp/ql/test/library-tests/dataflow/library/FlowSummariesFiltered.ql

@@ -3,7 +3,7 @@ private import semmle.code.csharp.dataflow.internal.DataFlowPrivate::Csv
 private import semmle.code.csharp.dataflow.ExternalFlow
 
 class IncludeFilteredSummarizedCallable extends IncludeSummarizedCallable {
-  IncludeFilteredSummarizedCallable() { exists(this) }
+  IncludeFilteredSummarizedCallable() { this instanceof SummarizedCallable }
 
   /**
    * Holds if flow is propagated between `input` and `output` and
@@ -13,12 +13,11 @@ class IncludeFilteredSummarizedCallable extends IncludeSummarizedCallable {
   override predicate relevantSummary(
     SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
   ) {
-    this.propagatesFlow(input, output, preservesValue) and
-    not exists(IncludeSummarizedCallable rsc, SummarizedCallable sc |
-      sc = rsc.asSummarizedCallable() and
-      isBaseCallableOrPrototype(sc) and
-      rsc.propagatesFlow(input, output, preservesValue) and
-      this.asSummarizedCallable().(UnboundCallable).overridesOrImplementsUnbound(sc)
+    this.(SummarizedCallable).propagatesFlow(input, output, preservesValue) and
+    not exists(IncludeSummarizedCallable rsc |
+      isBaseCallableOrPrototype(rsc) and
+      rsc.(SummarizedCallable).propagatesFlow(input, output, preservesValue) and
+      this.(UnboundCallable).overridesOrImplementsUnbound(rsc)
     )
   }
 }

csharp/ql/test/library-tests/frameworks/EntityFramework/FlowSummaries.ql

@@ -3,7 +3,7 @@ import shared.FlowSummaries
 import semmle.code.csharp.dataflow.ExternalFlow as ExternalFlow
 
 private class IncludeEFSummarizedCallable extends IncludeSummarizedCallable {
-  IncludeEFSummarizedCallable() { this.asSummarizedCallable() instanceof EFSummarizedCallable }
+  IncludeEFSummarizedCallable() { this instanceof EFSummarizedCallable }
 }
 
 query predicate sourceNode(DataFlow::Node node, string kind) {