Skip to content

Commit c8ac9ad

Browse files
committed
[alpha.webkit.UncountedCallArgsChecker] Check the safety of the object argument in a member function call.
This PR makes alpha.webkit.UncountedCallArgsChecker eplicitly check the safety of the object argument in a member function call. It also removes the exemption of local variables from this checker so that each local variable's safety is checked if it's used in a function call instead of relying on the local variable checker to find those since local variable checker currently has exemption for "for" and "if" statements.
1 parent 535da10 commit c8ac9ad

File tree

3 files changed

+69
-20
lines changed

3 files changed

+69
-20
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ bool isASafeCallArg(const Expr *E) {
8585
assert(E);
8686
if (auto *Ref = dyn_cast<DeclRefExpr>(E)) {
8787
if (auto *D = dyn_cast_or_null<VarDecl>(Ref->getFoundDecl())) {
88-
if (isa<ParmVarDecl>(D) || D->isLocalVarDecl())
88+
if (isa<ParmVarDecl>(D))
8989
return true;
9090
}
9191
}

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

Lines changed: 48 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,15 @@ class UncountedCallArgsChecker
7070
// or std::function call operator).
7171
unsigned ArgIdx = isa<CXXOperatorCallExpr>(CE) && isa_and_nonnull<CXXMethodDecl>(F);
7272

73+
if (auto *MemberCallExpr = dyn_cast<CXXMemberCallExpr>(CE)) {
74+
auto *E = MemberCallExpr->getImplicitObjectArgument();
75+
auto *ArgType = MemberCallExpr->getObjectType().getTypePtrOrNull();
76+
std::optional<bool> IsUncounted =
77+
isUncounted(ArgType->getAsCXXRecordDecl());
78+
if (IsUncounted && *IsUncounted && !isPtrOriginSafe(E))
79+
reportBugOnThis(E);
80+
}
81+
7382
for (auto P = F->param_begin();
7483
// FIXME: Also check variadic function parameters.
7584
// FIXME: Also check default function arguments. Probably a different
@@ -91,32 +100,36 @@ class UncountedCallArgsChecker
91100

92101
const auto *Arg = CE->getArg(ArgIdx);
93102

94-
std::pair<const clang::Expr *, bool> ArgOrigin =
95-
tryToFindPtrOrigin(Arg, true);
96-
97-
// Temporary ref-counted object created as part of the call argument
98-
// would outlive the call.
99-
if (ArgOrigin.second)
100-
continue;
101-
102-
if (isa<CXXNullPtrLiteralExpr>(ArgOrigin.first)) {
103-
// foo(nullptr)
104-
continue;
105-
}
106-
if (isa<IntegerLiteral>(ArgOrigin.first)) {
107-
// FIXME: Check the value.
108-
// foo(NULL)
109-
continue;
110-
}
111-
112-
if (isASafeCallArg(ArgOrigin.first))
103+
if (isPtrOriginSafe(Arg))
113104
continue;
114105

115106
reportBug(Arg, *P);
116107
}
117108
}
118109
}
119110

111+
bool isPtrOriginSafe(const Expr *Arg) const {
112+
std::pair<const clang::Expr *, bool> ArgOrigin =
113+
tryToFindPtrOrigin(Arg, true);
114+
115+
// Temporary ref-counted object created as part of the call argument
116+
// would outlive the call.
117+
if (ArgOrigin.second)
118+
return true;
119+
120+
if (isa<CXXNullPtrLiteralExpr>(ArgOrigin.first)) {
121+
// foo(nullptr)
122+
return true;
123+
}
124+
if (isa<IntegerLiteral>(ArgOrigin.first)) {
125+
// FIXME: Check the value.
126+
// foo(NULL)
127+
return true;
128+
}
129+
130+
return isASafeCallArg(ArgOrigin.first);
131+
}
132+
120133
bool shouldSkipCall(const CallExpr *CE) const {
121134
if (CE->getNumArgs() == 0)
122135
return false;
@@ -183,6 +196,22 @@ class UncountedCallArgsChecker
183196
Report->addRange(CallArg->getSourceRange());
184197
BR->emitReport(std::move(Report));
185198
}
199+
200+
void reportBugOnThis(const Expr *CallArg) const {
201+
assert(CallArg);
202+
203+
SmallString<100> Buf;
204+
llvm::raw_svector_ostream Os(Buf);
205+
206+
Os << "Call argument for 'this' parameter is uncounted and unsafe.";
207+
208+
const SourceLocation SrcLocToReport = CallArg->getSourceRange().getBegin();
209+
210+
PathDiagnosticLocation BSLoc(SrcLocToReport, BR->getSourceManager());
211+
auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
212+
Report->addRange(CallArg->getSourceRange());
213+
BR->emitReport(std::move(Report));
214+
}
186215
};
187216
} // namespace
188217

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
2+
3+
#include "mock-types.h"
4+
5+
class RefCounted {
6+
public:
7+
void ref() const;
8+
void deref() const;
9+
void someFunction();
10+
};
11+
12+
RefCounted* refCountedObj();
13+
14+
void test()
15+
{
16+
if (auto* obj = refCountedObj()) {
17+
obj->someFunction();
18+
// expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}}
19+
}
20+
}

0 commit comments

Comments
 (0)