From dbfc256f40c9153d5897f771f8b614ff7dae79cb Mon Sep 17 00:00:00 2001
From: Anders Schack-Mulligen <aschackmull@github.com>
Date: Tue, 14 Mar 2023 11:17:01 +0100
Subject: [PATCH] Java: Remove low-confidence dispatch to known neutrals.

---
 java/ql/lib/change-notes/2023-03-14-neutral-dispatch.md       | 4 ++++
 java/ql/lib/semmle/code/java/dataflow/FlowSummary.qll         | 2 ++
 .../semmle/code/java/dataflow/internal/DataFlowDispatch.qll   | 2 ++
 java/ql/lib/semmle/code/java/dispatch/ObjFlow.qll             | 2 ++
 java/ql/lib/semmle/code/java/dispatch/VirtualDispatch.qll     | 3 ++-
 5 files changed, 12 insertions(+), 1 deletion(-)
 create mode 100644 java/ql/lib/change-notes/2023-03-14-neutral-dispatch.md

diff --git a/java/ql/lib/change-notes/2023-03-14-neutral-dispatch.md b/java/ql/lib/change-notes/2023-03-14-neutral-dispatch.md
new file mode 100644
index 00000000000..f01e8700757
--- /dev/null
+++ b/java/ql/lib/change-notes/2023-03-14-neutral-dispatch.md
@@ -0,0 +1,4 @@
+---
+category: majorAnalysis
+---
+* Removed low-confidence call edges to known neutral call targets from the call graph used in data flow analysis. This includes, for example, custom `List.contains` implementations when the best inferrable type at the call site is simply `List`.
diff --git a/java/ql/lib/semmle/code/java/dataflow/FlowSummary.qll b/java/ql/lib/semmle/code/java/dataflow/FlowSummary.qll
index 09a5f05914f..6d6af4f82b4 100644
--- a/java/ql/lib/semmle/code/java/dataflow/FlowSummary.qll
+++ b/java/ql/lib/semmle/code/java/dataflow/FlowSummary.qll
@@ -171,6 +171,8 @@ class SummarizedCallableBase extends TSummarizedCallableBase {
 
 class SummarizedCallable = Impl::Public::SummarizedCallable;
 
+class NeutralCallable = Impl::Public::NeutralCallable;
+
 /**
  * An adapter class to add the flow summaries specified on `SyntheticCallable`
  * to `SummarizedCallable`.
diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowDispatch.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowDispatch.qll
index 22e79a2240d..875fbd43921 100644
--- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowDispatch.qll
+++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowDispatch.qll
@@ -11,6 +11,8 @@ private module DispatchImpl {
   private predicate hasHighConfidenceTarget(Call c) {
     exists(SummarizedCallable sc | sc.getACall() = c and not sc.isAutoGenerated())
     or
+    exists(NeutralCallable nc | nc.getACall() = c and nc.isManual())
+    or
     exists(Callable srcTgt |
       srcTgt = VirtualDispatch::viableCallable(c) and
       not VirtualDispatch::lowConfidenceDispatchTarget(c, srcTgt)
diff --git a/java/ql/lib/semmle/code/java/dispatch/ObjFlow.qll b/java/ql/lib/semmle/code/java/dispatch/ObjFlow.qll
index 81a303256d0..a85a2dd118e 100644
--- a/java/ql/lib/semmle/code/java/dispatch/ObjFlow.qll
+++ b/java/ql/lib/semmle/code/java/dispatch/ObjFlow.qll
@@ -236,6 +236,8 @@ private VirtualMethodAccess objectToString(ObjNode n) {
   result.getQualifier() = n.asExpr() and sink(n)
 }
 
+predicate objectToStringCall(VirtualMethodAccess ma) { ma = objectToString(_) }
+
 /**
  * Holds if the qualifier of the `Object.toString()` call `ma` might have type `t`.
  */
diff --git a/java/ql/lib/semmle/code/java/dispatch/VirtualDispatch.qll b/java/ql/lib/semmle/code/java/dispatch/VirtualDispatch.qll
index eb1878bf7e4..4b880542229 100644
--- a/java/ql/lib/semmle/code/java/dispatch/VirtualDispatch.qll
+++ b/java/ql/lib/semmle/code/java/dispatch/VirtualDispatch.qll
@@ -93,7 +93,8 @@ private module Dispatch {
       exists(RefType t | qualUnionType(ma, t, false) |
         lowConfidenceDispatchType(t.getSourceDeclaration())
       )
-    )
+    ) and
+    not ObjFlow::objectToStringCall(ma)
   }
 
   private predicate lowConfidenceDispatchType(SrcRefType t) {