[clang][analyzer] Teach the BlockInCriticalSectionChecker about O_NONBLOCK streams by flovent · Pull Request #127049 · llvm/llvm-project (original) (raw)

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-static-analyzer-1

Author: None (flovent)

Changes

this PR close #124474
when calling read and recv function for a non-block file descriptor or a invalid file descriptor(-1), it will not cause block inside a critical section.
this commit checks for non-block file descriptor assigned by open function with O_NONBLOCK flag.

Full diff: https://github.com/llvm/llvm-project/pull/127049.diff

2 Files Affected:

• (modified) clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp (+66-1)
• (added) clang/test/Analysis/issue-124474.cpp (+49)

diff --git a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp index 7460781799d08..db784f2cc77b2 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp @@ -145,7 +145,8 @@ using MutexDescriptor = std::variant<FirstArgMutexDescriptor, MemberMutexDescriptor, RAIIMutexDescriptor>;

-class BlockInCriticalSectionChecker : public Checkercheck::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 @@ -179,6 +180,8 @@ class BlockInCriticalSectionChecker : public Checkercheck::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"};

@@ -197,6 +200,8 @@ class BlockInCriticalSectionChecker : public Checkercheck::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;

@@ -205,11 +210,14 @@ class BlockInCriticalSectionChecker : public Checkercheck::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. @@ -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 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) && @@ -315,6 +342,27 @@ 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 LockDesc = checkDescriptorMatch(Call, C, /IsLock=/true)) {

@@ -322,9 +370,26 @@ void BlockInCriticalSectionChecker::checkPostCall(const CallEvent &Call, } else if (std::optional 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();
• 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()); diff --git a/clang/test/Analysis/issue-124474.cpp b/clang/test/Analysis/issue-124474.cpp new file mode 100644 index 0000000000000..09e3d4f3f9ad9 --- /dev/null +++ b/clang/test/Analysis/issue-124474.cpp @@ -0,0 +1,49 @@ +// 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
• struct lock_guard {
• lock_guard(std::mutex) {}
• ~lock_guard() {}
• };
• template
• struct unique_lock {
• unique_lock(std::mutex) {}
• ~unique_lock() {}
• };
• template
• struct not_real_lock {
• not_real_lock(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_guardstd::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);

+}