Skip to content

Commit d9c829c

Browse files
committed
[WebKit checkers] Treat NULL, 0, and nil like nullptr (llvm#157700)
This PR makes WebKit checkers treat NULL, 0, and nil like nullptr in various places.
1 parent fa7e102 commit d9c829c

File tree

10 files changed

+40
-9
lines changed

10 files changed

+40
-9
lines changed

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,16 @@ bool isASafeCallArg(const Expr *E) {
218218
return isa<CXXThisExpr>(E);
219219
}
220220

221+
bool isNullPtr(const clang::Expr *E) {
222+
if (isa<CXXNullPtrLiteralExpr>(E) || isa<GNUNullExpr>(E))
223+
return true;
224+
if (auto *Int = dyn_cast_or_null<IntegerLiteral>(E)) {
225+
if (Int->getValue().isZero())
226+
return true;
227+
}
228+
return false;
229+
}
230+
221231
bool isConstOwnerPtrMemberExpr(const clang::Expr *E) {
222232
if (auto *MCE = dyn_cast<CXXMemberCallExpr>(E)) {
223233
if (auto *Callee = MCE->getDirectCallee()) {
@@ -276,7 +286,7 @@ class EnsureFunctionVisitor
276286
bool VisitReturnStmt(const ReturnStmt *RS) {
277287
if (auto *RV = RS->getRetValue()) {
278288
RV = RV->IgnoreParenCasts();
279-
if (isa<CXXNullPtrLiteralExpr>(RV))
289+
if (isNullPtr(RV))
280290
return true;
281291
return isConstOwnerPtrMemberExpr(RV);
282292
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ bool tryToFindPtrOrigin(
6666
/// \returns Whether \p E is a safe call arugment.
6767
bool isASafeCallArg(const clang::Expr *E);
6868

69+
/// \returns true if E is nullptr or __null.
70+
bool isNullPtr(const clang::Expr *E);
71+
6972
/// \returns true if E is a MemberExpr accessing a const smart pointer type.
7073
bool isConstOwnerPtrMemberExpr(const clang::Expr *E);
7174

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ class ForwardDeclChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
268268
ArgExpr = ArgExpr->IgnoreParenCasts();
269269
}
270270
}
271-
if (isa<CXXNullPtrLiteralExpr>(ArgExpr) || isa<IntegerLiteral>(ArgExpr) ||
271+
if (isNullPtr(ArgExpr) || isa<IntegerLiteral>(ArgExpr) ||
272272
isa<CXXDefaultArgExpr>(ArgExpr))
273273
return;
274274
if (auto *DRE = dyn_cast<DeclRefExpr>(ArgExpr)) {

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -222,13 +222,11 @@ class RawPtrRefCallArgsChecker
222222
[&](const clang::Expr *ArgOrigin, bool IsSafe) {
223223
if (IsSafe)
224224
return true;
225-
if (isa<CXXNullPtrLiteralExpr>(ArgOrigin)) {
226-
// foo(nullptr)
225+
if (isNullPtr(ArgOrigin))
227226
return true;
228-
}
229227
if (isa<IntegerLiteral>(ArgOrigin)) {
230228
// FIXME: Check the value.
231-
// foo(NULL)
229+
// foo(123)
232230
return true;
233231
}
234232
if (isa<ObjCStringLiteral>(ArgOrigin))

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ class RawPtrRefLocalVarsChecker
301301
if (isa<CXXThisExpr>(InitArgOrigin))
302302
return true;
303303

304-
if (isa<CXXNullPtrLiteralExpr>(InitArgOrigin))
304+
if (isNullPtr(InitArgOrigin))
305305
return true;
306306

307307
if (isa<IntegerLiteral>(InitArgOrigin))

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,8 @@ class RetainPtrCtorAdoptChecker
181181
CreateOrCopyFnCall.insert(Arg); // Avoid double reporting.
182182
return;
183183
}
184-
if (Result == IsOwnedResult::Owned || Result == IsOwnedResult::Skip) {
184+
if (Result == IsOwnedResult::Owned || Result == IsOwnedResult::Skip ||
185+
isNullPtr(Arg)) {
185186
CreateOrCopyFnCall.insert(Arg);
186187
return;
187188
}
@@ -489,7 +490,7 @@ class RetainPtrCtorAdoptChecker
489490
continue;
490491
}
491492
}
492-
if (isa<CXXNullPtrLiteralExpr>(E))
493+
if (isNullPtr(E))
493494
return IsOwnedResult::NotOwned;
494495
if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
495496
auto QT = DRE->getType();

clang/test/Analysis/Checkers/WebKit/call-args-checked-const-member.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,12 +71,21 @@ class Foo {
7171
return m_obj5.get();
7272
}
7373

74+
CheckedObj* ensureObj6() {
75+
if (!m_obj6)
76+
const_cast<std::unique_ptr<CheckedObj>&>(m_obj6) = new CheckedObj;
77+
if (m_obj6->next())
78+
return (CheckedObj *)0;
79+
return m_obj6.get();
80+
}
81+
7482
private:
7583
const std::unique_ptr<CheckedObj> m_obj1;
7684
std::unique_ptr<CheckedObj> m_obj2;
7785
const std::unique_ptr<CheckedObj> m_obj3;
7886
const std::unique_ptr<CheckedObj> m_obj4;
7987
const std::unique_ptr<CheckedObj> m_obj5;
88+
const std::unique_ptr<CheckedObj> m_obj6;
8089
};
8190

8291
void Foo::bar() {
@@ -87,6 +96,7 @@ void Foo::bar() {
8796
badEnsureObj4().method();
8897
// expected-warning@-1{{Call argument for 'this' parameter is unchecked and unsafe}}
8998
ensureObj5()->method();
99+
ensureObj6()->method();
90100
}
91101

92102
} // namespace call_args_const_unique_ptr

clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ void basic_correct() {
1212
auto ns4 = adoptNS([ns3 mutableCopy]);
1313
auto ns5 = adoptNS([ns3 copyWithValue:3]);
1414
auto ns6 = retainPtr([ns3 next]);
15+
auto ns7 = retainPtr((SomeObj *)0);
16+
auto ns8 = adoptNS(nil);
1517
CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10));
1618
auto cf2 = adoptCF(SecTaskCreateFromSelf(kCFAllocatorDefault));
1719
auto cf3 = adoptCF(checked_cf_cast<CFArrayRef>(CFCopyArray(cf1)));
@@ -110,6 +112,10 @@ - (void)setValue:value {
110112
return adoptCF(rawBuffer);
111113
}
112114

115+
RetainPtr<SomeObj> return_nil() {
116+
return nil;
117+
}
118+
113119
RetainPtr<SomeObj> return_nullptr() {
114120
return nullptr;
115121
}

clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ void foo8(RefCountable* obj) {
121121
RefCountable *bar = foo->trivial() ? foo.get() : nullptr;
122122
// expected-warning@-1{{Local variable 'bar' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
123123
foo = nullptr;
124+
foo = (RefCountable *)0;
124125
bar->method();
125126
}
126127
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,8 @@ - (void)doWorkOnSelf {
455455
// expected-warning@-1{{Call argument is unretained and unsafe}}
456456
// expected-warning@-2{{Call argument is unretained and unsafe}}
457457
[self doWork:@"hello", RetainPtr<SomeObj> { provide() }.get(), RetainPtr<CFMutableArrayRef> { provide_cf() }.get()];
458+
[self doWork:__null];
459+
[self doWork:nil];
458460
}
459461

460462
- (SomeObj *)getSomeObj {

0 commit comments

Comments
 (0)