Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[alpha.webkit.webkit.RetainPtrCtorAdoptChecker] Add a new WebKit checker for correct use of RetainPtr, adoptNS, and adoptCF #128679

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rniwa
Copy link
Contributor

@rniwa rniwa commented Feb 25, 2025

Add a new WebKit checker to validate the correct use of RetainPtr constructor as well as adoptNS and adoptCF functions. adoptNS and adoptCf are used for +1 semantics and RetainPtr constructor is used for +0 semantics.

…ker for correct use of RetainPtr, adoptNS, and adoptCF

Add a new WebKit checker to validate the correct use of RetainPtr constructor as well as adoptNS and adoptCF functions.
adoptNS and adoptCf are used for +1 semantics and RetainPtr constructor is used for +0 semantics.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Feb 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 25, 2025

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Ryosuke Niwa (rniwa)

Changes

Add a new WebKit checker to validate the correct use of RetainPtr constructor as well as adoptNS and adoptCF functions. adoptNS and adoptCf are used for +1 semantics and RetainPtr constructor is used for +0 semantics.


Patch is 29.52 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/128679.diff

9 Files Affected:

  • (modified) clang/docs/analyzer/checkers.rst (+20)
  • (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+4)
  • (modified) clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt (+1)
  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+4-3)
  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h (+2-1)
  • (added) clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp (+347)
  • (modified) clang/test/Analysis/Checkers/WebKit/objc-mock-types.h (+132-14)
  • (added) clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm (+85)
  • (added) clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm (+85)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index c1eedb33e74d2..4cbd31f44d3f6 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -3713,6 +3713,26 @@ Here are some examples of situations that we warn about as they *might* be poten
       NSObject* unretained = retained.get(); // warn
     }
 
+webkit.RetainPtrCtorAdoptChecker
+""""""""""""""""""""""""""""""""
+The goal of this rule is to make sure the constructor of RetinPtr as well as adoptNS and adoptCF are used correctly.
+When creating a RetainPtr with +1 semantics, adoptNS or adoptCF should be used, and in +0 semantics, RetainPtr constructor should be used.
+Warn otherwise.
+
+These are examples of cases that we consider correct:
+
+  .. code-block:: cpp
+
+    RetainPtr ptr = adoptNS([[NSObject alloc] init]); // ok
+    RetainPtr ptr = CGImageGetColorSpace(image); // ok
+
+Here are some examples of cases that we consider incorrect use of RetainPtr constructor and adoptCF
+
+  .. code-block:: cpp
+
+    RetainPtr ptr = [[NSObject alloc] init]; // warn
+    auto ptr = adoptCF(CGImageGetColorSpace(image)); // warn
+
 Debug Checkers
 ---------------
 
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 410f841630660..9aa696d8803b1 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1786,4 +1786,8 @@ def UnretainedLocalVarsChecker : Checker<"UnretainedLocalVarsChecker">,
   HelpText<"Check unretained local variables.">,
   Documentation<HasDocumentation>;
 
+def RetainPtrCtorAdoptChecker : Checker<"RetainPtrCtorAdoptChecker">,
+  HelpText<"Check for correct use of RetainPtr constructor, adoptNS, and adoptCF">,
+  Documentation<HasDocumentation>;
+
 } // end alpha.webkit
diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 5910043440987..0b6b169d7b447 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -133,6 +133,7 @@ add_clang_library(clangStaticAnalyzerCheckers
   WebKit/MemoryUnsafeCastChecker.cpp
   WebKit/PtrTypesSemantics.cpp
   WebKit/RefCntblBaseVirtualDtorChecker.cpp
+  WebKit/RetainPtrCtorAdoptChecker.cpp
   WebKit/RawPtrRefCallArgsChecker.cpp
   WebKit/UncountedLambdaCapturesChecker.cpp
   WebKit/RawPtrRefLocalVarsChecker.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 7899b19854806..7e7bd49ca0bdb 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -225,15 +225,16 @@ void RetainTypeChecker::visitTypedef(const TypedefDecl *TD) {
     return;
 
   for (auto *Redecl : RT->getDecl()->getMostRecentDecl()->redecls()) {
-    if (Redecl->getAttr<ObjCBridgeAttr>()) {
+    if (Redecl->getAttr<ObjCBridgeAttr>() ||
+        Redecl->getAttr<ObjCBridgeMutableAttr>()) {
       CFPointees.insert(RT);
       return;
     }
   }
 }
 
-bool RetainTypeChecker::isUnretained(const QualType QT) {
-  if (ento::cocoa::isCocoaObjectRef(QT) && !IsARCEnabled)
+bool RetainTypeChecker::isUnretained(const QualType QT, bool ignoreARC) {
+  if (ento::cocoa::isCocoaObjectRef(QT) && (!IsARCEnabled || ignoreARC))
     return true;
   auto CanonicalType = QT.getCanonicalType();
   auto PointeeType = CanonicalType->getPointeeType();
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index fcc1a41dba78b..f69d374cbbf90 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -75,7 +75,8 @@ class RetainTypeChecker {
 public:
   void visitTranslationUnitDecl(const TranslationUnitDecl *);
   void visitTypedef(const TypedefDecl *);
-  bool isUnretained(const QualType);
+  bool isUnretained(const QualType, bool ignoreARC = false);
+  bool isARCEnabled() const { return IsARCEnabled; }
 };
 
 /// \returns true if \p Class is NS or CF objects AND not retained, false if
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
new file mode 100644
index 0000000000000..8727f89826807
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
@@ -0,0 +1,347 @@
+//=======- RefCntblBaseVirtualDtor.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/RecursiveASTVisitor.h"
+#include "clang/AST/StmtVisitor.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 "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/Analysis/RetainSummaryManager.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/SetVector.h"
+#include <optional>
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class RetainPtrCtorAdoptChecker
+    : public Checker<check::ASTDecl<TranslationUnitDecl>> {
+private:
+  BugType Bug;
+  mutable BugReporter *BR;
+  mutable std::unique_ptr<RetainSummaryManager> Summaries;
+  mutable RetainTypeChecker RTC;
+
+public:
+  RetainPtrCtorAdoptChecker()
+      : Bug(this,
+            "Correct use of RetainPtr, adoptNS, and adoptCF",
+            "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 RetainPtrCtorAdoptChecker *Checker;
+      Decl *DeclWithIssue{nullptr};
+
+      using Base = RecursiveASTVisitor<LocalVisitor>;
+
+      explicit LocalVisitor(const RetainPtrCtorAdoptChecker *Checker)
+          : Checker(Checker) {
+        assert(Checker);
+      }
+
+      bool shouldVisitTemplateInstantiations() const { return true; }
+      bool shouldVisitImplicitCode() const { return false; }
+
+      bool TraverseDecl(Decl *D) {
+        llvm::SaveAndRestore SavedDecl(DeclWithIssue);
+        if (D && (isa<FunctionDecl>(D) || isa<ObjCMethodDecl>(D)))
+          DeclWithIssue = D;
+        return Base::TraverseDecl(D);
+      }
+      
+      bool TraverseClassTemplateDecl(ClassTemplateDecl *CTD) {
+        if (safeGetName(CTD) == "RetainPtr")
+          return true; // Skip the contents of RetainPtr.
+        return Base::TraverseClassTemplateDecl(CTD);
+      }
+
+      bool VisitTypedefDecl(TypedefDecl *TD) {
+        Checker->RTC.visitTypedef(TD);
+        return true;
+      }
+
+      bool VisitCallExpr(const CallExpr *CE) {
+        Checker->visitCallExpr(CE, DeclWithIssue);
+        return true;
+      }
+
+      bool VisitCXXConstructExpr(const CXXConstructExpr *CE) {
+        Checker->visitConstructExpr(CE, DeclWithIssue);
+        return true;
+      }
+
+      llvm::SetVector<const CXXRecordDecl *> Decls;
+      llvm::DenseSet<const CXXRecordDecl *> CRTPs;
+    };
+
+    LocalVisitor visitor(this);
+    Summaries = std::make_unique<RetainSummaryManager>(TUD->getASTContext(),
+        true /* trackObjCAndCFObjects */, false /* trackOSObjects */);
+    RTC.visitTranslationUnitDecl(TUD);
+    visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
+  }
+
+  bool isAdoptFn(const Decl *FnDecl) const {
+    auto Name = safeGetName(FnDecl);
+    return Name == "adoptNS" || Name == "adoptCF" || Name == "adoptNSArc" ||
+           Name == "adoptCFArc";
+  }
+
+  bool isAdoptNS(const Decl *FnDecl) const {
+    auto Name = safeGetName(FnDecl);
+    return Name == "adoptNS" || Name == "adoptNSArc";
+  }
+
+  void visitCallExpr(const CallExpr *CE, const Decl* DeclWithIssue) const {
+    if (BR->getSourceManager().isInSystemHeader(CE->getExprLoc()))
+      return;
+
+    auto *F = CE->getDirectCallee();
+    if (!F)
+      return;
+
+    if (!isAdoptFn(F) || !CE->getNumArgs())
+      return;
+
+    auto *Arg = CE->getArg(0)->IgnoreParenCasts();
+    auto Result = isOwned(Arg);
+    auto Name = safeGetName(F);
+    if (Result == IsOwnedResult::Unknown)
+      reportUnknown(Name, CE, DeclWithIssue);
+    else if (Result == IsOwnedResult::NotOwned) {
+      if (RTC.isARCEnabled() && isAdoptNS(F)) {
+        if (!isAllocInit(Arg))
+          reportUseAfterFree(Name, CE, DeclWithIssue, "when ARC is disabled");
+      } else
+        reportUseAfterFree(Name, CE, DeclWithIssue);
+    }
+  }
+
+  void visitConstructExpr(const CXXConstructExpr *CE,
+                          const Decl* DeclWithIssue) const {
+    if (BR->getSourceManager().isInSystemHeader(CE->getExprLoc()))
+      return;
+
+    auto *Ctor = CE->getConstructor();
+    if (!Ctor)
+      return;
+
+    auto *Cls = Ctor->getParent();
+    if (!Cls)
+      return;
+
+    if (safeGetName(Cls) != "RetainPtr" || !CE->getNumArgs())
+      return;
+
+    // Ignore RetainPtr construction inside adoptNS, adoptCF, and retainPtr.
+    if (isAdoptFn(DeclWithIssue) || safeGetName(DeclWithIssue) == "retainPtr")
+      return;
+
+    std::string Name = "RetainPtr constructor";
+    auto *Arg = CE->getArg(0)->IgnoreParenCasts();
+    auto Result = isOwned(Arg);
+    if (Result == IsOwnedResult::Unknown)
+      reportUnknown(Name, CE, DeclWithIssue);
+    else if (Result == IsOwnedResult::Owned)
+      reportLeak(Name, CE, DeclWithIssue);
+    else if (RTC.isARCEnabled() && isAllocInit(Arg))
+      reportLeak(Name, CE, DeclWithIssue, "when ARC is disabled");
+  }
+
+  bool isAllocInit(const Expr *E) const {
+    auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E);
+    if (!ObjCMsgExpr)
+      return false;
+    auto Selector = ObjCMsgExpr->getSelector();
+    auto NameForFirstSlot = Selector.getNameForSlot(0);
+    if (NameForFirstSlot == "alloc" || NameForFirstSlot.starts_with("copy") ||
+        NameForFirstSlot.starts_with("mutableCopy"))
+      return true;
+    if (!NameForFirstSlot.starts_with("init"))
+      return false;
+    if (!ObjCMsgExpr->isInstanceMessage())
+      return false;
+    auto *Receiver = ObjCMsgExpr->getInstanceReceiver()->IgnoreParenCasts();
+    if (!Receiver)
+      return false;
+    auto *InnerObjCMsgExpr = dyn_cast<ObjCMessageExpr>(Receiver);
+    if (!InnerObjCMsgExpr)
+      return false;
+    auto InnerSelector = InnerObjCMsgExpr->getSelector();
+    return InnerSelector.getNameForSlot(0) == "alloc";
+  }
+
+  enum class IsOwnedResult { Unknown, Skip, Owned, NotOwned };
+  IsOwnedResult isOwned(const Expr *E) const {
+    while (1) {
+      if (isa<CXXNullPtrLiteralExpr>(E))
+        return IsOwnedResult::NotOwned;
+      if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
+        auto QT = DRE->getType();
+        if (isRetainPtrType(QT))
+          return IsOwnedResult::NotOwned;
+        QT = QT.getCanonicalType();
+        if (RTC.isUnretained(QT, true /* ignoreARC */))
+          return IsOwnedResult::NotOwned;
+        auto *PointeeType = QT->getPointeeType().getTypePtrOrNull();
+        if (PointeeType && PointeeType->isVoidType())
+          return IsOwnedResult::NotOwned; // Assume reading void* as +0.
+      }
+      if (auto *TE = dyn_cast<CXXBindTemporaryExpr>(E)) {
+        E = TE->getSubExpr();
+        continue;
+      }
+      if (auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E)) {
+        auto Summary = Summaries->getSummary(AnyCall(ObjCMsgExpr));
+        auto RetEffect = Summary->getRetEffect();
+        switch (RetEffect.getKind()) {
+          case RetEffect::NoRet:
+            return IsOwnedResult::Unknown;
+          case RetEffect::OwnedSymbol:
+            return IsOwnedResult::Owned;
+          case RetEffect::NotOwnedSymbol:
+            return IsOwnedResult::NotOwned;
+          case RetEffect::OwnedWhenTrackedReceiver:
+            if (auto *Receiver = ObjCMsgExpr->getInstanceReceiver()) {
+              E = Receiver->IgnoreParenCasts();
+              continue;
+            }
+            return IsOwnedResult::Unknown;
+          case RetEffect::NoRetHard:
+            return IsOwnedResult::Unknown;
+        }
+      }
+      if (auto *CXXCE = dyn_cast<CXXMemberCallExpr>(E)) {
+        if (auto *MD = CXXCE->getMethodDecl()) {
+          auto *Cls = MD->getParent();
+          if (auto *CD = dyn_cast<CXXConversionDecl>(MD)) {
+            auto QT = CD->getConversionType().getCanonicalType();
+            auto *ResultType = QT.getTypePtrOrNull();
+            if (safeGetName(Cls) == "RetainPtr" && ResultType &&
+                (ResultType->isPointerType() || ResultType->isReferenceType() ||
+                 ResultType->isObjCObjectPointerType()))
+              return IsOwnedResult::NotOwned;
+          }
+          if (safeGetName(MD) == "leakRef" && safeGetName(Cls) == "RetainPtr")
+            return IsOwnedResult::Owned;
+        }
+      }
+      if (auto *CE = dyn_cast<CallExpr>(E)) {
+        if (auto *Callee = CE->getDirectCallee()) {
+          if (isAdoptFn(Callee))
+            return IsOwnedResult::NotOwned;
+          if (safeGetName(Callee) == "__builtin___CFStringMakeConstantString")
+            return IsOwnedResult::NotOwned;
+          auto RetType = Callee->getReturnType();
+          if (isRetainPtrType(RetType))
+            return IsOwnedResult::NotOwned;
+        } else if (auto *CalleeExpr = CE->getCallee()) {
+          if (isa<CXXDependentScopeMemberExpr>(CalleeExpr))
+            return IsOwnedResult::Skip; // Wait for instantiation.
+        }
+        auto Summary = Summaries->getSummary(AnyCall(CE));
+        auto RetEffect = Summary->getRetEffect();
+        switch (RetEffect.getKind()) {
+          case RetEffect::NoRet:
+            return IsOwnedResult::Unknown;
+          case RetEffect::OwnedSymbol:
+            return IsOwnedResult::Owned;
+          case RetEffect::NotOwnedSymbol:
+            return IsOwnedResult::NotOwned;
+          case RetEffect::OwnedWhenTrackedReceiver:
+            return IsOwnedResult::Unknown;
+          case RetEffect::NoRetHard:
+            return IsOwnedResult::Unknown;
+        }
+      }
+      break;
+    }
+    return IsOwnedResult::Unknown;
+  }
+
+  template <typename ExprType>
+  void reportUnknown(std::string& Name, const ExprType *CE,
+                     const Decl* DeclWithIssue) const {
+    SmallString<100> Buf;
+    llvm::raw_svector_ostream Os(Buf);
+
+    Os << Name << " is called on an object in unknown state.";
+
+    PathDiagnosticLocation BSLoc(CE->getSourceRange().getBegin(),
+                                 BR->getSourceManager());
+    auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
+    Report->addRange(CE->getSourceRange());
+    Report->setDeclWithIssue(DeclWithIssue);
+    BR->emitReport(std::move(Report));
+  }
+
+  void reportUseAfterFree(std::string& Name, const CallExpr *CE,
+                          const Decl* DeclWithIssue,
+                          const char* condition = nullptr) const {
+    SmallString<100> Buf;
+    llvm::raw_svector_ostream Os(Buf);
+
+    Os << "Incorrect use of " << Name <<
+      ". The argument is +0 and results in an use-after-free";
+    if (condition)
+      Os << " " << condition;
+    Os << ".";
+
+    PathDiagnosticLocation BSLoc(CE->getSourceRange().getBegin(),
+                                 BR->getSourceManager());
+    auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
+    Report->addRange(CE->getSourceRange());
+    Report->setDeclWithIssue(DeclWithIssue);
+    BR->emitReport(std::move(Report));
+  }
+
+  void reportLeak(std::string& Name, const CXXConstructExpr *CE,
+                  const Decl* DeclWithIssue,
+                  const char* condition = nullptr) const {
+    SmallString<100> Buf;
+    llvm::raw_svector_ostream Os(Buf);
+
+    Os << "Incorrect use of " << Name <<
+      ". The argument is +1 and results in a memory leak";
+    if (condition)
+      Os << " " << condition;
+    Os << ".";
+
+    PathDiagnosticLocation BSLoc(CE->getSourceRange().getBegin(),
+                                 BR->getSourceManager());
+    auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
+    Report->addRange(CE->getSourceRange());
+    Report->setDeclWithIssue(DeclWithIssue);
+    BR->emitReport(std::move(Report));
+  }
+};
+} // namespace
+
+void ento::registerRetainPtrCtorAdoptChecker(CheckerManager &Mgr) {
+  Mgr.registerChecker<RetainPtrCtorAdoptChecker>();
+}
+
+bool ento::shouldRegisterRetainPtrCtorAdoptChecker(
+    const CheckerManager &mgr) {
+  return true;
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
index 7bb33bcb6cf44..9b13810d0c5c9 100644
--- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
@@ -1,30 +1,94 @@
 @class NSString;
 @class NSArray;
 @class NSMutableArray;
+#define nil ((id)0)
 #define CF_BRIDGED_TYPE(T) __attribute__((objc_bridge(T)))
+#define CF_BRIDGED_MUTABLE_TYPE(T) __attribute__((objc_bridge_mutable(T)))
 typedef CF_BRIDGED_TYPE(id) void * CFTypeRef;
+typedef signed char BOOL;
 typedef signed long CFIndex;
-typedef const struct __CFAllocator * CFAllocatorRef;
+typedef unsigned long NSUInteger;
+typedef const struct CF_BRIDGED_TYPE(id) __CFAllocator * CFAllocatorRef;
 typedef const struct CF_BRIDGED_TYPE(NSString) __CFString * CFStringRef;
 typedef const struct CF_BRIDGED_TYPE(NSArray) __CFArray * CFArrayRef;
-typedef struct CF_BRIDGED_TYPE(NSMutableArray) __CFArray * CFMutableArrayRef;
+typedef struct CF_BRIDGED_MUTABLE_TYPE(NSMutableArray) __CFArray * CFMutableArrayRef;
+typedef struct CF_BRIDGED_MUTABLE_TYPE(CFRunLoopRef) __CFRunLoop * CFRunLoopRef;
 extern const CFAllocatorRef kCFAllocatorDefault;
+typedef struct _NSZone NSZone;
 CFMutableArrayRef CFArrayCreateMutable(CFAllocatorRef allocator, CFIndex capacity);
 extern void CFArrayAppendValue(CFMutableArrayRef theArray, const void *value);
 CFArrayRef CFArrayCreate(CFAllocatorRef allocator, const void **values, CFIndex numValues);
 CFIndex CFArrayGetCount(CFArrayRef theArray);
+CFRunLoopRef CFRunLoopGetCurrent(void);
+CFRunLoopRef CFRunLoopGetMain(void);
 extern CFTypeRef CFRetain(CFTypeRef cf);
 extern void CFRelease(CFTypeRef cf);
+#define CFSTR(cStr) ((CFStringRef) __builtin___CFStringMakeConstantString ("" cStr ""))
+extern Class NSClassFromString(NSString *aClassName);
 
 __attribute__((objc_root_class))
 @interface NSObject
 + (instancetype) alloc;
++ (Class) class;
++ (Class) superclass;
 - (instancetype) init;
 - (instancetype)retain;
 - (void)release;
+- (BOOL)isKindOfClass:(Class)aClass;
+@end
+
+@protocol NSCopying
+- (id)copyWithZone:(NSZone *)zone;
+@end
+
+@protocol NSFastEnumeration
+- (int)countByEnumeratingWithState:(void *)state objects:(id *)objects count:(unsigned)count;
+- (void)protocolMethod;
+@end
+
+@interface NSEnumerator <NSFastEnumeration>
+@end
+
+@interface NSDictionary : NSObject <NSCopying>
+- (NSUInteger)count;
+- (id)objectForKey:(id)aKey;
+- (id)objectForKeyedSubscript:(id)aKey;
+- (NSEnumerator *)keyEnumerator;
++ (id)dictionary;
++ (id)dictionaryWithObject:(id)object forKey:(id <NSCopying>)key;
++ (instancetype)dictionaryWithObjects:(const id [])objects forKeys:(const id <NSCopying> [])keys count:(NSUInteger)cnt;
+@end
+
+@interface NSArray : NSObject <NSCopying, NSFastEnumeration>
+- (NSUInteger)count;
+- (NSEnumerator *)objectEnumerator;
++ (NSArray *)arrayWithObjects:(const id [])objects count:(NSUInteger)count;
+@end
+
+@interface NSString : NSObject <NSCopying>
+- (NSUInteger)length;
+- (NS...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Feb 25, 2025

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)

Changes

Add a new WebKit checker to validate the correct use of RetainPtr constructor as well as adoptNS and adoptCF functions. adoptNS and adoptCf are used for +1 semantics and RetainPtr constructor is used for +0 semantics.


Patch is 29.52 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/128679.diff

9 Files Affected:

  • (modified) clang/docs/analyzer/checkers.rst (+20)
  • (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+4)
  • (modified) clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt (+1)
  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+4-3)
  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h (+2-1)
  • (added) clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp (+347)
  • (modified) clang/test/Analysis/Checkers/WebKit/objc-mock-types.h (+132-14)
  • (added) clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm (+85)
  • (added) clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm (+85)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index c1eedb33e74d2..4cbd31f44d3f6 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -3713,6 +3713,26 @@ Here are some examples of situations that we warn about as they *might* be poten
       NSObject* unretained = retained.get(); // warn
     }
 
+webkit.RetainPtrCtorAdoptChecker
+""""""""""""""""""""""""""""""""
+The goal of this rule is to make sure the constructor of RetinPtr as well as adoptNS and adoptCF are used correctly.
+When creating a RetainPtr with +1 semantics, adoptNS or adoptCF should be used, and in +0 semantics, RetainPtr constructor should be used.
+Warn otherwise.
+
+These are examples of cases that we consider correct:
+
+  .. code-block:: cpp
+
+    RetainPtr ptr = adoptNS([[NSObject alloc] init]); // ok
+    RetainPtr ptr = CGImageGetColorSpace(image); // ok
+
+Here are some examples of cases that we consider incorrect use of RetainPtr constructor and adoptCF
+
+  .. code-block:: cpp
+
+    RetainPtr ptr = [[NSObject alloc] init]; // warn
+    auto ptr = adoptCF(CGImageGetColorSpace(image)); // warn
+
 Debug Checkers
 ---------------
 
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 410f841630660..9aa696d8803b1 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1786,4 +1786,8 @@ def UnretainedLocalVarsChecker : Checker<"UnretainedLocalVarsChecker">,
   HelpText<"Check unretained local variables.">,
   Documentation<HasDocumentation>;
 
+def RetainPtrCtorAdoptChecker : Checker<"RetainPtrCtorAdoptChecker">,
+  HelpText<"Check for correct use of RetainPtr constructor, adoptNS, and adoptCF">,
+  Documentation<HasDocumentation>;
+
 } // end alpha.webkit
diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 5910043440987..0b6b169d7b447 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -133,6 +133,7 @@ add_clang_library(clangStaticAnalyzerCheckers
   WebKit/MemoryUnsafeCastChecker.cpp
   WebKit/PtrTypesSemantics.cpp
   WebKit/RefCntblBaseVirtualDtorChecker.cpp
+  WebKit/RetainPtrCtorAdoptChecker.cpp
   WebKit/RawPtrRefCallArgsChecker.cpp
   WebKit/UncountedLambdaCapturesChecker.cpp
   WebKit/RawPtrRefLocalVarsChecker.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 7899b19854806..7e7bd49ca0bdb 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -225,15 +225,16 @@ void RetainTypeChecker::visitTypedef(const TypedefDecl *TD) {
     return;
 
   for (auto *Redecl : RT->getDecl()->getMostRecentDecl()->redecls()) {
-    if (Redecl->getAttr<ObjCBridgeAttr>()) {
+    if (Redecl->getAttr<ObjCBridgeAttr>() ||
+        Redecl->getAttr<ObjCBridgeMutableAttr>()) {
       CFPointees.insert(RT);
       return;
     }
   }
 }
 
-bool RetainTypeChecker::isUnretained(const QualType QT) {
-  if (ento::cocoa::isCocoaObjectRef(QT) && !IsARCEnabled)
+bool RetainTypeChecker::isUnretained(const QualType QT, bool ignoreARC) {
+  if (ento::cocoa::isCocoaObjectRef(QT) && (!IsARCEnabled || ignoreARC))
     return true;
   auto CanonicalType = QT.getCanonicalType();
   auto PointeeType = CanonicalType->getPointeeType();
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index fcc1a41dba78b..f69d374cbbf90 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -75,7 +75,8 @@ class RetainTypeChecker {
 public:
   void visitTranslationUnitDecl(const TranslationUnitDecl *);
   void visitTypedef(const TypedefDecl *);
-  bool isUnretained(const QualType);
+  bool isUnretained(const QualType, bool ignoreARC = false);
+  bool isARCEnabled() const { return IsARCEnabled; }
 };
 
 /// \returns true if \p Class is NS or CF objects AND not retained, false if
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
new file mode 100644
index 0000000000000..8727f89826807
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
@@ -0,0 +1,347 @@
+//=======- RefCntblBaseVirtualDtor.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/RecursiveASTVisitor.h"
+#include "clang/AST/StmtVisitor.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 "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/Analysis/RetainSummaryManager.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/SetVector.h"
+#include <optional>
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class RetainPtrCtorAdoptChecker
+    : public Checker<check::ASTDecl<TranslationUnitDecl>> {
+private:
+  BugType Bug;
+  mutable BugReporter *BR;
+  mutable std::unique_ptr<RetainSummaryManager> Summaries;
+  mutable RetainTypeChecker RTC;
+
+public:
+  RetainPtrCtorAdoptChecker()
+      : Bug(this,
+            "Correct use of RetainPtr, adoptNS, and adoptCF",
+            "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 RetainPtrCtorAdoptChecker *Checker;
+      Decl *DeclWithIssue{nullptr};
+
+      using Base = RecursiveASTVisitor<LocalVisitor>;
+
+      explicit LocalVisitor(const RetainPtrCtorAdoptChecker *Checker)
+          : Checker(Checker) {
+        assert(Checker);
+      }
+
+      bool shouldVisitTemplateInstantiations() const { return true; }
+      bool shouldVisitImplicitCode() const { return false; }
+
+      bool TraverseDecl(Decl *D) {
+        llvm::SaveAndRestore SavedDecl(DeclWithIssue);
+        if (D && (isa<FunctionDecl>(D) || isa<ObjCMethodDecl>(D)))
+          DeclWithIssue = D;
+        return Base::TraverseDecl(D);
+      }
+      
+      bool TraverseClassTemplateDecl(ClassTemplateDecl *CTD) {
+        if (safeGetName(CTD) == "RetainPtr")
+          return true; // Skip the contents of RetainPtr.
+        return Base::TraverseClassTemplateDecl(CTD);
+      }
+
+      bool VisitTypedefDecl(TypedefDecl *TD) {
+        Checker->RTC.visitTypedef(TD);
+        return true;
+      }
+
+      bool VisitCallExpr(const CallExpr *CE) {
+        Checker->visitCallExpr(CE, DeclWithIssue);
+        return true;
+      }
+
+      bool VisitCXXConstructExpr(const CXXConstructExpr *CE) {
+        Checker->visitConstructExpr(CE, DeclWithIssue);
+        return true;
+      }
+
+      llvm::SetVector<const CXXRecordDecl *> Decls;
+      llvm::DenseSet<const CXXRecordDecl *> CRTPs;
+    };
+
+    LocalVisitor visitor(this);
+    Summaries = std::make_unique<RetainSummaryManager>(TUD->getASTContext(),
+        true /* trackObjCAndCFObjects */, false /* trackOSObjects */);
+    RTC.visitTranslationUnitDecl(TUD);
+    visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
+  }
+
+  bool isAdoptFn(const Decl *FnDecl) const {
+    auto Name = safeGetName(FnDecl);
+    return Name == "adoptNS" || Name == "adoptCF" || Name == "adoptNSArc" ||
+           Name == "adoptCFArc";
+  }
+
+  bool isAdoptNS(const Decl *FnDecl) const {
+    auto Name = safeGetName(FnDecl);
+    return Name == "adoptNS" || Name == "adoptNSArc";
+  }
+
+  void visitCallExpr(const CallExpr *CE, const Decl* DeclWithIssue) const {
+    if (BR->getSourceManager().isInSystemHeader(CE->getExprLoc()))
+      return;
+
+    auto *F = CE->getDirectCallee();
+    if (!F)
+      return;
+
+    if (!isAdoptFn(F) || !CE->getNumArgs())
+      return;
+
+    auto *Arg = CE->getArg(0)->IgnoreParenCasts();
+    auto Result = isOwned(Arg);
+    auto Name = safeGetName(F);
+    if (Result == IsOwnedResult::Unknown)
+      reportUnknown(Name, CE, DeclWithIssue);
+    else if (Result == IsOwnedResult::NotOwned) {
+      if (RTC.isARCEnabled() && isAdoptNS(F)) {
+        if (!isAllocInit(Arg))
+          reportUseAfterFree(Name, CE, DeclWithIssue, "when ARC is disabled");
+      } else
+        reportUseAfterFree(Name, CE, DeclWithIssue);
+    }
+  }
+
+  void visitConstructExpr(const CXXConstructExpr *CE,
+                          const Decl* DeclWithIssue) const {
+    if (BR->getSourceManager().isInSystemHeader(CE->getExprLoc()))
+      return;
+
+    auto *Ctor = CE->getConstructor();
+    if (!Ctor)
+      return;
+
+    auto *Cls = Ctor->getParent();
+    if (!Cls)
+      return;
+
+    if (safeGetName(Cls) != "RetainPtr" || !CE->getNumArgs())
+      return;
+
+    // Ignore RetainPtr construction inside adoptNS, adoptCF, and retainPtr.
+    if (isAdoptFn(DeclWithIssue) || safeGetName(DeclWithIssue) == "retainPtr")
+      return;
+
+    std::string Name = "RetainPtr constructor";
+    auto *Arg = CE->getArg(0)->IgnoreParenCasts();
+    auto Result = isOwned(Arg);
+    if (Result == IsOwnedResult::Unknown)
+      reportUnknown(Name, CE, DeclWithIssue);
+    else if (Result == IsOwnedResult::Owned)
+      reportLeak(Name, CE, DeclWithIssue);
+    else if (RTC.isARCEnabled() && isAllocInit(Arg))
+      reportLeak(Name, CE, DeclWithIssue, "when ARC is disabled");
+  }
+
+  bool isAllocInit(const Expr *E) const {
+    auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E);
+    if (!ObjCMsgExpr)
+      return false;
+    auto Selector = ObjCMsgExpr->getSelector();
+    auto NameForFirstSlot = Selector.getNameForSlot(0);
+    if (NameForFirstSlot == "alloc" || NameForFirstSlot.starts_with("copy") ||
+        NameForFirstSlot.starts_with("mutableCopy"))
+      return true;
+    if (!NameForFirstSlot.starts_with("init"))
+      return false;
+    if (!ObjCMsgExpr->isInstanceMessage())
+      return false;
+    auto *Receiver = ObjCMsgExpr->getInstanceReceiver()->IgnoreParenCasts();
+    if (!Receiver)
+      return false;
+    auto *InnerObjCMsgExpr = dyn_cast<ObjCMessageExpr>(Receiver);
+    if (!InnerObjCMsgExpr)
+      return false;
+    auto InnerSelector = InnerObjCMsgExpr->getSelector();
+    return InnerSelector.getNameForSlot(0) == "alloc";
+  }
+
+  enum class IsOwnedResult { Unknown, Skip, Owned, NotOwned };
+  IsOwnedResult isOwned(const Expr *E) const {
+    while (1) {
+      if (isa<CXXNullPtrLiteralExpr>(E))
+        return IsOwnedResult::NotOwned;
+      if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
+        auto QT = DRE->getType();
+        if (isRetainPtrType(QT))
+          return IsOwnedResult::NotOwned;
+        QT = QT.getCanonicalType();
+        if (RTC.isUnretained(QT, true /* ignoreARC */))
+          return IsOwnedResult::NotOwned;
+        auto *PointeeType = QT->getPointeeType().getTypePtrOrNull();
+        if (PointeeType && PointeeType->isVoidType())
+          return IsOwnedResult::NotOwned; // Assume reading void* as +0.
+      }
+      if (auto *TE = dyn_cast<CXXBindTemporaryExpr>(E)) {
+        E = TE->getSubExpr();
+        continue;
+      }
+      if (auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E)) {
+        auto Summary = Summaries->getSummary(AnyCall(ObjCMsgExpr));
+        auto RetEffect = Summary->getRetEffect();
+        switch (RetEffect.getKind()) {
+          case RetEffect::NoRet:
+            return IsOwnedResult::Unknown;
+          case RetEffect::OwnedSymbol:
+            return IsOwnedResult::Owned;
+          case RetEffect::NotOwnedSymbol:
+            return IsOwnedResult::NotOwned;
+          case RetEffect::OwnedWhenTrackedReceiver:
+            if (auto *Receiver = ObjCMsgExpr->getInstanceReceiver()) {
+              E = Receiver->IgnoreParenCasts();
+              continue;
+            }
+            return IsOwnedResult::Unknown;
+          case RetEffect::NoRetHard:
+            return IsOwnedResult::Unknown;
+        }
+      }
+      if (auto *CXXCE = dyn_cast<CXXMemberCallExpr>(E)) {
+        if (auto *MD = CXXCE->getMethodDecl()) {
+          auto *Cls = MD->getParent();
+          if (auto *CD = dyn_cast<CXXConversionDecl>(MD)) {
+            auto QT = CD->getConversionType().getCanonicalType();
+            auto *ResultType = QT.getTypePtrOrNull();
+            if (safeGetName(Cls) == "RetainPtr" && ResultType &&
+                (ResultType->isPointerType() || ResultType->isReferenceType() ||
+                 ResultType->isObjCObjectPointerType()))
+              return IsOwnedResult::NotOwned;
+          }
+          if (safeGetName(MD) == "leakRef" && safeGetName(Cls) == "RetainPtr")
+            return IsOwnedResult::Owned;
+        }
+      }
+      if (auto *CE = dyn_cast<CallExpr>(E)) {
+        if (auto *Callee = CE->getDirectCallee()) {
+          if (isAdoptFn(Callee))
+            return IsOwnedResult::NotOwned;
+          if (safeGetName(Callee) == "__builtin___CFStringMakeConstantString")
+            return IsOwnedResult::NotOwned;
+          auto RetType = Callee->getReturnType();
+          if (isRetainPtrType(RetType))
+            return IsOwnedResult::NotOwned;
+        } else if (auto *CalleeExpr = CE->getCallee()) {
+          if (isa<CXXDependentScopeMemberExpr>(CalleeExpr))
+            return IsOwnedResult::Skip; // Wait for instantiation.
+        }
+        auto Summary = Summaries->getSummary(AnyCall(CE));
+        auto RetEffect = Summary->getRetEffect();
+        switch (RetEffect.getKind()) {
+          case RetEffect::NoRet:
+            return IsOwnedResult::Unknown;
+          case RetEffect::OwnedSymbol:
+            return IsOwnedResult::Owned;
+          case RetEffect::NotOwnedSymbol:
+            return IsOwnedResult::NotOwned;
+          case RetEffect::OwnedWhenTrackedReceiver:
+            return IsOwnedResult::Unknown;
+          case RetEffect::NoRetHard:
+            return IsOwnedResult::Unknown;
+        }
+      }
+      break;
+    }
+    return IsOwnedResult::Unknown;
+  }
+
+  template <typename ExprType>
+  void reportUnknown(std::string& Name, const ExprType *CE,
+                     const Decl* DeclWithIssue) const {
+    SmallString<100> Buf;
+    llvm::raw_svector_ostream Os(Buf);
+
+    Os << Name << " is called on an object in unknown state.";
+
+    PathDiagnosticLocation BSLoc(CE->getSourceRange().getBegin(),
+                                 BR->getSourceManager());
+    auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
+    Report->addRange(CE->getSourceRange());
+    Report->setDeclWithIssue(DeclWithIssue);
+    BR->emitReport(std::move(Report));
+  }
+
+  void reportUseAfterFree(std::string& Name, const CallExpr *CE,
+                          const Decl* DeclWithIssue,
+                          const char* condition = nullptr) const {
+    SmallString<100> Buf;
+    llvm::raw_svector_ostream Os(Buf);
+
+    Os << "Incorrect use of " << Name <<
+      ". The argument is +0 and results in an use-after-free";
+    if (condition)
+      Os << " " << condition;
+    Os << ".";
+
+    PathDiagnosticLocation BSLoc(CE->getSourceRange().getBegin(),
+                                 BR->getSourceManager());
+    auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
+    Report->addRange(CE->getSourceRange());
+    Report->setDeclWithIssue(DeclWithIssue);
+    BR->emitReport(std::move(Report));
+  }
+
+  void reportLeak(std::string& Name, const CXXConstructExpr *CE,
+                  const Decl* DeclWithIssue,
+                  const char* condition = nullptr) const {
+    SmallString<100> Buf;
+    llvm::raw_svector_ostream Os(Buf);
+
+    Os << "Incorrect use of " << Name <<
+      ". The argument is +1 and results in a memory leak";
+    if (condition)
+      Os << " " << condition;
+    Os << ".";
+
+    PathDiagnosticLocation BSLoc(CE->getSourceRange().getBegin(),
+                                 BR->getSourceManager());
+    auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
+    Report->addRange(CE->getSourceRange());
+    Report->setDeclWithIssue(DeclWithIssue);
+    BR->emitReport(std::move(Report));
+  }
+};
+} // namespace
+
+void ento::registerRetainPtrCtorAdoptChecker(CheckerManager &Mgr) {
+  Mgr.registerChecker<RetainPtrCtorAdoptChecker>();
+}
+
+bool ento::shouldRegisterRetainPtrCtorAdoptChecker(
+    const CheckerManager &mgr) {
+  return true;
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
index 7bb33bcb6cf44..9b13810d0c5c9 100644
--- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
@@ -1,30 +1,94 @@
 @class NSString;
 @class NSArray;
 @class NSMutableArray;
+#define nil ((id)0)
 #define CF_BRIDGED_TYPE(T) __attribute__((objc_bridge(T)))
+#define CF_BRIDGED_MUTABLE_TYPE(T) __attribute__((objc_bridge_mutable(T)))
 typedef CF_BRIDGED_TYPE(id) void * CFTypeRef;
+typedef signed char BOOL;
 typedef signed long CFIndex;
-typedef const struct __CFAllocator * CFAllocatorRef;
+typedef unsigned long NSUInteger;
+typedef const struct CF_BRIDGED_TYPE(id) __CFAllocator * CFAllocatorRef;
 typedef const struct CF_BRIDGED_TYPE(NSString) __CFString * CFStringRef;
 typedef const struct CF_BRIDGED_TYPE(NSArray) __CFArray * CFArrayRef;
-typedef struct CF_BRIDGED_TYPE(NSMutableArray) __CFArray * CFMutableArrayRef;
+typedef struct CF_BRIDGED_MUTABLE_TYPE(NSMutableArray) __CFArray * CFMutableArrayRef;
+typedef struct CF_BRIDGED_MUTABLE_TYPE(CFRunLoopRef) __CFRunLoop * CFRunLoopRef;
 extern const CFAllocatorRef kCFAllocatorDefault;
+typedef struct _NSZone NSZone;
 CFMutableArrayRef CFArrayCreateMutable(CFAllocatorRef allocator, CFIndex capacity);
 extern void CFArrayAppendValue(CFMutableArrayRef theArray, const void *value);
 CFArrayRef CFArrayCreate(CFAllocatorRef allocator, const void **values, CFIndex numValues);
 CFIndex CFArrayGetCount(CFArrayRef theArray);
+CFRunLoopRef CFRunLoopGetCurrent(void);
+CFRunLoopRef CFRunLoopGetMain(void);
 extern CFTypeRef CFRetain(CFTypeRef cf);
 extern void CFRelease(CFTypeRef cf);
+#define CFSTR(cStr) ((CFStringRef) __builtin___CFStringMakeConstantString ("" cStr ""))
+extern Class NSClassFromString(NSString *aClassName);
 
 __attribute__((objc_root_class))
 @interface NSObject
 + (instancetype) alloc;
++ (Class) class;
++ (Class) superclass;
 - (instancetype) init;
 - (instancetype)retain;
 - (void)release;
+- (BOOL)isKindOfClass:(Class)aClass;
+@end
+
+@protocol NSCopying
+- (id)copyWithZone:(NSZone *)zone;
+@end
+
+@protocol NSFastEnumeration
+- (int)countByEnumeratingWithState:(void *)state objects:(id *)objects count:(unsigned)count;
+- (void)protocolMethod;
+@end
+
+@interface NSEnumerator <NSFastEnumeration>
+@end
+
+@interface NSDictionary : NSObject <NSCopying>
+- (NSUInteger)count;
+- (id)objectForKey:(id)aKey;
+- (id)objectForKeyedSubscript:(id)aKey;
+- (NSEnumerator *)keyEnumerator;
++ (id)dictionary;
++ (id)dictionaryWithObject:(id)object forKey:(id <NSCopying>)key;
++ (instancetype)dictionaryWithObjects:(const id [])objects forKeys:(const id <NSCopying> [])keys count:(NSUInteger)cnt;
+@end
+
+@interface NSArray : NSObject <NSCopying, NSFastEnumeration>
+- (NSUInteger)count;
+- (NSEnumerator *)objectEnumerator;
++ (NSArray *)arrayWithObjects:(const id [])objects count:(NSUInteger)count;
+@end
+
+@interface NSString : NSObject <NSCopying>
+- (NSUInteger)length;
+- (NS...
[truncated]

Copy link

github-actions bot commented Feb 25, 2025

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff b335d5a8303250cb49901ecae7570adf61abbd3c 1d24671b5742059b484a4c8f755a1ee645446499 --extensions cpp,h -- clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
View the diff from clang-format here.
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
index fedf8ea30c..a97e497ad3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
@@ -301,8 +301,8 @@ public:
     SmallString<100> Buf;
     llvm::raw_svector_ostream Os(Buf);
 
-    Os << "Incorrect use of " << Name <<
-      ". The argument is +0 and results in an use-after-free";
+    Os << "Incorrect use of " << Name
+       << ". The argument is +0 and results in an use-after-free";
     if (condition)
       Os << " " << condition;
     Os << ".";

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:static analyzer clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants