Skip to content

Commit

Permalink
[analyzer] Use CDM::CLibrary instead of isGlobalCFunction() (#88267)
Browse files Browse the repository at this point in the history 
This commit updates several checkers to use call descriptions with the
matching mode `CDM::CLibrary` instead of checking
`Call.isGlobalCFunction()` after performing the match. This resolves
several TODOs in various checkers.

Note that both matching with `CDM::CLibrary` and calling
`isGlobalCFunction` leads to `CheckerContext::isCLibraryFunction()`
checks (so this change is close to being NFC), but if it is used via the
matching mode then the checker can automatically recognize the builtin
variants of the matched functions.

I'll also make similar changes in GenericTaintChecker, but that checker
has separate and inconsistent rules for handling the normal and the
builtin variant of several functions (e.g. `memcpy` and
`__builtin_memcpy`), so I'll put those changes into a separate commit.
  • Loading branch information
@NagyDonat
NagyDonat committed Apr 11, 2024
1 parent def6174 commit fe3b20d
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 111 deletions.
12 changes: 4 additions & 8 deletions clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -542,10 +542,10 @@ namespace {
class CFRetainReleaseChecker : public Checker<check::PreCall> {
mutable APIMisuse BT{this, "null passed to CF memory management function"};
const CallDescriptionSet ModelledCalls = {
{{"CFRetain"}, 1},
{{"CFRelease"}, 1},
{{"CFMakeCollectable"}, 1},
{{"CFAutorelease"}, 1},
{CDM::CLibrary, {"CFRetain"}, 1},
{CDM::CLibrary, {"CFRelease"}, 1},
{CDM::CLibrary, {"CFMakeCollectable"}, 1},
{CDM::CLibrary, {"CFAutorelease"}, 1},
};

public:
Expand All @@ -555,10 +555,6 @@ class CFRetainReleaseChecker : public Checker<check::PreCall> {

void CFRetainReleaseChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
// TODO: Make this check part of CallDescription.
if (!Call.isGlobalCFunction())
return;

// Check if we called CFRetain/CFRelease/CFMakeCollectable/CFAutorelease.
if (!ModelledCalls.contains(Call))
return;
Expand Down
115 changes: 72 additions & 43 deletions clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,82 +87,115 @@ class PthreadLockChecker : public Checker<check::PostCall, check::DeadSymbols,
CheckerKind CheckKind) const;
CallDescriptionMap<FnCheck> PThreadCallbacks = {
// Init.
{{{"pthread_mutex_init"}, 2}, &PthreadLockChecker::InitAnyLock},
{{CDM::CLibrary, {"pthread_mutex_init"}, 2},
&PthreadLockChecker::InitAnyLock},
// TODO: pthread_rwlock_init(2 arguments).
// TODO: lck_mtx_init(3 arguments).
// TODO: lck_mtx_alloc_init(2 arguments) => returns the mutex.
// TODO: lck_rw_init(3 arguments).
// TODO: lck_rw_alloc_init(2 arguments) => returns the mutex.

// Acquire.
{{{"pthread_mutex_lock"}, 1}, &PthreadLockChecker::AcquirePthreadLock},
{{{"pthread_rwlock_rdlock"}, 1}, &PthreadLockChecker::AcquirePthreadLock},
{{{"pthread_rwlock_wrlock"}, 1}, &PthreadLockChecker::AcquirePthreadLock},
{{{"lck_mtx_lock"}, 1}, &PthreadLockChecker::AcquireXNULock},
{{{"lck_rw_lock_exclusive"}, 1}, &PthreadLockChecker::AcquireXNULock},
{{{"lck_rw_lock_shared"}, 1}, &PthreadLockChecker::AcquireXNULock},
{{CDM::CLibrary, {"pthread_mutex_lock"}, 1},
&PthreadLockChecker::AcquirePthreadLock},
{{CDM::CLibrary, {"pthread_rwlock_rdlock"}, 1},
&PthreadLockChecker::AcquirePthreadLock},
{{CDM::CLibrary, {"pthread_rwlock_wrlock"}, 1},
&PthreadLockChecker::AcquirePthreadLock},
{{CDM::CLibrary, {"lck_mtx_lock"}, 1},
&PthreadLockChecker::AcquireXNULock},
{{CDM::CLibrary, {"lck_rw_lock_exclusive"}, 1},
&PthreadLockChecker::AcquireXNULock},
{{CDM::CLibrary, {"lck_rw_lock_shared"}, 1},
&PthreadLockChecker::AcquireXNULock},

// Try.
{{{"pthread_mutex_trylock"}, 1}, &PthreadLockChecker::TryPthreadLock},
{{{"pthread_rwlock_tryrdlock"}, 1}, &PthreadLockChecker::TryPthreadLock},
{{{"pthread_rwlock_trywrlock"}, 1}, &PthreadLockChecker::TryPthreadLock},
{{{"lck_mtx_try_lock"}, 1}, &PthreadLockChecker::TryXNULock},
{{{"lck_rw_try_lock_exclusive"}, 1}, &PthreadLockChecker::TryXNULock},
{{{"lck_rw_try_lock_shared"}, 1}, &PthreadLockChecker::TryXNULock},
{{CDM::CLibrary, {"pthread_mutex_trylock"}, 1},
&PthreadLockChecker::TryPthreadLock},
{{CDM::CLibrary, {"pthread_rwlock_tryrdlock"}, 1},
&PthreadLockChecker::TryPthreadLock},
{{CDM::CLibrary, {"pthread_rwlock_trywrlock"}, 1},
&PthreadLockChecker::TryPthreadLock},
{{CDM::CLibrary, {"lck_mtx_try_lock"}, 1},
&PthreadLockChecker::TryXNULock},
{{CDM::CLibrary, {"lck_rw_try_lock_exclusive"}, 1},
&PthreadLockChecker::TryXNULock},
{{CDM::CLibrary, {"lck_rw_try_lock_shared"}, 1},
&PthreadLockChecker::TryXNULock},

// Release.
{{{"pthread_mutex_unlock"}, 1}, &PthreadLockChecker::ReleaseAnyLock},
{{{"pthread_rwlock_unlock"}, 1}, &PthreadLockChecker::ReleaseAnyLock},
{{{"lck_mtx_unlock"}, 1}, &PthreadLockChecker::ReleaseAnyLock},
{{{"lck_rw_unlock_exclusive"}, 1}, &PthreadLockChecker::ReleaseAnyLock},
{{{"lck_rw_unlock_shared"}, 1}, &PthreadLockChecker::ReleaseAnyLock},
{{{"lck_rw_done"}, 1}, &PthreadLockChecker::ReleaseAnyLock},
{{CDM::CLibrary, {"pthread_mutex_unlock"}, 1},
&PthreadLockChecker::ReleaseAnyLock},
{{CDM::CLibrary, {"pthread_rwlock_unlock"}, 1},
&PthreadLockChecker::ReleaseAnyLock},
{{CDM::CLibrary, {"lck_mtx_unlock"}, 1},
&PthreadLockChecker::ReleaseAnyLock},
{{CDM::CLibrary, {"lck_rw_unlock_exclusive"}, 1},
&PthreadLockChecker::ReleaseAnyLock},
{{CDM::CLibrary, {"lck_rw_unlock_shared"}, 1},
&PthreadLockChecker::ReleaseAnyLock},
{{CDM::CLibrary, {"lck_rw_done"}, 1},
&PthreadLockChecker::ReleaseAnyLock},

// Destroy.
{{{"pthread_mutex_destroy"}, 1}, &PthreadLockChecker::DestroyPthreadLock},
{{{"lck_mtx_destroy"}, 2}, &PthreadLockChecker::DestroyXNULock},
{{CDM::CLibrary, {"pthread_mutex_destroy"}, 1},
&PthreadLockChecker::DestroyPthreadLock},
{{CDM::CLibrary, {"lck_mtx_destroy"}, 2},
&PthreadLockChecker::DestroyXNULock},
// TODO: pthread_rwlock_destroy(1 argument).
// TODO: lck_rw_destroy(2 arguments).
};

CallDescriptionMap<FnCheck> FuchsiaCallbacks = {
// Init.
{{{"spin_lock_init"}, 1}, &PthreadLockChecker::InitAnyLock},
{{CDM::CLibrary, {"spin_lock_init"}, 1},
&PthreadLockChecker::InitAnyLock},

// Acquire.
{{{"spin_lock"}, 1}, &PthreadLockChecker::AcquirePthreadLock},
{{{"spin_lock_save"}, 3}, &PthreadLockChecker::AcquirePthreadLock},
{{{"sync_mutex_lock"}, 1}, &PthreadLockChecker::AcquirePthreadLock},
{{{"sync_mutex_lock_with_waiter"}, 1},
{{CDM::CLibrary, {"spin_lock"}, 1},
&PthreadLockChecker::AcquirePthreadLock},
{{CDM::CLibrary, {"spin_lock_save"}, 3},
&PthreadLockChecker::AcquirePthreadLock},
{{CDM::CLibrary, {"sync_mutex_lock"}, 1},
&PthreadLockChecker::AcquirePthreadLock},
{{CDM::CLibrary, {"sync_mutex_lock_with_waiter"}, 1},
&PthreadLockChecker::AcquirePthreadLock},

// Try.
{{{"spin_trylock"}, 1}, &PthreadLockChecker::TryFuchsiaLock},
{{{"sync_mutex_trylock"}, 1}, &PthreadLockChecker::TryFuchsiaLock},
{{{"sync_mutex_timedlock"}, 2}, &PthreadLockChecker::TryFuchsiaLock},
{{CDM::CLibrary, {"spin_trylock"}, 1},
&PthreadLockChecker::TryFuchsiaLock},
{{CDM::CLibrary, {"sync_mutex_trylock"}, 1},
&PthreadLockChecker::TryFuchsiaLock},
{{CDM::CLibrary, {"sync_mutex_timedlock"}, 2},
&PthreadLockChecker::TryFuchsiaLock},

// Release.
{{{"spin_unlock"}, 1}, &PthreadLockChecker::ReleaseAnyLock},
{{{"spin_unlock_restore"}, 3}, &PthreadLockChecker::ReleaseAnyLock},
{{{"sync_mutex_unlock"}, 1}, &PthreadLockChecker::ReleaseAnyLock},
{{CDM::CLibrary, {"spin_unlock"}, 1},
&PthreadLockChecker::ReleaseAnyLock},
{{CDM::CLibrary, {"spin_unlock_restore"}, 3},
&PthreadLockChecker::ReleaseAnyLock},
{{CDM::CLibrary, {"sync_mutex_unlock"}, 1},
&PthreadLockChecker::ReleaseAnyLock},
};

CallDescriptionMap<FnCheck> C11Callbacks = {
// Init.
{{{"mtx_init"}, 2}, &PthreadLockChecker::InitAnyLock},
{{CDM::CLibrary, {"mtx_init"}, 2}, &PthreadLockChecker::InitAnyLock},

// Acquire.
{{{"mtx_lock"}, 1}, &PthreadLockChecker::AcquirePthreadLock},
{{CDM::CLibrary, {"mtx_lock"}, 1},
&PthreadLockChecker::AcquirePthreadLock},

// Try.
{{{"mtx_trylock"}, 1}, &PthreadLockChecker::TryC11Lock},
{{{"mtx_timedlock"}, 2}, &PthreadLockChecker::TryC11Lock},
{{CDM::CLibrary, {"mtx_trylock"}, 1}, &PthreadLockChecker::TryC11Lock},
{{CDM::CLibrary, {"mtx_timedlock"}, 2}, &PthreadLockChecker::TryC11Lock},

// Release.
{{{"mtx_unlock"}, 1}, &PthreadLockChecker::ReleaseAnyLock},
{{CDM::CLibrary, {"mtx_unlock"}, 1}, &PthreadLockChecker::ReleaseAnyLock},

// Destroy
{{{"mtx_destroy"}, 1}, &PthreadLockChecker::DestroyPthreadLock},
{{CDM::CLibrary, {"mtx_destroy"}, 1},
&PthreadLockChecker::DestroyPthreadLock},
};

ProgramStateRef resolvePossiblyDestroyedMutex(ProgramStateRef state,
Expand Down Expand Up @@ -258,13 +291,9 @@ REGISTER_MAP_WITH_PROGRAMSTATE(DestroyRetVal, const MemRegion *, SymbolRef)

void PthreadLockChecker::checkPostCall(const CallEvent &Call,
CheckerContext &C) const {
// An additional umbrella check that all functions modeled by this checker
// are global C functions.
// TODO: Maybe make this the default behavior of CallDescription
// with exactly one identifier?
// FIXME: Try to handle cases when the implementation was inlined rather
// than just giving up.
if (!Call.isGlobalCFunction() || C.wasInlined)
if (C.wasInlined)
return;

if (const FnCheck *Callback = PThreadCallbacks.lookup(Call))
Expand Down
10 changes: 2 additions & 8 deletions clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ class SimpleStreamChecker : public Checker<check::PostCall,
check::PreCall,
check::DeadSymbols,
check::PointerEscape> {
const CallDescription OpenFn{{"fopen"}, 2};
const CallDescription CloseFn{{"fclose"}, 1};
const CallDescription OpenFn{CDM::CLibrary, {"fopen"}, 2};
const CallDescription CloseFn{CDM::CLibrary, {"fclose"}, 1};

const BugType DoubleCloseBugType{this, "Double fclose",
"Unix Stream API Error"};
Expand Down Expand Up @@ -92,9 +92,6 @@ REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)

void SimpleStreamChecker::checkPostCall(const CallEvent &Call,
CheckerContext &C) const {
if (!Call.isGlobalCFunction())
return;

if (!OpenFn.matches(Call))
return;

Expand All @@ -111,9 +108,6 @@ void SimpleStreamChecker::checkPostCall(const CallEvent &Call,

void SimpleStreamChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
if (!Call.isGlobalCFunction())
return;

if (!CloseFn.matches(Call))
return;

Expand Down
69 changes: 35 additions & 34 deletions clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,85 +302,88 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,

private:
CallDescriptionMap<FnDescription> FnDescriptions = {
{{{"fopen"}, 2}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
{{{"fdopen"}, 2}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
{{{"freopen"}, 3},
{{CDM::CLibrary, {"fopen"}, 2},
{nullptr, &StreamChecker::evalFopen, ArgNone}},
{{CDM::CLibrary, {"fdopen"}, 2},
{nullptr, &StreamChecker::evalFopen, ArgNone}},
{{CDM::CLibrary, {"freopen"}, 3},
{&StreamChecker::preFreopen, &StreamChecker::evalFreopen, 2}},
{{{"tmpfile"}, 0}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
{{{"fclose"}, 1},
{{CDM::CLibrary, {"tmpfile"}, 0},
{nullptr, &StreamChecker::evalFopen, ArgNone}},
{{CDM::CLibrary, {"fclose"}, 1},
{&StreamChecker::preDefault, &StreamChecker::evalFclose, 0}},
{{{"fread"}, 4},
{{CDM::CLibrary, {"fread"}, 4},
{&StreamChecker::preRead,
std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, true), 3}},
{{{"fwrite"}, 4},
{{CDM::CLibrary, {"fwrite"}, 4},
{&StreamChecker::preWrite,
std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, false), 3}},
{{{"fgetc"}, 1},
{{CDM::CLibrary, {"fgetc"}, 1},
{&StreamChecker::preRead,
std::bind(&StreamChecker::evalFgetx, _1, _2, _3, _4, true), 0}},
{{{"fgets"}, 3},
{{CDM::CLibrary, {"fgets"}, 3},
{&StreamChecker::preRead,
std::bind(&StreamChecker::evalFgetx, _1, _2, _3, _4, false), 2}},
{{{"getc"}, 1},
{{CDM::CLibrary, {"getc"}, 1},
{&StreamChecker::preRead,
std::bind(&StreamChecker::evalFgetx, _1, _2, _3, _4, true), 0}},
{{{"fputc"}, 2},
{{CDM::CLibrary, {"fputc"}, 2},
{&StreamChecker::preWrite,
std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, true), 1}},
{{{"fputs"}, 2},
{{CDM::CLibrary, {"fputs"}, 2},
{&StreamChecker::preWrite,
std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, false), 1}},
{{{"putc"}, 2},
{{CDM::CLibrary, {"putc"}, 2},
{&StreamChecker::preWrite,
std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, true), 1}},
{{{"fprintf"}},
{{CDM::CLibrary, {"fprintf"}},
{&StreamChecker::preWrite,
std::bind(&StreamChecker::evalFprintf, _1, _2, _3, _4), 0}},
{{{"vfprintf"}, 3},
{{CDM::CLibrary, {"vfprintf"}, 3},
{&StreamChecker::preWrite,
std::bind(&StreamChecker::evalFprintf, _1, _2, _3, _4), 0}},
{{{"fscanf"}},
{{CDM::CLibrary, {"fscanf"}},
{&StreamChecker::preRead,
std::bind(&StreamChecker::evalFscanf, _1, _2, _3, _4), 0}},
{{{"vfscanf"}, 3},
{{CDM::CLibrary, {"vfscanf"}, 3},
{&StreamChecker::preRead,
std::bind(&StreamChecker::evalFscanf, _1, _2, _3, _4), 0}},
{{{"ungetc"}, 2},
{{CDM::CLibrary, {"ungetc"}, 2},
{&StreamChecker::preWrite,
std::bind(&StreamChecker::evalUngetc, _1, _2, _3, _4), 1}},
{{{"getdelim"}, 4},
{{CDM::CLibrary, {"getdelim"}, 4},
{&StreamChecker::preRead,
std::bind(&StreamChecker::evalGetdelim, _1, _2, _3, _4), 3}},
{{{"getline"}, 3},
{{CDM::CLibrary, {"getline"}, 3},
{&StreamChecker::preRead,
std::bind(&StreamChecker::evalGetdelim, _1, _2, _3, _4), 2}},
{{{"fseek"}, 3},
{{CDM::CLibrary, {"fseek"}, 3},
{&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
{{{"fseeko"}, 3},
{{CDM::CLibrary, {"fseeko"}, 3},
{&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
{{{"ftell"}, 1},
{{CDM::CLibrary, {"ftell"}, 1},
{&StreamChecker::preWrite, &StreamChecker::evalFtell, 0}},
{{{"ftello"}, 1},
{{CDM::CLibrary, {"ftello"}, 1},
{&StreamChecker::preWrite, &StreamChecker::evalFtell, 0}},
{{{"fflush"}, 1},
{{CDM::CLibrary, {"fflush"}, 1},
{&StreamChecker::preFflush, &StreamChecker::evalFflush, 0}},
{{{"rewind"}, 1},
{{CDM::CLibrary, {"rewind"}, 1},
{&StreamChecker::preDefault, &StreamChecker::evalRewind, 0}},
{{{"fgetpos"}, 2},
{{CDM::CLibrary, {"fgetpos"}, 2},
{&StreamChecker::preWrite, &StreamChecker::evalFgetpos, 0}},
{{{"fsetpos"}, 2},
{{CDM::CLibrary, {"fsetpos"}, 2},
{&StreamChecker::preDefault, &StreamChecker::evalFsetpos, 0}},
{{{"clearerr"}, 1},
{{CDM::CLibrary, {"clearerr"}, 1},
{&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0}},
{{{"feof"}, 1},
{{CDM::CLibrary, {"feof"}, 1},
{&StreamChecker::preDefault,
std::bind(&StreamChecker::evalFeofFerror, _1, _2, _3, _4, ErrorFEof),
0}},
{{{"ferror"}, 1},
{{CDM::CLibrary, {"ferror"}, 1},
{&StreamChecker::preDefault,
std::bind(&StreamChecker::evalFeofFerror, _1, _2, _3, _4, ErrorFError),
0}},
{{{"fileno"}, 1},
{{CDM::CLibrary, {"fileno"}, 1},
{&StreamChecker::preDefault, &StreamChecker::evalFileno, 0}},
};

Expand Down Expand Up @@ -540,8 +543,6 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
const FnDescription *lookupFn(const CallEvent &Call) const {
// Recognize "global C functions" with only integral or pointer arguments
// (and matching name) as stream functions.
if (!Call.isGlobalCFunction())
return nullptr;
for (auto *P : Call.parameters()) {
QualType T = P->getType();
if (!T->isIntegralOrEnumerationType() && !T->isPointerType() &&
Expand Down

0 comments on commit fe3b20d

Please sign in to comment.