[clang] WebKit Checkers should set DeclWithIssue. (PR #109389)

Ryosuke Niwa via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 20 01:44:39 PDT 2024


https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/109389

>From b8f95b5b809cbeb8199de6cd24e18a605189f722 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Thu, 19 Sep 2024 23:41:10 -0700
Subject: [PATCH 1/2] WebKit Checkers should set DeclWithIssue.

Set DeclWithIssue in alpha.webkit.UncountedCallArgsChecker and alpha.webkit.UncountedLocalVarsChecker.
---
 .../WebKit/UncountedCallArgsChecker.cpp       | 29 ++++++++++++++-----
 .../WebKit/UncountedLocalVarsChecker.cpp      | 21 ++++++++++----
 2 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index 81c2434ce64775..410e78c5418ee3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -18,6 +18,8 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/Support/SaveAndRestore.h"
 #include <optional>
 
 using namespace clang;
@@ -44,7 +46,11 @@ class UncountedCallArgsChecker
     // visit template instantiations or lambda classes. We
     // want to visit those, so we make our own RecursiveASTVisitor.
     struct LocalVisitor : public RecursiveASTVisitor<LocalVisitor> {
+      using Base = RecursiveASTVisitor<LocalVisitor>;
+
       const UncountedCallArgsChecker *Checker;
+      Decl *DeclWithIssue { nullptr };
+
       explicit LocalVisitor(const UncountedCallArgsChecker *Checker)
           : Checker(Checker) {
         assert(Checker);
@@ -56,12 +62,16 @@ class UncountedCallArgsChecker
       bool TraverseClassTemplateDecl(ClassTemplateDecl *Decl) {
         if (isRefType(safeGetName(Decl)))
           return true;
-        return RecursiveASTVisitor<LocalVisitor>::TraverseClassTemplateDecl(
-            Decl);
+        return Base::TraverseClassTemplateDecl(Decl);
+      }
+
+      bool TraverseDecl(Decl *D) {
+        llvm::SaveAndRestore SavedDecl(DeclWithIssue, D);
+        return Base::TraverseDecl(D);
       }
 
       bool VisitCallExpr(const CallExpr *CE) {
-        Checker->visitCallExpr(CE);
+        Checker->visitCallExpr(CE, DeclWithIssue);
         return true;
       }
     };
@@ -70,7 +80,7 @@ class UncountedCallArgsChecker
     visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
   }
 
-  void visitCallExpr(const CallExpr *CE) const {
+  void visitCallExpr(const CallExpr *CE, const Decl *D) const {
     if (shouldSkipCall(CE))
       return;
 
@@ -89,7 +99,7 @@ class UncountedCallArgsChecker
         QualType ArgType = MemberCallExpr->getObjectType();
         std::optional<bool> IsUncounted = isUncounted(ArgType);
         if (IsUncounted && *IsUncounted && !isPtrOriginSafe(E))
-          reportBugOnThis(E);
+          reportBugOnThis(E, D);
       }
 
       for (auto P = F->param_begin();
@@ -119,7 +129,7 @@ class UncountedCallArgsChecker
         if (isPtrOriginSafe(Arg))
           continue;
 
-        reportBug(Arg, *P);
+        reportBug(Arg, *P, D);
       }
     }
   }
@@ -240,7 +250,8 @@ class UncountedCallArgsChecker
             ClsName.ends_with("String"));
   }
 
