llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-analysis Author: Marco Elver (melver) <details> <summary>Changes</summary> When a temporary object with a [[noreturn]] destructor is constructed conditionally (e.g., inside a branch of a ternary operator), Clang's CFG inserts a TemporaryDtorsBranch decision block before dispatching to the destructor block. Thread Safety Analysis previously treated this block as a generic join point. Because the path through the temporary construction terminates in the destructor, and dropped held locks on the continuation path. This premature join resulted in spurious lockset mismatch warnings Teach neverReturns() to recognize when a block's single successor is a TemporaryDtorsBranch whose temporary was constructed in that block and whose destructor branch is noreturn. Assisted-by: Antigravity:gemini --- Full diff: https://github.com/llvm/llvm-project/pull/215691.diff 2 Files Affected: - (modified) clang/lib/Analysis/ThreadSafety.cpp (+21) - (modified) clang/test/SemaCXX/warn-thread-safety-analysis.cpp (+36) ``````````diff diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp index 5e656a3dbc1cb..3f6eb85c041e4 100644 --- a/clang/lib/Analysis/ThreadSafety.cpp +++ b/clang/lib/Analysis/ThreadSafety.cpp @@ -2815,6 +2815,27 @@ static bool neverReturns(const CFGBlock *B) { if (isa<CXXThrowExpr>(S->getStmt())) return true; } + + // If B constructed a temporary whose destructor is noreturn, control entering + // the decision block will always branch to the non-returning destructor. + if (B->succ_size() == 1) { + if (const CFGBlock *Succ = *B->succ_begin()) { + if (Succ->getTerminator().isTemporaryDtorsBranch() && + !Succ->succ_empty()) { + // The decision block's terminator is the CXXBindTemporaryExpr; if B + // bound this temporary, entering Succ from B always takes dtor edge. + const Stmt *Term = Succ->getTerminatorStmt(); + if (llvm::any_of(*B, [Term](const CFGElement &CE) { + auto CS = CE.getAs<CFGStmt>(); + return CS && CS->getStmt() == Term; + })) { + if (const auto *DtorBlock = Succ->succ_begin()->getReachableBlock()) + return neverReturns(DtorBlock); + } + } + } + } + return false; } diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp index 54b2c70940c24..45f65d07918d9 100644 --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -4782,9 +4782,24 @@ namespace UnreachableExitTest { class FemmeFatale { public: FemmeFatale(); + template <typename T> + FemmeFatale& operator<<(const T&) { return *this; } ~FemmeFatale() __attribute__((noreturn)); }; +class NonFatal { +public: + NonFatal(); + template <typename T> + NonFatal& operator<<(const T&) { return *this; } + ~NonFatal(); +}; + +struct Voidify { + template <typename T> + void operator&&(T&&) const&&; +}; + void exitNow() __attribute__((noreturn)); void exitDestruct(const MyString& ms) __attribute__((noreturn)); @@ -4813,6 +4828,27 @@ void test4() EXCLUSIVE_LOCKS_REQUIRED(fatalmu_) { exitDestruct("foo"); } +void test5() { + fatalmu_.TryLock() ? (void)0 : (void)FemmeFatale(); + fatalmu_.Unlock(); +} + +void test6() { + fatalmu_.TryLock() ? (void)0 : Voidify() && FemmeFatale() << "foo"; + fatalmu_.Unlock(); +} + +void test7() { + fatalmu_.TryLock() ? (void)0 : Voidify() && NonFatal() << "foo"; // \ + // expected-warning {{mutex 'fatalmu_' is not held on every path through here}} \ + // expected-note {{mutex acquired here}} + fatalmu_.Unlock(); // expected-warning {{releasing mutex 'fatalmu_' that was not held}} +} + +void test8() EXCLUSIVE_LOCKS_REQUIRED(fatalmu_) { + c ? (void)0 : (void)FemmeFatale(); +} + } // end namespace UnreachableExitTest `````````` </details> https://github.com/llvm/llvm-project/pull/215691 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
