Skip to content

[clang][analyzer] Teach the BlockInCriticalSectionChecker about O_NONBLOCK streams #127049

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Feb 17, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ using MutexDescriptor =
std::variant<FirstArgMutexDescriptor, MemberMutexDescriptor,
RAIIMutexDescriptor>;

class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
class BlockInCriticalSectionChecker
: public Checker<check::PostCall, check::DeadSymbols> {
private:
const std::array<MutexDescriptor, 8> MutexDescriptors{
// NOTE: There are standard library implementations where some methods
Expand Down Expand Up @@ -179,6 +180,8 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
{CDM::CLibrary, {"read"}},
{CDM::CLibrary, {"recv"}}};

const CallDescription OpenFunction{CDM::CLibrary, {"open"}, 2};

const BugType BlockInCritSectionBugType{
this, "Call to blocking function in critical section", "Blocking Error"};

Expand All @@ -197,6 +200,8 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
void handleUnlock(const MutexDescriptor &Mutex, const CallEvent &Call,
CheckerContext &C) const;

void handleOpen(const CallEvent &Call, CheckerContext &C) const;

[[nodiscard]] bool isBlockingInCritSection(const CallEvent &Call,
CheckerContext &C) const;

Expand All @@ -205,11 +210,14 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
/// Process lock.
/// Process blocking functions (sleep, getc, fgets, read, recv)
void checkPostCall(const CallEvent &Call, CheckerContext &C) const;

void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
};

} // end anonymous namespace

REGISTER_LIST_WITH_PROGRAMSTATE(ActiveCritSections, CritSectionMarker)
REGISTER_SET_WITH_PROGRAMSTATE(NonBlockFileDescriptor, SymbolRef)

// Iterator traits for ImmutableList data structure
// that enable the use of STL algorithms.
Expand Down Expand Up @@ -306,6 +314,25 @@ void BlockInCriticalSectionChecker::handleUnlock(
C.addTransition(State);
}

void BlockInCriticalSectionChecker::handleOpen(const CallEvent &Call,
CheckerContext &C) const {
const auto *Flag = Call.getArgExpr(1);
static std::optional<int> ValueOfONonBlockVFlag =
tryExpandAsInteger("O_NONBLOCK", C.getBugReporter().getPreprocessor());
if (!ValueOfONonBlockVFlag)
return;

SVal FlagSV = C.getState()->getSVal(Flag, C.getLocationContext());
const llvm::APSInt *FlagV = FlagSV.getAsInteger();
if (!FlagV)
return;

if ((*FlagV & ValueOfONonBlockVFlag.value()) != 0)
if (SymbolRef SR = Call.getReturnValue().getAsSymbol()) {
C.addTransition(C.getState()->add<NonBlockFileDescriptor>(SR));
}
}

bool BlockInCriticalSectionChecker::isBlockingInCritSection(
const CallEvent &Call, CheckerContext &C) const {
return BlockingFunctions.contains(Call) &&
Expand All @@ -315,16 +342,54 @@ bool BlockInCriticalSectionChecker::isBlockingInCritSection(
void BlockInCriticalSectionChecker::checkPostCall(const CallEvent &Call,
CheckerContext &C) const {
if (isBlockingInCritSection(Call, C)) {
// for 'read' and 'recv' call, check whether it's file descriptor(first
// argument) is
// created by 'open' API with O_NONBLOCK flag or is equal to -1, they will
// not cause block in these situations, don't report
StringRef FuncName = Call.getCalleeIdentifier()->getName();
if (FuncName == "read" || FuncName == "recv") {
const auto *Arg = Call.getArgExpr(0);
if (!Arg)
return;

SVal SV = C.getSVal(Arg);
if (const auto *IntValue = SV.getAsInteger()) {
if (*IntValue == -1)
return;
}

SymbolRef SR = C.getSVal(Arg).getAsSymbol();
if (SR && C.getState()->contains<NonBlockFileDescriptor>(SR)) {
return;
}
}
reportBlockInCritSection(Call, C);
} else if (std::optional<MutexDescriptor> LockDesc =
checkDescriptorMatch(Call, C, /*IsLock=*/true)) {
handleLock(*LockDesc, Call, C);
} else if (std::optional<MutexDescriptor> UnlockDesc =
checkDescriptorMatch(Call, C, /*IsLock=*/false)) {
handleUnlock(*UnlockDesc, Call, C);
} else if (OpenFunction.matches(Call)) {
handleOpen(Call, C);
}
}

void BlockInCriticalSectionChecker::checkDeadSymbols(SymbolReaper &SymReaper,
CheckerContext &C) const {
ProgramStateRef State = C.getState();

// Remove the dead symbols from the NonBlockFileDescriptor set.
NonBlockFileDescriptorTy Tracked = State->get<NonBlockFileDescriptor>();
for (SymbolRef SR : Tracked) {
if (SymReaper.isDead(SR)) {
State = State->remove<NonBlockFileDescriptor>(SR);
}
}

C.addTransition(State);
}

void BlockInCriticalSectionChecker::reportBlockInCritSection(
const CallEvent &Call, CheckerContext &C) const {
ExplodedNode *ErrNode = C.generateNonFatalErrorNode(C.getState());
Expand Down
50 changes: 50 additions & 0 deletions clang/test/Analysis/issue-124474.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// RUN: %clang_analyze_cc1 \
// RUN: -analyzer-checker=unix.BlockInCriticalSection \
// RUN: -std=c++11 \
// RUN: -analyzer-output text \
// RUN: -verify %s

// expected-no-diagnostics

namespace std {
struct mutex {
void lock() {}
void unlock() {}
};
template<typename T>
struct lock_guard {
lock_guard<T>(std::mutex) {}
~lock_guard<T>() {}
};
template<typename T>
struct unique_lock {
unique_lock<T>(std::mutex) {}
~unique_lock<T>() {}
};
template<typename T>
struct not_real_lock {
not_real_lock<T>(std::mutex) {}
};
} // namespace std

std::mutex mtx;
using ssize_t = long long;
using size_t = unsigned long long;
int open (const char *__file, int __oflag, ...);
ssize_t read(int fd, void *buf, size_t count);
void close(int fd);
#define O_RDONLY 00
# define O_NONBLOCK 04000

void foo()
{
std::lock_guard<std::mutex> lock(mtx);

const char *filename = "example.txt";
int fd = open(filename, O_RDONLY | O_NONBLOCK);

char buffer[200] = {};
read(fd, buffer, 199); // no-warning: fd is a non-block file descriptor
close(fd);
}