Skip to content

Commit 891ccf8

Browse files
committed
[alpha.webkit.UnretainedCallArgsChecker] Recognize [allocObj() init] pattern
Generalize the check for recognizing [[Obj alloc] init] to also recognize [allocObj() init]. We do this by utilizing isAllocInit function in RetainPtrCtorAdoptChecker.
1 parent 77a3d43 commit 891ccf8

File tree

5 files changed

+54
-51
lines changed

5 files changed

+54
-51
lines changed

clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,51 @@ bool isExprToGetCheckedPtrCapableMember(const clang::Expr *E) {
301301
return result && *result;
302302
}
303303

304+
bool isAllocInit(const Expr *E, const Expr **InnerExpr) {
305+
auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E);
306+
if (auto *POE = dyn_cast<PseudoObjectExpr>(E)) {
307+
if (unsigned ExprCount = POE->getNumSemanticExprs()) {
308+
auto *Expr = POE->getSemanticExpr(ExprCount - 1)->IgnoreParenCasts();
309+
ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(Expr);
310+
if (InnerExpr)
311+
*InnerExpr = ObjCMsgExpr;
312+
}
313+
}
314+
if (!ObjCMsgExpr)
315+
return false;
316+
auto Selector = ObjCMsgExpr->getSelector();
317+
auto NameForFirstSlot = Selector.getNameForSlot(0);
318+
if (NameForFirstSlot.starts_with("alloc") ||
319+
NameForFirstSlot.starts_with("copy") ||
320+
NameForFirstSlot.starts_with("mutableCopy"))
321+
return true;
322+
if (!NameForFirstSlot.starts_with("init") &&
323+
!NameForFirstSlot.starts_with("_init"))
324+
return false;
325+
if (!ObjCMsgExpr->isInstanceMessage())
326+
return false;
327+
auto *Receiver = ObjCMsgExpr->getInstanceReceiver();
328+
if (!Receiver)
329+
return false;
330+
Receiver = Receiver->IgnoreParenCasts();
331+
if (auto *Inner = dyn_cast<ObjCMessageExpr>(Receiver)) {
332+
if (InnerExpr)
333+
*InnerExpr = Inner;
334+
auto InnerSelector = Inner->getSelector();
335+
return InnerSelector.getNameForSlot(0).starts_with("alloc");
336+
} else if (auto *CE = dyn_cast<CallExpr>(Receiver)) {
337+
if (InnerExpr)
338+
*InnerExpr = CE;
339+
if (auto *Callee = CE->getDirectCallee()) {
340+
if (Callee->getDeclName().isIdentifier()) {
341+
auto CalleeName = Callee->getName();
342+
return CalleeName.starts_with("alloc");
343+
}
344+
}
345+
}
346+
return false;
347+
}
348+
304349
class EnsureFunctionVisitor
305350
: public ConstStmtVisitor<EnsureFunctionVisitor, bool> {
306351
public:

clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@ bool isConstOwnerPtrMemberExpr(const clang::Expr *E);
7676
/// supports CheckedPtr.
7777
bool isExprToGetCheckedPtrCapableMember(const clang::Expr *E);
7878

79+
/// \returns true if \p E is a [[alloc] init] pattern expression.
80+
/// Sets \p InnerExpr to the inner function call or selector invocation.
81+
bool isAllocInit(const Expr *E, const Expr **InnerExpr = nullptr);
82+
7983
/// \returns true if E is a CXXMemberCallExpr which returns a const smart
8084
/// pointer type.
8185
class EnsureFunctionAnalysis {

clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -176,16 +176,11 @@ class RawPtrRefCallArgsChecker
176176
if (BR->getSourceManager().isInSystemHeader(E->getExprLoc()))
177177
return;
178178

179-
auto Selector = E->getSelector();
180179
if (auto *Receiver = E->getInstanceReceiver()) {
181180
std::optional<bool> IsUnsafe = isUnsafePtr(E->getReceiverType());
182181
if (IsUnsafe && *IsUnsafe && !isPtrOriginSafe(Receiver)) {
183-
if (auto *InnerMsg = dyn_cast<ObjCMessageExpr>(Receiver)) {
184-
auto InnerSelector = InnerMsg->getSelector();
185-
if (InnerSelector.getNameForSlot(0) == "alloc" &&
186-
Selector.getNameForSlot(0).starts_with("init"))
187-
return;
188-
}
182+
if (isAllocInit(E))
183+
return;
189184
reportBugOnReceiver(Receiver, D);
190185
}
191186
}

clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -419,50 +419,6 @@ class RetainPtrCtorAdoptChecker
419419
return std::nullopt;
420420
}
421421

422-
bool isAllocInit(const Expr *E, const Expr **InnerExpr = nullptr) const {
423-
auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E);
424-
if (auto *POE = dyn_cast<PseudoObjectExpr>(E)) {
425-
if (unsigned ExprCount = POE->getNumSemanticExprs()) {
426-
auto *Expr = POE->getSemanticExpr(ExprCount - 1)->IgnoreParenCasts();
427-
ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(Expr);
428-
if (InnerExpr)
429-
*InnerExpr = ObjCMsgExpr;
430-
}
431-
}
432-
if (!ObjCMsgExpr)
433-
return false;
434-
auto Selector = ObjCMsgExpr->getSelector();
435-
auto NameForFirstSlot = Selector.getNameForSlot(0);
436-
if (NameForFirstSlot == "alloc" || NameForFirstSlot.starts_with("copy") ||
437-
NameForFirstSlot.starts_with("mutableCopy"))
438-
return true;
439-
if (!NameForFirstSlot.starts_with("init") &&
440-
!NameForFirstSlot.starts_with("_init"))
441-
return false;
442-
if (!ObjCMsgExpr->isInstanceMessage())
443-
return false;
444-
auto *Receiver = ObjCMsgExpr->getInstanceReceiver();
445-
if (!Receiver)
446-
return false;
447-
Receiver = Receiver->IgnoreParenCasts();
448-
if (auto *Inner = dyn_cast<ObjCMessageExpr>(Receiver)) {
449-
if (InnerExpr)
450-
*InnerExpr = Inner;
451-
auto InnerSelector = Inner->getSelector();
452-
return InnerSelector.getNameForSlot(0) == "alloc";
453-
} else if (auto *CE = dyn_cast<CallExpr>(Receiver)) {
454-
if (InnerExpr)
455-
*InnerExpr = CE;
456-
if (auto *Callee = CE->getDirectCallee()) {
457-
if (Callee->getDeclName().isIdentifier()) {
458-
auto CalleeName = Callee->getName();
459-
return CalleeName.starts_with("alloc");
460-
}
461-
}
462-
}
463-
return false;
464-
}
465-
466422
bool isCreateOrCopy(const Expr *E) const {
467423
auto *CE = dyn_cast<CallExpr>(E);
468424
if (!CE)

clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,8 @@ void foo() {
561561

562562
} // namespace ns_retained_return_value
563563

564+
SomeObj *allocObj();
565+
564566
@interface TestObject : NSObject
565567
- (void)doWork:(NSString *)msg, ...;
566568
- (void)doWorkOnSelf;
@@ -582,6 +584,7 @@ - (void)doWorkOnSelf {
582584
[self doWork:@"hello", RetainPtr<SomeObj> { provide() }.get(), RetainPtr<CFMutableArrayRef> { provide_cf() }.get(), OSObjectPtr { provide_dispatch() }.get()];
583585
[self doWork:__null];
584586
[self doWork:nil];
587+
adoptNS([allocObj() init]);
585588
}
586589

587590
- (SomeObj *)getSomeObj {

0 commit comments

Comments
 (0)