https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/128651
>From dc53e0602fcec63bdd1bc84325ecc16a3f3e293b Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <rn...@webkit.org> Date: Mon, 24 Feb 2025 23:39:13 -0800 Subject: [PATCH 1/3] [alpha.webkit.UnretainedLambdaCapturesChecker] Add a WebKit checker for lambda capturing NS or CF types. Add a new WebKit checker for checking that lambda captures of CF types use RetainPtr either when ARC is disabled or enabled, and those of NS types use RetainPtr when ARC is disabled. --- clang/docs/analyzer/checkers.rst | 12 + .../clang/StaticAnalyzer/Checkers/Checkers.td | 4 + .../StaticAnalyzer/Checkers/CMakeLists.txt | 4 +- ...cpp => RawPtrRefLambdaCapturesChecker.cpp} | 136 ++++++-- .../Checkers/WebKit/objc-mock-types.h | 146 ++++++++- ...mbda-captures-decl-protects-this-crash.cpp | 4 +- .../WebKit/uncounted-lambda-captures.cpp | 34 +- .../WebKit/unretained-lambda-captures-arc.mm | 273 ++++++++++++++++ .../WebKit/unretained-lambda-captures.mm | 296 ++++++++++++++++++ .../lib/StaticAnalyzer/Checkers/BUILD.gn | 2 +- 10 files changed, 851 insertions(+), 60 deletions(-) rename clang/lib/StaticAnalyzer/Checkers/WebKit/{UncountedLambdaCapturesChecker.cpp => RawPtrRefLambdaCapturesChecker.cpp} (77%) create mode 100644 clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-arc.mm create mode 100644 clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures.mm diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index c1eedb33e74d2..57733d45167f5 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -3487,6 +3487,18 @@ Raw pointers and references to an object which supports CheckedPtr or CheckedRef See `WebKit Guidelines for Safer C++ Programming <https://github.com/WebKit/WebKit/wiki/Safer-CPP-Guidelines>`_ for details. +alpha.webkit.UnretainedLambdaCapturesChecker +"""""""""""""""""""""""""""""""""""""""""""" +Raw pointers and references to unretained types can't be captured in lambdas. Only RetainPtr is allowed. + +.. code-block:: cpp + + void foo(NSObject *a, NSObject *b) { + [&, a](){ // warn about 'a' + do_something(b); // warn about 'b' + }; + }; + .. _alpha-webkit-UncountedCallArgsChecker: alpha.webkit.UncountedCallArgsChecker diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 410f841630660..06aa083671b7a 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -1766,6 +1766,10 @@ def NoUncheckedPtrMemberChecker : Checker<"NoUncheckedPtrMemberChecker">, HelpText<"Check for no unchecked member variables.">, Documentation<HasDocumentation>; +def UnretainedLambdaCapturesChecker : Checker<"UnretainedLambdaCapturesChecker">, + HelpText<"Check unretained lambda captures.">, + Documentation<HasDocumentation>; + def UncountedCallArgsChecker : Checker<"UncountedCallArgsChecker">, HelpText<"Check uncounted call arguments.">, Documentation<HasDocumentation>; diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index 5910043440987..9aac200cd7370 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -128,14 +128,14 @@ add_clang_library(clangStaticAnalyzerCheckers VLASizeChecker.cpp ValistChecker.cpp VirtualCallChecker.cpp - WebKit/RawPtrRefMemberChecker.cpp WebKit/ASTUtils.cpp WebKit/MemoryUnsafeCastChecker.cpp WebKit/PtrTypesSemantics.cpp WebKit/RefCntblBaseVirtualDtorChecker.cpp WebKit/RawPtrRefCallArgsChecker.cpp - WebKit/UncountedLambdaCapturesChecker.cpp + WebKit/RawPtrRefLambdaCapturesChecker.cpp WebKit/RawPtrRefLocalVarsChecker.cpp + WebKit/RawPtrRefMemberChecker.cpp LINK_LIBS clangAST diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp similarity index 77% rename from clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp rename to clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp index 506442f352288..314b3dc72c8e5 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp @@ -21,15 +21,23 @@ using namespace clang; using namespace ento; namespace { -class UncountedLambdaCapturesChecker +class RawPtrRefLambdaCapturesChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> { private: - BugType Bug{this, "Lambda capture of uncounted variable", - "WebKit coding guidelines"}; + BugType Bug; mutable BugReporter *BR = nullptr; TrivialFunctionAnalysis TFA; +protected: + mutable std::optional<RetainTypeChecker> RTC; + public: + RawPtrRefLambdaCapturesChecker(const char *description) + : Bug(this, description, "WebKit coding guidelines") {} + + virtual std::optional<bool> isUnsafePtr(QualType) const = 0; + virtual const char *ptrKind(QualType QT) const = 0; + void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR, BugReporter &BRArg) const { BR = &BRArg; @@ -38,7 +46,7 @@ class UncountedLambdaCapturesChecker // visit template instantiations or lambda classes. We // want to visit those, so we make our own RecursiveASTVisitor. struct LocalVisitor : DynamicRecursiveASTVisitor { - const UncountedLambdaCapturesChecker *Checker; + const RawPtrRefLambdaCapturesChecker *Checker; llvm::DenseSet<const DeclRefExpr *> DeclRefExprsToIgnore; llvm::DenseSet<const LambdaExpr *> LambdasToIgnore; llvm::DenseSet<const ValueDecl *> ProtectedThisDecls; @@ -46,7 +54,7 @@ class UncountedLambdaCapturesChecker QualType ClsType; - explicit LocalVisitor(const UncountedLambdaCapturesChecker *Checker) + explicit LocalVisitor(const RawPtrRefLambdaCapturesChecker *Checker) : Checker(Checker) { assert(Checker); ShouldVisitTemplateInstantiations = true; @@ -60,16 +68,23 @@ class UncountedLambdaCapturesChecker return DynamicRecursiveASTVisitor::TraverseCXXMethodDecl(CXXMD); } + bool VisitTypedefDecl(TypedefDecl *TD) override { + if (Checker->RTC) + Checker->RTC->visitTypedef(TD); + return true; + } + bool shouldCheckThis() { - auto result = - !ClsType.isNull() ? isUnsafePtr(ClsType, false) : std::nullopt; + auto result = !ClsType.isNull() ? + Checker->isUnsafePtr(ClsType) : std::nullopt; return result && *result; } bool VisitLambdaExpr(LambdaExpr *L) override { if (LambdasToIgnore.contains(L)) return true; - Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L)); + Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L), + ClsType); return true; } @@ -97,7 +112,8 @@ class UncountedLambdaCapturesChecker if (!L) return true; LambdasToIgnore.insert(L); - Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L)); + Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L), + ClsType); return true; } @@ -123,7 +139,8 @@ class UncountedLambdaCapturesChecker LambdasToIgnore.insert(L); if (!Param->hasAttr<NoEscapeAttr>()) Checker->visitLambdaExpr(L, shouldCheckThis() && - !hasProtectedThis(L)); + !hasProtectedThis(L), + ClsType); } ++ArgIndex; } @@ -144,7 +161,8 @@ class UncountedLambdaCapturesChecker LambdasToIgnore.insert(L); if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape) Checker->visitLambdaExpr(L, shouldCheckThis() && - !hasProtectedThis(L)); + !hasProtectedThis(L), + ClsType); } ++ArgIndex; } @@ -169,14 +187,22 @@ class UncountedLambdaCapturesChecker auto *CtorArg = CE->getArg(0)->IgnoreParenCasts(); if (!CtorArg) return nullptr; - if (auto *Lambda = dyn_cast<LambdaExpr>(CtorArg)) { + auto *InnerCE = dyn_cast_or_null<CXXConstructExpr>(CtorArg); + if (InnerCE && InnerCE->getNumArgs()) + CtorArg = InnerCE->getArg(0)->IgnoreParenCasts(); + auto updateIgnoreList = [&] { ConstructToIgnore.insert(CE); + if (InnerCE) + ConstructToIgnore.insert(InnerCE); + }; + if (auto *Lambda = dyn_cast<LambdaExpr>(CtorArg)) { + updateIgnoreList(); return Lambda; } if (auto *TempExpr = dyn_cast<CXXBindTemporaryExpr>(CtorArg)) { E = TempExpr->getSubExpr()->IgnoreParenCasts(); if (auto *Lambda = dyn_cast<LambdaExpr>(E)) { - ConstructToIgnore.insert(CE); + updateIgnoreList(); return Lambda; } } @@ -189,10 +215,14 @@ class UncountedLambdaCapturesChecker auto *Init = VD->getInit(); if (!Init) return nullptr; + if (auto *Lambda = dyn_cast<LambdaExpr>(Init)) { + updateIgnoreList(); + return Lambda; + } TempExpr = dyn_cast<CXXBindTemporaryExpr>(Init->IgnoreParenCasts()); if (!TempExpr) return nullptr; - ConstructToIgnore.insert(CE); + updateIgnoreList(); return dyn_cast_or_null<LambdaExpr>(TempExpr->getSubExpr()); } @@ -226,7 +256,7 @@ class UncountedLambdaCapturesChecker DeclRefExprsToIgnore.insert(ArgRef); LambdasToIgnore.insert(L); Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L), - /* ignoreParamVarDecl */ true); + ClsType, /* ignoreParamVarDecl */ true); } bool hasProtectedThis(LambdaExpr *L) { @@ -293,10 +323,13 @@ class UncountedLambdaCapturesChecker }; LocalVisitor visitor(this); + if (RTC) + RTC->visitTranslationUnitDecl(TUD); visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD)); } void visitLambdaExpr(LambdaExpr *L, bool shouldCheckThis, + const QualType T, bool ignoreParamVarDecl = false) const { if (TFA.isTrivial(L->getBody())) return; @@ -306,13 +339,13 @@ class UncountedLambdaCapturesChecker if (ignoreParamVarDecl && isa<ParmVarDecl>(CapturedVar)) continue; QualType CapturedVarQualType = CapturedVar->getType(); - auto IsUncountedPtr = isUnsafePtr(CapturedVar->getType(), false); + auto IsUncountedPtr = isUnsafePtr(CapturedVar->getType()); if (IsUncountedPtr && *IsUncountedPtr) reportBug(C, CapturedVar, CapturedVarQualType); } else if (C.capturesThis() && shouldCheckThis) { if (ignoreParamVarDecl) // this is always a parameter to this function. continue; - reportBugOnThisPtr(C); + reportBugOnThisPtr(C, T); } } } @@ -320,6 +353,9 @@ class UncountedLambdaCapturesChecker void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar, const QualType T) const { assert(CapturedVar); + + if (isa<ImplicitParamDecl>(CapturedVar) && !Capture.getLocation().isValid()) + return; // Ignore implicit captruing of self. SmallString<100> Buf; llvm::raw_svector_ostream Os(Buf); @@ -329,22 +365,22 @@ class UncountedLambdaCapturesChecker } else { Os << "Implicitly captured "; } - if (T->isPointerType()) { + if (isa<PointerType>(T) || isa<ObjCObjectPointerType>(T)) { Os << "raw-pointer "; } else { - assert(T->isReferenceType()); Os << "reference "; } - printQuotedQualifiedName(Os, Capture.getCapturedVar()); - Os << " to ref-counted type or CheckedPtr-capable type is unsafe."; + printQuotedQualifiedName(Os, CapturedVar); + Os << " to " << ptrKind(T) << " type is unsafe."; PathDiagnosticLocation BSLoc(Capture.getLocation(), BR->getSourceManager()); auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc); BR->emitReport(std::move(Report)); } - void reportBugOnThisPtr(const LambdaCapture &Capture) const { + void reportBugOnThisPtr(const LambdaCapture &Capture, + const QualType T) const { SmallString<100> Buf; llvm::raw_svector_ostream Os(Buf); @@ -354,14 +390,57 @@ class UncountedLambdaCapturesChecker Os << "Implicitly captured "; } - Os << "raw-pointer 'this' to ref-counted type or CheckedPtr-capable type " - "is unsafe."; + Os << "raw-pointer 'this' to " << ptrKind(T) << " type is unsafe."; PathDiagnosticLocation BSLoc(Capture.getLocation(), BR->getSourceManager()); auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc); BR->emitReport(std::move(Report)); } }; + +class UncountedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker { +public: + UncountedLambdaCapturesChecker() + : RawPtrRefLambdaCapturesChecker("Lambda capture of uncounted or " + "unchecked variable") {} + + std::optional<bool> isUnsafePtr(QualType QT) const final { + auto result1 = isUncountedPtr(QT); + auto result2 = isUncheckedPtr(QT); + if (result1 && *result1) + return true; + if (result2 && *result2) + return true; + if (result1) + return *result1; + return result2; + } + + const char *ptrKind(QualType QT) const final { + if (isUncounted(QT)) + return "uncounted"; + return "unchecked"; + } + +}; + +class UnretainedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker { +public: + UnretainedLambdaCapturesChecker() + : RawPtrRefLambdaCapturesChecker("Lambda capture of unretained " + "variables") { + RTC = RetainTypeChecker(); + } + + std::optional<bool> isUnsafePtr(QualType QT) const final { + return RTC->isUnretained(QT); + } + + const char *ptrKind(QualType QT) const final { + return "unretained"; + } +}; + } // namespace void ento::registerUncountedLambdaCapturesChecker(CheckerManager &Mgr) { @@ -372,3 +451,12 @@ bool ento::shouldRegisterUncountedLambdaCapturesChecker( const CheckerManager &mgr) { return true; } + +void ento::registerUnretainedLambdaCapturesChecker(CheckerManager &Mgr) { + Mgr.registerChecker<UnretainedLambdaCapturesChecker>(); +} + +bool ento::shouldRegisterUnretainedLambdaCapturesChecker( + 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; +- (NSString *)stringByAppendingString:(NSString *)aString; +- ( const char *)UTF8String; +- (id)initWithUTF8String:(const char *)nullTerminatedCString; ++ (id)stringWithUTF8String:(const char *)nullTerminatedCString; +@end + +@interface NSMutableString : NSString +@end + +@interface NSValue : NSObject <NSCopying> +- (void)getValue:(void *)value; +@end + +@interface NSNumber : NSValue +- (char)charValue; +- (id)initWithInt:(int)value; ++ (NSNumber *)numberWithInt:(int)value; @end @interface SomeObj : NSObject +- (SomeObj *)mutableCopy; +- (SomeObj *)copyWithValue:(int)value; - (void)doWork; - (SomeObj *)other; - (SomeObj *)next; @@ -57,28 +121,34 @@ template <typename T> struct RetainPtr { PtrType t; RetainPtr() : t(nullptr) { } - RetainPtr(PtrType t) : t(t) { if (t) - CFRetain(t); + CFRetain(toCFTypeRef(t)); } - RetainPtr(RetainPtr&& o) - : RetainPtr(o.t) - { - o.t = nullptr; - } - RetainPtr(const RetainPtr& o) + RetainPtr(RetainPtr&&); + RetainPtr(const RetainPtr&); + template <typename U> + RetainPtr(const RetainPtr<U>& o) : RetainPtr(o.t) + {} + RetainPtr operator=(const RetainPtr& o) { + if (t) + CFRelease(toCFTypeRef(t)); + t = o.t; + if (t) + CFRetain(toCFTypeRef(t)); + return *this; } - RetainPtr operator=(const RetainPtr& o) + template <typename U> + RetainPtr operator=(const RetainPtr<U>& o) { if (t) - CFRelease(t); + CFRelease(toCFTypeRef(t)); t = o.t; if (t) - CFRetain(t); + CFRetain(toCFTypeRef(t)); return *this; } ~RetainPtr() { @@ -86,7 +156,7 @@ template <typename T> struct RetainPtr { } void clear() { if (t) - CFRelease(t); + CFRelease(toCFTypeRef(t)); t = nullptr; } void swap(RetainPtr& o) { @@ -102,10 +172,19 @@ template <typename T> struct RetainPtr { swap(o); return *this; } + PtrType leakRef() + { + PtrType s = t; + t = nullptr; + return s; + } operator PtrType() const { return t; } operator bool() const { return t; } private: + CFTypeRef toCFTypeRef(id ptr) { return (__bridge CFTypeRef)ptr; } + CFTypeRef toCFTypeRef(const void* ptr) { return (CFTypeRef)ptr; } + template <typename U> friend RetainPtr<U> adoptNS(U*); template <typename U> friend RetainPtr<U> adoptCF(U); @@ -113,9 +192,26 @@ template <typename T> struct RetainPtr { RetainPtr(PtrType t, AdoptTag) : t(t) { } }; +template <typename T> +RetainPtr<T>::RetainPtr(RetainPtr<T>&& o) + : RetainPtr(o.t) +{ + o.t = nullptr; +} + +template <typename T> +RetainPtr<T>::RetainPtr(const RetainPtr<T>& o) + : RetainPtr(o.t) +{ +} + template <typename T> RetainPtr<T> adoptNS(T* t) { +#if __has_feature(objc_arc) + return t; +#else return RetainPtr<T>(t, RetainPtr<T>::Adopt); +#endif } template <typename T> @@ -123,9 +219,31 @@ RetainPtr<T> adoptCF(T t) { return RetainPtr<T>(t, RetainPtr<T>::Adopt); } +template<typename T> inline RetainPtr<T> retainPtr(T ptr) +{ + return ptr; +} + +template<typename T> inline RetainPtr<T> retainPtr(T* ptr) +{ + return ptr; +} + +inline NSObject *bridge_cast(CFTypeRef object) +{ + return (__bridge NSObject *)object; +} + +inline CFTypeRef bridge_cast(NSObject *object) +{ + return (__bridge CFTypeRef)object; +} + } using WTF::RetainPtr; using WTF::adoptNS; using WTF::adoptCF; +using WTF::retainPtr; using WTF::downcast; +using WTF::bridge_cast; \ No newline at end of file diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures-decl-protects-this-crash.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures-decl-protects-this-crash.cpp index 840433db5133a..0c10c69a97eda 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures-decl-protects-this-crash.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures-decl-protects-this-crash.cpp @@ -28,9 +28,9 @@ struct Obj { void foo(Foo foo) { bar([this](auto baz) { - // expected-warning@-1{{Captured raw-pointer 'this' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured raw-pointer 'this' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} bar([this, foo = *baz, foo2 = !baz](auto&&) { - // expected-warning@-1{{Captured raw-pointer 'this' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured raw-pointer 'this' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} someFunction(); }); }); diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp index 82058bf13d137..1746d2b93d469 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp @@ -94,22 +94,22 @@ void callAsync(const WTF::Function<void()>&); void raw_ptr() { RefCountable* ref_countable = make_obj(); auto foo1 = [ref_countable](){ - // expected-warning@-1{{Captured raw-pointer 'ref_countable' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} ref_countable->method(); }; auto foo2 = [&ref_countable](){ - // expected-warning@-1{{Captured raw-pointer 'ref_countable' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} ref_countable->method(); }; auto foo3 = [&](){ ref_countable->method(); - // expected-warning@-1{{Implicitly captured raw-pointer 'ref_countable' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Implicitly captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} ref_countable = nullptr; }; auto foo4 = [=](){ ref_countable->method(); - // expected-warning@-1{{Implicitly captured raw-pointer 'ref_countable' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Implicitly captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} }; call(foo1); @@ -128,13 +128,13 @@ void references() { RefCountable automatic; RefCountable& ref_countable_ref = automatic; auto foo1 = [ref_countable_ref](){ ref_countable_ref.constMethod(); }; - // expected-warning@-1{{Captured reference 'ref_countable_ref' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} auto foo2 = [&ref_countable_ref](){ ref_countable_ref.method(); }; - // expected-warning@-1{{Captured reference 'ref_countable_ref' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} auto foo3 = [&](){ ref_countable_ref.method(); }; - // expected-warning@-1{{Implicitly captured reference 'ref_countable_ref' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Implicitly captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} auto foo4 = [=](){ ref_countable_ref.constMethod(); }; - // expected-warning@-1{{Implicitly captured reference 'ref_countable_ref' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Implicitly captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} call(foo1); call(foo2); @@ -187,7 +187,7 @@ void noescape_lambda() { otherObj->method(); }, [&](RefCountable& obj) { otherObj->method(); - // expected-warning@-1{{Implicitly captured raw-pointer 'otherObj' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Implicitly captured raw-pointer 'otherObj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} }); ([&] { someObj->method(); @@ -217,7 +217,7 @@ struct RefCountableWithLambdaCapturingThis { void method_captures_this_unsafe() { auto lambda = [&]() { nonTrivial(); - // expected-warning@-1{{Implicitly captured raw-pointer 'this' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Implicitly captured raw-pointer 'this' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} }; call(lambda); } @@ -225,7 +225,7 @@ struct RefCountableWithLambdaCapturingThis { void method_captures_this_unsafe_capture_local_var_explicitly() { RefCountable* x = make_obj(); call([this, protectedThis = RefPtr { this }, x]() { - // expected-warning@-1{{Captured raw-pointer 'x' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured raw-pointer 'x' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} nonTrivial(); x->method(); }); @@ -234,7 +234,7 @@ struct RefCountableWithLambdaCapturingThis { void method_captures_this_with_other_protected_var() { RefCountable* x = make_obj(); call([this, protectedX = RefPtr { x }]() { - // expected-warning@-1{{Captured raw-pointer 'this' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured raw-pointer 'this' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} nonTrivial(); protectedX->method(); }); @@ -243,7 +243,7 @@ struct RefCountableWithLambdaCapturingThis { void method_captures_this_unsafe_capture_local_var_explicitly_with_deref() { RefCountable* x = make_obj(); call([this, protectedThis = Ref { *this }, x]() { - // expected-warning@-1{{Captured raw-pointer 'x' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured raw-pointer 'x' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} nonTrivial(); x->method(); }); @@ -252,7 +252,7 @@ struct RefCountableWithLambdaCapturingThis { void method_captures_this_unsafe_local_var_via_vardecl() { RefCountable* x = make_obj(); auto lambda = [this, protectedThis = Ref { *this }, x]() { - // expected-warning@-1{{Captured raw-pointer 'x' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured raw-pointer 'x' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} nonTrivial(); x->method(); }; @@ -330,7 +330,7 @@ struct RefCountableWithLambdaCapturingThis { void method_nested_lambda3() { callAsync([this, protectedThis = RefPtr { this }] { callAsync([this] { - // expected-warning@-1{{Captured raw-pointer 'this' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured raw-pointer 'this' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} nonTrivial(); }); }); @@ -380,10 +380,10 @@ void lambda_converted_to_function(RefCountable* obj) { callFunction([&]() { obj->method(); - // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} }); callFunctionOpaque([&]() { obj->method(); - // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} }); } diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-arc.mm new file mode 100644 index 0000000000000..4e3d9c2708d96 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-arc.mm @@ -0,0 +1,273 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedLambdaCapturesChecker -fobjc-arc -verify %s + +#include "objc-mock-types.h" + +namespace std { + +template <typename T> +class unique_ptr { +private: + T *t; + +public: + unique_ptr() : t(nullptr) { } + unique_ptr(T *t) : t(t) { } + ~unique_ptr() { + if (t) + delete t; + } + template <typename U> unique_ptr(unique_ptr<U>&& u) + : t(u.t) + { + u.t = nullptr; + } + T *get() const { return t; } + T *operator->() const { return t; } + T &operator*() const { return *t; } + unique_ptr &operator=(T *) { return *this; } + explicit operator bool() const { return !!t; } +}; + +}; + +namespace WTF { + +namespace Detail { + +template<typename Out, typename... In> +class CallableWrapperBase { +public: + virtual ~CallableWrapperBase() { } + virtual Out call(In...) = 0; +}; + +template<typename, typename, typename...> class CallableWrapper; + +template<typename CallableType, typename Out, typename... In> +class CallableWrapper : public CallableWrapperBase<Out, In...> { +public: + explicit CallableWrapper(CallableType& callable) + : m_callable(callable) { } + Out call(In... in) final { return m_callable(in...); } + +private: + CallableType m_callable; +}; + +} // namespace Detail + +template<typename> class Function; + +template<typename Out, typename... In> Function<Out(In...)> adopt(Detail::CallableWrapperBase<Out, In...>*); + +template <typename Out, typename... In> +class Function<Out(In...)> { +public: + using Impl = Detail::CallableWrapperBase<Out, In...>; + + Function() = default; + + template<typename FunctionType> + Function(FunctionType f) + : m_callableWrapper(new Detail::CallableWrapper<FunctionType, Out, In...>(f)) { } + + Out operator()(In... in) const { return m_callableWrapper->call(in...); } + explicit operator bool() const { return !!m_callableWrapper; } + +private: + enum AdoptTag { Adopt }; + Function(Impl* impl, AdoptTag) + : m_callableWrapper(impl) + { + } + + friend Function adopt<Out, In...>(Impl*); + + std::unique_ptr<Impl> m_callableWrapper; +}; + +template<typename Out, typename... In> Function<Out(In...)> adopt(Detail::CallableWrapperBase<Out, In...>* impl) +{ + return Function<Out(In...)>(impl, Function<Out(In...)>::Adopt); +} + +template <typename KeyType, typename ValueType> +class HashMap { +public: + HashMap(); + HashMap([[clang::noescape]] const Function<ValueType()>&); + void ensure(const KeyType&, [[clang::noescape]] const Function<ValueType()>&); + bool operator+([[clang::noescape]] const Function<ValueType()>&) const; + static void ifAny(HashMap, [[clang::noescape]] const Function<bool(ValueType)>&); + +private: + ValueType* m_table { nullptr }; +}; + +} // namespace WTF + +struct A { + static void b(); +}; + +SomeObj* make_obj(); +CFMutableArrayRef make_cf(); + +void someFunction(); +template <typename Callback> void call(Callback callback) { + someFunction(); + callback(); +} +void callAsync(const WTF::Function<void()>&); + +void raw_ptr() { + SomeObj* obj = make_obj(); + auto foo1 = [obj](){ + [obj doWork]; + }; + call(foo1); + + auto foo2 = [&obj](){ + [obj doWork]; + }; + auto foo3 = [&](){ + [obj doWork]; + obj = nullptr; + }; + auto foo4 = [=](){ + [obj doWork]; + }; + + auto cf = make_cf(); + auto bar1 = [cf](){ + // expected-warning@-1{{Captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + CFArrayAppendValue(cf, nullptr); + }; + auto bar2 = [&cf](){ + // expected-warning@-1{{Captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + CFArrayAppendValue(cf, nullptr); + }; + auto bar3 = [&](){ + CFArrayAppendValue(cf, nullptr); + // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + cf = nullptr; + }; + auto bar4 = [=](){ + CFArrayAppendValue(cf, nullptr); + // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }; + + call(foo1); + call(foo2); + call(foo3); + call(foo4); + + call(bar1); + call(bar2); + call(bar3); + call(bar4); +} + +void quiet() { +// This code is not expected to trigger any warnings. + SomeObj *obj; + + auto foo3 = [&]() {}; + auto foo4 = [=]() {}; + + call(foo3); + call(foo4); + + obj = nullptr; +} + +template <typename Callback> +void map(SomeObj* start, [[clang::noescape]] Callback&& callback) +{ + while (start) { + callback(start); + start = [start next]; + } +} + +template <typename Callback1, typename Callback2> +void doubleMap(SomeObj* start, [[clang::noescape]] Callback1&& callback1, Callback2&& callback2) +{ + while (start) { + callback1(start); + callback2(start); + start = [start next]; + } +} + +template <typename Callback1, typename Callback2> +void get_count_cf(CFArrayRef array, [[clang::noescape]] Callback1&& callback1, Callback2&& callback2) +{ + auto count = CFArrayGetCount(array); + callback1(count); + callback2(count); +} + +void noescape_lambda() { + SomeObj* someObj = make_obj(); + SomeObj* otherObj = make_obj(); + map(make_obj(), [&](SomeObj *obj) { + [otherObj doWork]; + }); + doubleMap(make_obj(), [&](SomeObj *obj) { + [otherObj doWork]; + }, [&](SomeObj *obj) { + [otherObj doWork]; + }); + ([&] { + [someObj doWork]; + })(); + + CFMutableArrayRef someCF = make_cf(); + get_count_cf(make_cf(), [&](CFIndex count) { + CFArrayAppendValue(someCF, nullptr); + }, [&](CFIndex count) { + CFArrayAppendValue(someCF, nullptr); + // expected-warning@-1{{Implicitly captured reference 'someCF' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }); +} + +void callFunctionOpaque(WTF::Function<void()>&&); +void callFunction(WTF::Function<void()>&& function) { + someFunction(); + function(); +} + +void lambda_converted_to_function(SomeObj* obj, CFMutableArrayRef cf) +{ + callFunction([&]() { + [obj doWork]; + CFArrayAppendValue(cf, nullptr); + // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }); + callFunctionOpaque([&]() { + [obj doWork]; + CFArrayAppendValue(cf, nullptr); + // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }); +} + +@interface ObjWithSelf : NSObject { + RetainPtr<id> delegate; +} +-(void)doWork; +-(void)run; +@end + +@implementation ObjWithSelf +-(void)doWork { + auto doWork = [&] { + someFunction(); + [delegate doWork]; + }; + callFunctionOpaque(doWork); +} +-(void)run { + someFunction(); +} +@end \ No newline at end of file diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures.mm b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures.mm new file mode 100644 index 0000000000000..073eff9386baa --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures.mm @@ -0,0 +1,296 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedLambdaCapturesChecker -verify %s + +#include "objc-mock-types.h" + +namespace std { + +template <typename T> +class unique_ptr { +private: + T *t; + +public: + unique_ptr() : t(nullptr) { } + unique_ptr(T *t) : t(t) { } + ~unique_ptr() { + if (t) + delete t; + } + template <typename U> unique_ptr(unique_ptr<U>&& u) + : t(u.t) + { + u.t = nullptr; + } + T *get() const { return t; } + T *operator->() const { return t; } + T &operator*() const { return *t; } + unique_ptr &operator=(T *) { return *this; } + explicit operator bool() const { return !!t; } +}; + +}; + +namespace WTF { + +namespace Detail { + +template<typename Out, typename... In> +class CallableWrapperBase { +public: + virtual ~CallableWrapperBase() { } + virtual Out call(In...) = 0; +}; + +template<typename, typename, typename...> class CallableWrapper; + +template<typename CallableType, typename Out, typename... In> +class CallableWrapper : public CallableWrapperBase<Out, In...> { +public: + explicit CallableWrapper(CallableType& callable) + : m_callable(callable) { } + Out call(In... in) final { return m_callable(in...); } + +private: + CallableType m_callable; +}; + +} // namespace Detail + +template<typename> class Function; + +template<typename Out, typename... In> Function<Out(In...)> adopt(Detail::CallableWrapperBase<Out, In...>*); + +template <typename Out, typename... In> +class Function<Out(In...)> { +public: + using Impl = Detail::CallableWrapperBase<Out, In...>; + + Function() = default; + + template<typename FunctionType> + Function(FunctionType f) + : m_callableWrapper(new Detail::CallableWrapper<FunctionType, Out, In...>(f)) { } + + Out operator()(In... in) const { return m_callableWrapper->call(in...); } + explicit operator bool() const { return !!m_callableWrapper; } + +private: + enum AdoptTag { Adopt }; + Function(Impl* impl, AdoptTag) + : m_callableWrapper(impl) + { + } + + friend Function adopt<Out, In...>(Impl*); + + std::unique_ptr<Impl> m_callableWrapper; +}; + +template<typename Out, typename... In> Function<Out(In...)> adopt(Detail::CallableWrapperBase<Out, In...>* impl) +{ + return Function<Out(In...)>(impl, Function<Out(In...)>::Adopt); +} + +template <typename KeyType, typename ValueType> +class HashMap { +public: + HashMap(); + HashMap([[clang::noescape]] const Function<ValueType()>&); + void ensure(const KeyType&, [[clang::noescape]] const Function<ValueType()>&); + bool operator+([[clang::noescape]] const Function<ValueType()>&) const; + static void ifAny(HashMap, [[clang::noescape]] const Function<bool(ValueType)>&); + +private: + ValueType* m_table { nullptr }; +}; + +} // namespace WTF + +struct A { + static void b(); +}; + +SomeObj* make_obj(); +CFMutableArrayRef make_cf(); + +void someFunction(); +template <typename Callback> void call(Callback callback) { + someFunction(); + callback(); +} +void callAsync(const WTF::Function<void()>&); + +void raw_ptr() { + SomeObj* obj = make_obj(); + auto foo1 = [obj](){ + // expected-warning@-1{{Captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + [obj doWork]; + }; + call(foo1); + + auto foo2 = [&obj](){ + // expected-warning@-1{{Captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + [obj doWork]; + }; + auto foo3 = [&](){ + [obj doWork]; + // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + obj = nullptr; + }; + auto foo4 = [=](){ + [obj doWork]; + // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }; + + auto cf = make_cf(); + auto bar1 = [cf](){ + // expected-warning@-1{{Captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + CFArrayAppendValue(cf, nullptr); + }; + auto bar2 = [&cf](){ + // expected-warning@-1{{Captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + CFArrayAppendValue(cf, nullptr); + }; + auto bar3 = [&](){ + CFArrayAppendValue(cf, nullptr); + // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + cf = nullptr; + }; + auto bar4 = [=](){ + CFArrayAppendValue(cf, nullptr); + // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }; + + call(foo1); + call(foo2); + call(foo3); + call(foo4); + + call(bar1); + call(bar2); + call(bar3); + call(bar4); + + // Confirm that the checker respects [[clang::suppress]]. + SomeObj* suppressed_obj = nullptr; + [[clang::suppress]] auto foo5 = [suppressed_obj](){ + [suppressed_obj doWork]; + }; + // no warning. + call(foo5); + + // Confirm that the checker respects [[clang::suppress]]. + CFMutableArrayRef suppressed_cf = nullptr; + [[clang::suppress]] auto bar5 = [suppressed_cf](){ + CFArrayAppendValue(suppressed_cf, nullptr); + }; + // no warning. + call(bar5); +} + +void quiet() { +// This code is not expected to trigger any warnings. + SomeObj *obj; + + auto foo3 = [&]() {}; + auto foo4 = [=]() {}; + + call(foo3); + call(foo4); + + obj = nullptr; +} + +template <typename Callback> +void map(SomeObj* start, [[clang::noescape]] Callback&& callback) +{ + while (start) { + callback(start); + start = [start next]; + } +} + +template <typename Callback1, typename Callback2> +void doubleMap(SomeObj* start, [[clang::noescape]] Callback1&& callback1, Callback2&& callback2) +{ + while (start) { + callback1(start); + callback2(start); + start = [start next]; + } +} + +template <typename Callback1, typename Callback2> +void get_count_cf(CFArrayRef array, [[clang::noescape]] Callback1&& callback1, Callback2&& callback2) +{ + auto count = CFArrayGetCount(array); + callback1(count); + callback2(count); +} + +void noescape_lambda() { + SomeObj* someObj = make_obj(); + SomeObj* otherObj = make_obj(); + map(make_obj(), [&](SomeObj *obj) { + [otherObj doWork]; + }); + doubleMap(make_obj(), [&](SomeObj *obj) { + [otherObj doWork]; + }, [&](SomeObj *obj) { + [otherObj doWork]; + // expected-warning@-1{{Implicitly captured raw-pointer 'otherObj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }); + ([&] { + [someObj doWork]; + })(); + + CFMutableArrayRef someCF = make_cf(); + get_count_cf(make_cf(), [&](CFIndex count) { + CFArrayAppendValue(someCF, nullptr); + }, [&](CFIndex count) { + CFArrayAppendValue(someCF, nullptr); + // expected-warning@-1{{Implicitly captured reference 'someCF' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }); +} + +void callFunctionOpaque(WTF::Function<void()>&&); +void callFunction(WTF::Function<void()>&& function) { + someFunction(); + function(); +} + +void lambda_converted_to_function(SomeObj* obj, CFMutableArrayRef cf) +{ + callFunction([&]() { + [obj doWork]; + // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + CFArrayAppendValue(cf, nullptr); + // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }); + callFunctionOpaque([&]() { + [obj doWork]; + // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + CFArrayAppendValue(cf, nullptr); + // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }); +} + +@interface ObjWithSelf : NSObject { + RetainPtr<id> delegate; +} +-(void)doWork; +-(void)run; +@end + +@implementation ObjWithSelf +-(void)doWork { + auto doWork = [&] { + someFunction(); + [delegate doWork]; + }; + callFunctionOpaque(doWork); +} +-(void)run { + someFunction(); +} +@end \ No newline at end of file diff --git a/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn b/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn index d9c3257536639..856fce4d655bb 100644 --- a/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn +++ b/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn @@ -140,10 +140,10 @@ static_library("Checkers") { "WebKit/MemoryUnsafeCastChecker.cpp", "WebKit/PtrTypesSemantics.cpp", "WebKit/RawPtrRefCallArgsChecker.cpp", + "WebKit/RawPtrRefLambdaCapturesChecker.cpp", "WebKit/RawPtrRefLocalVarsChecker.cpp", "WebKit/RawPtrRefMemberChecker.cpp", "WebKit/RefCntblBaseVirtualDtorChecker.cpp", - "WebKit/UncountedLambdaCapturesChecker.cpp", "cert/InvalidPtrChecker.cpp", ] } >From edba566bbfd0e2be53210842c02fb9d4d44f5d85 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <rn...@webkit.org> Date: Mon, 24 Feb 2025 23:57:21 -0800 Subject: [PATCH 2/3] Fix formatting. --- .../WebKit/RawPtrRefLambdaCapturesChecker.cpp | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp index 314b3dc72c8e5..4a297a6c0ab91 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp @@ -75,8 +75,8 @@ class RawPtrRefLambdaCapturesChecker } bool shouldCheckThis() { - auto result = !ClsType.isNull() ? - Checker->isUnsafePtr(ClsType) : std::nullopt; + auto result = + !ClsType.isNull() ? Checker->isUnsafePtr(ClsType) : std::nullopt; return result && *result; } @@ -138,9 +138,8 @@ class RawPtrRefLambdaCapturesChecker if (auto *L = findLambdaInArg(Arg)) { LambdasToIgnore.insert(L); if (!Param->hasAttr<NoEscapeAttr>()) - Checker->visitLambdaExpr(L, shouldCheckThis() && - !hasProtectedThis(L), - ClsType); + Checker->visitLambdaExpr( + L, shouldCheckThis() && !hasProtectedThis(L), ClsType); } ++ArgIndex; } @@ -160,9 +159,8 @@ class RawPtrRefLambdaCapturesChecker if (auto *L = findLambdaInArg(Arg)) { LambdasToIgnore.insert(L); if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape) - Checker->visitLambdaExpr(L, shouldCheckThis() && - !hasProtectedThis(L), - ClsType); + Checker->visitLambdaExpr( + L, shouldCheckThis() && !hasProtectedThis(L), ClsType); } ++ArgIndex; } @@ -328,8 +326,7 @@ class RawPtrRefLambdaCapturesChecker visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD)); } - void visitLambdaExpr(LambdaExpr *L, bool shouldCheckThis, - const QualType T, + void visitLambdaExpr(LambdaExpr *L, bool shouldCheckThis, const QualType T, bool ignoreParamVarDecl = false) const { if (TFA.isTrivial(L->getBody())) return; @@ -353,7 +350,7 @@ class RawPtrRefLambdaCapturesChecker void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar, const QualType T) const { assert(CapturedVar); - + if (isa<ImplicitParamDecl>(CapturedVar) && !Capture.getLocation().isValid()) return; // Ignore implicit captruing of self. @@ -421,7 +418,6 @@ class UncountedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker { return "uncounted"; return "unchecked"; } - }; class UnretainedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker { @@ -436,9 +432,7 @@ class UnretainedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker { return RTC->isUnretained(QT); } - const char *ptrKind(QualType QT) const final { - return "unretained"; - } + const char *ptrKind(QualType QT) const final { return "unretained"; } }; } // namespace >From 295596e3147ea2b1cfbe2537f5439f3806cd92e5 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <rn...@webkit.org> Date: Sun, 9 Mar 2025 10:12:02 -0700 Subject: [PATCH 3/3] Clarify the correct use of NS and CF types with respect to ARC --- clang/docs/analyzer/checkers.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 57733d45167f5..678554b0fcc28 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -3489,7 +3489,7 @@ See `WebKit Guidelines for Safer C++ Programming <https://github.com/WebKit/WebK alpha.webkit.UnretainedLambdaCapturesChecker """""""""""""""""""""""""""""""""""""""""""" -Raw pointers and references to unretained types can't be captured in lambdas. Only RetainPtr is allowed. +Raw pointers and references to NS or CF types can't be captured in lambdas. Only RetainPtr is allowed for CF types regardless of whether ARC is enabled or disabled, and only RetainPtr is allowed for NS types when ARC is disabled. .. code-block:: cpp _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits