Skip to content

Commit 6d64f8e

Browse files
NagyDonatNagyDonat
authored
[analyzer] Use explicit call description mode in more checkers (#90974)
This commit explicitly specifies the matching mode (C library function, any non-method function, or C++ method) for the `CallDescription`s constructed in various checkers. Some code was simplified to use `CallDescriptionSet`s instead of individual `CallDescription`s. This change won't cause major functional changes, but isn't NFC because it ensures that e.g. call descriptions for a non-method function won't accidentally match a method that has the same name. Separate commits have already performed this change in other checkers: - easy cases: e2f1cba - MallocChecker: d6d84b5 - iterator checkers: 06eedff - InvalidPtr checker: 024281d ... and follow-up commits will handle the remaining checkers. My goal is to ensure that the call description mode is always explicitly specified and eliminate (or strongly restrict) the vague "may be either a method or a simple function" mode that's the current default.
1 parent dcc7ef3 commit 6d64f8e

File tree

5 files changed

+61
-67
lines changed

5 files changed

+61
-67
lines changed

‎clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -148,27 +148,28 @@ using MutexDescriptor =
148148
class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
149149
private:
150150
const std::array<MutexDescriptor, 8> MutexDescriptors{
151-
MemberMutexDescriptor(
152-
CallDescription(/*QualifiedName=*/{"std", "mutex", "lock"},
153-
/*RequiredArgs=*/0),
154-
CallDescription({"std", "mutex", "unlock"}, 0)),
155-
FirstArgMutexDescriptor(CallDescription({"pthread_mutex_lock"}, 1),
156-
CallDescription({"pthread_mutex_unlock"}, 1)),
157-
FirstArgMutexDescriptor(CallDescription({"mtx_lock"}, 1),
158-
CallDescription({"mtx_unlock"}, 1)),
159-
FirstArgMutexDescriptor(CallDescription({"pthread_mutex_trylock"}, 1),
160-
CallDescription({"pthread_mutex_unlock"}, 1)),
161-
FirstArgMutexDescriptor(CallDescription({"mtx_trylock"}, 1),
162-
CallDescription({"mtx_unlock"}, 1)),
163-
FirstArgMutexDescriptor(CallDescription({"mtx_timedlock"}, 1),
164-
CallDescription({"mtx_unlock"}, 1)),
151+
MemberMutexDescriptor({/*MatchAs=*/CDM::CXXMethod,
152+
/*QualifiedName=*/{"std", "mutex", "lock"},
153+
/*RequiredArgs=*/0},
154+
{CDM::CXXMethod, {"std", "mutex", "unlock"}, 0}),
155+
FirstArgMutexDescriptor({CDM::CLibrary, {"pthread_mutex_lock"}, 1},
156+
{CDM::CLibrary, {"pthread_mutex_unlock"}, 1}),
157+
FirstArgMutexDescriptor({CDM::CLibrary, {"mtx_lock"}, 1},
158+
{CDM::CLibrary, {"mtx_unlock"}, 1}),
159+
FirstArgMutexDescriptor({CDM::CLibrary, {"pthread_mutex_trylock"}, 1},
160+
{CDM::CLibrary, {"pthread_mutex_unlock"}, 1}),
161+
FirstArgMutexDescriptor({CDM::CLibrary, {"mtx_trylock"}, 1},
162+
{CDM::CLibrary, {"mtx_unlock"}, 1}),
163+
FirstArgMutexDescriptor({CDM::CLibrary, {"mtx_timedlock"}, 1},
164+
{CDM::CLibrary, {"mtx_unlock"}, 1}),
165165
RAIIMutexDescriptor("lock_guard"),
166166
RAIIMutexDescriptor("unique_lock")};
167167

168-
const std::array<CallDescription, 5> BlockingFunctions{
169-
ArrayRef{StringRef{"sleep"}}, ArrayRef{StringRef{"getc"}},
170-
ArrayRef{StringRef{"fgets"}}, ArrayRef{StringRef{"read"}},
171-
ArrayRef{StringRef{"recv"}}};
168+
const CallDescriptionSet BlockingFunctions{{CDM::CLibrary, {"sleep"}},
169+
{CDM::CLibrary, {"getc"}},
170+
{CDM::CLibrary, {"fgets"}},
171+
{CDM::CLibrary, {"read"}},
172+
{CDM::CLibrary, {"recv"}}};
172173

173174
const BugType BlockInCritSectionBugType{
174175
this, "Call to blocking function in critical section", "Blocking Error"};
@@ -291,8 +292,7 @@ void BlockInCriticalSectionChecker::handleUnlock(
291292

292293
bool BlockInCriticalSectionChecker::isBlockingInCritSection(
293294
const CallEvent &Call, CheckerContext &C) const {
294-
return llvm::any_of(BlockingFunctions,
295-
[&Call](auto &&Fn) { return Fn.matches(Call); }) &&
295+
return BlockingFunctions.contains(Call) &&
296296
!C.getState()->get<ActiveCritSections>().isEmpty();
297297
}
298298

‎clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,8 @@ class CStringChecker : public Checker< eval::Call,
189189
};
190190

191191
// These require a bit of special handling.
192-
CallDescription StdCopy{{"std", "copy"}, 3},
193-
StdCopyBackward{{"std", "copy_backward"}, 3};
192+
CallDescription StdCopy{CDM::SimpleFunc, {"std", "copy"}, 3},
193+
StdCopyBackward{CDM::SimpleFunc, {"std", "copy_backward"}, 3};
194194

195195
FnCheck identifyCall(const CallEvent &Call, CheckerContext &C) const;
196196
void evalMemcpy(CheckerContext &C, const CallEvent &Call, CharKind CK) const;

‎clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp

Lines changed: 25 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,28 @@ namespace {
3535
class InnerPointerChecker
3636
: public Checker<check::DeadSymbols, check::PostCall> {
3737

38-
CallDescription AppendFn, AssignFn, AddressofFn, AddressofFn_, ClearFn,
39-
CStrFn, DataFn, DataMemberFn, EraseFn, InsertFn, PopBackFn, PushBackFn,
40-
ReplaceFn, ReserveFn, ResizeFn, ShrinkToFitFn, SwapFn;
38+
CallDescriptionSet InvalidatingMemberFunctions{
39+
CallDescription(CDM::CXXMethod, {"std", "basic_string", "append"}),
40+
CallDescription(CDM::CXXMethod, {"std", "basic_string", "assign"}),
41+
CallDescription(CDM::CXXMethod, {"std", "basic_string", "clear"}),
42+
CallDescription(CDM::CXXMethod, {"std", "basic_string", "erase"}),
43+
CallDescription(CDM::CXXMethod, {"std", "basic_string", "insert"}),
44+
CallDescription(CDM::CXXMethod, {"std", "basic_string", "pop_back"}),
45+
CallDescription(CDM::CXXMethod, {"std", "basic_string", "push_back"}),
46+
CallDescription(CDM::CXXMethod, {"std", "basic_string", "replace"}),
47+
CallDescription(CDM::CXXMethod, {"std", "basic_string", "reserve"}),
48+
CallDescription(CDM::CXXMethod, {"std", "basic_string", "resize"}),
49+
CallDescription(CDM::CXXMethod, {"std", "basic_string", "shrink_to_fit"}),
50+
CallDescription(CDM::CXXMethod, {"std", "basic_string", "swap"})};
51+
52+
CallDescriptionSet AddressofFunctions{
53+
CallDescription(CDM::SimpleFunc, {"std", "addressof"}),
54+
CallDescription(CDM::SimpleFunc, {"std", "__addressof"})};
55+
56+
CallDescriptionSet InnerPointerAccessFunctions{
57+
CallDescription(CDM::CXXMethod, {"std", "basic_string", "c_str"}),
58+
CallDescription(CDM::SimpleFunc, {"std", "data"}, 1),
59+
CallDescription(CDM::CXXMethod, {"std", "basic_string", "data"})};
4160

4261
public:
4362
class InnerPointerBRVisitor : public BugReporterVisitor {
@@ -71,30 +90,10 @@ class InnerPointerChecker
7190
}
7291
};
7392

74-
InnerPointerChecker()
75-
: AppendFn({"std", "basic_string", "append"}),
76-
AssignFn({"std", "basic_string", "assign"}),
77-
AddressofFn({"std", "addressof"}), AddressofFn_({"std", "__addressof"}),
78-
ClearFn({"std", "basic_string", "clear"}),
79-
CStrFn({"std", "basic_string", "c_str"}), DataFn({"std", "data"}, 1),
80-
DataMemberFn({"std", "basic_string", "data"}),
81-
EraseFn({"std", "basic_string", "erase"}),
82-
InsertFn({"std", "basic_string", "insert"}),
83-
PopBackFn({"std", "basic_string", "pop_back"}),
84-
PushBackFn({"std", "basic_string", "push_back"}),
85-
ReplaceFn({"std", "basic_string", "replace"}),
86-
ReserveFn({"std", "basic_string", "reserve"}),
87-
ResizeFn({"std", "basic_string", "resize"}),
88-
ShrinkToFitFn({"std", "basic_string", "shrink_to_fit"}),
89-
SwapFn({"std", "basic_string", "swap"}) {}
90-
9193
/// Check whether the called member function potentially invalidates
9294
/// pointers referring to the container object's inner buffer.
9395
bool isInvalidatingMemberFunction(const CallEvent &Call) const;
9496

95-
/// Check whether the called function returns a raw inner pointer.
96-
bool isInnerPointerAccessFunction(const CallEvent &Call) const;
97-
9897
/// Mark pointer symbols associated with the given memory region released
9998
/// in the program state.
10099
void markPtrSymbolsReleased(const CallEvent &Call, ProgramStateRef State,
@@ -127,14 +126,7 @@ bool InnerPointerChecker::isInvalidatingMemberFunction(
127126
return false;
128127
}
129128
return isa<CXXDestructorCall>(Call) ||
130-
matchesAny(Call, AppendFn, AssignFn, ClearFn, EraseFn, InsertFn,
131-
PopBackFn, PushBackFn, ReplaceFn, ReserveFn, ResizeFn,
132-
ShrinkToFitFn, SwapFn);
133-
}
134-
135-
bool InnerPointerChecker::isInnerPointerAccessFunction(
136-
const CallEvent &Call) const {
137-
return matchesAny(Call, CStrFn, DataFn, DataMemberFn);
129+
InvalidatingMemberFunctions.contains(Call);
138130
}
139131

140132
void InnerPointerChecker::markPtrSymbolsReleased(const CallEvent &Call,
@@ -181,7 +173,7 @@ void InnerPointerChecker::checkFunctionArguments(const CallEvent &Call,
181173

182174
// std::addressof functions accepts a non-const reference as an argument,
183175
// but doesn't modify it.
184-
if (matchesAny(Call, AddressofFn, AddressofFn_))
176+
if (AddressofFunctions.contains(Call))
185177
continue;
186178

187179
markPtrSymbolsReleased(Call, State, ArgRegion, C);
@@ -221,7 +213,7 @@ void InnerPointerChecker::checkPostCall(const CallEvent &Call,
221213
}
222214
}
223215

224-
if (isInnerPointerAccessFunction(Call)) {
216+
if (InnerPointerAccessFunctions.contains(Call)) {
225217

226218
if (isa<SimpleFunctionCall>(Call)) {
227219
// NOTE: As of now, we only have one free access function: std::data.

‎clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -86,14 +86,14 @@ class SmartPtrModeling
8686
using SmartPtrMethodHandlerFn =
8787
void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
8888
CallDescriptionMap<SmartPtrMethodHandlerFn> SmartPtrMethodHandlers{
89-
{{{"reset"}}, &SmartPtrModeling::handleReset},
90-
{{{"release"}}, &SmartPtrModeling::handleRelease},
91-
{{{"swap"}, 1}, &SmartPtrModeling::handleSwapMethod},
92-
{{{"get"}}, &SmartPtrModeling::handleGet}};
93-
const CallDescription StdSwapCall{{"std", "swap"}, 2};
94-
const CallDescription StdMakeUniqueCall{{"std", "make_unique"}};
95-
const CallDescription StdMakeUniqueForOverwriteCall{
96-
{"std", "make_unique_for_overwrite"}};
89+
{{CDM::CXXMethod, {"reset"}}, &SmartPtrModeling::handleReset},
90+
{{CDM::CXXMethod, {"release"}}, &SmartPtrModeling::handleRelease},
91+
{{CDM::CXXMethod, {"swap"}, 1}, &SmartPtrModeling::handleSwapMethod},
92+
{{CDM::CXXMethod, {"get"}}, &SmartPtrModeling::handleGet}};
93+
const CallDescription StdSwapCall{CDM::SimpleFunc, {"std", "swap"}, 2};
94+
const CallDescriptionSet MakeUniqueVariants{
95+
{CDM::SimpleFunc, {"std", "make_unique"}},
96+
{CDM::SimpleFunc, {"std", "make_unique_for_overwrite"}}};
9797
};
9898
} // end of anonymous namespace
9999

@@ -296,7 +296,7 @@ bool SmartPtrModeling::evalCall(const CallEvent &Call,
296296
return handleSwap(State, Call.getArgSVal(0), Call.getArgSVal(1), C);
297297
}
298298

299-
if (matchesAny(Call, StdMakeUniqueCall, StdMakeUniqueForOverwriteCall)) {
299+
if (MakeUniqueVariants.contains(Call)) {
300300
if (!ModelSmartPtrDereference)
301301
return false;
302302

‎clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -388,17 +388,19 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
388388
};
389389

390390
CallDescriptionMap<FnDescription> FnTestDescriptions = {
391-
{{{"StreamTesterChecker_make_feof_stream"}, 1},
391+
{{CDM::SimpleFunc, {"StreamTesterChecker_make_feof_stream"}, 1},
392392
{nullptr,
393393
std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4, ErrorFEof,
394394
false),
395395
0}},
396-
{{{"StreamTesterChecker_make_ferror_stream"}, 1},
396+
{{CDM::SimpleFunc, {"StreamTesterChecker_make_ferror_stream"}, 1},
397397
{nullptr,
398398
std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4,
399399
ErrorFError, false),
400400
0}},
401-
{{{"StreamTesterChecker_make_ferror_indeterminate_stream"}, 1},
401+
{{CDM::SimpleFunc,
402+
{"StreamTesterChecker_make_ferror_indeterminate_stream"},
403+
1},
402404
{nullptr,
403405
std::bind(&StreamChecker::evalSetFeofFerror, _1, _2, _3, _4,
404406
ErrorFError, true),

0 commit comments

Comments
 (0)