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/2] [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/2] 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); } + _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits