Skip to content

Commit 5566404

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 b626282 commit 5566404

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
@@ -313,6 +313,51 @@ bool isExprToGetCheckedPtrCapableMember(const clang::Expr *E) {
313313
return result && *result;
314314
}
315315

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

597597
} // namespace sel_string
598598

599+
SomeObj *allocObj();
600+
599601
@interface TestObject : NSObject
600602
- (void)doWork:(NSString *)msg, ...;
601603
- (void)doWorkOnSelf;
@@ -618,6 +620,7 @@ - (void)doWorkOnSelf {
618620
[self doWork:__null];
619621
[self doWork:nil];
620622
[NSApp run];
623+
adoptNS([allocObj() init]);
621624
}
622625

623626
- (SomeObj *)getSomeObj {

0 commit comments

Comments
 (0)