[clang] [alpha.webkit.NoDeleteChecker] Better diagnostics (PR #182614)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 20 15:01:59 PST 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Ryosuke Niwa (rniwa)
<details>
<summary>Changes</summary>
This PR improves NoDeleteChecker's bug report so that it's more actionable. Namely, when a function fails "triviali function analysis", we report the first statement or parameter which failed the triviality check.
---
Full diff: https://github.com/llvm/llvm-project/pull/182614.diff
4 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp (+26-7)
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+19-8)
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h (+8-4)
- (modified) clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp (+73-23)
``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp
index abbdc2967e859..2a9d216823392 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp
@@ -12,6 +12,7 @@
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DynamicRecursiveASTVisitor.h"
+#include "clang/AST/QualTypeNames.h"
#include "clang/Analysis/DomainSpecific/CocoaConventions.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
@@ -85,7 +86,7 @@ class NoDeleteChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
}
void visitFunctionDecl(const FunctionDecl *FD) const {
- if (!FD->doesThisDeclarationHaveABody())
+ if (!FD->doesThisDeclarationHaveABody() || FD->isDependentContext())
return;
if (!hasNoDeleteAnnotation(FD))
@@ -95,8 +96,15 @@ class NoDeleteChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
if (!Body)
return;
- auto hasTrivialDtor = [&](VarDecl *D) { return TFA.hasTrivialDtor(D); };
- if (llvm::all_of(FD->parameters(), hasTrivialDtor) && TFA.isTrivial(Body))
+ NamedDecl* ParamDecl = nullptr;
+ for (auto* D : FD->parameters()) {
+ if (!TFA.hasTrivialDtor(D)) {
+ ParamDecl = D;
+ break;
+ }
+ }
+ const Stmt *OffendingStmt = nullptr;
+ if (!ParamDecl && TFA.isTrivial(Body, &OffendingStmt))
return;
SmallString<100> Buf;
@@ -104,13 +112,24 @@ class NoDeleteChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
Os << "A function ";
printQuotedName(Os, FD);
- Os << " has [[clang::annotate_type(\"webkit.nodelete\")]] but it contains "
- "code that could destruct an object";
+ Os << " has [[clang::annotate_type(\"webkit.nodelete\")]] but it contains ";
+ SourceLocation SrcLocToReport;
+ SourceRange Range;
+ if (ParamDecl) {
+ Os << "a parameter ";
+ printQuotedName(Os, ParamDecl);
+ Os << " which could destruct an object.";
+ SrcLocToReport = FD->getBeginLoc();
+ Range = ParamDecl->getSourceRange();
+ } else {
+ Os << "code that could destruct an object.";
+ SrcLocToReport = OffendingStmt->getBeginLoc();
+ Range = OffendingStmt->getSourceRange();
+ }
- const SourceLocation SrcLocToReport = FD->getBeginLoc();
PathDiagnosticLocation BSLoc(SrcLocToReport, BR->getSourceManager());
auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
- Report->addRange(FD->getSourceRange());
+ Report->addRange(Range);
Report->setDeclWithIssue(FD);
BR->emitReport(std::move(Report));
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 8cd64c12b7a73..463050d4d3b36 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -483,8 +483,11 @@ class TrivialFunctionAnalysisVisitor
// Returns false if at least one child is non-trivial.
bool VisitChildren(const Stmt *S) {
for (const Stmt *Child : S->children()) {
- if (Child && !Visit(Child))
+ if (Child && !Visit(Child)) {
+ if (OffendingStmt && !*OffendingStmt)
+ *OffendingStmt = Child;
return false;
+ }
}
return true;
@@ -493,7 +496,7 @@ class TrivialFunctionAnalysisVisitor
template <typename StmtOrDecl, typename CheckFunction>
bool WithCachedResult(const StmtOrDecl *S, CheckFunction Function) {
auto CacheIt = Cache.find(S);
- if (CacheIt != Cache.end())
+ if (CacheIt != Cache.end() && !OffendingStmt)
return CacheIt->second;
// Treat a recursive statement to be trivial until proven otherwise.
@@ -553,10 +556,13 @@ class TrivialFunctionAnalysisVisitor
public:
using CacheTy = TrivialFunctionAnalysis::CacheTy;
- TrivialFunctionAnalysisVisitor(CacheTy &Cache) : Cache(Cache) {}
+ TrivialFunctionAnalysisVisitor(CacheTy &Cache,
+ const Stmt **OffendingStmt = nullptr)
+ : Cache(Cache), OffendingStmt(OffendingStmt) {}
bool IsFunctionTrivial(const Decl *D) {
- return WithCachedResult(D, [&]() {
+ const Stmt** SavedOffendingStmt = std::exchange(OffendingStmt, nullptr);
+ auto Result = WithCachedResult(D, [&]() {
if (auto *FnDecl = dyn_cast<FunctionDecl>(D)) {
if (isNoDeleteFunction(FnDecl))
return true;
@@ -578,6 +584,8 @@ class TrivialFunctionAnalysisVisitor
return false;
return Visit(Body);
});
+ OffendingStmt = SavedOffendingStmt;
+ return Result;
}
bool HasTrivialDestructor(const VarDecl *VD) {
@@ -903,17 +911,20 @@ class TrivialFunctionAnalysisVisitor
private:
CacheTy &Cache;
CacheTy RecursiveFn;
+ const Stmt** OffendingStmt;
};
bool TrivialFunctionAnalysis::isTrivialImpl(
- const Decl *D, TrivialFunctionAnalysis::CacheTy &Cache) {
- TrivialFunctionAnalysisVisitor V(Cache);
+ const Decl *D, TrivialFunctionAnalysis::CacheTy &Cache,
+ const Stmt** OffendingStmt) {
+ TrivialFunctionAnalysisVisitor V(Cache, OffendingStmt);
return V.IsFunctionTrivial(D);
}
bool TrivialFunctionAnalysis::isTrivialImpl(
- const Stmt *S, TrivialFunctionAnalysis::CacheTy &Cache) {
- TrivialFunctionAnalysisVisitor V(Cache);
+ const Stmt *S, TrivialFunctionAnalysis::CacheTy &Cache,
+ const Stmt** OffendingStmt) {
+ TrivialFunctionAnalysisVisitor V(Cache, OffendingStmt);
return V.IsStatementTrivial(S);
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index 8a696a789c65b..844dd787e1123 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -168,8 +168,12 @@ bool isSingleton(const NamedDecl *F);
class TrivialFunctionAnalysis {
public:
/// \returns true if \p D is a "trivial" function.
- bool isTrivial(const Decl *D) const { return isTrivialImpl(D, TheCache); }
- bool isTrivial(const Stmt *S) const { return isTrivialImpl(S, TheCache); }
+ bool isTrivial(const Decl *D, const Stmt **OffendingStmt = nullptr) const {
+ return isTrivialImpl(D, TheCache, OffendingStmt);
+ }
+ bool isTrivial(const Stmt *S, const Stmt **OffendingStmt = nullptr) const {
+ return isTrivialImpl(S, TheCache, OffendingStmt);
+ }
bool hasTrivialDtor(const VarDecl *VD) const {
return hasTrivialDtorImpl(VD, TheCache);
}
@@ -181,8 +185,8 @@ class TrivialFunctionAnalysis {
llvm::DenseMap<llvm::PointerUnion<const Decl *, const Stmt *>, bool>;
mutable CacheTy TheCache{};
- static bool isTrivialImpl(const Decl *D, CacheTy &Cache);
- static bool isTrivialImpl(const Stmt *S, CacheTy &Cache);
+ static bool isTrivialImpl(const Decl *D, CacheTy &Cache, const Stmt**);
+ static bool isTrivialImpl(const Stmt *S, CacheTy &Cache, const Stmt**);
static bool hasTrivialDtorImpl(const VarDecl *VD, CacheTy &Cache);
};
diff --git a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
index 98f4017e5e3fd..16597ff011e55 100644
--- a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
@@ -3,15 +3,14 @@
#include "mock-types.h"
void someFunction();
-void [[clang::annotate_type("webkit.nodelete")]] safeFunction();
+RefCountable* [[clang::annotate_type("webkit.nodelete")]] safeFunction();
void functionWithoutNoDeleteAnnotation() {
someFunction();
}
void [[clang::annotate_type("webkit.nodelete")]] callsUnsafe() {
- // expected-warning at -1{{A function 'callsUnsafe' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
- someFunction();
+ someFunction(); // expected-warning{{A function 'callsUnsafe' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
}
void [[clang::annotate_type("webkit.nodelete")]] callsSafe() {
@@ -20,14 +19,72 @@ void [[clang::annotate_type("webkit.nodelete")]] callsSafe() {
void [[clang::annotate_type("webkit.nodelete")]] declWithNoDelete();
void declWithNoDelete() {
- // expected-warning at -1{{A function 'declWithNoDelete' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
- someFunction();
+ someFunction(); // expected-warning{{A function 'declWithNoDelete' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
}
-
void defWithNoDelete();
void [[clang::annotate_type("webkit.nodelete")]] defWithNoDelete() {
-// expected-warning at -1{{A function 'defWithNoDelete' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
- someFunction();
+ someFunction(); // expected-warning{{A function 'defWithNoDelete' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+}
+
+void [[clang::annotate_type("webkit.nodelete")]] funncWithUnsafeParam(Ref<RefCountable> t) {
+ // expected-warning at -1{{A function 'funncWithUnsafeParam' has [[clang::annotate_type("webkit.nodelete")]] but it contains a parameter 't' which could destruct an object}}
+}
+
+void [[clang::annotate_type("webkit.nodelete")]] funncWithUnsafeParam(unsigned safe, Ref<RefCountable> unsafe) {
+ // expected-warning at -1{{A function 'funncWithUnsafeParam' has [[clang::annotate_type("webkit.nodelete")]] but it contains a parameter 'unsafe' which could destruct an object}}
+}
+
+void [[clang::annotate_type("webkit.nodelete")]] funncWithUnsafeParam(Ref<RefCountable> unsafe, unsigned safe) {
+ // expected-warning at -1{{A function 'funncWithUnsafeParam' has [[clang::annotate_type("webkit.nodelete")]] but it contains a parameter 'unsafe' which could destruct an object}}
+}
+
+void [[clang::annotate_type("webkit.nodelete")]] funncWithSafeParam(Ref<RefCountable>& safe1, Ref<RefCountable>* safe2) {
+}
+
+void [[clang::annotate_type("webkit.nodelete")]] callsUnsafeInDoWhile() {
+ do {
+ someFunction(); // expected-warning{{A function 'callsUnsafeInDoWhile' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+ } while(0);
+}
+
+void [[clang::annotate_type("webkit.nodelete")]] callsUnsafeInIf(bool safe) {
+ if (safe)
+ safeFunction();
+ else
+ someFunction(); // expected-warning{{A function 'callsUnsafeInIf' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+}
+
+void [[clang::annotate_type("webkit.nodelete")]] declaresUnsafeVar(bool safe) {
+ if (safe) {
+ auto* t = safeFunction();
+ } else {
+ RefPtr<RefCountable> t = safeFunction();
+ // expected-warning at -1{{A function 'declaresUnsafeVar' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+ }
+}
+
+void [[clang::annotate_type("webkit.nodelete")]] declaresVarInIf(bool safe) {
+ if (RefPtr<RefCountable> t = safeFunction()) {
+ // expected-warning at -1{{A function 'declaresVarInIf' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+ t->method();
+ }
+}
+
+template <typename T>
+struct TemplatedClass {
+ void [[clang::annotate_type("webkit.nodelete")]] methodCallsUnsafe(T* t) {
+ t->method(); // expected-warning{{A function 'methodCallsUnsafe' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+ }
+ void [[clang::annotate_type("webkit.nodelete")]] methodCallsSafe(T* t) {
+ t->trivial();
+ }
+};
+
+using TemplatedToRefCountable = TemplatedClass<RefCountable>;
+void useTemplatedToRefCountable() {
+ TemplatedToRefCountable c;
+ c.methodCallsUnsafe(nullptr);
+ c.methodCallsSafe(nullptr);
}
class WeakRefCountable : public CanMakeWeakPtr<WeakRefCountable> {
@@ -54,8 +111,7 @@ class SomeClass {
void [[clang::annotate_type("webkit.nodelete")]] someMethod();
void [[clang::annotate_type("webkit.nodelete")]] unsafeMethod() {
- // expected-warning at -1{{A function 'unsafeMethod' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
- someFunction();
+ someFunction(); // expected-warning{{A function 'unsafeMethod' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
}
void [[clang::annotate_type("webkit.nodelete")]] safeMethod() {
safeFunction();
@@ -63,8 +119,7 @@ class SomeClass {
virtual void [[clang::annotate_type("webkit.nodelete")]] someVirtualMethod();
virtual void [[clang::annotate_type("webkit.nodelete")]] unsafeVirtualMethod() {
- // expected-warning at -1{{A function 'unsafeVirtualMethod' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
- someFunction();
+ someFunction(); // expected-warning{{A function 'unsafeVirtualMethod' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
}
virtual void [[clang::annotate_type("webkit.nodelete")]] safeVirtualMethod() {
safeFunction();
@@ -72,8 +127,7 @@ class SomeClass {
static void [[clang::annotate_type("webkit.nodelete")]] someStaticMethod();
static void [[clang::annotate_type("webkit.nodelete")]] unsafeStaticMethod() {
- // expected-warning at -1{{A function 'unsafeStaticMethod' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
- someFunction();
+ someFunction(); // expected-warning{{A function 'unsafeStaticMethod' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
}
static void [[clang::annotate_type("webkit.nodelete")]] safeStaticMethod() {
safeFunction();
@@ -82,8 +136,7 @@ class SomeClass {
virtual void [[clang::annotate_type("webkit.nodelete")]] anotherVirtualMethod();
void [[clang::annotate_type("webkit.nodelete")]] setObj(RefCountable* obj) {
- // expected-warning at -1{{A function 'setObj' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
- m_obj = obj;
+ m_obj = obj; // expected-warning{{A function 'setObj' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
}
void [[clang::annotate_type("webkit.nodelete")]] swapObj(RefPtr<RefCountable>&& obj) {
@@ -91,8 +144,7 @@ class SomeClass {
}
void [[clang::annotate_type("webkit.nodelete")]] clearObj(RefCountable* obj) {
- // expected-warning at -1{{A function 'clearObj' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
- m_obj = nullptr;
+ m_obj = nullptr; // expected-warning{{A function 'clearObj' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
}
void [[clang::annotate_type("webkit.nodelete")]] deposeArg(WeakRefCountable&& unused) {
@@ -108,8 +160,7 @@ class SomeClass {
}
void [[clang::annotate_type("webkit.nodelete")]] deposeLocal() {
- // expected-warning at -1{{A function 'deposeLocal' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
- RefPtr<RefCountable> obj = std::move(m_obj);
+ RefPtr<RefCountable> obj = std::move(m_obj); // expected-warning{{A function 'deposeLocal' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
}
RefPtr<RefCountable> [[clang::annotate_type("webkit.nodelete")]] copyRefPtr() {
@@ -141,8 +192,7 @@ class IntermediateClass : public SomeClass {
class DerivedClass : public IntermediateClass {
void anotherVirtualMethod() override {
- // expected-warning at -1{{A function 'anotherVirtualMethod' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
- someFunction();
+ someFunction(); // expected-warning{{A function 'anotherVirtualMethod' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
}
};
@@ -201,6 +251,6 @@ void [[clang::annotate_type("webkit.nodelete")]] makeData() {
}
void [[clang::annotate_type("webkit.nodelete")]] makeSubData() {
- // expected-warning at -1{{A function 'makeSubData' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
SubData::create()->doSomething();
+ // expected-warning at -1{{A function 'makeSubData' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/182614
More information about the cfe-commits
mailing list