Merge pull request #19268 from hvitved/rust/summarized-callable-base

Rust: Make `SummarizedCallable` extend `Function` instead of `string`

Author: hvitved

rust/ql/lib/codeql/rust/dataflow/FlowSummary.qll

@@ -2,42 +2,17 @@
 
 private import rust
 private import internal.FlowSummaryImpl as Impl
-private import codeql.rust.elements.internal.CallExprBaseImpl::Impl as CallExprBaseImpl
 
 // import all instances below
 private module Summaries {
   private import codeql.rust.Frameworks
   private import codeql.rust.dataflow.internal.ModelsAsData
 }
 
-/** Provides the `Range` class used to define the extent of `LibraryCallable`. */
-module LibraryCallable {
-  /** A callable defined in library code, identified by a unique string. */
-  abstract class Range extends string {
-    bindingset[this]
-    Range() { any() }
-
-    /** Gets a call to this library callable. */
-    CallExprBase getACall() {
-      exists(Resolvable r, string crate |
-        r = CallExprBaseImpl::getCallResolvable(result) and
-        this = crate + r.getResolvedPath()
-      |
-        crate = r.getResolvedCrateOrigin() + "::_::"
-        or
-        not r.hasResolvedCrateOrigin() and
-        crate = ""
-      )
-    }
-  }
-}
-
-final class LibraryCallable = LibraryCallable::Range;
-
 /** Provides the `Range` class used to define the extent of `SummarizedCallable`. */
 module SummarizedCallable {
   /** A callable with a flow summary, identified by a unique string. */
-  abstract class Range extends LibraryCallable::Range, Impl::Public::SummarizedCallable {
+  abstract class Range extends Impl::Public::SummarizedCallable {
     bindingset[this]
     Range() { any() }
 

rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

@@ -45,10 +45,12 @@ final class DataFlowCallable extends TDataFlowCallable {
   /**
    * Gets the underlying library callable, if any.
    */
-  LibraryCallable asLibraryCallable() { this = TLibraryCallable(result) }
+  SummarizedCallable asSummarizedCallable() { this = TSummarizedCallable(result) }
 
   /** Gets a textual representation of this callable. */
-  string toString() { result = [this.asCfgScope().toString(), this.asLibraryCallable().toString()] }
+  string toString() {
+    result = [this.asCfgScope().toString(), this.asSummarizedCallable().toString()]
+  }
 
   /** Gets the location of this callable. */
   Location getLocation() { result = this.asCfgScope().getLocation() }
@@ -65,12 +67,9 @@ final class DataFlowCall extends TDataFlowCall {
   }
 
   DataFlowCallable getEnclosingCallable() {
-    result = TCfgScope(this.asCallCfgNode().getExpr().getEnclosingCfgScope())
+    result.asCfgScope() = this.asCallCfgNode().getExpr().getEnclosingCfgScope()
     or
-    exists(FlowSummaryImpl::Public::SummarizedCallable c |
-      this.isSummaryCall(c, _) and
-      result = TLibraryCallable(c)
-    )
+    this.isSummaryCall(result.asSummarizedCallable(), _)
   }
 
   string toString() {
@@ -401,9 +400,11 @@ module RustDataFlow implements InputSig<Location> {
 
   /** Gets a viable implementation of the target of the given `Call`. */
   DataFlowCallable viableCallable(DataFlowCall call) {
-    result.asCfgScope() = call.asCallCfgNode().getCall().getStaticTarget()
-    or
-    result.asLibraryCallable().getACall() = call.asCallCfgNode().getCall()
+    exists(Callable target | target = call.asCallCfgNode().getCall().getStaticTarget() |
+      target = result.asCfgScope()
+      or
+      target = result.asSummarizedCallable()
+    )
   }
 
   /**
@@ -757,7 +758,7 @@ module RustDataFlow implements InputSig<Location> {
   predicate allowParameterReturnInSelf(ParameterNode p) {
     exists(DataFlowCallable c, ParameterPosition pos |
       p.isParameterOf(c, pos) and
-      FlowSummaryImpl::Private::summaryAllowParameterReturnInSelf(c.asLibraryCallable(), pos)
+      FlowSummaryImpl::Private::summaryAllowParameterReturnInSelf(c.asSummarizedCallable(), pos)
     )
     or
     VariableCapture::Flow::heuristicAllowInstanceParameterReturnInSelf(p.(ClosureParameterNode)
@@ -968,7 +969,7 @@ private module Cached {
   cached
   newtype TDataFlowCallable =
     TCfgScope(CfgScope scope) or
-    TLibraryCallable(LibraryCallable c)
+    TSummarizedCallable(SummarizedCallable c)
 
   /** This is the local flow predicate that is exposed. */
   cached

rust/ql/lib/codeql/rust/dataflow/internal/FlowSummaryImpl.qll

@@ -13,7 +13,7 @@ module Input implements InputSig<Location, RustDataFlow> {
   private import codeql.rust.elements.internal.CallExprBaseImpl::Impl as CallExprBaseImpl
   private import codeql.rust.frameworks.stdlib.Stdlib
 
-  class SummarizedCallableBase = string;
+  class SummarizedCallableBase = Function;
 
   abstract private class SourceSinkBase extends AstNode {
     /** Gets the associated call. */
@@ -138,7 +138,7 @@ private import Make<Location, RustDataFlow, Input> as Impl
 
 private module StepsInput implements Impl::Private::StepsInputSig {
   DataFlowCall getACall(Public::SummarizedCallable sc) {
-    result.asCallCfgNode().getCall() = sc.(LibraryCallable).getACall()
+    result.asCallCfgNode().getCall().getStaticTarget() = sc
   }
 
   RustDataFlow::Node getSourceNode(Input::SourceBase source, Impl::Private::SummaryComponent sc) {

rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll

@@ -47,6 +47,7 @@ private import rust
 private import codeql.rust.dataflow.FlowSummary
 private import codeql.rust.dataflow.FlowSource
 private import codeql.rust.dataflow.FlowSink
+private import codeql.rust.elements.internal.CallExprBaseImpl::Impl as CallExprBaseImpl
 
 /**
  * Holds if in a call to the function with canonical path `path`, defined in the
@@ -120,7 +121,12 @@ private class SummarizedCallableFromModel extends SummarizedCallable::Range {
 
   SummarizedCallableFromModel() {
     summaryModel(crate, path, _, _, _, _, _) and
-    this = crate + "::_::" + path
+    exists(CallExprBase call, Resolvable r |
+      call.getStaticTarget() = this and
+      r = CallExprBaseImpl::getCallResolvable(call) and
+      r.getResolvedPath() = path and
+      r.getResolvedCrateOrigin() = crate
+    )
   }
 
   override predicate propagatesFlow(

rust/ql/lib/codeql/rust/dataflow/internal/Node.qll

@@ -44,7 +44,7 @@ abstract class NodePublic extends TNode {
 
 abstract class Node extends NodePublic {
   /** Gets the enclosing callable. */
-  DataFlowCallable getEnclosingCallable() { result = TCfgScope(this.getCfgScope()) }
+  DataFlowCallable getEnclosingCallable() { result.asCfgScope() = this.getCfgScope() }
 
   /** Do not call: use `getEnclosingCallable()` instead. */
   abstract CfgScope getCfgScope();
@@ -102,9 +102,9 @@ class FlowSummaryNode extends Node, TFlowSummaryNode {
   }
 
   override DataFlowCallable getEnclosingCallable() {
-    result.asLibraryCallable() = this.getSummarizedCallable()
-    or
     result.asCfgScope() = this.getCfgScope()
+    or
+    result.asSummarizedCallable() = this.getSummarizedCallable()
   }
 
   override Location getLocation() {
@@ -195,7 +195,7 @@ final class SummaryParameterNode extends ParameterNode, FlowSummaryNode {
   }
 
   override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
-    this.getSummarizedCallable() = c.asLibraryCallable() and pos = pos_
+    this.getSummarizedCallable() = c.asSummarizedCallable() and pos = pos_
   }
 }
 

rust/ql/lib/codeql/rust/frameworks/stdlib/Clone.qll

@@ -6,13 +6,9 @@ private import codeql.rust.dataflow.FlowSummary
 /** A `clone` method. */
 final class CloneCallable extends SummarizedCallable::Range {
   CloneCallable() {
-    // NOTE: The function target may not exist in the database, so we base this
-    // on method calls.
-    exists(MethodCallExpr c |
-      c.getIdentifier().getText() = "clone" and
-      c.getArgList().getNumberOfArgs() = 0 and
-      this = c.getResolvedCrateOrigin() + "::_::" + c.getResolvedPath()
-    )
+    this.getParamList().hasSelfParam() and
+    this.getParamList().getNumberOfParams() = 0 and
+    this.getName().getText() = "clone"
   }
 
   final override predicate propagatesFlow(

rust/ql/lib/codeql/rust/internal/PathResolution.qll

@@ -1522,8 +1522,8 @@ private module Debug {
   private Locatable getRelevantLocatable() {
     exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
       result.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
-      filepath.matches("%/test.rs") and
-      startline = 74
+      filepath.matches("%/main.rs") and
+      startline = 52
     )
   }
 

rust/ql/lib/codeql/rust/internal/TypeInference.qll

@@ -1170,6 +1170,7 @@ final class MethodCall extends Call {
  * Holds if a method for `type` with the name `name` and the arity `arity`
  * exists in `impl`.
  */
+pragma[nomagic]
 private predicate methodCandidate(Type type, string name, int arity, Impl impl) {
   type = impl.getSelfTy().(TypeMention).resolveType() and
   exists(Function f |
@@ -1579,8 +1580,8 @@ private module Debug {
   private Locatable getRelevantLocatable() {
     exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
       result.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
-      filepath.matches("%/main.rs") and
-      startline = 1718
+      filepath.matches("%/sqlx.rs") and
+      startline = [56 .. 60]
     )
   }
 

rust/ql/test/library-tests/dataflow/global/viableCallable.expected

@@ -75,10 +75,10 @@
 | main.rs:279:17:279:25 | source(...) | main.rs:1:1:3:1 | fn source |
 | main.rs:280:9:280:15 | sink(...) | main.rs:5:1:7:1 | fn sink |
 | main.rs:283:5:283:17 | sink(...) | main.rs:5:1:7:1 | fn sink |
-| main.rs:287:13:287:55 | ...::block_on(...) | file://:0:0:0:0 | repo:https://github.com/rust-lang/futures-rs:futures-executor::_::crate::local_pool::block_on |
+| main.rs:287:13:287:55 | ...::block_on(...) | file://:0:0:0:0 | fn block_on |
 | main.rs:287:41:287:54 | async_source(...) | main.rs:268:1:272:1 | fn async_source |
 | main.rs:288:5:288:11 | sink(...) | main.rs:5:1:7:1 | fn sink |
-| main.rs:290:5:290:62 | ...::block_on(...) | file://:0:0:0:0 | repo:https://github.com/rust-lang/futures-rs:futures-executor::_::crate::local_pool::block_on |
+| main.rs:290:5:290:62 | ...::block_on(...) | file://:0:0:0:0 | fn block_on |
 | main.rs:290:33:290:61 | test_async_await_async_part(...) | main.rs:274:1:284:1 | fn test_async_await_async_part |
 | main.rs:294:5:294:22 | data_out_of_call(...) | main.rs:16:1:19:1 | fn data_out_of_call |
 | main.rs:295:5:295:35 | data_out_of_call_side_effect1(...) | main.rs:35:1:40:1 | fn data_out_of_call_side_effect1 |