Skip to content

Conversation

@rniwa
Copy link
Contributor

@rniwa rniwa commented Sep 27, 2025

Treat accessing NSApp without retaining it as safe

Treat accessing NSApp without retaining it as safe
@rniwa rniwa requested a review from t-rasmud September 27, 2025 08:19
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Sep 27, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 27, 2025

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)

Changes

Treat accessing NSApp without retaining it as safe


Full diff: https://github.com/llvm/llvm-project/pull/160990.diff

3 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp (+2-2)
  • (modified) clang/test/Analysis/Checkers/WebKit/objc-mock-types.h (+16)
  • (modified) clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm (+1)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index 00a1b8b6e7e895..2e407eb7cf1e9f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -31,9 +31,9 @@ bool tryToFindPtrOrigin(
     if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
       if (auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl())) {
         auto QT = VD->getType();
-        if (VD->hasGlobalStorage() && QT.isConstQualified()) {
+        auto IsImmortal = safeGetName(VD) == "NSApp";
+        if (VD->hasGlobalStorage() && (IsImmortal || QT.isConstQualified()))
           return callback(E, true);
-        }
       }
     }
     if (auto *tempExpr = dyn_cast<MaterializeTemporaryExpr>(E)) {
diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
index 39dee1746158b5..7a199530ca52f3 100644
--- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
@@ -163,6 +163,22 @@ __attribute__((objc_root_class))
 + (NSNumber *)numberWithBool:(BOOL)value;
 @end
 
+@interface NSResponder : NSObject
+@end
+
+@interface NSApplication : NSResponder
+
+extern NSApplication * NSApp;
+
+@property (class, readonly, strong) NSApplication *sharedApplication;
+
+- (void)finishLaunching;
+- (void)run;
+- (void)stop:(id)sender;
+- (void)terminate:(id)sender;
+
+@end
+
 @interface SomeObj : NSObject
 - (instancetype)_init;
 - (SomeObj *)mutableCopy;
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
index c9d2fe861bb498..03ef200fb51f58 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
@@ -582,6 +582,7 @@ - (void)doWorkOnSelf {
   [self doWork:@"hello", RetainPtr<SomeObj> { provide() }.get(), RetainPtr<CFMutableArrayRef> { provide_cf() }.get(), OSObjectPtr { provide_dispatch() }.get()];
   [self doWork:__null];
   [self doWork:nil];
+  [NSApp run];
 }
 
 - (SomeObj *)getSomeObj {

Copy link
Contributor

@t-rasmud t-rasmud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@rniwa
Copy link
Contributor Author

rniwa commented Oct 7, 2025

Thanks for the review!

@rniwa rniwa merged commit 872c431 into llvm:main Oct 7, 2025
12 checks passed
@rniwa rniwa deleted the fix-webkit-recognize-ns-app branch October 7, 2025 19:09
rniwa added a commit to rniwa/llvm-project that referenced this pull request Oct 7, 2025
)

Treat accessing NSApp without retaining it as safe
rniwa added a commit to rniwa/llvm-project that referenced this pull request Oct 16, 2025
)

Treat accessing NSApp without retaining it as safe

(cherry picked from commit 872c431)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:static analyzer clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants