Introduce a new WebKit checker for a unchecked call arguments (#113708) (#114522)

This PR introduces alpha.webkit.UncheckedCallArgsChecker which detects a
function argument which is a raw reference or a raw pointer to a
CheckedPtr capable object.

Author: rniwa

clang/docs/analyzer/checkers.rst

@@ -3560,6 +3560,12 @@ We also define a set of safe transformations which if passed a safe value as an
 - casts
 - unary operators like ``&`` or ``*``
 
+alpha.webkit.UncheckedCallArgsChecker
+"""""""""""""""""""""""""""""""""""""
+The goal of this rule is to make sure that lifetime of any dynamically allocated CheckedPtr capable object passed as a call argument keeps its memory region past the end of the call. This applies to call to any function, method, lambda, function pointer or functor. CheckedPtr capable objects aren't supposed to be allocated on stack so we check arguments for parameters of raw pointers and references to unchecked types.
+
+The rules of when to use and not to use CheckedPtr / CheckedRef are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects.
+
 alpha.webkit.UncountedLocalVarsChecker
 """"""""""""""""""""""""""""""""""""""
 The goal of this rule is to make sure that any uncounted local variable is backed by a ref-counted object with lifetime that is strictly larger than the scope of the uncounted local variable. To be on the safe side we require the scope of an uncounted variable to be embedded in the scope of ref-counted object that backs it.

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

@@ -1760,6 +1760,10 @@ def UncountedCallArgsChecker : Checker<"UncountedCallArgsChecker">,
   HelpText<"Check uncounted call arguments.">,
   Documentation<HasDocumentation>;
 
+def UncheckedCallArgsChecker : Checker<"UncheckedCallArgsChecker">,
+  HelpText<"Check unchecked call arguments.">,
+  Documentation<HasDocumentation>;
+
 def UncountedLocalVarsChecker : Checker<"UncountedLocalVarsChecker">,
   HelpText<"Check uncounted local variables.">,
   Documentation<HasDocumentation>;

clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt

@@ -134,7 +134,7 @@ add_clang_library(clangStaticAnalyzerCheckers
   WebKit/ASTUtils.cpp
   WebKit/PtrTypesSemantics.cpp
   WebKit/RefCntblBaseVirtualDtorChecker.cpp
-  WebKit/UncountedCallArgsChecker.cpp
+  WebKit/RawPtrRefCallArgsChecker.cpp
   WebKit/UncountedLambdaCapturesChecker.cpp
   WebKit/RawPtrRefLocalVarsChecker.cpp
 

clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp

@@ -174,6 +174,16 @@ std::optional<bool> isUncounted(const QualType T) {
   return isUncounted(T->getAsCXXRecordDecl());
 }
 
+std::optional<bool> isUnchecked(const QualType T) {
+  if (auto *Subst = dyn_cast<SubstTemplateTypeParmType>(T)) {
+    if (auto *Decl = Subst->getAssociatedDecl()) {
+      if (isCheckedPtr(safeGetName(Decl)))
+        return false;
+    }
+  }
+  return isUnchecked(T->getAsCXXRecordDecl());
+}
+
 std::optional<bool> isUncounted(const CXXRecordDecl* Class)
 {
   // Keep isRefCounted first as it's cheaper.
@@ -188,7 +198,7 @@ std::optional<bool> isUncounted(const CXXRecordDecl* Class)
 }
 
 std::optional<bool> isUnchecked(const CXXRecordDecl *Class) {
-  if (isCheckedPtr(Class))
+  if (!Class || isCheckedPtr(Class))
     return false; // Cheaper than below
   return isCheckedPtrCapable(Class);
 }

clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h

@@ -55,10 +55,18 @@ bool isCheckedPtr(const clang::CXXRecordDecl *Class);
 /// not, std::nullopt if inconclusive.
 std::optional<bool> isUncounted(const clang::QualType T);
 
+/// \returns true if \p Class is CheckedPtr capable AND not checked, false if
+/// not, std::nullopt if inconclusive.
+std::optional<bool> isUnchecked(const clang::QualType T);
+
 /// \returns true if \p Class is ref-countable AND not ref-counted, false if
 /// not, std::nullopt if inconclusive.
 std::optional<bool> isUncounted(const clang::CXXRecordDecl* Class);
 
+/// \returns true if \p Class is CheckedPtr capable AND not checked, false if
+/// not, std::nullopt if inconclusive.
+std::optional<bool> isUnchecked(const clang::CXXRecordDecl *Class);
+
 /// \returns true if \p T is either a raw pointer or reference to an uncounted
 /// class, false if not, std::nullopt if inconclusive.
 std::optional<bool> isUncountedPtr(const clang::QualType T);

clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp renamed to clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp

@@ -1,4 +1,4 @@
-//=======- UncountedCallArgsChecker.cpp --------------------------*- C++ -*-==//
+//=======- RawPtrRefCallArgsChecker.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.
@@ -27,16 +27,20 @@ using namespace ento;
 
 namespace {
 
-class UncountedCallArgsChecker
+class RawPtrRefCallArgsChecker
     : public Checker<check::ASTDecl<TranslationUnitDecl>> {
-  BugType Bug{this,
-            "Uncounted call argument for a raw pointer/reference parameter",
-            "WebKit coding guidelines"};
+  BugType Bug;
   mutable BugReporter *BR;
 
   TrivialFunctionAnalysis TFA;
 
 public:
+  RawPtrRefCallArgsChecker(const char *description)
+      : Bug(this, description, "WebKit coding guidelines") {}
+
+  virtual std::optional<bool> isUnsafeType(QualType) const = 0;
+  virtual std::optional<bool> isUnsafePtr(QualType) const = 0;
+  virtual const char *ptrKind() const = 0;
 
   void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
                     BugReporter &BRArg) const {
@@ -48,10 +52,10 @@ class UncountedCallArgsChecker
     struct LocalVisitor : public RecursiveASTVisitor<LocalVisitor> {
       using Base = RecursiveASTVisitor<LocalVisitor>;
 
-      const UncountedCallArgsChecker *Checker;
+      const RawPtrRefCallArgsChecker *Checker;
       Decl *DeclWithIssue{nullptr};
 
-      explicit LocalVisitor(const UncountedCallArgsChecker *Checker)
+      explicit LocalVisitor(const RawPtrRefCallArgsChecker *Checker)
           : Checker(Checker) {
         assert(Checker);
       }
@@ -89,7 +93,8 @@ class UncountedCallArgsChecker
     if (auto *F = CE->getDirectCallee()) {
       // Skip the first argument for overloaded member operators (e. g. lambda
       // or std::function call operator).
-      unsigned ArgIdx = isa<CXXOperatorCallExpr>(CE) && isa_and_nonnull<CXXMethodDecl>(F);
+      unsigned ArgIdx =
+          isa<CXXOperatorCallExpr>(CE) && isa_and_nonnull<CXXMethodDecl>(F);
 
       if (auto *MemberCallExpr = dyn_cast<CXXMemberCallExpr>(CE)) {
         if (auto *MD = MemberCallExpr->getMethodDecl()) {
@@ -102,8 +107,8 @@ class UncountedCallArgsChecker
         }
         auto *E = MemberCallExpr->getImplicitObjectArgument();
         QualType ArgType = MemberCallExpr->getObjectType().getCanonicalType();
-        std::optional<bool> IsUncounted = isUncounted(ArgType);
-        if (IsUncounted && *IsUncounted && !isPtrOriginSafe(E))
+        std::optional<bool> IsUnsafe = isUnsafeType(ArgType);
+        if (IsUnsafe && *IsUnsafe && !isPtrOriginSafe(E))
           reportBugOnThis(E, D);
       }
 
@@ -119,7 +124,7 @@ class UncountedCallArgsChecker
 
         QualType ArgType = (*P)->getType().getCanonicalType();
         // FIXME: more complex types (arrays, references to raw pointers, etc)
-        std::optional<bool> IsUncounted = isUncountedPtr(ArgType);
+        std::optional<bool> IsUncounted = isUnsafePtr(ArgType);
         if (!IsUncounted || !(*IsUncounted))
           continue;
 
@@ -265,7 +270,7 @@ class UncountedCallArgsChecker
       Os << " for parameter ";
       printQuotedQualifiedName(Os, Param);
     }
-    Os << " is uncounted and unsafe.";
+    Os << " is " << ptrKind() << " and unsafe.";
 
     const SourceLocation SrcLocToReport =
         isa<CXXDefaultArgExpr>(CallArg) ? Param->getDefaultArg()->getExprLoc()
@@ -283,15 +288,53 @@ class UncountedCallArgsChecker
 
     const SourceLocation SrcLocToReport = CallArg->getSourceRange().getBegin();
 
+    SmallString<100> Buf;
+    llvm::raw_svector_ostream Os(Buf);
+    Os << "Call argument for 'this' parameter is " << ptrKind();
+    Os << " and unsafe.";
+
     PathDiagnosticLocation BSLoc(SrcLocToReport, BR->getSourceManager());
-    auto Report = std::make_unique<BasicBugReport>(
-        Bug, "Call argument for 'this' parameter is uncounted and unsafe.",
-        BSLoc);
+    auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
     Report->addRange(CallArg->getSourceRange());
     Report->setDeclWithIssue(DeclWithIssue);
     BR->emitReport(std::move(Report));
   }
 };
+
+class UncountedCallArgsChecker final : public RawPtrRefCallArgsChecker {
+public:
+  UncountedCallArgsChecker()
+      : RawPtrRefCallArgsChecker("Uncounted call argument for a raw "
+                                 "pointer/reference parameter") {}
+
+  std::optional<bool> isUnsafeType(QualType QT) const final {
+    return isUncounted(QT);
+  }
+
+  std::optional<bool> isUnsafePtr(QualType QT) const final {
+    return isUncountedPtr(QT);
+  }
+
+  const char *ptrKind() const final { return "uncounted"; }
+};
+
+class UncheckedCallArgsChecker final : public RawPtrRefCallArgsChecker {
+public:
+  UncheckedCallArgsChecker()
+      : RawPtrRefCallArgsChecker("Unchecked call argument for a raw "
+                                 "pointer/reference parameter") {}
+
+  std::optional<bool> isUnsafeType(QualType QT) const final {
+    return isUnchecked(QT);
+  }
+
+  std::optional<bool> isUnsafePtr(QualType QT) const final {
+    return isUncheckedPtr(QT);
+  }
+
+  const char *ptrKind() const final { return "unchecked"; }
+};
+
 } // namespace
 
 void ento::registerUncountedCallArgsChecker(CheckerManager &Mgr) {
@@ -301,3 +344,11 @@ void ento::registerUncountedCallArgsChecker(CheckerManager &Mgr) {
 bool ento::shouldRegisterUncountedCallArgsChecker(const CheckerManager &) {
   return true;
 }
+
+void ento::registerUncheckedCallArgsChecker(CheckerManager &Mgr) {
+  Mgr.registerChecker<UncheckedCallArgsChecker>();
+}
+
+bool ento::shouldRegisterUncheckedCallArgsChecker(const CheckerManager &) {
+  return true;
+}