https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/132833
>From ca50deea7d8096b065d2bf7aa5e007afc8f0a954 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <rn...@webkit.org> Date: Mon, 24 Mar 2025 14:22:28 -0700 Subject: [PATCH 1/4] [alpha.webkit.RawPtrRefMemberChecker] The checker doesn't warn Objective-C types in ivars. This PR fixes the bug that we weren't generating warnings when a raw poiner is used to point to a NS type in Objective-C ivars. Also fix the bug that we weren't suppressing this warning in system headers. --- .../Checkers/WebKit/RawPtrRefMemberChecker.cpp | 10 ++++++++++ .../Analysis/Checkers/WebKit/mock-system-header.h | 11 +++++++++++ .../Analysis/Checkers/WebKit/unretained-members.mm | 12 +++++++++++- 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp index dc4e2c7d168fb..6d20869043358 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp @@ -111,6 +111,8 @@ class RawPtrRefMemberChecker } void visitObjCDecl(const ObjCContainerDecl *CD) const { + if (BR->getSourceManager().isInSystemHeader(CD->getLocation())) + return; if (auto *ID = dyn_cast<ObjCInterfaceDecl>(CD)) { for (auto *Ivar : ID->ivars()) visitIvarDecl(CD, Ivar); @@ -133,6 +135,14 @@ class RawPtrRefMemberChecker std::optional<bool> IsCompatible = isPtrCompatible(QT, IvarCXXRD); if (IsCompatible && *IsCompatible) reportBug(Ivar, IvarType, IvarCXXRD, CD); + } else { + std::optional<bool> IsCompatible = isPtrCompatible(QT, nullptr); + auto *PointeeType = IvarType->getPointeeType().getTypePtrOrNull(); + if (IsCompatible && *IsCompatible) { + auto *Desugared = PointeeType->getUnqualifiedDesugaredType(); + if (auto *ObjCType = dyn_cast_or_null<ObjCInterfaceType>(Desugared)) + reportBug(Ivar, IvarType, ObjCType->getDecl(), CD); + } } } diff --git a/clang/test/Analysis/Checkers/WebKit/mock-system-header.h b/clang/test/Analysis/Checkers/WebKit/mock-system-header.h index e993fd697ffab..b377f5098c002 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-system-header.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-system-header.h @@ -29,3 +29,14 @@ enum os_log_type_t : uint8_t { typedef struct os_log_s *os_log_t; os_log_t os_log_create(const char *subsystem, const char *category); 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; + +#ifdef __OBJC__ +@class NSString; +@interface SystemObject : NSObject { + NSString *ns_string; + CFStringRef cf_string; +} +@end +#endif diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm index e068a583c18c5..79f7a05caa1be 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm @@ -1,7 +1,8 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.NoUnretainedMemberChecker -verify %s #include "objc-mock-types.h" - +#include "mock-system-header.h" +#if 0 namespace members { struct Foo { @@ -58,3 +59,12 @@ void forceTmplToInstantiate(FooTmpl<SomeObj, CFMutableArrayRef>) {} void forceTmplToInstantiate(RefPtr<SomeObj>) {} } +#endif + +@interface AnotherObject : NSObject { + NSString *ns_string; + // expected-warning@-1{{Instance variable 'ns_string' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} + CFStringRef cf_string; + // expected-warning@-1{{Instance variable 'cf_string' in 'AnotherObject' is a retainable type 'CFStringRef'; member variables must be a RetainPtr}} +} +@end >From d72d24363c7ba16463a1f5fe28442dd2740d2d95 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <rn...@webkit.org> Date: Mon, 24 Mar 2025 16:00:52 -0700 Subject: [PATCH 2/4] Revert erroneous #if 0 ~ #endif --- clang/test/Analysis/Checkers/WebKit/unretained-members.mm | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm index 79f7a05caa1be..483ce4e2763ea 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm @@ -2,7 +2,7 @@ #include "objc-mock-types.h" #include "mock-system-header.h" -#if 0 + namespace members { struct Foo { @@ -59,7 +59,6 @@ void forceTmplToInstantiate(FooTmpl<SomeObj, CFMutableArrayRef>) {} void forceTmplToInstantiate(RefPtr<SomeObj>) {} } -#endif @interface AnotherObject : NSObject { NSString *ns_string; >From c3796e7e626ee22f89f5b9d5850ed89a0f723c65 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <rn...@webkit.org> Date: Thu, 27 Mar 2025 15:42:46 -0700 Subject: [PATCH 3/4] Also add the support for detecting raw pointer property with a test. --- .../WebKit/RawPtrRefMemberChecker.cpp | 37 +++++++++++++++++-- .../Checkers/WebKit/unretained-members.mm | 2 + 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp index 6d20869043358..adcf9fce83b90 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp @@ -113,6 +113,12 @@ class RawPtrRefMemberChecker void visitObjCDecl(const ObjCContainerDecl *CD) const { if (BR->getSourceManager().isInSystemHeader(CD->getLocation())) return; + + ObjCContainerDecl::PropertyMap map; + CD->collectPropertiesToImplement(map); + for (auto it : map) + visitObjCPropertyDecl(CD, it.second); + if (auto *ID = dyn_cast<ObjCInterfaceDecl>(CD)) { for (auto *Ivar : ID->ivars()) visitIvarDecl(CD, Ivar); @@ -146,6 +152,28 @@ class RawPtrRefMemberChecker } } + void visitObjCPropertyDecl(const ObjCContainerDecl *CD, + const ObjCPropertyDecl *PD) const { + auto QT = PD->getType(); + const Type *PropType = QT.getTypePtrOrNull(); + if (!PropType) + return; + if (auto *PropCXXRD = PropType->getPointeeCXXRecordDecl()) { + std::optional<bool> IsCompatible = isPtrCompatible(QT, PropCXXRD); + fprintf(stderr, "IsCompatible=%d\n", IsCompatible ? *IsCompatible : -1); + if (IsCompatible && *IsCompatible) + reportBug(PD, PropType, PropCXXRD, CD); + } else { + std::optional<bool> IsCompatible = isPtrCompatible(QT, nullptr); + auto *PointeeType = PropType->getPointeeType().getTypePtrOrNull(); + if (IsCompatible && *IsCompatible) { + auto *Desugared = PointeeType->getUnqualifiedDesugaredType(); + if (auto *ObjCType = dyn_cast_or_null<ObjCInterfaceType>(Desugared)) + reportBug(PD, PropType, ObjCType->getDecl(), CD); + } + } + } + bool shouldSkipDecl(const RecordDecl *RD) const { if (!RD->isThisDeclarationADefinition()) return true; @@ -191,9 +219,12 @@ class RawPtrRefMemberChecker SmallString<100> Buf; llvm::raw_svector_ostream Os(Buf); - if (isa<ObjCContainerDecl>(ClassCXXRD)) - Os << "Instance variable "; - else + if (isa<ObjCContainerDecl>(ClassCXXRD)) { + if (isa<ObjCPropertyDecl>(Member)) + Os << "Property "; + else + Os << "Instance variable "; + } else Os << "Member variable "; printQuotedName(Os, Member); Os << " in "; diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm index 483ce4e2763ea..92d70a94427c0 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm @@ -66,4 +66,6 @@ @interface AnotherObject : NSObject { CFStringRef cf_string; // expected-warning@-1{{Instance variable 'cf_string' in 'AnotherObject' is a retainable type 'CFStringRef'; member variables must be a RetainPtr}} } +@property(nonatomic, strong) NSString *prop_string; +// expected-warning@-1{{Property 'prop_string' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} @end >From a46ce85a481b353bf7d89cc27ab09e82cf74eef2 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <rn...@webkit.org> Date: Fri, 28 Mar 2025 12:31:47 -0700 Subject: [PATCH 4/4] Remove fprintf logging for debugging purposes --- .../StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp index adcf9fce83b90..89df1a725ab92 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp @@ -160,7 +160,6 @@ class RawPtrRefMemberChecker return; if (auto *PropCXXRD = PropType->getPointeeCXXRecordDecl()) { std::optional<bool> IsCompatible = isPtrCompatible(QT, PropCXXRD); - fprintf(stderr, "IsCompatible=%d\n", IsCompatible ? *IsCompatible : -1); if (IsCompatible && *IsCompatible) reportBug(PD, PropType, PropCXXRD, CD); } else { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits