Skip to content

Commit 58bad28

Browse files
NagyDonatNagyDonat
authored
[analyzer][NFC] Require explicit matching mode for CallDescriptions (#92454)
This commit deletes the "simple" constructor of `CallDescription` which did not require a `CallDescription::Mode` argument and always used the "wildcard" mode `CDM::Unspecified`. A few months ago, this vague matching mode was used by many checkers, which caused bugs like #81597 and #88181. Since then, my commits improved the available matching modes and ensured that all checkers explicitly specify the right matching mode. After those commits, the only remaining references to the "simple" constructor were some unit tests; this commit updates them to use an explicitly specified matching mode (often `CDM::SimpleFunc`). The mode `CDM::Unspecified` was not deleted in this commit because it's still a reasonable choice in `GenericTaintChecker` and a few unit tests.
1 parent 37c6b9f commit 58bad28

File tree

8 files changed

+51
-56
lines changed

8 files changed

+51
-56
lines changed

‎clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,10 @@ class CallDescription {
5656
/// overloaded operator, a constructor or a destructor).
5757
CXXMethod,
5858

59-
/// Match any CallEvent that is not an ObjCMethodCall.
60-
/// FIXME: Previously this was the default behavior of CallDescription, but
61-
/// its use should be replaced by a more specific mode almost everywhere.
59+
/// Match any CallEvent that is not an ObjCMethodCall. This should not be
60+
/// used when the checker looks for a concrete function (and knows whether
61+
/// it is a method); but GenericTaintChecker uses this mode to match
62+
/// functions whose name was configured by the user.
6263
Unspecified,
6364

6465
/// FIXME: Add support for ObjCMethodCall events (I'm not adding it because
@@ -100,13 +101,6 @@ class CallDescription {
100101
MaybeCount RequiredArgs = std::nullopt,
101102
MaybeCount RequiredParams = std::nullopt);
102103

103-
/// Construct a CallDescription with default flags.
104-
CallDescription(ArrayRef<StringRef> QualifiedName,
105-
MaybeCount RequiredArgs = std::nullopt,
106-
MaybeCount RequiredParams = std::nullopt);
107-
108-
CallDescription(std::nullptr_t) = delete;
109-
110104
/// Get the name of the function that this object matches.
111105
StringRef getFunctionName() const { return QualifiedName.back(); }
112106

‎clang/lib/StaticAnalyzer/Core/CallDescription.cpp

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,6 @@ ento::CallDescription::CallDescription(Mode MatchAs,
4848
[](StringRef From) { return From.str(); });
4949
}
5050

51-
/// Construct a CallDescription with default flags.
52-
ento::CallDescription::CallDescription(ArrayRef<StringRef> QualifiedName,
53-
MaybeCount RequiredArgs /*= None*/,
54-
MaybeCount RequiredParams /*= None*/)
55-
: CallDescription(Mode::Unspecified, QualifiedName, RequiredArgs,
56-
RequiredParams) {}
57-
5851
bool ento::CallDescription::matches(const CallEvent &Call) const {
5952
// FIXME: Add ObjC Message support.
6053
if (Call.getKind() == CE_ObjCMessage)

‎clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,13 @@ class InterestingnessTestChecker : public Checker<check::PreCall> {
3333
const CallEvent &, CheckerContext &)>;
3434

3535
CallDescriptionMap<HandlerFn> Handlers = {
36-
{{{"setInteresting"}, 1}, &InterestingnessTestChecker::handleInteresting},
37-
{{{"setNotInteresting"}, 1},
36+
{{CDM::SimpleFunc, {"setInteresting"}, 1},
37+
&InterestingnessTestChecker::handleInteresting},
38+
{{CDM::SimpleFunc, {"setNotInteresting"}, 1},
3839
&InterestingnessTestChecker::handleNotInteresting},
39-
{{{"check"}, 1}, &InterestingnessTestChecker::handleCheck},
40-
{{{"bug"}, 1}, &InterestingnessTestChecker::handleBug},
40+
{{CDM::SimpleFunc, {"check"}, 1},
41+
&InterestingnessTestChecker::handleCheck},
42+
{{CDM::SimpleFunc, {"bug"}, 1}, &InterestingnessTestChecker::handleBug},
4143
};
4244

4345
void handleInteresting(const CallEvent &Call, CheckerContext &C) const;

‎clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -138,26 +138,28 @@ class CallDescriptionAction : public ASTFrontendAction {
138138
TEST(CallDescription, SimpleNameMatching) {
139139
EXPECT_TRUE(tooling::runToolOnCode(
140140
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
141-
{{{"bar"}}, false}, // false: there's no call to 'bar' in this code.
142-
{{{"foo"}}, true}, // true: there's a call to 'foo' in this code.
141+
{{CDM::SimpleFunc, {"bar"}},
142+
false}, // false: there's no call to 'bar' in this code.
143+
{{CDM::SimpleFunc, {"foo"}},
144+
true}, // true: there's a call to 'foo' in this code.
143145
})),
144146
"void foo(); void bar() { foo(); }"));
145147
}
146148

147149
TEST(CallDescription, RequiredArguments) {
148150
EXPECT_TRUE(tooling::runToolOnCode(
149151
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
150-
{{{"foo"}, 1}, true},
151-
{{{"foo"}, 2}, false},
152+
{{CDM::SimpleFunc, {"foo"}, 1}, true},
153+
{{CDM::SimpleFunc, {"foo"}, 2}, false},
152154
})),
153155
"void foo(int); void foo(int, int); void bar() { foo(1); }"));
154156
}
155157

156158
TEST(CallDescription, LackOfRequiredArguments) {
157159
EXPECT_TRUE(tooling::runToolOnCode(
158160
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
159-
{{{"foo"}, std::nullopt}, true},
160-
{{{"foo"}, 2}, false},
161+
{{CDM::SimpleFunc, {"foo"}, std::nullopt}, true},
162+
{{CDM::SimpleFunc, {"foo"}, 2}, false},
161163
})),
162164
"void foo(int); void foo(int, int); void bar() { foo(1); }"));
163165
}
@@ -187,7 +189,7 @@ TEST(CallDescription, QualifiedNames) {
187189
const std::string Code = (Twine{MockStdStringHeader} + AdditionalCode).str();
188190
EXPECT_TRUE(tooling::runToolOnCode(
189191
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
190-
{{{"std", "basic_string", "c_str"}}, true},
192+
{{CDM::CXXMethod, {"std", "basic_string", "c_str"}}, true},
191193
})),
192194
Code));
193195
}
@@ -202,7 +204,8 @@ TEST(CallDescription, MatchConstructor) {
202204
EXPECT_TRUE(tooling::runToolOnCode(
203205
std::unique_ptr<FrontendAction>(
204206
new CallDescriptionAction<CXXConstructExpr>({
205-
{{{"std", "basic_string", "basic_string"}, 2, 2}, true},
207+
{{CDM::CXXMethod, {"std", "basic_string", "basic_string"}, 2, 2},
208+
true},
206209
})),
207210
Code));
208211
}
@@ -228,7 +231,7 @@ TEST(CallDescription, MatchConversionOperator) {
228231
})code";
229232
EXPECT_TRUE(tooling::runToolOnCode(
230233
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
231-
{{{"aaa", "bbb", "Bar", "operator int"}}, true},
234+
{{CDM::CXXMethod, {"aaa", "bbb", "Bar", "operator int"}}, true},
232235
})),
233236
Code));
234237
}
@@ -252,7 +255,7 @@ TEST(CallDescription, RejectOverQualifiedNames) {
252255
// FIXME: We should **not** match.
253256
EXPECT_TRUE(tooling::runToolOnCode(
254257
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
255-
{{{"std", "container", "data"}}, true},
258+
{{CDM::CXXMethod, {"std", "container", "data"}}, true},
256259
})),
257260
Code));
258261
}
@@ -272,7 +275,7 @@ TEST(CallDescription, DontSkipNonInlineNamespaces) {
272275
SCOPED_TRACE("my v1 bar");
273276
EXPECT_TRUE(tooling::runToolOnCode(
274277
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
275-
{{{"my", "v1", "bar"}}, true},
278+
{{CDM::SimpleFunc, {"my", "v1", "bar"}}, true},
276279
})),
277280
Code));
278281
}
@@ -281,7 +284,7 @@ TEST(CallDescription, DontSkipNonInlineNamespaces) {
281284
SCOPED_TRACE("my bar");
282285
EXPECT_TRUE(tooling::runToolOnCode(
283286
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
284-
{{{"my", "bar"}}, true},
287+
{{CDM::SimpleFunc, {"my", "bar"}}, true},
285288
})),
286289
Code));
287290
}
@@ -303,15 +306,15 @@ TEST(CallDescription, SkipTopInlineNamespaces) {
303306
SCOPED_TRACE("my v1 bar");
304307
EXPECT_TRUE(tooling::runToolOnCode(
305308
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
306-
{{{"my", "v1", "bar"}}, true},
309+
{{CDM::SimpleFunc, {"my", "v1", "bar"}}, true},
307310
})),
308311
Code));
309312
}
310313
{
311314
SCOPED_TRACE("v1 bar");
312315
EXPECT_TRUE(tooling::runToolOnCode(
313316
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
314-
{{{"v1", "bar"}}, true},
317+
{{CDM::SimpleFunc, {"v1", "bar"}}, true},
315318
})),
316319
Code));
317320
}
@@ -338,7 +341,7 @@ TEST(CallDescription, SkipAnonimousNamespaces) {
338341

339342
EXPECT_TRUE(tooling::runToolOnCode(
340343
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
341-
{{{"std", "container", "data"}}, true},
344+
{{CDM::CXXMethod, {"std", "container", "data"}}, true},
342345
})),
343346
Code));
344347
}
@@ -376,7 +379,7 @@ TEST(CallDescription, AliasNames) {
376379
SCOPED_TRACE("std container data");
377380
EXPECT_TRUE(tooling::runToolOnCode(
378381
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
379-
{{{"std", "container", "data"}}, true},
382+
{{CDM::CXXMethod, {"std", "container", "data"}}, true},
380383
})),
381384
UseAliasInSpellingCode));
382385
}
@@ -385,7 +388,7 @@ TEST(CallDescription, AliasNames) {
385388
SCOPED_TRACE("std cont data");
386389
EXPECT_TRUE(tooling::runToolOnCode(
387390
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
388-
{{{"std", "cont", "data"}}, false},
391+
{{CDM::CXXMethod, {"std", "cont", "data"}}, false},
389392
})),
390393
UseAliasInSpellingCode));
391394
}
@@ -399,7 +402,7 @@ TEST(CallDescription, AliasNames) {
399402
SCOPED_TRACE("std container data");
400403
EXPECT_TRUE(tooling::runToolOnCode(
401404
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
402-
{{{"std", "container", "data"}}, true},
405+
{{CDM::CXXMethod, {"std", "container", "data"}}, true},
403406
})),
404407
UseAliasInSpellingCode));
405408
}
@@ -408,7 +411,7 @@ TEST(CallDescription, AliasNames) {
408411
SCOPED_TRACE("std cont data");
409412
EXPECT_TRUE(tooling::runToolOnCode(
410413
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
411-
{{{"std", "cont", "data"}}, false},
414+
{{CDM::CXXMethod, {"std", "cont", "data"}}, false},
412415
})),
413416
UseAliasInSpellingCode));
414417
}
@@ -431,7 +434,7 @@ TEST(CallDescription, AliasSingleNamespace) {
431434
SCOPED_TRACE("aaa bbb ccc bar");
432435
EXPECT_TRUE(tooling::runToolOnCode(
433436
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
434-
{{{"aaa", "bbb", "ccc", "bar"}}, true},
437+
{{CDM::SimpleFunc, {"aaa", "bbb", "ccc", "bar"}}, true},
435438
})),
436439
Code));
437440
}
@@ -440,7 +443,7 @@ TEST(CallDescription, AliasSingleNamespace) {
440443
SCOPED_TRACE("aaa bbb_alias ccc bar");
441444
EXPECT_TRUE(tooling::runToolOnCode(
442445
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
443-
{{{"aaa", "bbb_alias", "ccc", "bar"}}, false},
446+
{{CDM::SimpleFunc, {"aaa", "bbb_alias", "ccc", "bar"}}, false},
444447
})),
445448
Code));
446449
}
@@ -462,7 +465,7 @@ TEST(CallDescription, AliasMultipleNamespaces) {
462465
SCOPED_TRACE("aaa bbb ccc bar");
463466
EXPECT_TRUE(tooling::runToolOnCode(
464467
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
465-
{{{"aaa", "bbb", "ccc", "bar"}}, true},
468+
{{CDM::SimpleFunc, {"aaa", "bbb", "ccc", "bar"}}, true},
466469
})),
467470
Code));
468471
}
@@ -471,7 +474,7 @@ TEST(CallDescription, AliasMultipleNamespaces) {
471474
SCOPED_TRACE("aaa_bbb_ccc bar");
472475
EXPECT_TRUE(tooling::runToolOnCode(
473476
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
474-
{{{"aaa_bbb_ccc", "bar"}}, false},
477+
{{CDM::SimpleFunc, {"aaa_bbb_ccc", "bar"}}, false},
475478
})),
476479
Code));
477480
}
@@ -480,9 +483,9 @@ TEST(CallDescription, AliasMultipleNamespaces) {
480483
TEST(CallDescription, NegativeMatchQualifiedNames) {
481484
EXPECT_TRUE(tooling::runToolOnCode(
482485
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
483-
{{{"foo", "bar"}}, false},
484-
{{{"bar", "foo"}}, false},
485-
{{{"foo"}}, true},
486+
{{CDM::Unspecified, {"foo", "bar"}}, false},
487+
{{CDM::Unspecified, {"bar", "foo"}}, false},
488+
{{CDM::Unspecified, {"foo"}}, true},
486489
})),
487490
"void foo(); struct bar { void foo(); }; void test() { foo(); }"));
488491
}
@@ -598,7 +601,7 @@ TEST(CallDescription, MatchBuiltins) {
598601

599602
class CallDescChecker
600603
: public Checker<check::PreCall, check::PreStmt<CallExpr>> {
601-
CallDescriptionSet Set = {{{"bar"}, 0}};
604+
CallDescriptionSet Set = {{CDM::SimpleFunc, {"bar"}, 0}};
602605

603606
public:
604607
void checkPreCall(const CallEvent &Call, CheckerContext &C) const {

‎clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ using namespace ento;
1818

1919
namespace {
2020
class EvalCallBase : public Checker<eval::Call> {
21-
const CallDescription Foo = {{"foo"}, 0};
21+
const CallDescription Foo = {CDM::SimpleFunc, {"foo"}, 0};
2222

2323
public:
2424
bool evalCall(const CallEvent &Call, CheckerContext &C) const {

‎clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,11 @@ class FalsePositiveGenerator : public Checker<eval::Call> {
3030
using HandlerFn = bool (Self::*)(const CallEvent &Call,
3131
CheckerContext &) const;
3232
CallDescriptionMap<HandlerFn> Callbacks = {
33-
{{{"reachedWithContradiction"}, 0}, &Self::reachedWithContradiction},
34-
{{{"reachedWithNoContradiction"}, 0}, &Self::reachedWithNoContradiction},
35-
{{{"reportIfCanBeTrue"}, 1}, &Self::reportIfCanBeTrue},
33+
{{CDM::SimpleFunc, {"reachedWithContradiction"}, 0},
34+
&Self::reachedWithContradiction},
35+
{{CDM::SimpleFunc, {"reachedWithNoContradiction"}, 0},
36+
&Self::reachedWithNoContradiction},
37+
{{CDM::SimpleFunc, {"reportIfCanBeTrue"}, 1}, &Self::reportIfCanBeTrue},
3638
};
3739

3840
bool report(CheckerContext &C, ProgramStateRef State,

‎clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ class DescriptiveNameChecker : public Checker<check::PreCall> {
3838

3939
private:
4040
const BugType Bug{this, "DescriptiveNameBug"};
41-
const CallDescription HandlerFn = {{"reportDescriptiveName"}, 1};
41+
const CallDescription HandlerFn = {
42+
CDM::SimpleFunc, {"reportDescriptiveName"}, 1};
4243
};
4344

4445
void addDescriptiveNameChecker(AnalysisASTConsumer &AnalysisConsumer,

‎clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,17 +86,17 @@ class StatefulChecker : public Checker<check::PreCall> {
8686

8787
public:
8888
void checkPreCall(const CallEvent &Call, CheckerContext &C) const {
89-
if (CallDescription{{"preventError"}, 0}.matches(Call)) {
89+
if (CallDescription{CDM::SimpleFunc, {"preventError"}, 0}.matches(Call)) {
9090
C.addTransition(C.getState()->set<ErrorPrevented>(true));
9191
return;
9292
}
9393

94-
if (CallDescription{{"allowError"}, 0}.matches(Call)) {
94+
if (CallDescription{CDM::SimpleFunc, {"allowError"}, 0}.matches(Call)) {
9595
C.addTransition(C.getState()->set<ErrorPrevented>(false));
9696
return;
9797
}
9898

99-
if (CallDescription{{"error"}, 0}.matches(Call)) {
99+
if (CallDescription{CDM::SimpleFunc, {"error"}, 0}.matches(Call)) {
100100
if (C.getState()->get<ErrorPrevented>())
101101
return;
102102
const ExplodedNode *N = C.generateErrorNode();

0 commit comments

Comments
 (0)