Skip to content

Commit cc9b8f5

Browse files
rniwahaoNoQ
authored andcommitted
[analyzer] Check the safety of the object argument in a member function call. (llvm#81400)
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. (cherry picked from commit 3a49dfb)
1 parent 81fa098 commit cc9b8f5

File tree

2 files changed

+63
-19
lines changed

2 files changed

+63
-19
lines changed

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

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

74+
if (auto *MemberCallExpr = dyn_cast<CXXMemberCallExpr>(CE)) {
75+
auto *E = MemberCallExpr->getImplicitObjectArgument();
76+
QualType ArgType = MemberCallExpr->getObjectType();
77+
std::optional<bool> IsUncounted =
78+
isUncounted(ArgType->getAsCXXRecordDecl());
79+
if (IsUncounted && *IsUncounted && !isPtrOriginSafe(E))
80+
reportBugOnThis(E);
81+
}
82+
7483
for (auto P = F->param_begin();
7584
// FIXME: Also check variadic function parameters.
7685
// FIXME: Also check default function arguments. Probably a different
@@ -95,32 +104,36 @@ class UncountedCallArgsChecker
95104
if (auto *defaultArg = dyn_cast<CXXDefaultArgExpr>(Arg))
96105
Arg = defaultArg->getExpr();
97106

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

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

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

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)