Skip to content

Commit 20ed04d

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 8ad4dc7 commit 20ed04d

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
@@ -217,6 +217,16 @@ bool isASafeCallArg(const Expr *E) {
217217
return isa<CXXThisExpr>(E);
218218
}
219219

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

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
@@ -272,7 +272,7 @@ class ForwardDeclChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
272272
ArgExpr = ArgExpr->IgnoreParenCasts();
273273
}
274274
}
275-
if (isa<CXXNullPtrLiteralExpr>(ArgExpr) || isa<IntegerLiteral>(ArgExpr) ||
275+
if (isNullPtr(ArgExpr) || isa<IntegerLiteral>(ArgExpr) ||
276276
isa<CXXDefaultArgExpr>(ArgExpr))
277277
return;
278278
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
@@ -217,13 +217,11 @@ class RawPtrRefCallArgsChecker
217217
[&](const clang::Expr *ArgOrigin, bool IsSafe) {
218218
if (IsSafe)
219219
return true;
220-
if (isa<CXXNullPtrLiteralExpr>(ArgOrigin)) {
221-
// foo(nullptr)
220+
if (isNullPtr(ArgOrigin))
222221
return true;
223-
}
224222
if (isa<IntegerLiteral>(ArgOrigin)) {
225223
// FIXME: Check the value.
226-
// foo(NULL)
224+
// foo(123)
227225
return true;
228226
}
229227
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
@@ -295,7 +295,7 @@ class RawPtrRefLocalVarsChecker
295295
if (isa<CXXThisExpr>(InitArgOrigin))
296296
return true;
297297

298-
if (isa<CXXNullPtrLiteralExpr>(InitArgOrigin))
298+
if (isNullPtr(InitArgOrigin))
299299
return true;
300300

301301
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
@@ -177,7 +177,8 @@ class RetainPtrCtorAdoptChecker
177177
CreateOrCopyFnCall.insert(Arg); // Avoid double reporting.
178178
return;
179179
}
180-
if (Result == IsOwnedResult::Owned || Result == IsOwnedResult::Skip) {
180+
if (Result == IsOwnedResult::Owned || Result == IsOwnedResult::Skip ||
181+
isNullPtr(Arg)) {
181182
CreateOrCopyFnCall.insert(Arg);
182183
return;
183184
}
@@ -486,7 +487,7 @@ class RetainPtrCtorAdoptChecker
486487
continue;
487488
}
488489
}
489-
if (isa<CXXNullPtrLiteralExpr>(E))
490+
if (isNullPtr(E))
490491
return IsOwnedResult::NotOwned;
491492
if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
492493
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
@@ -13,6 +13,8 @@ void basic_correct() {
1313
auto ns4 = adoptNS([ns3 mutableCopy]);
1414
auto ns5 = adoptNS([ns3 copyWithValue:3]);
1515
auto ns6 = retainPtr([ns3 next]);
16+
auto ns7 = retainPtr((SomeObj *)0);
17+
auto ns8 = adoptNS(nil);
1618
CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10));
1719
auto cf2 = adoptCF(SecTaskCreateFromSelf(kCFAllocatorDefault));
1820
auto cf3 = adoptCF(checked_cf_cast<CFArrayRef>(CFCopyArray(cf1)));
@@ -111,6 +113,10 @@ - (void)setValue:value {
111113
return adoptCF(rawBuffer);
112114
}
113115

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

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
@@ -456,6 +456,8 @@ - (void)doWorkOnSelf {
456456
// expected-warning@-1{{Call argument is unretained and unsafe}}
457457
// expected-warning@-2{{Call argument is unretained and unsafe}}
458458
[self doWork:@"hello", RetainPtr<SomeObj> { provide() }.get(), RetainPtr<CFMutableArrayRef> { provide_cf() }.get()];
459+
[self doWork:__null];
460+
[self doWork:nil];
459461
}
460462

461463
- (SomeObj *)getSomeObj {

0 commit comments

Comments
 (0)