[clang] [alpha.webkit.UncountedCallArgsChecker] Fix a false negative when a call argument is a local variable. (PR #129974)

[Ryosuke Niwa via cfe-commits]

https://github.com/rniwa created https://github.com/llvm/llvm-project/pull/129974

isASafeCallArg erroneously returns true when a call argument is a local variable regardless of its type. This is incorrect. We should only allow any local variable of a safe pointer type.

Fix the bug by moving the logic to check for a function parameter and local variable from isASafeCallArg to a lambda in isPtrOriginSafe, and check that the local variable is a safe pointer type.

Also fix a bug in isPtrOfType that it was not recognizing DeducedTemplateSpecializationType.

>From 1f0d65a410f88adb5e9737d5c78bef1cd4e3b0df Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Wed, 5 Mar 2025 18:06:52 -0800
Subject: [PATCH] [alpha.webkit.UncountedCallArgsChecker] Fix a false negative
 when a call argument is a local variable.

isASafeCallArg erroneously returns true when a call argument is a local variable regardless of its type.
This is incorrect. We should only allow any local variable of a safe pointer type.

Fix the bug by moving the logic to check for a function parameter and local variable from isASafeCallArg
to a lambda in isPtrOriginSafe, and check that the local variable is a safe pointer type.

Also fix a bug in isPtrOfType that it was not recognizing DeducedTemplateSpecializationType.
---
 .../StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp |  2 +-
 .../Checkers/WebKit/PtrTypesSemantics.cpp       | 17 ++++++++++-------
 .../WebKit/RawPtrRefCallArgsChecker.cpp         |  8 ++++++++
 .../test/Analysis/Checkers/WebKit/call-args.cpp |  9 ++++++++-
 4 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index dc86c4fcc64b1..73b6afd3642c7 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -141,7 +141,7 @@ bool isASafeCallArg(const Expr *E) {
   assert(E);
   if (auto *Ref = dyn_cast<DeclRefExpr>(E)) {
     if (auto *D = dyn_cast_or_null<VarDecl>(Ref->getFoundDecl())) {
-      if (isa<ParmVarDecl>(D) || D->isLocalVarDecl())
+      if (isa<ParmVarDecl>(D))
         return true;
     }
   }
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 7899b19854806..cbb207bcb9df0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -162,13 +162,16 @@ static bool isPtrOfType(const clang::QualType T, Predicate Pred) {
       type = elaboratedT->desugar();
       continue;
     }
-    auto *SpecialT = type->getAs<TemplateSpecializationType>();
-    if (!SpecialT)
-      return false;
-    auto *Decl = SpecialT->getTemplateName().getAsTemplateDecl();
-    if (!Decl)
-      return false;
-    return Pred(Decl->getNameAsString());
+    if (auto *SpecialT = type->getAs<TemplateSpecializationType>()) {
+      auto *Decl = SpecialT->getTemplateName().getAsTemplateDecl();
+      if (!Decl)
+        return false;
+      return Pred(Decl->getNameAsString());
+    } else if (auto *DTS = type->getAs<DeducedTemplateSpecializationType>()) {
+      auto *Decl = DTS->getTemplateName().getAsTemplateDecl();
+      return Pred(Decl->getNameAsString());
+    } else
+      break;
   }
   return false;
 }
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
index d633fbcbd798b..fbb6a06afbdac 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
@@ -158,6 +158,14 @@ class RawPtrRefCallArgsChecker
             // foo(NULL)
             return true;
           }
+          if (auto *Ref = dyn_cast<DeclRefExpr>(ArgOrigin)) {
+            if (auto *D = dyn_cast_or_null<VarDecl>(Ref->getFoundDecl())) {
+              if (isa<ParmVarDecl>(D))
+                return true; // Parameters are transitively safe.
+              if (D->isLocalVarDecl() && isSafePtrType(D->getType()))
+                return true;
+            }
+          }
           if (isASafeCallArg(ArgOrigin))
             return true;
           if (EFA.isACallToEnsureFn(ArgOrigin))
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args.cpp b/clang/test/Analysis/Checkers/WebKit/call-args.cpp
index b4613d5090f29..4fc5a574f9e69 100644
--- a/clang/test/Analysis/Checkers/WebKit/call-args.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/call-args.cpp
@@ -374,7 +374,14 @@ namespace call_with_explicit_temporary_obj {
   }
 }
 
-namespace call_with_explicit_construct {
+namespace call_via_local_var {
+  RefCountable* provide();
+  void bar(RefCountable*);
+  void foo() {
+    auto* obj = provide();
+    bar(obj);
+    // expected-warning at -1{{Call argument is uncounted and unsafe}}
+  }
 }
 
 namespace call_with_adopt_ref {