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 <rn...@webkit.org> Date: Thu, 19 Sep 2024 23:41:10 -0700 Subject: [PATCH 1/5] 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 <rn...@webkit.org> Date: Fri, 20 Sep 2024 01:44:16 -0700 Subject: [PATCH 2/5] 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); } >From e7aa34bbf882abf4b4816d3190806089d943ad48 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <rn...@webkit.org> Date: Fri, 20 Sep 2024 01:45:20 -0700 Subject: [PATCH 3/5] Fix more formatting. --- .../StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp index 410e78c5418ee3..c63464b23aa712 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp @@ -49,7 +49,7 @@ class UncountedCallArgsChecker using Base = RecursiveASTVisitor<LocalVisitor>; const UncountedCallArgsChecker *Checker; - Decl *DeclWithIssue { nullptr }; + Decl *DeclWithIssue{nullptr}; explicit LocalVisitor(const UncountedCallArgsChecker *Checker) : Checker(Checker) { >From b4fe97e0677dae2f97a3b14fc6206adaa7fe6963 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <rn...@webkit.org> Date: Thu, 26 Sep 2024 22:43:56 -0700 Subject: [PATCH 4/5] Also handle ObjCMethodDecl --- .../Checkers/WebKit/UncountedCallArgsChecker.cpp | 4 +++- .../Checkers/WebKit/UncountedLocalVarsChecker.cpp | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp index c63464b23aa712..00a9acd9694478 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp @@ -66,7 +66,9 @@ class UncountedCallArgsChecker } bool TraverseDecl(Decl *D) { - llvm::SaveAndRestore SavedDecl(DeclWithIssue, D); + llvm::SaveAndRestore SavedDecl(DeclWithIssue); + if (D && (isa<FunctionDecl>(D) || isa<ObjCMethodDecl>(D))) + DeclWithIssue = D; return Base::TraverseDecl(D); } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp index fac1a3454ec880..9d0a3bb5da7325 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp @@ -137,7 +137,7 @@ class UncountedLocalVarsChecker bool TraverseDecl(Decl *D) { llvm::SaveAndRestore SavedDecl(DeclWithIssue); - if (D && isa<FunctionDecl>(D)) + if (D && (isa<FunctionDecl>(D) || isa<ObjCMethodDecl>(D))) DeclWithIssue = D; return Base::TraverseDecl(D); } >From 603036579805b0b0184695ea101a4153ae64dc27 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <rn...@webkit.org> Date: Thu, 26 Sep 2024 23:26:34 -0700 Subject: [PATCH 5/5] Fix formatting. --- .../StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp index 00a9acd9694478..0a2e48969d666e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp @@ -68,7 +68,7 @@ class UncountedCallArgsChecker bool TraverseDecl(Decl *D) { llvm::SaveAndRestore SavedDecl(DeclWithIssue); if (D && (isa<FunctionDecl>(D) || isa<ObjCMethodDecl>(D))) - DeclWithIssue = D; + DeclWithIssue = D; return Base::TraverseDecl(D); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits