Merge pull request #11408 from rniwa/merge-webkit-checker-fixes-2025-09-23

adrian-prantl · web-flow · commit 88cb9d6ac1c9 · 2025-09-14T13:01:28.000-07:00
Merge webkit checker fixes 2025 09 23
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -91,6 +91,13 @@ bool tryToFindPtrOrigin(
       continue;
     }
     if (auto *call = dyn_cast<CallExpr>(E)) {
+      if (auto *Callee = call->getCalleeDecl()) {
+        if (Callee->hasAttr<CFReturnsRetainedAttr>() ||
+            Callee->hasAttr<NSReturnsRetainedAttr>()) {
+          return callback(E, true);
+        }
+      }
+
       if (auto *memberCall = dyn_cast<CXXMemberCallExpr>(call)) {
         if (auto *decl = memberCall->getMethodDecl()) {
           std::optional<bool> IsGetterOfRefCt = isGetterOfSafePtr(decl);
@@ -153,6 +160,29 @@ bool tryToFindPtrOrigin(
         if (Name == "__builtin___CFStringMakeConstantString" ||
             Name == "NSClassFromString")
           return callback(E, true);
+      } else if (auto *CalleeE = call->getCallee()) {
+        if (auto *E = dyn_cast<DeclRefExpr>(CalleeE->IgnoreParenCasts())) {
+          if (isSingleton(E->getFoundDecl()))
+            return callback(E, true);
+        }
+      }
+
+      // Sometimes, canonical type erroneously turns Ref<T> into T.
+      // Workaround this problem by checking again if the original type was
+      // a SubstTemplateTypeParmType of a safe smart pointer type (e.g. Ref).
+      if (auto *CalleeDecl = call->getCalleeDecl()) {
+        if (auto *FD = dyn_cast<FunctionDecl>(CalleeDecl)) {
+          auto RetType = FD->getReturnType();
+          if (auto *Subst = dyn_cast<SubstTemplateTypeParmType>(RetType)) {
+            if (auto *SubstType = Subst->desugar().getTypePtr()) {
+              if (auto *RD = dyn_cast<RecordType>(SubstType)) {
+                if (auto *CXX = dyn_cast<CXXRecordDecl>(RD->getDecl()))
+                  if (isSafePtr(CXX))
+                    return callback(E, true);
+              }
+            }
+          }
+        }
       }
     }
     if (auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E)) {
@@ -208,13 +238,25 @@ bool isASafeCallArg(const Expr *E) {
         return true;
     }
   }
+  if (isa<CXXTemporaryObjectExpr>(E))
+    return true; // A temporary lives until the end of this statement.
   if (isConstOwnerPtrMemberExpr(E))
     return true;
 
   // TODO: checker for method calls on non-refcounted objects
   return isa<CXXThisExpr>(E);
 }
 