-  void reportBug(const Expr *CallArg, const ParmVarDecl *Param) const {
+  void reportBug(const Expr *CallArg, const ParmVarDecl *Param,
+                 const Decl *DeclWithIssue) const {
     assert(CallArg);
 
     SmallString<100> Buf;
@@ -261,10 +272,11 @@ class UncountedCallArgsChecker
     PathDiagnosticLocation BSLoc(SrcLocToReport, BR->getSourceManager());
     auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
     Report->addRange(CallArg->getSourceRange());
+    Report->setDeclWithIssue(DeclWithIssue);
     BR->emitReport(std::move(Report));
   }
 
-  void reportBugOnThis(const Expr *CallArg) const {
+  void reportBugOnThis(const Expr *CallArg, const Decl *DeclWithIssue) const {
     assert(CallArg);
 
     const SourceLocation SrcLocToReport = CallArg->getSourceRange().getBegin();
@@ -274,6 +286,7 @@ class UncountedCallArgsChecker
         Bug, "Call argument for 'this' parameter is uncounted and unsafe.",
         BSLoc);
     Report->addRange(CallArg->getSourceRange());
+    Report->setDeclWithIssue(DeclWithIssue);
     BR->emitReport(std::move(Report));
   }
 };
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
index 274da0baf2ce5c..30f10d7e9f91e7 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
@@ -121,6 +121,7 @@ class UncountedLocalVarsChecker
     // want to visit those, so we make our own RecursiveASTVisitor.
     struct LocalVisitor : public RecursiveASTVisitor<LocalVisitor> {
       const UncountedLocalVarsChecker *Checker;
+      Decl *DeclWithIssue { nullptr };
 
       TrivialFunctionAnalysis TFA;
 
@@ -134,10 +135,17 @@ class UncountedLocalVarsChecker
       bool shouldVisitTemplateInstantiations() const { return true; }
       bool shouldVisitImplicitCode() const { return false; }
 
+      bool TraverseDecl(Decl *D) {
+        llvm::SaveAndRestore SavedDecl(DeclWithIssue);
+        if (D && isa<FunctionDecl>(D))
+            DeclWithIssue = D;
+        return Base::TraverseDecl(D);
+      }
+
       bool VisitVarDecl(VarDecl *V) {
         auto *Init = V->getInit();
         if (Init && V->isLocalVarDecl())
-          Checker->visitVarDecl(V, Init);
+          Checker->visitVarDecl(V, Init, DeclWithIssue);
         return true;
       }
 
@@ -145,7 +153,7 @@ class UncountedLocalVarsChecker
         if (BO->isAssignmentOp()) {
           if (auto *VarRef = dyn_cast<DeclRefExpr>(BO->getLHS())) {
             if (auto *V = dyn_cast<VarDecl>(VarRef->getDecl()))
-              Checker->visitVarDecl(V, BO->getRHS());
+              Checker->visitVarDecl(V, BO->getRHS(), DeclWithIssue);
           }
         }
         return true;
@@ -186,7 +194,8 @@ class UncountedLocalVarsChecker
     visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
   }
 
-  void visitVarDecl(const VarDecl *V, const Expr *Value) const {
+  void visitVarDecl(const VarDecl *V, const Expr *Value,
+                    const Decl *DeclWithIssue) const {
     if (shouldSkipVarDecl(V))
       return;
 
@@ -240,7 +249,7 @@ class UncountedLocalVarsChecker
               }))
         return;
 
-      reportBug(V, Value);
+      reportBug(V, Value, DeclWithIssue);
     }
   }
 
@@ -249,7 +258,8 @@ class UncountedLocalVarsChecker
     return BR->getSourceManager().isInSystemHeader(V->getLocation());
   }
 
-  void reportBug(const VarDecl *V, const Expr *Value) const {
+  void reportBug(const VarDecl *V, const Expr *Value,
+                 const Decl *DeclWithIssue) const {
     assert(V);
     SmallString<100> Buf;
     llvm::raw_svector_ostream Os(Buf);
@@ -278,6 +288,7 @@ class UncountedLocalVarsChecker
       PathDiagnosticLocation BSLoc(V->getLocation(), BR->getSourceManager());
       auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
       Report->addRange(V->getSourceRange());
+      Report->setDeclWithIssue(DeclWithIssue);
       BR->emitReport(std::move(Report));
     }
   }

>From 840252746a4b4cdf28526c424a9d9e92f9d3673a Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 20 Sep 2024 01:44:16 -0700
Subject: [PATCH 2/2] Fix formatting.

---
 .../Checkers/WebKit/UncountedLocalVarsChecker.cpp             | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
index 30f10d7e9f91e7..fac1a3454ec880 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
@@ -121,7 +121,7 @@ class UncountedLocalVarsChecker
     // want to visit those, so we make our own RecursiveASTVisitor.
     struct LocalVisitor : public RecursiveASTVisitor<LocalVisitor> {
       const UncountedLocalVarsChecker *Checker;
-      Decl *DeclWithIssue { nullptr };
+      Decl *DeclWithIssue{nullptr};
 
       TrivialFunctionAnalysis TFA;
 
@@ -138,7 +138,7 @@ class UncountedLocalVarsChecker
       bool TraverseDecl(Decl *D) {
         llvm::SaveAndRestore SavedDecl(DeclWithIssue);
         if (D && isa<FunctionDecl>(D))
-            DeclWithIssue = D;
+          DeclWithIssue = D;
         return Base::TraverseDecl(D);
       }
 



More information about the cfe-commits mailing list