Author: flovent Date: 2025-02-17T15:35:40+01:00 New Revision: 9d487050a144b895950a6fd48b993513a714e69d
URL: https://github.com/llvm/llvm-project/commit/9d487050a144b895950a6fd48b993513a714e69d DIFF: https://github.com/llvm/llvm-project/commit/9d487050a144b895950a6fd48b993513a714e69d.diff LOG: [clang][analyzer] Teach the BlockInCriticalSectionChecker about O_NONBLOCK streams (#127049) this PR close #124474 when calling `read` and `recv` function for a non-block file descriptor or a invalid file descriptor(`-1`), it will not cause block inside a critical section. this commit checks for non-block file descriptor assigned by `open` function with `O_NONBLOCK` flag. --------- Co-authored-by: Balazs Benics <benicsbal...@gmail.com> Added: clang/test/Analysis/Inputs/system-header-simulator-cxx-std-locks.h clang/test/Analysis/issue-124474.cpp Modified: clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp index 7460781799d08..bf35bee70870b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp @@ -145,6 +145,57 @@ using MutexDescriptor = std::variant<FirstArgMutexDescriptor, MemberMutexDescriptor, RAIIMutexDescriptor>; +class SuppressNonBlockingStreams : public BugReporterVisitor { +private: + const CallDescription OpenFunction{CDM::CLibrary, {"open"}, 2}; + SymbolRef StreamSym; + const int NonBlockMacroVal; + bool Satisfied = false; + +public: + SuppressNonBlockingStreams(SymbolRef StreamSym, int NonBlockMacroVal) + : StreamSym(StreamSym), NonBlockMacroVal(NonBlockMacroVal) {} + + static void *getTag() { + static bool Tag; + return &Tag; + } + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.AddPointer(getTag()); + } + + PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override { + if (Satisfied) + return nullptr; + + std::optional<StmtPoint> Point = N->getLocationAs<StmtPoint>(); + if (!Point) + return nullptr; + + const auto *CE = Point->getStmtAs<CallExpr>(); + if (!CE || !OpenFunction.matchesAsWritten(*CE)) + return nullptr; + + if (N->getSVal(CE).getAsSymbol() != StreamSym) + return nullptr; + + Satisfied = true; + + // Check if open's second argument contains O_NONBLOCK + const llvm::APSInt *FlagVal = N->getSVal(CE->getArg(1)).getAsInteger(); + if (!FlagVal) + return nullptr; + + if ((*FlagVal & NonBlockMacroVal) != 0) + BR.markInvalid(getTag(), nullptr); + + return nullptr; + } +}; + class BlockInCriticalSectionChecker : public Checker<check::PostCall> { private: const std::array<MutexDescriptor, 8> MutexDescriptors{ @@ -182,6 +233,9 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall> { 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, @@ -337,6 +391,28 @@ void BlockInCriticalSectionChecker::reportBlockInCritSection( << "' inside of critical section"; auto R = std::make_unique<PathSensitiveBugReport>(BlockInCritSectionBugType, os.str(), ErrNode); + // 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") { + 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; + + if (SymbolRef SR = SV.getAsSymbol()) { + if (!O_NONBLOCKValue) + O_NONBLOCKValue = tryExpandAsInteger( + "O_NONBLOCK", C.getBugReporter().getPreprocessor()); + if (*O_NONBLOCKValue) + R->addVisitor<SuppressNonBlockingStreams>(SR, **O_NONBLOCKValue); + } + } R->addRange(Call.getSourceRange()); R->markInteresting(Call.getReturnValue()); C.emitReport(std::move(R)); 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..054dd5405e1be --- /dev/null +++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx-std-locks.h @@ -0,0 +1,13 @@ +#pragma clang system_header + +namespace std { +struct mutex { + void lock(); + void unlock(); +}; + +template <typename T> struct lock_guard { + lock_guard(std::mutex &); + ~lock_guard(); +}; +} // namespace std diff --git a/clang/test/Analysis/issue-124474.cpp b/clang/test/Analysis/issue-124474.cpp new file mode 100644 index 0000000000000..ae30c4db552c1 --- /dev/null +++ b/clang/test/Analysis/issue-124474.cpp @@ -0,0 +1,37 @@ +// RUN: %clang_analyze_cc1 \ +// RUN: -analyzer-checker=core,unix.BlockInCriticalSection \ +// RUN: -analyzer-output text -verify %s + +// expected-no-diagnostics + +#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, ...); +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 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); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
