Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[alpha.webkit.RawPtrRefMemberChecker] The checker doesn't warn Objective-C types in ivars. #132833

Merged
merged 5 commits into from
Mar 29, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -111,6 +111,14 @@ 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);
@@ -133,6 +141,35 @@ 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);
}
}
}

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);
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);
}
}
}

@@ -181,9 +218,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 ";
11 changes: 11 additions & 0 deletions clang/test/Analysis/Checkers/WebKit/mock-system-header.h
Original file line number Diff line number Diff line change
@@ -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 {
NSString *ns_string;
CFStringRef cf_string;
}
@end
#endif
11 changes: 11 additions & 0 deletions clang/test/Analysis/Checkers/WebKit/unretained-members.mm
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.NoUnretainedMemberChecker -verify %s

#include "objc-mock-types.h"
#include "mock-system-header.h"

namespace members {

@@ -58,3 +59,13 @@ void forceTmplToInstantiate(FooTmpl<SomeObj, CFMutableArrayRef>) {}

void forceTmplToInstantiate(RefPtr<SomeObj>) {}
}

@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}}
}
@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
Copy link
Contributor

Choose a reason for hiding this comment

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

This is more of a question for my understanding: Do we expect the same behavior for raw pointers that are properties?

@interface AnotherObject : NSObject
@property(nonatomic,strong) NSString *prop_string;
@end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a test case for properties?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out I have to enumerate properties separately from ivars. Added a test with code fix.