Skip to content

Commit 94d6401

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 86b89a6 commit 94d6401

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
@@ -309,6 +309,51 @@ bool isExprToGetCheckedPtrCapableMember(const clang::Expr *E) {
309309
return result && *result;
310310
}
311311

312+
bool isAllocInit(const Expr *E, const Expr **InnerExpr) {
313+
auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E);
314+
if (auto *POE = dyn_cast<PseudoObjectExpr>(E)) {
315+
if (unsigned ExprCount = POE->getNumSemanticExprs()) {
316+
auto *Expr = POE->getSemanticExpr(ExprCount - 1)->IgnoreParenCasts();
317+
ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(Expr);
318+
if (InnerExpr)
319+
*InnerExpr = ObjCMsgExpr;
320+
}
321+
}
322+
if (!ObjCMsgExpr)
323+
return false;
324+
auto Selector = ObjCMsgExpr->getSelector();
325+
auto NameForFirstSlot = Selector.getNameForSlot(0);
326+
if (NameForFirstSlot.starts_with("alloc") ||
327+
NameForFirstSlot.starts_with("copy") ||
328+
NameForFirstSlot.starts_with("mutableCopy"))
329+
return true;
330+
if (!NameForFirstSlot.starts_with("init") &&
331+
!NameForFirstSlot.starts_with("_init"))
332+
return false;
333+
if (!ObjCMsgExpr->isInstanceMessage())
334+
return false;
335+
auto *Receiver = ObjCMsgExpr->getInstanceReceiver();
336+
if (!Receiver)
337+
return false;
338+
Receiver = Receiver->IgnoreParenCasts();
339+
if (auto *Inner = dyn_cast<ObjCMessageExpr>(Receiver)) {
340+
if (InnerExpr)
341+
*InnerExpr = Inner;
342+
auto InnerSelector = Inner->getSelector();
343+
return InnerSelector.getNameForSlot(0).starts_with("alloc");
344+
} else if (auto *CE = dyn_cast<CallExpr>(Receiver)) {
345+
if (InnerExpr)
346+
*InnerExpr = CE;
347+
if (auto *Callee = CE->getDirectCallee()) {
348+
if (Callee->getDeclName().isIdentifier()) {
349+
auto CalleeName = Callee->getName();
350+
return CalleeName.starts_with("alloc");
351+
}
352+
}
353+
}
354+
return false;
355+
}
356+
312357
class EnsureFunctionVisitor
313358
: public ConstStmtVisitor<EnsureFunctionVisitor, bool> {
314359
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
@@ -423,50 +423,6 @@ class RetainPtrCtorAdoptChecker
423423
return std::nullopt;
424424
}
425425

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

579579
} // autoreleased
580580

581+
SomeObj *allocObj();
582+
581583
@interface TestObject : NSObject
582584
- (void)doWork:(NSString *)msg, ...;
583585
- (void)doWorkOnSelf;
@@ -600,6 +602,7 @@ - (void)doWorkOnSelf {
600602
[self doWork:__null];
601603
[self doWork:nil];
602604
[NSApp run];
605+
adoptNS([allocObj() init]);
603606
}
604607

605608
- (SomeObj *)getSomeObj {

0 commit comments

Comments
 (0)