Python: Added recursion guard

to ensure that the call graph seen by type tracking
does not include summary calls resolved by type tracking.

(I tried inserting a similar test into the Ruby codebase,
 and it still compiled)

To get this to compile, I had to move the resolution of summary calls
out of the data flow nodes and into the `viableCallable` predicate.
This means that we now have a potential summary call for each
cfg call node. (I tried using the base class, `DataFlowCall`, for this
but calls to `map` got identified as class calls and would no longer
be associated with a summary.)

It is possible that the "NonLIbrary"-layers the were inserted into the
hierarchy can be removed again.

Author: yoff

python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatchPointsTo.qll

@@ -636,16 +636,22 @@ class SpecialCall extends DataFlowSourceCall, TSpecialCall {
   }
 }
 
-/** A call to a summarized callable. */
+/**
+ * A call to a summarized callable, a `LibraryCallable`.
+ *
+ * This is a potential call, really. It has an empty charpred, so any `CallNode` is allowed.
+ * It is here to stake out territory, as otherwise a call to `map` would be considered a `ClassCall`
+ * and not be available for a summary.
+ */
 class LibraryCall extends NormalCall {
-  LibraryCallable callable;
-
-  LibraryCall() { call = callable.getACall() }
-
   // TODO: Implement Python calling convention?
   override Node getArg(int n) { result = TCfgNode(call.getArg(n)) }
 
-  override DataFlowCallable getCallable() { result.asLibraryCallable() = callable }
+  // We cannot refer to a `LibraryCallable` here,
+  // as that could in turn refer to type tracking.
+  // This call will be tied to a `LibraryCallable` via
+  // `getViableCallabe` when the global data flow is assembled.
+  override DataFlowCallable getCallable() { none() }
 }
 
 /**
@@ -747,7 +753,18 @@ private class SummaryPostUpdateNode extends SummaryNode, PostUpdateNode {
 }
 
 /** Gets a viable run-time target for the call `call`. */
-DataFlowCallable viableCallable(DataFlowSourceCall call) { result = call.getCallable() }
+DataFlowCallable viableCallable(DataFlowSourceCall call) {
+  result = call.getCallable()
+  or
+  // A call to a library callable with a flow summary
+  // In this situation we can not resolve the callable from the call,
+  // as that would make data flow depend on type tracking.
+  // Instead we reolve the call from the summary.
+  exists(LibraryCallable callable |
+    result = TLibraryCallable(callable) and
+    call.getNode() = callable.getACall()
+  )
+}
 
 private newtype TReturnKind = TNormalReturnKind()
 

python/ql/lib/semmle/python/dataflow/new/internal/TypeTrackerSpecific.qll

@@ -43,7 +43,13 @@ private DataFlowPrivate::DataFlowCallable getCallableForArgument(
   )
 }
 
-/** Holds if `nodeFrom` steps to `nodeTo` by being passed as a parameter in a call. */
+/**
+ * Holds if `nodeFrom` steps to `nodeTo` by being passed as a parameter in a call.
+ *
+ * Flow into summarized library methods is not included, as that will lead to negative
+ * recursion (or, at best, terrible performance), since identifying calls to library
+ * methods is done using API graphs (which uses type tracking).
+ */
 predicate callStep(DataFlowPublic::ArgumentNode nodeFrom, DataFlowPrivate::ParameterNodeImpl nodeTo) {
   // TODO: Support special methods?
   exists(DataFlowPrivate::DataFlowCallable callable, int i |

python/ql/test/experimental/dataflow/summaries/TestSummaries.qll

@@ -2,6 +2,25 @@ private import python
 private import semmle.python.dataflow.new.FlowSummary
 private import semmle.python.ApiGraphs
 
+/** This module ensures that the `callStep` predicate in
+ * our type tracker implelemtation does not refer to the
+ * `getACall` predicate on `SummarizedCallable`.
+ */
+module RecursionGuard {
+  private import semmle.python.dataflow.new.internal.TypeTrackerSpecific as TT
+
+  private class RecursionGuard extends SummarizedCallable {
+    RecursionGuard() { this = "RecursionGuard" }
+
+    override CallNode getACall() {
+      result.getFunction().(NameNode).getId() = this and
+      (TT::callStep(_, _) implies any())
+    }
+
+    override DataFlow::ArgumentNode getACallback() { result.asExpr().(Name).getId() = this }
+  }
+}
+
 private class SummarizedCallableIdentity extends SummarizedCallable {
   SummarizedCallableIdentity() { this = "identity" }