[clang] 1127dd7 - [WebKit Checkers] Treat a NS/CF global defined in a system header as a safe pointer origin (#161146)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 14 16:52:22 PDT 2025
Author: Ryosuke Niwa
Date: 2025-10-14T16:52:19-07:00
New Revision: 1127dd775426238cdc57b2581fbb91a0252e06ae
URL: https://github.com/llvm/llvm-project/commit/1127dd775426238cdc57b2581fbb91a0252e06ae
DIFF: https://github.com/llvm/llvm-project/commit/1127dd775426238cdc57b2581fbb91a0252e06ae.diff
LOG: [WebKit Checkers] Treat a NS/CF global defined in a system header as a safe pointer origin (#161146)
Added:
clang/test/Analysis/Checkers/WebKit/unretained-obj-arg.mm
Modified:
clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
clang/test/Analysis/Checkers/WebKit/mock-system-header.h
clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm
Removed:
################################################################################
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<bool(const clang::CXXRecordDecl *)> isSafePtr,
std::function<bool(const clang::QualType)> isSafePtrType,
+ std::function<bool(const clang::Decl *)> isSafeGlobalDecl,
std::function<bool(const clang::Expr *, bool)> callback) {
while (E) {
if (auto *DRE = dyn_cast<DeclRefExpr>(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<MaterializeTemporaryExpr>(E)) {
@@ -71,9 +74,11 @@ bool tryToFindPtrOrigin(
}
if (auto *Expr = dyn_cast<ConditionalOperator>(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<CastExpr>(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<bool(const clang::CXXRecordDecl *)> isSafePtr,
std::function<bool(const clang::QualType)> isSafePtrType,
+ std::function<bool(const clang::Decl *)> isSafeGlobalDecl,
std::function<bool(const clang::Expr *, bool)> 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<check::ASTDecl<TranslationUnitDecl>> {
BugType Bug;
- mutable BugReporter *BR;
TrivialFunctionAnalysis TFA;
EnsureFunctionAnalysis EFA;
protected:
+ mutable BugReporter *BR;
mutable std::optional<RetainTypeChecker> 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<ObjCMessageExpr>(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<check::ASTDecl<TranslationUnitDecl>> {
BugType Bug;
- mutable BugReporter *BR;
EnsureFunctionAnalysis EFA;
protected:
+ mutable BugReporter *BR;
mutable std::optional<RetainTypeChecker> 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<ObjCMessageExpr>(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 at -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 at -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 at -1{{Call argument is unretained and unsafe}}
+ consumeNSString(NSApplicationDidBecomeActiveNotification);
+ consumeNSString(LocalGlobalNSString);
+ // expected-warning at -1{{Call argument is unretained and unsafe}}
+}
More information about the cfe-commits
mailing list