[clang] [clang][analyzer] fix false positive of BlockInCriticalSectionChecker (PR #126752)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: None (flovent)

<details>
<summary>Changes</summary>

this PR close #<!-- -->124474 

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


2 Files Affected:

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


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
index 7460781799d08a..cfff5a25e7a50f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
@@ -145,6 +145,92 @@ using MutexDescriptor =
     std::variant<FirstArgMutexDescriptor, MemberMutexDescriptor,
                  RAIIMutexDescriptor>;
 
+class NonBlockOpenVisitor : public BugReporterVisitor {
+private:
+  const VarRegion *VR;
+  const CallExpr *OpenCallExpr;
+  int O_NONBLOCKV;
+  bool Satisfied;
+  CallDescription OpenFunction{CDM::CLibrary, {"open"}, 2};
+
+public:
+  NonBlockOpenVisitor(const VarRegion *VR, int O_NONBLOCKV)
+      : VR(VR), OpenCallExpr(nullptr), O_NONBLOCKV(O_NONBLOCKV),
+        Satisfied(false) {}
+
+  static void *getTag() {
+    static int Tag = 0;
+    return static_cast<void *>(&Tag);
+  }
+
+  void Profile(llvm::FoldingSetNodeID &ID) const override {
+    ID.AddPointer(getTag());
+    ID.AddPointer(VR);
+  }
+
+  PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
+                                   BugReporterContext &BRC,
+                                   PathSensitiveBugReport &BR) override {
+    if (Satisfied)
+      return nullptr;
+
+    // check for open call's 2th argument
+    if (std::optional<StmtPoint> P = N->getLocationAs<StmtPoint>()) {
+      if (OpenCallExpr && OpenCallExpr == P->getStmtAs<CallExpr>()) {
+        Satisfied = true;
+        const auto *openFlagExpr = OpenCallExpr->getArg(1);
+        SVal flagSV = N->getSVal(openFlagExpr);
+        const llvm::APSInt *flagV = flagSV.getAsInteger();
+        if (!flagV)
+          return nullptr;
+
+        if ((*flagV & O_NONBLOCKV) != 0)
+          BR.markInvalid(getTag(), nullptr);
+
+        return nullptr;
+      }
+    }
+
+    const ExplodedNode *Pred = N->getFirstPred();
+    SVal presv = Pred->getState()->getSVal(VR);
+    SVal sv = N->getState()->getSVal(VR);
+
+    // check if file descirptor's SVal changed between nodes
+    if (sv == presv)
+      return nullptr;
+
+    std::optional<PostStore> P = N->getLocationAs<PostStore>();
+    if (!P)
+      return nullptr;
+
+    if (const auto *DS = P->getStmtAs<DeclStmt>()) {
+      // variable initialization
+      // int fd = open(...)
+      const VarDecl *VD = VR->getDecl();
+      if (DS->getSingleDecl() == VR->getDecl()) {
+        const auto *InitCall = dyn_cast_if_present<CallExpr>(VD->getInit());
+        if (!InitCall || !OpenFunction.matchesAsWritten(*InitCall))
+          return nullptr;
+
+        OpenCallExpr = InitCall;
+      }
+    } else if (const auto *BO = P->getStmtAs<BinaryOperator>()) {
+      // assignment
+      // fd = open(...)
+      const auto *DRE = dyn_cast<DeclRefExpr>(BO->getLHS());
+      if (DRE && DRE->getDecl() == VR->getDecl()) {
+        const auto *InitCall = dyn_cast<CallExpr>(BO->getRHS());
+        if (!InitCall || !OpenFunction.matchesAsWritten(*InitCall))
+          return nullptr;
+
+        OpenCallExpr = InitCall;
+      }
+    }
+
+    return nullptr;
+  }
+};
+
 class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
 private:
   const std::array<MutexDescriptor, 8> MutexDescriptors{
@@ -339,6 +425,36 @@ void BlockInCriticalSectionChecker::reportBlockInCritSection(
                                                     os.str(), ErrNode);
   R->addRange(Call.getSourceRange());
   R->markInteresting(Call.getReturnValue());
+  // for 'read' call, check whether it's file descriptor(first argument) is
+  // created by 'open' API with O_NONBLOCK flag and don't report for this
+  // situation.
+  if (Call.getCalleeIdentifier()->getName() == "read") {
+    do {
+      const auto *arg = Call.getArgExpr(0);
+      if (!arg)
+        break;
+
+      const auto *DRE = dyn_cast<DeclRefExpr>(arg->IgnoreImpCasts());
+      if (!DRE)
+        break;
+
+      const auto *VD = dyn_cast_if_present<VarDecl>(DRE->getDecl());
+      if (!VD)
+        break;
+
+      const VarRegion *VR = C.getState()->getRegion(VD, C.getLocationContext());
+      if (!VR)
+        break;
+
+      std::optional<int> O_NONBLOCKV = tryExpandAsInteger(
+          "O_NONBLOCK", C.getBugReporter().getPreprocessor());
+      if (!O_NONBLOCKV)
+        break;
+
+      R->addVisitor(
+          std::make_unique<NonBlockOpenVisitor>(VR, O_NONBLOCKV.value()));
+    } while (false);
+  }
   C.emitReport(std::move(R));
 }
 
diff --git a/clang/test/Analysis/issue-124474.cpp b/clang/test/Analysis/issue-124474.cpp
new file mode 100644
index 00000000000000..f6d67ae967af98
--- /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<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);
+}
\ No newline at end of file

``````````

</details>


https://github.com/llvm/llvm-project/pull/126752