https://github.com/flovent created
https://github.com/llvm/llvm-project/pull/126752
this PR close #124474
>From 6234834150a7c83361b5202fe41dad283bbdec3b Mon Sep 17 00:00:00 2001
From: flovent <flb...@protonmail.com>
Date: Tue, 11 Feb 2025 23:52:36 +0800
Subject: [PATCH] [clang][analyzer] fix false positive of
BlockInCriticalSectionChecker
---
.../BlockInCriticalSectionChecker.cpp | 116 ++++++++++++++++++
clang/test/Analysis/issue-124474.cpp | 49 ++++++++
2 files changed, 165 insertions(+)
create mode 100644 clang/test/Analysis/issue-124474.cpp
diff --git
a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
index 7460781799d08..cfff5a25e7a50 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 0000000000000..f6d67ae967af9
--- /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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
