[clang] [clang][analyzer] Fix false positive of BlockInCriticalSectionChecker (PR #127049)

via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 14 05:11:37 PST 2025


================
@@ -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);
+}
+
----------------
flovent wrote:

i can try to change the way back to BugVisitor, it does seems BugVisitor way has less overhead

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


More information about the cfe-commits mailing list