Skip to content

Commit 06f0758

Browse files
authored
[alpha.webkit.UnretainedCallArgsChecker] Recognize [allocObj() init] pattern (#161019)
Generalize the check for recognizing [[Obj alloc] init] to also recognize [allocObj() init]. We do this by utilizing isAllocInit function in RetainPtrCtorAdoptChecker.
1 parent f9326ff commit 06f0758

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
@@ -321,6 +321,51 @@ bool isExprToGetCheckedPtrCapableMember(const clang::Expr *E) {
321321
return result && *result;
322322
}
323323

324+
bool isAllocInit(const Expr *E, const Expr **InnerExpr) {
325+
auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E);
326+
if (auto *POE = dyn_cast<PseudoObjectExpr>(E)) {
327+
if (unsigned ExprCount = POE->getNumSemanticExprs()) {
328+
auto *Expr = POE->getSemanticExpr(ExprCount - 1)->IgnoreParenCasts();
329+
ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(Expr);
330+
if (InnerExpr)
331+
*InnerExpr = ObjCMsgExpr;
332+
}
333+
}
334+
if (!ObjCMsgExpr)
335+
return false;
336+
auto Selector = ObjCMsgExpr->getSelector();
337+
auto NameForFirstSlot = Selector.getNameForSlot(0);
338+
if (NameForFirstSlot.starts_with("alloc") ||
339+
NameForFirstSlot.starts_with("copy") ||
340+
NameForFirstSlot.starts_with("mutableCopy"))
341+
return true;
342+
if (!NameForFirstSlot.starts_with("init") &&
343+
!NameForFirstSlot.starts_with("_init"))
344+
return false;
345+
if (!ObjCMsgExpr->isInstanceMessage())
346+
return false;
347+
auto *Receiver = ObjCMsgExpr->getInstanceReceiver();
348+
if (!Receiver)
349+
return false;
350+
Receiver = Receiver->IgnoreParenCasts();
351+
if (auto *Inner = dyn_cast<ObjCMessageExpr>(Receiver)) {
352+
if (InnerExpr)
353+
*InnerExpr = Inner;
354+
auto InnerSelector = Inner->getSelector();
355+
return InnerSelector.getNameForSlot(0).starts_with("alloc");
356+
} else if (auto *CE = dyn_cast<CallExpr>(Receiver)) {
357+
if (InnerExpr)
358+
*InnerExpr = CE;
359+
if (auto *Callee = CE->getDirectCallee()) {
360+
if (Callee->getDeclName().isIdentifier()) {
361+
auto CalleeName = Callee->getName();
362+
return CalleeName.starts_with("alloc");
363+
}
364+
}
365+
}
366+
return false;
367+
}
368+
324369
class EnsureFunctionVisitor
325370
: public ConstStmtVisitor<EnsureFunctionVisitor, bool> {
326371
public:

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

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

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

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

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

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

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

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -445,50 +445,6 @@ class RetainPtrCtorAdoptChecker
445445
return std::nullopt;
446446
}
447447

448-
bool isAllocInit(const Expr *E, const Expr **InnerExpr = nullptr) const {
449-
auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E);
450-
if (auto *POE = dyn_cast<PseudoObjectExpr>(E)) {
451-
if (unsigned ExprCount = POE->getNumSemanticExprs()) {
452-
auto *Expr = POE->getSemanticExpr(ExprCount - 1)->IgnoreParenCasts();
453-
ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(Expr);
454-
if (InnerExpr)
455-
*InnerExpr = ObjCMsgExpr;
456-
}
457-
}
458-
if (!ObjCMsgExpr)
459-
return false;
460-
auto Selector = ObjCMsgExpr->getSelector();
461-
auto NameForFirstSlot = Selector.getNameForSlot(0);
462-
if (NameForFirstSlot == "alloc" || NameForFirstSlot.starts_with("copy") ||
463-
NameForFirstSlot.starts_with("mutableCopy"))
464-
return true;
465-
if (!NameForFirstSlot.starts_with("init") &&
466-
!NameForFirstSlot.starts_with("_init"))
467-
return false;
468-
if (!ObjCMsgExpr->isInstanceMessage())
469-
return false;
470-
auto *Receiver = ObjCMsgExpr->getInstanceReceiver();
471-
if (!Receiver)
472-
return false;
473-
Receiver = Receiver->IgnoreParenCasts();
474-
if (auto *Inner = dyn_cast<ObjCMessageExpr>(Receiver)) {
475-
if (InnerExpr)
476-
*InnerExpr = Inner;
477-
auto InnerSelector = Inner->getSelector();
478-
return InnerSelector.getNameForSlot(0) == "alloc";
479-
} else if (auto *CE = dyn_cast<CallExpr>(Receiver)) {
480-
if (InnerExpr)
481-
*InnerExpr = CE;
482-
if (auto *Callee = CE->getDirectCallee()) {
483-
if (Callee->getDeclName().isIdentifier()) {
484-
auto CalleeName = Callee->getName();
485-
return CalleeName.starts_with("alloc");
486-
}
487-
}
488-
}
489-
return false;
490-
}
491-
492448
bool isCreateOrCopy(const Expr *E) const {
493449
auto *CE = dyn_cast<CallExpr>(E);
494450
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
@@ -625,6 +625,8 @@ void foo() {
625625

626626
} // namespace template_function
627627

628+
SomeObj *allocObj();
629+
628630
@interface TestObject : NSObject
629631
- (void)doWork:(NSString *)msg, ...;
630632
- (void)doWorkOnSelf;
@@ -647,6 +649,7 @@ - (void)doWorkOnSelf {
647649
[self doWork:__null];
648650
[self doWork:nil];
649651
[NSApp run];
652+
adoptNS([allocObj() init]);
650653
}
651654

652655
- (SomeObj *)getSomeObj {

0 commit comments

Comments
 (0)