https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/177839
>From b156a8a9e683eda84a64658d0a0af24895613071 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <[email protected]> Date: Sat, 24 Jan 2026 22:28:12 -0800 Subject: [PATCH 1/2] Add nodelete annotation for WebKit checkers 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. --- .../Checkers/WebKit/PtrTypesSemantics.cpp | 11 +++++++++++ .../Analysis/Checkers/WebKit/uncounted-obj-arg.cpp | 6 ++++++ 2 files changed, 17 insertions(+) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index e056c35d6cf45..92fd612c11bc3 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -508,6 +508,17 @@ class TrivialFunctionAnalysisVisitor if (auto *FnDecl = dyn_cast<FunctionDecl>(D)) { if (FnDecl->isVirtualAsWritten()) return false; + auto ReturnType = FnDecl->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.nodelete") + return true; + } + } + } + } } return WithCachedResult(D, [&]() { if (auto *CtorDecl = dyn_cast<CXXConstructorDecl>(D)) { diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp index b83eaedf264e4..14a81aba1adc1 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp @@ -389,6 +389,9 @@ 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(); + static RefCounted& singleton() { static RefCounted s_RefCounted; s_RefCounted.ref(); @@ -581,6 +584,9 @@ class UnrelatedClass { getFieldTrivial().trivial70(); // no-warning getFieldTrivial().trivial71(); // no-warning + getFieldTrivial().nodelete1(); // no-warning + getFieldTrivial().nodelete2(); // no-warning + RefCounted::singleton().trivial18(); // no-warning RefCounted::singleton().someFunction(); // no-warning RefCounted::otherSingleton().trivial18(); // no-warning >From b19f97dc6d1a0f3ed9c4c5d059c91f121d20fca4 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <[email protected]> Date: Mon, 26 Jan 2026 15:53:15 -0800 Subject: [PATCH 2/2] Introduce alpha.webkit.NoDeleteChecker Add alpha.webkit.NoDeleteChecker which validates soundness of webkit.nodelete annotation by examining the function body. --- clang/docs/analyzer/checkers.rst | 14 ++ .../clang/StaticAnalyzer/Checkers/Checkers.td | 4 + .../StaticAnalyzer/Checkers/CMakeLists.txt | 1 + .../Checkers/WebKit/NoDeleteChecker.cpp | 127 ++++++++++++++++++ .../Checkers/WebKit/PtrTypesSemantics.cpp | 53 +++++--- .../Checkers/WebKit/PtrTypesSemantics.h | 4 + .../WebKit/RawPtrRefCallArgsChecker.cpp | 2 +- .../Checkers/WebKit/nodelete-annotation.cpp | 67 +++++++++ .../Checkers/WebKit/uncounted-obj-arg.cpp | 2 + 9 files changed, 251 insertions(+), 23 deletions(-) create mode 100644 clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp create mode 100644 clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index e449593e95d21..9ebd667d4c80e 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -3680,6 +3680,20 @@ 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 fucntion 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(); + }; + 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..8c6050a4bb1bd --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp @@ -0,0 +1,127 @@ +//=======- 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 "ASTUtils.h" +#include "DiagOutputUtils.h" +#include "PtrTypesSemantics.h" +#include "clang/AST/CXXInheritance.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/RecursiveASTVisitor.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" +#include "llvm/ADT/DenseSet.h" +#include "llvm/Support/SaveAndRestore.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 RecursiveASTVisitor. + struct LocalVisitor : public RecursiveASTVisitor<LocalVisitor> { + const NoDeleteChecker *Checker; + Decl *DeclWithIssue{nullptr}; + + explicit LocalVisitor(const NoDeleteChecker *Checker) + : Checker(Checker) { + assert(Checker); + } + + bool shouldVisitTemplateInstantiations() const { return true; } + bool shouldVisitImplicitCode() const { return false; } + + bool VisitFunctionDecl(FunctionDecl *FD) { + 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 92fd612c11bc3..15abfab7734dc 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,17 @@ 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,19 +524,10 @@ class TrivialFunctionAnalysisVisitor bool IsFunctionTrivial(const Decl *D) { if (auto *FnDecl = dyn_cast<FunctionDecl>(D)) { + if (isNoDeleteFunction(FnDecl)) + return true; if (FnDecl->isVirtualAsWritten()) return false; - auto ReturnType = FnDecl->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.nodelete") - return true; - } - } - } - } } return WithCachedResult(D, [&]() { if (auto *CtorDecl = dyn_cast<CXXConstructorDecl>(D)) { 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..70305b7cee69e --- /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@-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@-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@-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@-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@-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@-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@-1{{A function 'anotherVirtualMethod' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}} + someFunction(); + } +}; \ No newline at end of file diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp index 14a81aba1adc1..c60126947217d 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp @@ -391,6 +391,7 @@ class RefCounted { 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; @@ -586,6 +587,7 @@ class UnrelatedClass { getFieldTrivial().nodelete1(); // no-warning getFieldTrivial().nodelete2(); // no-warning + getFieldTrivial().nodelete3(); // no-warning RefCounted::singleton().trivial18(); // no-warning RefCounted::singleton().someFunction(); // no-warning _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
