diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index 319aacac188d5..c1a5000f8b647 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -26,6 +26,7 @@ bool tryToFindPtrOrigin( const Expr *E, bool StopAtFirstRefCountedObj, std::function isSafePtr, std::function isSafePtrType, + std::function isSafeGlobalDecl, std::function callback) { while (E) { if (auto *DRE = dyn_cast(E)) { @@ -34,6 +35,8 @@ bool tryToFindPtrOrigin( auto IsImmortal = safeGetName(VD) == "NSApp"; if (VD->hasGlobalStorage() && (IsImmortal || QT.isConstQualified())) return callback(E, true); + if (VD->hasGlobalStorage() && isSafeGlobalDecl(VD)) + return callback(E, true); } } if (auto *tempExpr = dyn_cast(E)) { @@ -71,9 +74,11 @@ bool tryToFindPtrOrigin( } if (auto *Expr = dyn_cast(E)) { return tryToFindPtrOrigin(Expr->getTrueExpr(), StopAtFirstRefCountedObj, - isSafePtr, isSafePtrType, callback) && + isSafePtr, isSafePtrType, isSafeGlobalDecl, + callback) && tryToFindPtrOrigin(Expr->getFalseExpr(), StopAtFirstRefCountedObj, - isSafePtr, isSafePtrType, callback); + isSafePtr, isSafePtrType, isSafeGlobalDecl, + callback); } if (auto *cast = dyn_cast(E)) { if (StopAtFirstRefCountedObj) { diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h index 3a009d65efea6..9fff456b7e8b8 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h @@ -56,6 +56,7 @@ bool tryToFindPtrOrigin( const clang::Expr *E, bool StopAtFirstRefCountedObj, std::function isSafePtr, std::function isSafePtrType, + std::function isSafeGlobalDecl, std::function callback); /// For \p E referring to a ref-countable/-counted pointer/reference we return diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp index 9585ceb40f95e..791e70998477f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp @@ -29,12 +29,12 @@ namespace { class RawPtrRefCallArgsChecker : public Checker> { BugType Bug; - mutable BugReporter *BR; TrivialFunctionAnalysis TFA; EnsureFunctionAnalysis EFA; protected: + mutable BugReporter *BR; mutable std::optional RTC; public: @@ -46,6 +46,7 @@ class RawPtrRefCallArgsChecker virtual bool isSafePtr(const CXXRecordDecl *Record) const = 0; virtual bool isSafePtrType(const QualType type) const = 0; virtual bool isSafeExpr(const Expr *) const { return false; } + virtual bool isSafeDecl(const Decl *) const { return false; } virtual const char *ptrKind() const = 0; void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR, @@ -214,6 +215,7 @@ class RawPtrRefCallArgsChecker Arg, /*StopAtFirstRefCountedObj=*/true, [&](const clang::CXXRecordDecl *Record) { return isSafePtr(Record); }, [&](const clang::QualType T) { return isSafePtrType(T); }, + [&](const clang::Decl *D) { return isSafeDecl(D); }, [&](const clang::Expr *ArgOrigin, bool IsSafe) { if (IsSafe) return true; @@ -479,6 +481,11 @@ class UnretainedCallArgsChecker final : public RawPtrRefCallArgsChecker { isa(E); } + bool isSafeDecl(const Decl *D) const final { + // Treat NS/CF globals in system header as immortal. + return BR->getSourceManager().isInSystemHeader(D->getLocation()); + } + const char *ptrKind() const final { return "unretained"; } }; diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp index dd9701fbbb017..c13df47920f72 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp @@ -166,10 +166,10 @@ bool isGuardedScopeEmbeddedInGuardianScope(const VarDecl *Guarded, class RawPtrRefLocalVarsChecker : public Checker> { BugType Bug; - mutable BugReporter *BR; EnsureFunctionAnalysis EFA; protected: + mutable BugReporter *BR; mutable std::optional RTC; public: @@ -180,6 +180,7 @@ class RawPtrRefLocalVarsChecker virtual bool isSafePtr(const CXXRecordDecl *) const = 0; virtual bool isSafePtrType(const QualType) const = 0; virtual bool isSafeExpr(const Expr *) const { return false; } + virtual bool isSafeDecl(const Decl *) const { return false; } virtual const char *ptrKind() const = 0; void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR, @@ -288,6 +289,7 @@ class RawPtrRefLocalVarsChecker return isSafePtr(Record); }, [&](const clang::QualType Type) { return isSafePtrType(Type); }, + [&](const clang::Decl *D) { return isSafeDecl(D); }, [&](const clang::Expr *InitArgOrigin, bool IsSafe) { if (!InitArgOrigin || IsSafe) return true; @@ -443,6 +445,10 @@ class UnretainedLocalVarsChecker final : public RawPtrRefLocalVarsChecker { return ento::cocoa::isCocoaObjectRef(E->getType()) && isa(E); } + bool isSafeDecl(const Decl *D) const final { + // Treat NS/CF globals in system header as immortal. + return BR->getSourceManager().isInSystemHeader(D->getLocation()); + } const char *ptrKind() const final { return "unretained"; } }; diff --git a/clang/test/Analysis/Checkers/WebKit/mock-system-header.h b/clang/test/Analysis/Checkers/WebKit/mock-system-header.h index 1e44de8eb62ad..d55b3abd34f4c 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-system-header.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-system-header.h @@ -34,6 +34,8 @@ void os_log_msg(os_log_t oslog, os_log_type_t type, const char *msg, ...); typedef const struct __attribute__((objc_bridge(NSString))) __CFString * CFStringRef; +extern CFStringRef const kCFURLTagNamesKey; + #ifdef __OBJC__ @class NSString; @interface SystemObject { @@ -41,4 +43,8 @@ typedef const struct __attribute__((objc_bridge(NSString))) __CFString * CFStrin CFStringRef cf_string; } @end + +typedef NSString *NSNotificationName; +extern "C" NSNotificationName NSApplicationDidBecomeActiveNotification; + #endif diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm index 72ba05e9e3a71..f49e7bdb3e79c 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm @@ -1,8 +1,11 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedLocalVarsChecker -verify %s #import "objc-mock-types.h" +#import "mock-system-header.h" void someFunction(); +extern "C" CFStringRef LocalGlobalCFString; +extern "C" NSString *LocalGlobalNSString; namespace raw_ptr { void foo() { @@ -547,6 +550,29 @@ void foo() { } // autoreleased +namespace ns_global { + +void consumeCFString(CFStringRef); +void consumeNSString(NSString *); + +void cf() { + auto *str = kCFURLTagNamesKey; + consumeCFString(str); + auto *localStr = LocalGlobalCFString; + // expected-warning@-1{{Local variable 'localStr' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + consumeCFString(localStr); +} + +void ns() { + auto *str = NSApplicationDidBecomeActiveNotification; + consumeNSString(str); + auto *localStr = LocalGlobalNSString; + // expected-warning@-1{{Local variable 'localStr' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + consumeNSString(localStr); +} + +} + bool doMoreWorkOpaque(OtherObj*); SomeObj* provide(); diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-obj-arg.mm b/clang/test/Analysis/Checkers/WebKit/unretained-obj-arg.mm new file mode 100644 index 0000000000000..5c78b21d6c94f --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unretained-obj-arg.mm @@ -0,0 +1,18 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedCallArgsChecker -verify %s + +#import "mock-types.h" +#import "mock-system-header.h" + +void consumeCFString(CFStringRef); +extern "C" CFStringRef LocalGlobalCFString; +void consumeNSString(NSString *); +extern "C" NSString *LocalGlobalNSString; + +void foo() { + consumeCFString(kCFURLTagNamesKey); + consumeCFString(LocalGlobalCFString); + // expected-warning@-1{{Call argument is unretained and unsafe}} + consumeNSString(NSApplicationDidBecomeActiveNotification); + consumeNSString(LocalGlobalNSString); + // expected-warning@-1{{Call argument is unretained and unsafe}} +}