[clang] [WebKit checkers] Treat global const variables as safe (PR #136170)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: Ryosuke Niwa (rniwa)

<details>
<summary>Changes</summary>

This PR makes WebKit checkers treat a variable with global storage as safe instead of constraining to ones that start with k or _k.

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


5 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp (+5-6) 
- (modified) clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm (+18) 
- (modified) clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm (+19) 
- (modified) clang/test/Analysis/Checkers/WebKit/unretained-local-vars-arc.mm (+20) 
- (modified) clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm (+21) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index c36c925c0d2d2..88b98756b2ca8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -30,12 +30,11 @@ bool tryToFindPtrOrigin(
     std::function<bool(const clang::Expr *, bool)> callback) {
   while (E) {
     if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
-      auto *ValDecl = DRE->getDecl();
-      auto QT = ValDecl->getType();
-      auto ValName = ValDecl->getName();
-      if (ValDecl && (ValName.starts_with('k') || ValName.starts_with("_k")) &&
-          QT.isConstQualified()) { // Treat constants such as kCF* as safe.
-        return callback(E, true);
+      if (auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl())) {
+        auto QT = VD->getType();
+        if (VD->hasGlobalStorage() && QT.isConstQualified()) {
+          return callback(E, true);
+        }
       }
     }
     if (auto *tempExpr = dyn_cast<MaterializeTemporaryExpr>(E)) {
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm
index 4207c1836079f..fa866258a2f6d 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm
@@ -18,6 +18,24 @@ void foo() {
 
 } // namespace raw_ptr
 
+namespace const_global {
+
+extern NSString * const SomeConstant;
+extern CFDictionaryRef const SomeDictionary;
+void doWork(NSString *str, CFDictionaryRef dict);
+void use_const_global() {
+  doWork(SomeConstant, SomeDictionary);
+}
+
+NSString *provide_str();
+CFDictionaryRef provide_dict();
+void use_const_local() {
+  doWork(provide_str(), provide_dict());
+  // expected-warning at -1{{Call argument for parameter 'dict' is unretained and unsafe}}
+}
+
+} // namespace const_global
+
 @interface AnotherObj : NSObject
 - (void)foo:(SomeObj *)obj;
 - (SomeObj *)getSomeObj;
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
index eb36b49313d42..c33d53b047c2e 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
@@ -415,6 +415,25 @@ void idcf(CFTypeRef obj) {
 
 } // ptr_conversion
 
+namespace const_global {
+
+extern NSString * const SomeConstant;
+extern CFDictionaryRef const SomeDictionary;
+void doWork(NSString *str, CFDictionaryRef dict);
+void use_const_global() {
+  doWork(SomeConstant, SomeDictionary);
+}
+
+NSString *provide_str();
+CFDictionaryRef provide_dict();
+void use_const_local() {
+  doWork(provide_str(), provide_dict());
+  // expected-warning at -1{{Call argument for parameter 'str' is unretained and unsafe}}
+  // expected-warning at -2{{Call argument for parameter 'dict' is unretained and unsafe}}
+}
+
+} // namespace const_global
+
 @interface TestObject : NSObject
 - (void)doWork:(NSString *)msg, ...;
 - (void)doWorkOnSelf;
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-arc.mm
index 92a718f7e3a4c..a84bee8529645 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-arc.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-arc.mm
@@ -25,6 +25,26 @@ void bar() {
 
 } // namespace raw_ptr
 
+namespace const_global {
+
+extern NSString * const SomeConstant;
+extern CFDictionaryRef const SomeDictionary;
+void doWork(NSString *, CFDictionaryRef);
+void use_const_global() {
+  doWork(SomeConstant, SomeDictionary);
+}
+
+NSString *provide_str();
+CFDictionaryRef provide_dict();
+void use_const_local() {
+  NSString * const str = provide_str();
+  CFDictionaryRef dict = provide_dict();
+  // expected-warning at -1{{Local variable 'dict' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}}
+  doWork(str, dict);
+}
+
+} // namespace const_global
+
 @interface AnotherObj : NSObject
 - (void)foo:(SomeObj *)obj;
 @end
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm
index a71a80ea3d647..10f7c9acb7a3c 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm
@@ -387,6 +387,27 @@ unsigned ccf(CFTypeRef obj) {
 
 } // ptr_conversion
 
+namespace const_global {
+
+extern NSString * const SomeConstant;
+extern CFDictionaryRef const SomeDictionary;
+void doWork(NSString *, CFDictionaryRef);
+void use_const_global() {
+  doWork(SomeConstant, SomeDictionary);
+}
+
+NSString *provide_str();
+CFDictionaryRef provide_dict();
+void use_const_local() {
+  NSString * const str = provide_str();
+  // expected-warning at -1{{Local variable 'str' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}}
+  CFDictionaryRef dict = provide_dict();
+  // expected-warning at -1{{Local variable 'dict' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}}
+  doWork(str, dict);
+}
+
+} // namespace const_global
+
 bool doMoreWorkOpaque(OtherObj*);
 SomeObj* provide();
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/136170