Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,16 @@ bool isASafeCallArg(const Expr *E) {
return isa<CXXThisExpr>(E);
}

bool isNullPtr(const clang::Expr *E) {
if (isa<CXXNullPtrLiteralExpr>(E) || isa<GNUNullExpr>(E))
return true;
if (auto *Int = dyn_cast_or_null<IntegerLiteral>(E)) {
if (Int->getValue().isZero())
return true;
}
return false;
}

bool isConstOwnerPtrMemberExpr(const clang::Expr *E) {
if (auto *MCE = dyn_cast<CXXMemberCallExpr>(E)) {
if (auto *Callee = MCE->getDirectCallee()) {
Expand Down Expand Up @@ -273,7 +283,7 @@ class EnsureFunctionVisitor
bool VisitReturnStmt(const ReturnStmt *RS) {
if (auto *RV = RS->getRetValue()) {
RV = RV->IgnoreParenCasts();
if (isa<CXXNullPtrLiteralExpr>(RV))
if (isNullPtr(RV))
return true;
return isConstOwnerPtrMemberExpr(RV);
}
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ bool tryToFindPtrOrigin(
/// \returns Whether \p E is a safe call arugment.
bool isASafeCallArg(const clang::Expr *E);

/// \returns true if E is nullptr or __null.
bool isNullPtr(const clang::Expr *E);

/// \returns true if E is a MemberExpr accessing a const smart pointer type.
bool isConstOwnerPtrMemberExpr(const clang::Expr *E);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ class ForwardDeclChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
ArgExpr = ArgExpr->IgnoreParenCasts();
}
}
if (isa<CXXNullPtrLiteralExpr>(ArgExpr) || isa<IntegerLiteral>(ArgExpr) ||
if (isNullPtr(ArgExpr) || isa<IntegerLiteral>(ArgExpr) ||
isa<CXXDefaultArgExpr>(ArgExpr))
return;
if (auto *DRE = dyn_cast<DeclRefExpr>(ArgExpr)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,13 +217,11 @@ class RawPtrRefCallArgsChecker
[&](const clang::Expr *ArgOrigin, bool IsSafe) {
if (IsSafe)
return true;
if (isa<CXXNullPtrLiteralExpr>(ArgOrigin)) {
// foo(nullptr)
if (isNullPtr(ArgOrigin))
return true;
}
if (isa<IntegerLiteral>(ArgOrigin)) {
// FIXME: Check the value.
// foo(NULL)
// foo(123)
return true;
}
if (isa<ObjCStringLiteral>(ArgOrigin))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ class RawPtrRefLocalVarsChecker
if (isa<CXXThisExpr>(InitArgOrigin))
return true;

if (isa<CXXNullPtrLiteralExpr>(InitArgOrigin))
if (isNullPtr(InitArgOrigin))
return true;

if (isa<IntegerLiteral>(InitArgOrigin))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ class RetainPtrCtorAdoptChecker
CreateOrCopyFnCall.insert(Arg); // Avoid double reporting.
return;
}
if (Result == IsOwnedResult::Owned || Result == IsOwnedResult::Skip) {
if (Result == IsOwnedResult::Owned || Result == IsOwnedResult::Skip ||
isNullPtr(Arg)) {
CreateOrCopyFnCall.insert(Arg);
return;
}
Expand Down Expand Up @@ -486,7 +487,7 @@ class RetainPtrCtorAdoptChecker
continue;
}
}
if (isa<CXXNullPtrLiteralExpr>(E))
if (isNullPtr(E))
return IsOwnedResult::NotOwned;
if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
auto QT = DRE->getType();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,21 @@ class Foo {
return m_obj5.get();
}

CheckedObj* ensureObj6() {
if (!m_obj6)
const_cast<std::unique_ptr<CheckedObj>&>(m_obj6) = new CheckedObj;
if (m_obj6->next())
return (CheckedObj *)0;
return m_obj6.get();
}

private:
const std::unique_ptr<CheckedObj> m_obj1;
std::unique_ptr<CheckedObj> m_obj2;
const std::unique_ptr<CheckedObj> m_obj3;
const std::unique_ptr<CheckedObj> m_obj4;
const std::unique_ptr<CheckedObj> m_obj5;
const std::unique_ptr<CheckedObj> m_obj6;
};

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

} // namespace call_args_const_unique_ptr
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ void basic_correct() {
auto ns4 = adoptNS([ns3 mutableCopy]);
auto ns5 = adoptNS([ns3 copyWithValue:3]);
auto ns6 = retainPtr([ns3 next]);
auto ns7 = retainPtr((SomeObj *)0);
auto ns8 = adoptNS(nil);
CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10));
auto cf2 = adoptCF(SecTaskCreateFromSelf(kCFAllocatorDefault));
auto cf3 = adoptCF(checked_cf_cast<CFArrayRef>(CFCopyArray(cf1)));
Expand Down Expand Up @@ -111,6 +113,10 @@ - (void)setValue:value {
return adoptCF(rawBuffer);
}

RetainPtr<SomeObj> return_nil() {
return nil;
}

RetainPtr<SomeObj> return_nullptr() {
return nullptr;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ void foo8(RefCountable* obj) {
RefCountable *bar = foo->trivial() ? foo.get() : nullptr;
// expected-warning@-1{{Local variable 'bar' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
foo = nullptr;
foo = (RefCountable *)0;
bar->method();
}
}
Expand Down
2 changes: 2 additions & 0 deletions clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,8 @@ - (void)doWorkOnSelf {
// expected-warning@-1{{Call argument is unretained and unsafe}}
// expected-warning@-2{{Call argument is unretained and unsafe}}
[self doWork:@"hello", RetainPtr<SomeObj> { provide() }.get(), RetainPtr<CFMutableArrayRef> { provide_cf() }.get()];
[self doWork:__null];
[self doWork:nil];
}

- (SomeObj *)getSomeObj {
Expand Down