[WebKit checkers] Treat a weak property / variable as safe #163689
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-static-analyzer-1 Author: Ryosuke Niwa (rniwa) ChangesTreat a weak Objective-C property, ivar, member variable, and local variable as safe. Full diff: https://github.com/llvm/llvm-project/pull/163689.diff 9 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
index 033eb8cc299b0..9e5742e442af9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
@@ -496,6 +496,8 @@ class UnretainedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker {
}
std::optional<bool> isUnsafePtr(QualType QT) const final {
+ if (QT.hasStrongOrWeakObjCLifetime())
+ return false;
return RTC->isUnretained(QT);
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
index c13df47920f72..f2235e7c25ab2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
@@ -433,6 +433,8 @@ class UnretainedLocalVarsChecker final : public RawPtrRefLocalVarsChecker {
RTC = RetainTypeChecker();
}
std::optional<bool> isUnsafePtr(const QualType T) const final {
+ if (T.hasStrongOrWeakObjCLifetime())
+ return false;
return RTC->isUnretained(T);
}
bool isSafePtr(const CXXRecordDecl *Record) const final {
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index ace639ce7ab18..e4cbd290127bb 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
@@ -231,8 +231,10 @@ class RawPtrRefMemberChecker
// "assign" property doesn't retain even under ARC so treat it as unsafe.
bool ignoreARC =
!PD->isReadOnly() && PD->getSetterKind() == ObjCPropertyDecl::Assign;
+ bool IsWeak = PD->getPropertyAttributes() & ObjCPropertyAttribute::kind_weak;
+ bool HasSafeAttr = PD->isRetaining() || IsWeak;
auto IsUnsafePtr = isUnsafePtr(QT, ignoreARC);
- return {IsUnsafePtr && *IsUnsafePtr && !PD->isRetaining(), PropType};
+ return {IsUnsafePtr && *IsUnsafePtr && !HasSafeAttr, PropType};
}
bool shouldSkipDecl(const RecordDecl *RD) const {
@@ -363,6 +365,8 @@ class NoUnretainedMemberChecker final : public RawPtrRefMemberChecker {
}
std::optional<bool> isUnsafePtr(QualType QT, bool ignoreARC) const final {
+ if (QT.hasStrongOrWeakObjCLifetime())
+ return false;
return RTC->isUnretained(QT, ignoreARC);
}
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-weak-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-weak-arc.mm
new file mode 100644
index 0000000000000..a52bc7c9a5572
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-weak-arc.mm
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedLambdaCapturesChecker -fobjc-runtime-has-weak -fobjc-weak -fobjc-arc -verify %s
+// expected-no-diagnostics
+
+#include "objc-mock-types.h"
+
+void someFunction();
+template <typename Callback> void call(Callback callback) {
+ someFunction();
+ callback();
+}
+
+NSString *provideStr();
+SomeObj *provideSomeObj();
+
+void foo() {
+ __weak NSString *weakStr = provideStr();
+ __weak SomeObj *weakObj = provideSomeObj();
+ auto lambda = [weakStr, weakObj]() {
+ return [weakStr length] + [weakObj value];
+ };
+ call(lambda);
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-weak.mm b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-weak.mm
new file mode 100644
index 0000000000000..7439d7f8bb93b
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-weak.mm
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedLambdaCapturesChecker -fobjc-runtime-has-weak -fobjc-weak -verify %s
+// expected-no-diagnostics
+
+#include "objc-mock-types.h"
+
+void someFunction();
+template <typename Callback> void call(Callback callback) {
+ someFunction();
+ callback();
+}
+
+NSString *provideStr();
+SomeObj *provideSomeObj();
+
+void foo() {
+ __weak NSString *weakStr = provideStr();
+ __weak SomeObj *weakObj = provideSomeObj();
+ auto lambda = [weakStr, weakObj]() {
+ return [weakStr length] + [weakObj value];
+ };
+ call(lambda);
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-weak-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-weak-arc.mm
new file mode 100644
index 0000000000000..8c709b5921227
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-weak-arc.mm
@@ -0,0 +1,13 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedLocalVarsChecker -fobjc-runtime-has-weak -fobjc-weak -fobjc-arc -verify %s
+// expected-no-diagnostics
+
+#include "objc-mock-types.h"
+
+NSString *provideStr();
+SomeObj *provideSomeObj();
+
+int foo() {
+ __weak NSString *weakStr = provideStr();
+ __weak SomeObj *weakObj = provideSomeObj();
+ return [weakStr length] + [weakObj value];
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-weak.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-weak.mm
new file mode 100644
index 0000000000000..3ac4ff9d1e4cb
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-weak.mm
@@ -0,0 +1,13 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedLocalVarsChecker -fobjc-runtime-has-weak -fobjc-weak -verify %s
+// expected-no-diagnostics
+
+#include "objc-mock-types.h"
+
+NSString *provideStr();
+SomeObj *provideSomeObj();
+
+int foo() {
+ __weak NSString *weakStr = provideStr();
+ __weak SomeObj *weakObj = provideSomeObj();
+ return [weakStr length] + [weakObj value];
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members-weak-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members-weak-arc.mm
new file mode 100644
index 0000000000000..c0aaac09e68d8
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-members-weak-arc.mm
@@ -0,0 +1,29 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.NoUnretainedMemberChecker -fobjc-runtime-has-weak -fobjc-weak -fobjc-arc -verify %s
+// expected-no-diagnostics
+
+#include "objc-mock-types.h"
+
+struct Foo {
+ __weak NSString *weakPtr = nullptr;
+ Foo();
+ ~Foo();
+ void bar();
+};
+
+@interface ObjectWithWeakProperty : NSObject
+@property(nonatomic, weak) NSString *weak_prop;
+@end
+
+@implementation ObjectWithWeakProperty
+@end
+
+NS_REQUIRES_PROPERTY_DEFINITIONS
+@interface NoSynthesisObjectWithWeakProperty : NSObject
+@property(nonatomic, readonly, weak) NSString *weak_prop;
+@end
+
+@implementation NoSynthesisObjectWithWeakProperty
+- (NSString *)weak_prop {
+ return nil;
+}
+@end
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members-weak.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members-weak.mm
new file mode 100644
index 0000000000000..422cf6189446d
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-members-weak.mm
@@ -0,0 +1,31 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.NoUnretainedMemberChecker -fobjc-runtime-has-weak -fobjc-weak -verify %s
+// expected-no-diagnostics
+
+#include "objc-mock-types.h"
+
+struct Foo {
+ __weak NSString *weakPtr = nullptr;
+ Foo();
+ ~Foo();
+ void bar();
+};
+
+@interface ObjectWithWeakProperty : NSObject
+@property(nonatomic, weak) NSString *weak_prop;
+@end
+
+@implementation ObjectWithWeakProperty
+@end
+
+NS_REQUIRES_PROPERTY_DEFINITIONS
+@interface NoSynthesisObjectWithWeakProperty : NSObject
+@property(nonatomic, readonly, weak) NSString *weak_prop;
+@end
+
+@implementation NoSynthesisObjectWithWeakProperty {
+ __weak NSNumber *weak_ivar;
+}
+- (NSString *)weak_prop {
+ return nil;
+}
+@end
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Treat a weak Objective-C property, ivar, member variable, and local variable as safe.
rniwa
force-pushed
the
fix-webkit-objc-weak
branch
from
e2a5d28 to
e0b9579
Compare
Contributor
|
I don't think I can help much on this one. |
steakhal
approved these changes
Dec 9, 2025
Contributor
steakhal
left a comment
steakhal
left a comment
There was a problem hiding this comment.
I have not much to say. I'll just rubber stamp this. I don't think I have spotted anything that would make me worried. I also don't have the context to be confident that this is safe.
Contributor
Author
|
Thanks for the rubber stamp! |
rniwa
added a commit
to rniwa/llvm-project
that referenced
this pull request
Dec 12, 2025
Treat a weak Objective-C property, ivar, member variable, and local variable as safe. (cherry picked from commit f9326ff)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Treat a weak Objective-C property, ivar, member variable, and local variable as safe.