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 <[email protected]> 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@-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(); + } +}; 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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
