https://github.com/flovent updated
https://github.com/llvm/llvm-project/pull/127049
>From c916dadbaf6021eda606d76784115698a9800571 Mon Sep 17 00:00:00 2001
From: flovent <flb...@protonmail.com>
Date: Thu, 13 Feb 2025 20:17:20 +0800
Subject: [PATCH 1/3] [clang][analyzer] fix false positive of
BlockInCriticalSectionChecker
---
.../BlockInCriticalSectionChecker.cpp | 67 ++++++++++++++++++-
clang/test/Analysis/issue-124474.cpp | 49 ++++++++++++++
2 files changed, 115 insertions(+), 1 deletion(-)
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..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 Checker<check::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
Checker<check::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
Checker<check::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
Checker<check::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<int> 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<MutexDescriptor> LockDesc =
checkDescriptorMatch(Call, C, /*IsLock=*/true)) {
@@ -322,9 +370,26 @@ void BlockInCriticalSectionChecker::checkPostCall(const
CallEvent &Call,
} 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);
+}
+
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<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);
+}
>From f2f928798e715ccd2739bfa44d3d09fffce25b16 Mon Sep 17 00:00:00 2001
From: flovent <flb...@protonmail.com>
Date: Thu, 13 Feb 2025 20:32:53 +0800
Subject: [PATCH 2/3] add new line
---
clang/test/Analysis/issue-124474.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/test/Analysis/issue-124474.cpp
b/clang/test/Analysis/issue-124474.cpp
index 09e3d4f3f9ad9..7136bbb316c67 100644
--- a/clang/test/Analysis/issue-124474.cpp
+++ b/clang/test/Analysis/issue-124474.cpp
@@ -47,3 +47,4 @@ void foo()
read(fd, buffer, 199); // no-warning: fd is a non-block file descriptor
close(fd);
}
+
>From 20bec01386e957146250819540a97bedaa97066d Mon Sep 17 00:00:00 2001
From: flovent <flb...@protonmail.com>
Date: Fri, 14 Feb 2025 21:05:38 +0800
Subject: [PATCH 3/3] update based on review advice
---
.../BlockInCriticalSectionChecker.cpp | 32 ++++++------
.../system-header-simulator-cxx-std-locks.h | 17 ++++++
clang/test/Analysis/issue-124474.cpp | 52 ++++++++-----------
3 files changed, 54 insertions(+), 47 deletions(-)
create mode 100644
clang/test/Analysis/Inputs/system-header-simulator-cxx-std-locks.h
diff --git
a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
index db784f2cc77b2..4a59c9f39eb7b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
@@ -185,6 +185,9 @@ class BlockInCriticalSectionChecker
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,
@@ -317,17 +320,16 @@ void BlockInCriticalSectionChecker::handleUnlock(
void BlockInCriticalSectionChecker::handleOpen(const CallEvent &Call,
CheckerContext &C) const {
const auto *Flag = Call.getArgExpr(1);
- static std::optional<int> 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 (!O_NONBLOCKValue)
+ O_NONBLOCKValue =
+ tryExpandAsInteger("O_NONBLOCK", C.getBugReporter().getPreprocessor());
+
+ if (*O_NONBLOCKValue && ((*FlagV & **O_NONBLOCKValue) != 0))
if (SymbolRef SR = Call.getReturnValue().getAsSymbol()) {
C.addTransition(C.getState()->add<NonBlockFileDescriptor>(SR));
}
@@ -348,18 +350,16 @@ void BlockInCriticalSectionChecker::checkPostCall(const
CallEvent &Call,
// 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)
+ 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;
- 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)) {
+ SymbolRef SR = SV.getAsSymbol();
+ if (SR && state->contains<NonBlockFileDescriptor>(SR)) {
return;
}
}
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..91e7c9f63fb55
--- /dev/null
+++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx-std-locks.h
@@ -0,0 +1,17 @@
+// This is a fake system header with divide-by-zero bugs introduced in
+// c++ std library functions. We use these bugs to test hard-coded
+// suppression of diagnostics within standard library functions that are known
+// to produce false positives.
+
+#pragma clang system_header
+namespace std {
+struct mutex {
+ void lock() {}
+ void unlock() {}
+};
+
+template <typename T> struct lock_guard {
+ lock_guard<T>(std::mutex) {}
+ ~lock_guard<T>() {}
+};
+} // namespace std
\ No newline at end of file
diff --git a/clang/test/Analysis/issue-124474.cpp
b/clang/test/Analysis/issue-124474.cpp
index 7136bbb316c67..d4861691a13ac 100644
--- a/clang/test/Analysis/issue-124474.cpp
+++ b/clang/test/Analysis/issue-124474.cpp
@@ -6,45 +6,35 @@
// 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
+#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, ...);
+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
+#define O_RDONLY 00
+#define O_NONBLOCK 04000
-void foo()
-{
- std::lock_guard<std::mutex> lock(mtx);
+void foo() {
+ std::lock_guard<std::mutex> lock(mtx);
- const char *filename = "example.txt";
- int fd = open(filename, O_RDONLY | O_NONBLOCK);
+ 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);
+ 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);
+}
+
\ 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
