Skip to content

Commit 6f94c58

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.
1 parent ec5f4a4 commit 6f94c58

File tree

2 files changed

+66
-19
lines changed

2 files changed

+66
-19
lines changed

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
@@ -94,32 +103,36 @@ class UncountedCallArgsChecker
94103
if (auto *defaultArg = dyn_cast<CXXDefaultArgExpr>(Arg))
95104
Arg = defaultArg->getExpr();
96105

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

118109
reportBug(Arg, *P);
119110
}
120111
}
121112
}
122113

114+
bool isPtrOriginSafe(const Expr *Arg) const {
115+
std::pair<const clang::Expr *, bool> ArgOrigin =
116+
tryToFindPtrOrigin(Arg, true);
117+
118+
// Temporary ref-counted object created as part of the call argument
119+
// would outlive the call.
120+
if (ArgOrigin.second)
121+
return true;
122+
123+
if (isa<CXXNullPtrLiteralExpr>(ArgOrigin.first)) {
124+
// foo(nullptr)
125+
return true;
126+
}
127+
if (isa<IntegerLiteral>(ArgOrigin.first)) {
128+
// FIXME: Check the value.
129+
// foo(NULL)
130+
return true;
131+
}
132+
133+
return isASafeCallArg(ArgOrigin.first);
134+
}
135+
123136
bool shouldSkipCall(const CallExpr *CE) const {
124137
if (CE->getNumArgs() == 0)
125138
return false;
@@ -196,6 +209,22 @@ class UncountedCallArgsChecker
196209
Report->addRange(CallArg->getSourceRange());
197210
BR->emitReport(std::move(Report));
198211
}
212+
213+
void reportBugOnThis(const Expr *CallArg) const {
214+
assert(CallArg);
215+
216+
SmallString<100> Buf;
217+
llvm::raw_svector_ostream Os(Buf);
218+
219+
Os << "Call argument for 'this' parameter is uncounted and unsafe.";
220+
221+
const SourceLocation SrcLocToReport = CallArg->getSourceRange().getBegin();
222+
223+
PathDiagnosticLocation BSLoc(SrcLocToReport, BR->getSourceManager());
224+
auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
225+
Report->addRange(CallArg->getSourceRange());
226+
BR->emitReport(std::move(Report));
227+
}
199228
};
200229
} // namespace
201230

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
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+
refCountedObj()->someFunction();
17+
// expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}}
18+
}

0 commit comments

Comments
 (0)