[clang] 9d48705 - [clang][analyzer] Teach the BlockInCriticalSectionChecker about O_NONBLOCK streams (#127049)

[via cfe-commits]

Author: flovent
Date: 2025-02-17T15:35:40+01:00
New Revision: 9d487050a144b895950a6fd48b993513a714e69d

URL: https://github.com/llvm/llvm-project/commit/9d487050a144b895950a6fd48b993513a714e69d
DIFF: https://github.com/llvm/llvm-project/commit/9d487050a144b895950a6fd48b993513a714e69d.diff

LOG: [clang][analyzer] Teach the BlockInCriticalSectionChecker about O_NONBLOCK streams (#127049)

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.

---------

Co-authored-by: Balazs Benics <benicsbalazs at gmail.com>

Added: 
    clang/test/Analysis/Inputs/system-header-simulator-cxx-std-locks.h
    clang/test/Analysis/issue-124474.cpp

Modified: 
    clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
index 7460781799d08..bf35bee70870b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
@@ -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{
@@ -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,
@@ -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));

diff  --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx-std-locks.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx-std-locks.h
new file mode 100644
index 0000000000000..054dd5405e1be
--- /dev/null
+++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx-std-locks.h
@@ -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

diff  --git a/clang/test/Analysis/issue-124474.cpp b/clang/test/Analysis/issue-124474.cpp
new file mode 100644
index 0000000000000..ae30c4db552c1
--- /dev/null
+++ b/clang/test/Analysis/issue-124474.cpp
@@ -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);
+}