[alpha.webkit.RetainPtrCtorAdoptChecker] Support adopt(cast(copy(~)) (llvm#132316)

This PR adds the support for recognizing calling adoptCF/adoptNS on the
result of a cast operation on the return value of a function which
creates NS or CF types. It also fixes a bug that we weren't reporting
memory leaks when CF types are created without ever calling RetainPtr's
constructor, adoptCF, or adoptNS.

To do this, this PR adds a new mechanism to report a memory leak
whenever create or copy CF functions are invoked unless this CallExpr
has already been visited while validating a call to adoptCF. Also added
an early exit when isOwned returns IsOwnedResult::Skip due to an
unresolved template argument.

Author: rniwa

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

@@ -125,6 +125,16 @@ bool isCheckedPtr(const std::string &Name) {
   return Name == "CheckedPtr" || Name == "CheckedRef";
 }
 
+bool isSmartPtrClass(const std::string &Name) {
+  return isRefType(Name) || isCheckedPtr(Name) || isRetainPtr(Name) ||
+         Name == "WeakPtr" || Name == "WeakPtr" || Name == "WeakPtrFactory" ||
+         Name == "WeakPtrFactoryWithBitField" || Name == "WeakPtrImplBase" ||
+         Name == "WeakPtrImplBaseSingleThread" || Name == "ThreadSafeWeakPtr" ||
+         Name == "ThreadSafeWeakOrStrongPtr" ||
+         Name == "ThreadSafeWeakPtrControlBlock" ||
+         Name == "ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr";
+}
+
 bool isCtorOfRefCounted(const clang::FunctionDecl *F) {
   assert(F);
   const std::string &FunctionName = safeGetName(F);
@@ -222,8 +232,13 @@ void RetainTypeChecker::visitTypedef(const TypedefDecl *TD) {
 
   auto PointeeQT = QT->getPointeeType();
   const RecordType *RT = PointeeQT->getAs<RecordType>();
-  if (!RT)
+  if (!RT) {
+    if (TD->hasAttr<ObjCBridgeAttr>() || TD->hasAttr<ObjCBridgeMutableAttr>()) {
+      if (auto *Type = TD->getTypeForDecl())
+        RecordlessTypes.insert(Type);
+    }
     return;
+  }
 
   for (auto *Redecl : RT->getDecl()->getMostRecentDecl()->redecls()) {
     if (Redecl->getAttr<ObjCBridgeAttr>() ||
@@ -240,6 +255,17 @@ bool RetainTypeChecker::isUnretained(const QualType QT, bool ignoreARC) {
   auto CanonicalType = QT.getCanonicalType();
   auto PointeeType = CanonicalType->getPointeeType();
   auto *RT = dyn_cast_or_null<RecordType>(PointeeType.getTypePtrOrNull());
+  if (!RT) {
+    auto *Type = QT.getTypePtrOrNull();
+    while (Type) {
+      if (RecordlessTypes.contains(Type))
+        return true;
+      auto *ET = dyn_cast_or_null<ElaboratedType>(Type);
+      if (!ET)
+        break;
+      Type = ET->desugar().getTypePtrOrNull();
+    }
+  }
   return RT && CFPointees.contains(RT);
 }
 

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

@@ -70,6 +70,7 @@ std::optional<bool> isUnchecked(const clang::QualType T);
 /// underlying pointer type.
 class RetainTypeChecker {
   llvm::DenseSet<const RecordType *> CFPointees;
+  llvm::DenseSet<const Type *> RecordlessTypes;
   bool IsARCEnabled{false};
 
 public:
@@ -135,6 +136,9 @@ bool isCheckedPtr(const std::string &Name);
 /// \returns true if \p Name is RetainPtr or its variant, false if not.
 bool isRetainPtr(const std::string &Name);
 
+/// \returns true if \p Name is a smart pointer type name, false if not.
+bool isSmartPtrClass(const std::string &Name);
+
 /// \returns true if \p M is getter of a ref-counted class, false if not.
 std::optional<bool> isGetterOfSafePtr(const clang::CXXMethodDecl *Method);
 

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

@@ -72,7 +72,7 @@ class RawPtrRefCallArgsChecker
       bool shouldVisitImplicitCode() const { return false; }
 
       bool TraverseClassTemplateDecl(ClassTemplateDecl *Decl) {
-        if (isRefType(safeGetName(Decl)))
+        if (isSmartPtrClass(safeGetName(Decl)))
           return true;
         return Base::TraverseClassTemplateDecl(Decl);
       }

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

@@ -267,6 +267,12 @@ class RawPtrRefLocalVarsChecker
           return Base::TraverseCompoundStmt(CS);
         return true;
       }
+
+      bool TraverseClassTemplateDecl(ClassTemplateDecl *Decl) {
+        if (isSmartPtrClass(safeGetName(Decl)))
+          return true;
+        return Base::TraverseClassTemplateDecl(Decl);
+      }
     };
 
     LocalVisitor visitor(this);

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

@@ -34,6 +34,7 @@ class RetainPtrCtorAdoptChecker
   mutable BugReporter *BR;
   mutable std::unique_ptr<RetainSummaryManager> Summaries;
   mutable llvm::DenseSet<const ValueDecl *> CreateOrCopyOutArguments;
+  mutable llvm::DenseSet<const Expr *> CreateOrCopyFnCall;
   mutable RetainTypeChecker RTC;
 
 public:
@@ -119,7 +120,7 @@ class RetainPtrCtorAdoptChecker
       return;
 
     if (!isAdoptFn(F) || !CE->getNumArgs()) {
-      rememberOutArguments(CE, F);
+      checkCreateOrCopyFunction(CE, F, DeclWithIssue);
       return;
     }
 
@@ -128,24 +129,29 @@ class RetainPtrCtorAdoptChecker
     auto Name = safeGetName(F);
     if (Result == IsOwnedResult::Unknown)
       Result = IsOwnedResult::NotOwned;
-    if (Result == IsOwnedResult::NotOwned && !isAllocInit(Arg) &&
-        !isCreateOrCopy(Arg)) {
-      if (auto *DRE = dyn_cast<DeclRefExpr>(Arg)) {
-        if (CreateOrCopyOutArguments.contains(DRE->getDecl()))
-          return;
-      }
-      if (RTC.isARCEnabled() && isAdoptNS(F))
-        reportUseAfterFree(Name, CE, DeclWithIssue, "when ARC is disabled");
-      else
-        reportUseAfterFree(Name, CE, DeclWithIssue);
+    if (isAllocInit(Arg) || isCreateOrCopy(Arg)) {
+      CreateOrCopyFnCall.insert(Arg); // Avoid double reporting.
+      return;
     }
+    if (Result == IsOwnedResult::Owned || Result == IsOwnedResult::Skip)
+      return;
+
+    if (auto *DRE = dyn_cast<DeclRefExpr>(Arg)) {
+      if (CreateOrCopyOutArguments.contains(DRE->getDecl()))
+        return;
+    }
+    if (RTC.isARCEnabled() && isAdoptNS(F))
+      reportUseAfterFree(Name, CE, DeclWithIssue, "when ARC is disabled");
+    else
+      reportUseAfterFree(Name, CE, DeclWithIssue);
   }
 
-  void rememberOutArguments(const CallExpr *CE,
-                            const FunctionDecl *Callee) const {
+  void checkCreateOrCopyFunction(const CallExpr *CE, const FunctionDecl *Callee,
+                                 const Decl *DeclWithIssue) const {
     if (!isCreateOrCopyFunction(Callee))
       return;
 
+    bool hasOutArgument = false;
     unsigned ArgCount = CE->getNumArgs();
     for (unsigned ArgIndex = 0; ArgIndex < ArgCount; ++ArgIndex) {
       auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts();
@@ -164,7 +170,12 @@ class RetainPtrCtorAdoptChecker
       if (!Decl)
         continue;
       CreateOrCopyOutArguments.insert(Decl);
+      hasOutArgument = true;
     }
+    if (!RTC.isUnretained(Callee->getReturnType()))
+      return;
+    if (!hasOutArgument && !CreateOrCopyFnCall.contains(CE))
+      reportLeak(CE, DeclWithIssue);
   }
 
   void visitConstructExpr(const CXXConstructExpr *CE,
@@ -190,6 +201,13 @@ class RetainPtrCtorAdoptChecker
     std::string Name = "RetainPtr constructor";
     auto *Arg = CE->getArg(0)->IgnoreParenCasts();
     auto Result = isOwned(Arg);
+
+    if (isCreateOrCopy(Arg))
+      CreateOrCopyFnCall.insert(Arg); // Avoid double reporting.
+
+    if (Result == IsOwnedResult::Skip)
+      return;
+
     if (Result == IsOwnedResult::Unknown)
       Result = IsOwnedResult::NotOwned;
     if (Result == IsOwnedResult::Owned)
@@ -315,11 +333,22 @@ class RetainPtrCtorAdoptChecker
         if (auto *Callee = CE->getDirectCallee()) {
           if (isAdoptFn(Callee))
             return IsOwnedResult::NotOwned;
-          if (safeGetName(Callee) == "__builtin___CFStringMakeConstantString")
+          auto Name = safeGetName(Callee);
+          if (Name == "__builtin___CFStringMakeConstantString")
             return IsOwnedResult::NotOwned;
+          if ((Name == "checked_cf_cast" || Name == "dynamic_cf_cast" ||
+               Name == "checked_objc_cast" || Name == "dynamic_objc_cast") &&
+              CE->getNumArgs() == 1) {
+            E = CE->getArg(0)->IgnoreParenCasts();
+            continue;
+          }
           auto RetType = Callee->getReturnType();
           if (isRetainPtrType(RetType))
             return IsOwnedResult::NotOwned;
+          if (isCreateOrCopyFunction(Callee)) {
+            CreateOrCopyFnCall.insert(CE);
+            return IsOwnedResult::Owned;
+          }
         } else if (auto *CalleeExpr = CE->getCallee()) {
           if (isa<CXXDependentScopeMemberExpr>(CalleeExpr))
             return IsOwnedResult::Skip; // Wait for instantiation.
@@ -383,6 +412,20 @@ class RetainPtrCtorAdoptChecker
     Report->setDeclWithIssue(DeclWithIssue);
     BR->emitReport(std::move(Report));
   }
+
+  void reportLeak(const CallExpr *CE, const Decl *DeclWithIssue) const {
+    SmallString<100> Buf;
+    llvm::raw_svector_ostream Os(Buf);
+
+    Os << "The return value is +1 and results in a memory leak.";
+
+    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
 

clang/test/Analysis/Checkers/WebKit/objc-mock-types.h

@@ -24,6 +24,9 @@ typedef struct CF_BRIDGED_MUTABLE_TYPE(CFRunLoopRef) __CFRunLoop * CFRunLoopRef;
 
 extern const CFAllocatorRef kCFAllocatorDefault;
 typedef struct _NSZone NSZone;
+
+CFTypeID CFGetTypeID(CFTypeRef cf);
+
 CFTypeID CFGetTypeID(CFTypeRef cf);
 CFTypeID CFArrayGetTypeID();
 CFMutableArrayRef CFArrayCreateMutable(CFAllocatorRef allocator, CFIndex capacity);
@@ -281,12 +284,12 @@ template<typename T> inline RetainPtr<T> retainPtr(T* ptr)
 
 inline NSObject *bridge_cast(CFTypeRef object)
 {
-    return (__bridge NSObject *)object;
+  return (__bridge NSObject *)object;
 }
 
 inline CFTypeRef bridge_cast(NSObject *object)
 {
-    return (__bridge CFTypeRef)object;
+  return (__bridge CFTypeRef)object;
 }
 
 inline id bridge_id_cast(CFTypeRef object)
@@ -386,35 +389,35 @@ template <typename> struct CFTypeTrait;
 
 template<typename T> T dynamic_cf_cast(CFTypeRef object)
 {
-    if (!object)
-        return nullptr;
+  if (!object)
+    return nullptr;
 
-    if (CFGetTypeID(object) != CFTypeTrait<T>::typeID())
-        return nullptr;
+  if (CFGetTypeID(object) != CFTypeTrait<T>::typeID())
+    return nullptr;
 
-    return static_cast<T>(const_cast<CF_BRIDGED_TYPE(id) void*>(object));
+  return static_cast<T>(const_cast<CF_BRIDGED_TYPE(id) void*>(object));
 }
 
 template<typename T> T checked_cf_cast(CFTypeRef object)
 {
-    if (!object)
-        return nullptr;
+  if (!object)
+    return nullptr;
 
-    if (CFGetTypeID(object) != CFTypeTrait<T>::typeID())
-      WTFCrash();
+  if (CFGetTypeID(object) != CFTypeTrait<T>::typeID())
+    WTFCrash();
 
-    return static_cast<T>(const_cast<CF_BRIDGED_TYPE(id) void*>(object));
+  return static_cast<T>(const_cast<CF_BRIDGED_TYPE(id) void*>(object));
 }
 
 template<typename T, typename U> RetainPtr<T> dynamic_cf_cast(RetainPtr<U>&& object)
 {
-    if (!object)
-        return nullptr;
+  if (!object)
+    return nullptr;
 
-    if (CFGetTypeID(object.get()) != CFTypeTrait<T>::typeID())
-        return nullptr;
+  if (CFGetTypeID(object.get()) != CFTypeTrait<T>::typeID())
+    return nullptr;
 
-    return adoptCF(static_cast<T>(const_cast<CF_BRIDGED_TYPE(id) void*>(object.leakRef())));
+  return adoptCF(static_cast<T>(const_cast<CF_BRIDGED_TYPE(id) void*>(object.leakRef())));
 }
 
 } // namespace WTF
@@ -448,4 +451,4 @@ using WTF::is_objc;
 using WTF::checked_objc_cast;
 using WTF::dynamic_objc_cast;
 using WTF::checked_cf_cast;
-using WTF::dynamic_cf_cast;
+using WTF::dynamic_cf_cast;

clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm

@@ -2,6 +2,8 @@
 
 #include "objc-mock-types.h"
 
+CFTypeRef CFCopyArray(CFArrayRef);
+
 void basic_correct() {
   auto ns1 = adoptNS([SomeObj alloc]);
   auto ns2 = adoptNS([[SomeObj alloc] init]);
@@ -11,6 +13,7 @@ void basic_correct() {
   auto ns6 = retainPtr([ns3 next]);
   CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10));
   auto cf2 = adoptCF(SecTaskCreateFromSelf(kCFAllocatorDefault));
+  auto cf3 = adoptCF(checked_cf_cast<CFArrayRef>(CFCopyArray(cf1)));
 }
 
 CFMutableArrayRef provide_cf();
@@ -26,6 +29,8 @@ void basic_wrong() {
   // expected-warning@-1{{Incorrect use of adoptCF. The argument is +0 and results in an use-after-free [alpha.webkit.RetainPtrCtorAdoptChecker]}}
   RetainPtr<CFTypeRef> cf3 = SecTaskCreateFromSelf(kCFAllocatorDefault);
   // expected-warning@-1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+  CFCopyArray(cf1);
+  // expected-warning@-1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
 }
 
 RetainPtr<CVPixelBufferRef> cf_out_argument() {
@@ -67,7 +72,7 @@ void adopt_retainptr() {
 
 class MemberInit {
 public:
-  MemberInit(CFMutableArrayRef array, NSString *str, CFRunLoopRef runLoop)
+  MemberInit(RetainPtr<CFMutableArrayRef>&& array, NSString *str, CFRunLoopRef runLoop)
     : m_array(array)
     , m_str(str)
     , m_runLoop(runLoop)
@@ -79,7 +84,7 @@ void adopt_retainptr() {
   RetainPtr<CFRunLoopRef> m_runLoop;
 };
 void create_member_init() {
-  MemberInit init { CFArrayCreateMutable(kCFAllocatorDefault, 10), @"hello", CFRunLoopGetCurrent() };
+  MemberInit init { adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10)), @"hello", CFRunLoopGetCurrent() };
 }
 
 RetainPtr<CFStringRef> cfstr() {

clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm

@@ -2,6 +2,9 @@
 
 #include "objc-mock-types.h"
 
+CFTypeRef CFCopyArray(CFArrayRef);
+void* CreateCopy();
+
 void basic_correct() {
   auto ns1 = adoptNS([SomeObj alloc]);
   auto ns2 = adoptNS([[SomeObj alloc] init]);
@@ -11,6 +14,8 @@ void basic_correct() {
   auto ns6 = retainPtr([ns3 next]);
   CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10));
   auto cf2 = adoptCF(SecTaskCreateFromSelf(kCFAllocatorDefault));
+  auto cf3 = adoptCF(checked_cf_cast<CFArrayRef>(CFCopyArray(cf1)));
+  CreateCopy();
 }
 
 CFMutableArrayRef provide_cf();
@@ -26,6 +31,8 @@ void basic_wrong() {
   // expected-warning@-1{{Incorrect use of adoptCF. The argument is +0 and results in an use-after-free [alpha.webkit.RetainPtrCtorAdoptChecker]}}
   RetainPtr<CFTypeRef> cf3 = SecTaskCreateFromSelf(kCFAllocatorDefault);
   // expected-warning@-1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+  CFCopyArray(cf1);
+  // expected-warning@-1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
 }
 
 RetainPtr<CVPixelBufferRef> cf_out_argument() {
@@ -67,7 +74,7 @@ void adopt_retainptr() {
 
 class MemberInit {
 public:
-  MemberInit(CFMutableArrayRef array, NSString *str, CFRunLoopRef runLoop)
+  MemberInit(RetainPtr<CFMutableArrayRef>&& array, NSString *str, CFRunLoopRef runLoop)
     : m_array(array)
     , m_str(str)
     , m_runLoop(runLoop)
@@ -79,7 +86,7 @@ void adopt_retainptr() {
   RetainPtr<CFRunLoopRef> m_runLoop;
 };
 void create_member_init() {
-  MemberInit init { CFArrayCreateMutable(kCFAllocatorDefault, 10), @"hello", CFRunLoopGetCurrent() };
+  MemberInit init { adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10)), @"hello", CFRunLoopGetCurrent() };
 }
 
 RetainPtr<CFStringRef> cfstr() {