+bool isNullPtr(const clang::Expr *E) {
+  if (isa<CXXNullPtrLiteralExpr>(E) || isa<GNUNullExpr>(E))
+    return true;
+  if (auto *Int = dyn_cast_or_null<IntegerLiteral>(E)) {
+    if (Int->getValue().isZero())
+      return true;
+  }
+  return false;
+}
+
 bool isConstOwnerPtrMemberExpr(const clang::Expr *E) {
   if (auto *MCE = dyn_cast<CXXMemberCallExpr>(E)) {
     if (auto *Callee = MCE->getDirectCallee()) {
@@ -273,7 +315,7 @@ class EnsureFunctionVisitor
   bool VisitReturnStmt(const ReturnStmt *RS) {
     if (auto *RV = RS->getRetValue()) {
       RV = RV->IgnoreParenCasts();
-      if (isa<CXXNullPtrLiteralExpr>(RV))
+      if (isNullPtr(RV))
         return true;
       return isConstOwnerPtrMemberExpr(RV);
     }
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
@@ -66,6 +66,9 @@ bool tryToFindPtrOrigin(
 /// \returns Whether \p E is a safe call arugment.
 bool isASafeCallArg(const clang::Expr *E);
 
+/// \returns true if E is nullptr or __null.
+bool isNullPtr(const clang::Expr *E);
+
 /// \returns true if E is a MemberExpr accessing a const smart pointer type.
 bool isConstOwnerPtrMemberExpr(const clang::Expr *E);
 
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp
@@ -272,7 +272,7 @@ class ForwardDeclChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
           ArgExpr = ArgExpr->IgnoreParenCasts();
       }
     }
-    if (isa<CXXNullPtrLiteralExpr>(ArgExpr) || isa<IntegerLiteral>(ArgExpr) ||
+    if (isNullPtr(ArgExpr) || isa<IntegerLiteral>(ArgExpr) ||
         isa<CXXDefaultArgExpr>(ArgExpr))
       return;
     if (auto *DRE = dyn_cast<DeclRefExpr>(ArgExpr)) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -492,7 +492,7 @@ bool isTrivialBuiltinFunction(const FunctionDecl *F) {
          Name.starts_with("os_log") || Name.starts_with("_os_log");
 }
 
-bool isSingleton(const FunctionDecl *F) {
+bool isSingleton(const NamedDecl *F) {
   assert(F);
   // FIXME: check # of params == 1
   if (auto *MethodDecl = dyn_cast<CXXMethodDecl>(F)) {
@@ -679,6 +679,10 @@ class TrivialFunctionAnalysisVisitor
     return IsFunctionTrivial(Callee);
   }
 
+  bool VisitGCCAsmStmt(const GCCAsmStmt *AS) {
+    return AS->getAsmString() == "brk #0xc471";
+  }
+
   bool
   VisitSubstNonTypeTemplateParmExpr(const SubstNonTypeTemplateParmExpr *E) {
     // Non-type template paramter is compile time constant and trivial.
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -21,6 +21,7 @@ class CXXMethodDecl;
 class CXXRecordDecl;
 class Decl;
 class FunctionDecl;
+class NamedDecl;
 class QualType;
 class RecordType;
 class Stmt;
@@ -156,7 +157,7 @@ bool isPtrConversion(const FunctionDecl *F);
 bool isTrivialBuiltinFunction(const FunctionDecl *F);
 
 /// \returns true if \p F is a static singleton function.
-bool isSingleton(const FunctionDecl *F);
+bool isSingleton(const NamedDecl *F);
 
 /// An inter-procedural analysis facility that detects functions with "trivial"
 /// behavior with respect to reference counting, such as simple field getters.
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
@@ -217,13 +217,11 @@ class RawPtrRefCallArgsChecker
         [&](const clang::Expr *ArgOrigin, bool IsSafe) {
           if (IsSafe)
             return true;
-          if (isa<CXXNullPtrLiteralExpr>(ArgOrigin)) {
-            // foo(nullptr)
+          if (isNullPtr(ArgOrigin))
             return true;
-          }
           if (isa<IntegerLiteral>(ArgOrigin)) {
             // FIXME: Check the value.
-            // foo(NULL)
+            // foo(123)
             return true;
           }
           if (isa<ObjCStringLiteral>(ArgOrigin))
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
@@ -232,14 +232,11 @@ class RawPtrRefLambdaCapturesChecker
         if (!Init)
           return nullptr;
         if (auto *Lambda = dyn_cast<LambdaExpr>(Init)) {
+          DeclRefExprsToIgnore.insert(DRE);
           updateIgnoreList();
           return Lambda;
         }
-        TempExpr = dyn_cast<CXXBindTemporaryExpr>(Init->IgnoreParenCasts());
-        if (!TempExpr)
-          return nullptr;
-        updateIgnoreList();
-        return dyn_cast_or_null<LambdaExpr>(TempExpr->getSubExpr());
+        return nullptr;
       }
 
       void checkCalleeLambda(CallExpr *CE) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
@@ -295,7 +295,7 @@ class RawPtrRefLocalVarsChecker
                 if (isa<CXXThisExpr>(InitArgOrigin))
                   return true;
 
-                if (isa<CXXNullPtrLiteralExpr>(InitArgOrigin))
+                if (isNullPtr(InitArgOrigin))
                   return true;
 
                 if (isa<IntegerLiteral>(InitArgOrigin))
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
@@ -177,7 +177,8 @@ class RetainPtrCtorAdoptChecker
       CreateOrCopyFnCall.insert(Arg); // Avoid double reporting.
       return;
     }
-    if (Result == IsOwnedResult::Owned || Result == IsOwnedResult::Skip) {
+    if (Result == IsOwnedResult::Owned || Result == IsOwnedResult::Skip ||
+        isNullPtr(Arg)) {
       CreateOrCopyFnCall.insert(Arg);
       return;
     }
@@ -486,7 +487,7 @@ class RetainPtrCtorAdoptChecker
           continue;
         }
       }
-      if (isa<CXXNullPtrLiteralExpr>(E))
+      if (isNullPtr(E))
         return IsOwnedResult::NotOwned;
       if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
         auto QT = DRE->getType();
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-checked-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-checked-const-member.cpp
@@ -71,12 +71,21 @@ class Foo {
     return m_obj5.get();
   }
 
+  CheckedObj* ensureObj6() {
+    if (!m_obj6)
+      const_cast<std::unique_ptr<CheckedObj>&>(m_obj6) = new CheckedObj;
+    if (m_obj6->next())
+      return (CheckedObj *)0;
+    return m_obj6.get();
+  }
+
 private:
   const std::unique_ptr<CheckedObj> m_obj1;
   std::unique_ptr<CheckedObj> m_obj2;
   const std::unique_ptr<CheckedObj> m_obj3;
   const std::unique_ptr<CheckedObj> m_obj4;
   const std::unique_ptr<CheckedObj> m_obj5;
+  const std::unique_ptr<CheckedObj> m_obj6;
 };
 
 void Foo::bar() {
@@ -87,6 +96,7 @@ void Foo::bar() {
   badEnsureObj4().method();
   // expected-warning@-1{{Call argument for 'this' parameter is unchecked and unsafe}}
   ensureObj5()->method();
+  ensureObj6()->method();
 }
 
 } // namespace call_args_const_unique_ptr
diff --git a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm
@@ -13,6 +13,8 @@ void basic_correct() {
   auto ns4 = adoptNS([ns3 mutableCopy]);
   auto ns5 = adoptNS([ns3 copyWithValue:3]);
   auto ns6 = retainPtr([ns3 next]);
+  auto ns7 = retainPtr((SomeObj *)0);
+  auto ns8 = adoptNS(nil);
   CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10));
   auto cf2 = adoptCF(SecTaskCreateFromSelf(kCFAllocatorDefault));
   auto cf3 = adoptCF(checked_cf_cast<CFArrayRef>(CFCopyArray(cf1)));
@@ -111,6 +113,10 @@ - (void)setValue:value {
   return adoptCF(rawBuffer);
 }
 
+RetainPtr<SomeObj> return_nil() {
+  return nil;
+}
+
 RetainPtr<SomeObj> return_nullptr() {
   return nullptr;
 }
diff --git a/clang/test/Analysis/Checkers/WebKit/template-wrapper-call-arg.cpp b/clang/test/Analysis/Checkers/WebKit/template-wrapper-call-arg.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
+// expected-no-diagnostics
+
+#include "mock-types.h"
+
+struct Obj {
+  void ref() const;
+  void deref() const;
+
+  void someFunction();
+};
+
+template<typename T> class Wrapper {
+public:
+  T obj();
+};
+
+static void foo(Wrapper<Ref<Obj>>&& wrapper)
+{
+  wrapper.obj()->someFunction();
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/trivial-code-check-asm-brk.cpp b/clang/test/Analysis/Checkers/WebKit/trivial-code-check-asm-brk.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 -triple arm-darwin -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
+// expected-no-diagnostics
+
+void crash()
+{
+  __asm__ volatile ("brk #0xc471");
+  __builtin_unreachable();
+}
+
+class SomeObj {
+public:
+  void ref();
+  void deref();
+
+  void someWork() { crash(); }
+};
+
+SomeObj* provide();
+
+void doSomeWork() {
+  provide()->someWork();
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/unchecked-call-arg.cpp b/clang/test/Analysis/Checkers/WebKit/unchecked-call-arg.cpp
@@ -32,3 +32,8 @@ void foo() {
   provide()->doWork();
   // expected-warning@-1{{Call argument for 'this' parameter is unchecked and unsafe}}
 }
+
+void doWorkWithObject(const CheckedObject&);
+void bar() {
+  doWorkWithObject(CheckedObject());
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
@@ -448,4 +448,27 @@ void ranges_for_each(RefCountable* obj) {
     obj->method();
     ++(*static_cast<unsigned*>(item));
   });
-}
+}
+
+class RefCountedObj {
+public:
+  void ref();
+  void deref();
+
+  void call() const;
+  void callLambda([[clang::noescape]] const WTF::Function<void ()>& callback) const;
+  void doSomeWork() const;
+};
+
+void RefCountedObj::callLambda([[clang::noescape]] const WTF::Function<void ()>& callback) const
+{
+    callback();
+}
+
+void RefCountedObj::call() const
+{
+    auto lambda = [&] {
+        doSomeWork();
+    };
+    callLambda(lambda);
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
@@ -121,6 +121,7 @@ void foo8(RefCountable* obj) {
     RefCountable *bar = foo->trivial() ? foo.get() : nullptr;
     // expected-warning@-1{{Local variable 'bar' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
     foo = nullptr;
+    foo = (RefCountable *)0;
     bar->method();
   }
 }
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm

Original file line number	Diff line number	Diff line change
`@@ -272,7 +272,7 @@ class ForwardDeclChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {`
`272`	`272`	`ArgExpr = ArgExpr->IgnoreParenCasts();`
`273`	`273`	`}`
`274`	`274`	`}`
`275`		`- if (isa<CXXNullPtrLiteralExpr>(ArgExpr) \|\| isa<IntegerLiteral>(ArgExpr) \|\|`
	`275`	`+ if (isNullPtr(ArgExpr) \|\| isa<IntegerLiteral>(ArgExpr) \|\|`
`276`	`276`	`isa<CXXDefaultArgExpr>(ArgExpr))`
`277`	`277`	`return;`
`278`	`278`	`if (auto *DRE = dyn_cast<DeclRefExpr>(ArgExpr)) {`
Original file line number	Diff line number	Diff line change
`@@ -177,7 +177,8 @@ class RetainPtrCtorAdoptChecker`
`177`	`177`	`CreateOrCopyFnCall.insert(Arg); // Avoid double reporting.`
`178`	`178`	`return;`
`179`	`179`	`}`
`180`		`- if (Result == IsOwnedResult::Owned \|\| Result == IsOwnedResult::Skip) {`
	`180`	`+ if (Result == IsOwnedResult::Owned \|\| Result == IsOwnedResult::Skip \|\|`
	`181`	`+ isNullPtr(Arg)) {`
`181`	`182`	`CreateOrCopyFnCall.insert(Arg);`
`182`	`183`	`return;`
`183`	`184`	`}`
`@@ -486,7 +487,7 @@ class RetainPtrCtorAdoptChecker`
`486`	`487`	`continue;`
`487`	`488`	`}`
`488`	`489`	`}`
`489`		`- if (isa<CXXNullPtrLiteralExpr>(E))`
	`490`	`+ if (isNullPtr(E))`
`490`	`491`	`return IsOwnedResult::NotOwned;`
`491`	`492`	`if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {`
`492`	`493`	`auto QT = DRE->getType();`