[clang] 2158b83 - Add nodelete annotation for WebKit checkers with a new checker for validation (#177839)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 27 09:18:50 PST 2026
Author: Ryosuke Niwa
Date: 2026-01-27T09:18:45-08:00
New Revision: 2158b83f61a788a4027f5f22d24f86eac43f42a5
URL: https://github.com/llvm/llvm-project/commit/2158b83f61a788a4027f5f22d24f86eac43f42a5
DIFF: https://github.com/llvm/llvm-project/commit/2158b83f61a788a4027f5f22d24f86eac43f42a5.diff
LOG: Add nodelete annotation for WebKit checkers with a new checker for validation (#177839)
This PR adds the support for specifying
`[[clang::annotate_type("webkit.nodelete")]]` on a function return type,
in which case, the whole function is considered "trivial" or more
precisely that it does not trigger any destruction of an object.
This PR also introduces alpha.webkit.NoDeleteChecker which validates
soundness of the annotation by examining the function body. The checker
will warn if `[[clang::annotate_type("webkit.nodelete")]]` is specified
on a function with a body which does not pass the triviality test.
---------
Co-authored-by: Balazs Benics <benicsbalazs at gmail.com>
Added:
clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp
clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
Modified:
clang/docs/analyzer/checkers.rst
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
Removed:
################################################################################
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index ecddda1f0737f..f6c37656c9fe2 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -3722,6 +3722,23 @@ This applies to:
For types like this, instead of using built in casts, the programmer will use helper functions that internally perform the appropriate type check and disable static analysis.
+alpha.webkit.NoDeleteChecker
+"""""""""""""""""""""""""""""""
+Check that ``[[clang::annotate_type("webkit.nodelete")]]`` annotation does not appear on a function which could delete an object.
+
+.. code-block:: cpp
+
+ void [[clang::annotate_type("webkit.nodelete")]] someFunction(RefCountable* obj) { // warn
+ delete obj;
+ };
+
+ Foo [[clang::annotate_type("webkit.nodelete")]] trivialFunction(RefCountable* obj) {
+ return obj->anotherTrivialFunction();
+ };
+
+``[[clang::annotate_type("webkit.nodelete")]]`` annotation makes the function ignored for the purpose of other WebKit smart pointer checkers.
+For example, ``alpha.webkit.UncountedCallArgsChecker`` will ignore a function call with this annotation.
+
alpha.webkit.NoUncheckedPtrMemberChecker
""""""""""""""""""""""""""""""""""""""""
Raw pointers and references to an object which supports CheckedPtr or CheckedRef can't be used as class members. Only CheckedPtr, CheckedRef, RefPtr, or Ref are allowed.
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 91738e6a29664..6a409944849e6 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1710,6 +1710,10 @@ def MemoryUnsafeCastChecker : Checker<"MemoryUnsafeCastChecker">,
HelpText<"Check for memory unsafe casts from base type to derived type.">,
Documentation<HasDocumentation>;
+def NoDeleteChecker : Checker<"NoDeleteChecker">,
+ HelpText<"Check for [[clang::annotate_type(\"webkit.nodelete\")]] annotation">,
+ Documentation<HasDocumentation>;
+
def NoUncheckedPtrMemberChecker : Checker<"NoUncheckedPtrMemberChecker">,
HelpText<"Check for no unchecked member variables.">,
Documentation<HasDocumentation>;
diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 29d2c4512d470..2df36d8e672ae 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -132,6 +132,7 @@ add_clang_library(clangStaticAnalyzerCheckers
WebKit/ASTUtils.cpp
WebKit/ForwardDeclChecker.cpp
WebKit/MemoryUnsafeCastChecker.cpp
+ WebKit/NoDeleteChecker.cpp
WebKit/PtrTypesSemantics.cpp
WebKit/RefCntblBaseVirtualDtorChecker.cpp
WebKit/RetainPtrCtorAdoptChecker.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp
new file mode 100644
index 0000000000000..397fda8cd67ed
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp
@@ -0,0 +1,128 @@
+//=======- NoDeleteChecker.cpp -----------------------------------*- C++ -*-==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "DiagOutputUtils.h"
+#include "PtrTypesSemantics.h"
+#include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
+#include "clang/Analysis/DomainSpecific/CocoaConventions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class NoDeleteChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
+ BugType Bug;
+ mutable BugReporter *BR = nullptr;
+ mutable TrivialFunctionAnalysis TFA;
+
+public:
+ NoDeleteChecker()
+ : Bug(this,
+ "Incorrect [[clang::annotate_type(\"webkit.nodelete\")]] "
+ "annotation",
+ "WebKit coding guidelines") {}
+
+ void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
+ BugReporter &BRArg) const {
+ BR = &BRArg;
+
+ // The calls to checkAST* from AnalysisConsumer don't
+ // visit template instantiations or lambda classes. We
+ // want to visit those, so we make our own visitor.
+ struct LocalVisitor final : public ConstDynamicRecursiveASTVisitor {
+ const NoDeleteChecker *Checker;
+ Decl *DeclWithIssue{nullptr};
+
+ explicit LocalVisitor(const NoDeleteChecker *Checker) : Checker(Checker) {
+ assert(Checker);
+ ShouldVisitTemplateInstantiations = true;
+ ShouldWalkTypesOfTypeLocs = true;
+ ShouldVisitImplicitCode = false;
+ ShouldVisitLambdaBody = true;
+ }
+
+ bool VisitFunctionDecl(const FunctionDecl *FD) override {
+ Checker->visitFunctionDecl(FD);
+ return true;
+ }
+ };
+
+ LocalVisitor visitor(this);
+ visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
+ }
+
+ void visitFunctionDecl(const FunctionDecl *FD) const {
+ if (!FD->doesThisDeclarationHaveABody())
+ return;
+
+ bool HasNoDeleteAnnotation = isNoDeleteFunction(FD);
+ if (auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
+ if (auto *Cls = MD->getParent(); Cls && MD->isVirtual()) {
+ CXXBasePaths Paths;
+ Paths.setOrigin(Cls);
+
+ Cls->lookupInBases(
+ [&](const CXXBaseSpecifier *Base, CXXBasePath &) {
+ const Type *T = Base->getType().getTypePtrOrNull();
+ if (!T)
+ return false;
+
+ const CXXRecordDecl *R = T->getAsCXXRecordDecl();
+ for (const CXXMethodDecl *BaseMD : R->methods()) {
+ if (BaseMD->getCorrespondingMethodInClass(Cls) == MD) {
+ if (isNoDeleteFunction(FD)) {
+ HasNoDeleteAnnotation = true;
+ return false;
+ }
+ }
+ }
+ return true;
+ },
+ Paths, /*LookupInDependent =*/true);
+ }
+ }
+
+ auto Body = FD->getBody();
+ if (!Body || TFA.isTrivial(Body))
+ return;
+
+ SmallString<100> Buf;
+ llvm::raw_svector_ostream Os(Buf);
+
+ Os << "A function ";
+ printQuotedName(Os, FD);
+ Os << " has [[clang::annotate_type(\"webkit.nodelete\")]] but it contains "
+ "code that could destruct an object";
+
+ 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->setDeclWithIssue(FD);
+ BR->emitReport(std::move(Report));
+ }
+};
+
+} // namespace
+
+void ento::registerNoDeleteChecker(CheckerManager &Mgr) {
+ Mgr.registerChecker<NoDeleteChecker>();
+}
+
+bool ento::shouldRegisterNoDeleteChecker(const CheckerManager &) {
+ return true;
+}
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index e056c35d6cf45..c47dabf2ec5b0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -406,6 +406,28 @@ bool isSmartPtr(const CXXRecordDecl *R) {
return false;
}
+enum class WebKitAnnotation : uint8_t {
+ None,
+ PointerConversion,
+ NoDelete,
+};
+
+static WebKitAnnotation typeAnnotationForReturnType(const FunctionDecl *FD) {
+ auto RetType = FD->getReturnType();
+ auto *Attr = dyn_cast_or_null<AttributedType>(RetType.getTypePtrOrNull());
+ if (!Attr)
+ return WebKitAnnotation::None;
+ auto *AnnotateType = dyn_cast_or_null<AnnotateTypeAttr>(Attr->getAttr());
+ if (!AnnotateType)
+ return WebKitAnnotation::None;
+ auto Annotation = AnnotateType->getAnnotation();
+ if (Annotation == "webkit.pointerconversion")
+ return WebKitAnnotation::PointerConversion;
+ if (Annotation == "webkit.nodelete")
+ return WebKitAnnotation::NoDelete;
+ return WebKitAnnotation::None;
+}
+
bool isPtrConversion(const FunctionDecl *F) {
assert(F);
if (isCtorOfRefCounted(F))
@@ -423,21 +445,16 @@ bool isPtrConversion(const FunctionDecl *F) {
FunctionName == "checked_objc_cast")
return true;
- auto ReturnType = F->getReturnType();
- if (auto *Type = ReturnType.getTypePtrOrNull()) {
- if (auto *AttrType = dyn_cast<AttributedType>(Type)) {
- if (auto *Attr = AttrType->getAttr()) {
- if (auto *AnnotateType = dyn_cast<AnnotateTypeAttr>(Attr)) {
- if (AnnotateType->getAnnotation() == "webkit.pointerconversion")
- return true;
- }
- }
- }
- }
+ if (typeAnnotationForReturnType(F) == WebKitAnnotation::PointerConversion)
+ return true;
return false;
}
+bool isNoDeleteFunction(const FunctionDecl *F) {
+ return typeAnnotationForReturnType(F) == WebKitAnnotation::NoDelete;
+}
+
bool isTrivialBuiltinFunction(const FunctionDecl *F) {
if (!F || !F->getDeclName().isIdentifier())
return false;
@@ -506,6 +523,8 @@ class TrivialFunctionAnalysisVisitor
bool IsFunctionTrivial(const Decl *D) {
if (auto *FnDecl = dyn_cast<FunctionDecl>(D)) {
+ if (isNoDeleteFunction(FnDecl))
+ return true;
if (FnDecl->isVirtualAsWritten())
return false;
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index d5ced4d857241..431357a2150be 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -152,6 +152,10 @@ std::optional<bool> isGetterOfSafePtr(const clang::CXXMethodDecl *Method);
/// pointer types.
bool isPtrConversion(const FunctionDecl *F);
+/// \returns true if \p F's return type is annotated with
+/// [[clang::annotate_type("webkit.nodelete")]].
+bool isNoDeleteFunction(const FunctionDecl *F);
+
/// \returns true if \p F is a builtin function which is considered trivial.
bool isTrivialBuiltinFunction(const FunctionDecl *F);
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
index dcc14a0aecdf7..b2d87bc267700 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
@@ -241,7 +241,7 @@ class RawPtrRefCallArgsChecker
if (BR->getSourceManager().isInSystemHeader(CE->getExprLoc()))
return true;
- if (Callee && TFA.isTrivial(Callee) && !Callee->isVirtualAsWritten())
+ if (Callee && TFA.isTrivial(Callee))
return true;
if (isTrivialBuiltinFunction(Callee))
diff --git a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
new file mode 100644
index 0000000000000..b23fd007ff789
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
@@ -0,0 +1,67 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.NoDeleteChecker -verify %s
+
+void someFunction();
+void [[clang::annotate_type("webkit.nodelete")]] safeFunction();
+
+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();
+}
+
+void [[clang::annotate_type("webkit.nodelete")]] callsSafe() {
+ safeFunction();
+}
+
+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();
+}
+
+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();
+}
+
+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();
+ }
+ void [[clang::annotate_type("webkit.nodelete")]] safeMethod() {
+ safeFunction();
+ }
+
+ 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();
+ }
+ virtual void [[clang::annotate_type("webkit.nodelete")]] safeVirtualMethod() {
+ safeFunction();
+ }
+
+ 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();
+ }
+ static void [[clang::annotate_type("webkit.nodelete")]] safeStaticMethod() {
+ safeFunction();
+ }
+
+ virtual void [[clang::annotate_type("webkit.nodelete")]] anotherVirtualMethod();
+};
+
+class IntermediateClass : public SomeClass {
+ void anotherVirtualMethod() override;
+};
+
+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();
+ }
+};
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
index b83eaedf264e4..c60126947217d 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
@@ -389,6 +389,10 @@ class RefCounted {
unsigned trivial71() { return std::bit_cast<unsigned>(nullptr); }
unsigned trivial72() { Number n { 5 }; return WTF::move(n).value(); }
+ unsigned [[clang::annotate_type("webkit.nodelete")]] nodelete1();
+ void [[clang::annotate_type("webkit.nodelete")]] nodelete2();
+ virtual void [[clang::annotate_type("webkit.nodelete")]] nodelete3();
+
static RefCounted& singleton() {
static RefCounted s_RefCounted;
s_RefCounted.ref();
@@ -581,6 +585,10 @@ class UnrelatedClass {
getFieldTrivial().trivial70(); // no-warning
getFieldTrivial().trivial71(); // no-warning
+ getFieldTrivial().nodelete1(); // no-warning
+ getFieldTrivial().nodelete2(); // no-warning
+ getFieldTrivial().nodelete3(); // no-warning
+
RefCounted::singleton().trivial18(); // no-warning
RefCounted::singleton().someFunction(); // no-warning
RefCounted::otherSingleton().trivial18(); // no-warning
More information about the cfe-commits
mailing list