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 all 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,6 +145,57 @@ using MutexDescriptor =
std::variant<FirstArgMutexDescriptor, MemberMutexDescriptor,
RAIIMutexDescriptor>;

class SuppressNonBlockingStreams : public BugReporterVisitor {
private:
const CallDescription OpenFunction{CDM::CLibrary, {"open"}, 2};
SymbolRef StreamSym;
const int NonBlockMacroVal;
bool Satisfied = false;

public:
SuppressNonBlockingStreams(SymbolRef StreamSym, int NonBlockMacroVal)
: StreamSym(StreamSym), NonBlockMacroVal(NonBlockMacroVal) {}

static void *getTag() {
static bool Tag;
return &Tag;
}

void Profile(llvm::FoldingSetNodeID &ID) const override {
ID.AddPointer(getTag());
}

PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
BugReporterContext &BRC,
PathSensitiveBugReport &BR) override {
if (Satisfied)
return nullptr;

std::optional<StmtPoint> Point = N->getLocationAs<StmtPoint>();
if (!Point)
return nullptr;

const auto *CE = Point->getStmtAs<CallExpr>();
if (!CE || !OpenFunction.matchesAsWritten(*CE))
return nullptr;

if (N->getSVal(CE).getAsSymbol() != StreamSym)
return nullptr;

Satisfied = true;

// Check if open's second argument contains O_NONBLOCK
const llvm::APSInt *FlagVal = N->getSVal(CE->getArg(1)).getAsInteger();
if (!FlagVal)
return nullptr;

if ((*FlagVal & NonBlockMacroVal) != 0)
BR.markInvalid(getTag(), nullptr);

return nullptr;
}
};

class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
private:
const std::array<MutexDescriptor, 8> MutexDescriptors{
Expand Down Expand Up @@ -182,6 +233,9 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
const BugType BlockInCritSectionBugType{
this, "Call to blocking function in critical section", "Blocking Error"};

using O_NONBLOCKValueTy = std::optional<int>;
mutable std::optional<O_NONBLOCKValueTy> O_NONBLOCKValue;

void reportBlockInCritSection(const CallEvent &call, CheckerContext &C) const;

[[nodiscard]] const NoteTag *createCritSectionNote(CritSectionMarker M,
Expand Down Expand Up @@ -337,6 +391,28 @@ void BlockInCriticalSectionChecker::reportBlockInCritSection(
<< "' inside of critical section";
auto R = std::make_unique<PathSensitiveBugReport>(BlockInCritSectionBugType,
os.str(), ErrNode);
// 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") {
SVal SV = Call.getArgSVal(0);
SValBuilder &SVB = C.getSValBuilder();
ProgramStateRef state = C.getState();
ConditionTruthVal CTV =
state->areEqual(SV, SVB.makeIntVal(-1, C.getASTContext().IntTy));
if (CTV.isConstrainedTrue())
return;

if (SymbolRef SR = SV.getAsSymbol()) {
if (!O_NONBLOCKValue)
O_NONBLOCKValue = tryExpandAsInteger(
"O_NONBLOCK", C.getBugReporter().getPreprocessor());
if (*O_NONBLOCKValue)
R->addVisitor<SuppressNonBlockingStreams>(SR, **O_NONBLOCKValue);
}
}
R->addRange(Call.getSourceRange());
R->markInteresting(Call.getReturnValue());
C.emitReport(std::move(R));
Expand Down
13 changes: 13 additions & 0 deletions clang/test/Analysis/Inputs/system-header-simulator-cxx-std-locks.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#pragma clang system_header

namespace std {
struct mutex {
void lock();
void unlock();
};

template <typename T> struct lock_guard {
lock_guard(std::mutex &);
~lock_guard();
};
} // namespace std
37 changes: 37 additions & 0 deletions clang/test/Analysis/issue-124474.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// RUN: %clang_analyze_cc1 \
// RUN: -analyzer-checker=core,unix.BlockInCriticalSection \
// RUN: -analyzer-output text -verify %s

// expected-no-diagnostics

#include "Inputs/system-header-simulator-cxx-std-locks.h"

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 or equals to -1
close(fd);
}

void foo1(int fd) {
std::lock_guard<std::mutex> lock(mtx);

const char *filename = "example.txt";
char buffer[200] = {};
if (fd == -1)
read(fd, buffer, 199); // no-warning: consider file descriptor is a symbol equals to -1
close(fd);
}