https://github.com/steakhal created https://github.com/llvm/llvm-project/pull/208729
BlockInCriticalSectionChecker registers the pre-call for the RAII ctors and Dtors - and also the raw 'lock' and 'unlock' handlers. However, pre-call does not prevent inlining. This means that (in the likely case of) that the body is present, the analyzer will model the effect of the lock twice. This happens on libc++ unique_lock. We really should have eval-called the ctor/dtor to avoid the inlining of those, but here we are. rdar://175814310 From 72fc83a3e77a5fd0e957af7ac767bd58d17c947f Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Tue, 16 Jun 2026 15:24:51 +0100 Subject: [PATCH] [analyzer] Prevent inlining RAII ctors/dtors BlockInCriticalSectionChecker registers the pre-call for the RAII ctors and Dtors - and also the raw 'lock' and 'unlock' handlers. However, pre-call does not prevent inlining. This means that (in the likely case of) that the body is present, the analyzer will model the effect of the lock twice. This happens on libc++ unique_lock. We really should have eval-called the ctor/dtor to avoid the inlining of those, but here we are. rdar://175814310 --- .../BlockInCriticalSectionChecker.cpp | 34 +++++++++--- .../block-in-critical-section-raii.cpp | 53 +++++++++++++++++++ 2 files changed, 81 insertions(+), 6 deletions(-) create mode 100644 clang/test/Analysis/block-in-critical-section-raii.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp index 03c576270797b..ad26324cdab76 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp @@ -212,7 +212,8 @@ class SuppressNonBlockingStreams : public BugReporterVisitor { } }; -class BlockInCriticalSectionChecker : public Checker<check::PostCall> { +class BlockInCriticalSectionChecker + : public Checker<check::PostCall, eval::Call> { private: const std::array<MutexDescriptor, 9> MutexDescriptors{ // NOTE: There are standard library implementations where some methods @@ -276,6 +277,9 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall> { /// Process lock. /// Process blocking functions (sleep, getc, fgets, read, recv) void checkPostCall(const CallEvent &Call, CheckerContext &C) const; + + // Process RAII lock guard object ctors/dtors. + bool evalCall(const CallEvent &Call, CheckerContext &C) const; }; } // end anonymous namespace @@ -373,15 +377,33 @@ void BlockInCriticalSectionChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { if (isBlockingInCritSection(Call, C)) { 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)) { + return; + } + + if (std::optional<MutexDescriptor> LockDesc = + checkDescriptorMatch(Call, C, /*IsLock=*/true)) { + if (!std::holds_alternative<RAIIMutexDescriptor>(*LockDesc)) + handleLock(*LockDesc, Call, C); + return; + } + if (std::optional<MutexDescriptor> UnlockDesc = + checkDescriptorMatch(Call, C, /*IsLock=*/false)) { handleUnlock(*UnlockDesc, Call, C); } } +bool BlockInCriticalSectionChecker::evalCall(const CallEvent &Call, + CheckerContext &C) const { + if (std::optional<MutexDescriptor> LockDesc = + checkDescriptorMatch(Call, C, /*IsLock=*/true)) { + if (std::holds_alternative<RAIIMutexDescriptor>(*LockDesc)) { + handleLock(*LockDesc, Call, C); + return true; + } + } + return false; +} + void BlockInCriticalSectionChecker::reportBlockInCritSection( const CallEvent &Call, CheckerContext &C) const { ExplodedNode *ErrNode = C.generateNonFatalErrorNode(C.getState()); diff --git a/clang/test/Analysis/block-in-critical-section-raii.cpp b/clang/test/Analysis/block-in-critical-section-raii.cpp new file mode 100644 index 0000000000000..4bc4c94c74259 --- /dev/null +++ b/clang/test/Analysis/block-in-critical-section-raii.cpp @@ -0,0 +1,53 @@ +// RUN: %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core,unix.BlockInCriticalSection + +void sleep(int x); + +namespace std { +struct mutex { + void lock(); + void unlock(); +}; +// libc++-shaped unique_lock: ctor and dtor have non-empty bodies that call +// the underlying mutex's lock()/unlock() member functions. +template <class M> +struct unique_lock { + M *m_; + bool owns_; + explicit unique_lock(M &m) : m_(&m), owns_(true) { m_->lock(); } + ~unique_lock() { + if (owns_) + m_->unlock(); + } +}; +} // namespace std + +// Recursive use of the RAII guard. Without the fix, deep recursion would +// prevent the dtor body from inlining at the inlining-stack-depth limit; +// the inner mutex unlock would not fire, used to emit a leak FP. +struct C { + std::mutex m; + bool aborted; + bool getAborted() { + std::unique_lock<std::mutex> lk(m); + return aborted; + } + void recurse() { + if (getAborted()) + recurse(); + sleep(1); // no-warning + } +}; + +// Direct case: RAII guard scoped to a function body. Constructor inlining + +// destructor inlining must not double-count, even when both bodies inline. +void callee(std::mutex &m) { + std::unique_lock<std::mutex> lk(m); + // The lock IS held here so blocking is correctly diagnosed. + sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}} +} // dtor fires here, releases the lock; no further sleep should warn. + +void top(std::mutex &m) { + callee(m); + sleep(1); // no-warning: the lock from `callee` was released by ~unique_lock before this sleep. +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
