Skip to content

Conversation

PiJoules
Copy link
Contributor

@PiJoules PiJoules commented Sep 4, 2025

This reverts commit 0a2eb85.

Reverting because this leads to a false positive with -Wthread-safety. See #156760.

…4078)"

This reverts commit 0a2eb85.

Reverting because this leads to a false positive with -Wthread-safety.
See llvm#156760.
@PiJoules PiJoules requested a review from a team as a code owner September 4, 2025 20:23
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 4, 2025
@PiJoules PiJoules requested a review from philnik777 September 4, 2025 20:24
@llvmbot
Copy link
Member

llvmbot commented Sep 4, 2025

@llvm/pr-subscribers-libcxx

Author: None (PiJoules)

Changes

This reverts commit 0a2eb85.

Reverting because this leads to a false positive with -Wthread-safety. See #156760.


Full diff: https://github.com/llvm/llvm-project/pull/156964.diff

3 Files Affected:

  • (modified) libcxx/include/__config (+1-1)
  • (modified) libcxx/include/mutex (+5-14)
  • (removed) libcxx/test/extensions/clang/thread/thread.mutex/lock.verify.cpp (-47)
diff --git a/libcxx/include/__config b/libcxx/include/__config
index c197851f1c8fe..77a71b6cf1cae 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -934,7 +934,7 @@ typedef __char32_t char32_t;
 #    endif
 #  endif
 
-#  if __has_attribute(__no_thread_safety_analysis__)
+#  if defined(__FreeBSD__) && defined(__clang__) && __has_attribute(__no_thread_safety_analysis__)
 #    define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS __attribute__((__no_thread_safety_analysis__))
 #  else
 #    define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS
diff --git a/libcxx/include/mutex b/libcxx/include/mutex
index 58474e0ca2b7a..78d8c8a9bcc6e 100644
--- a/libcxx/include/mutex
+++ b/libcxx/include/mutex
@@ -320,7 +320,7 @@ public:
 };
 
 template <class _L0, class _L1>
-_LIBCPP_NO_THREAD_SAFETY_ANALYSIS _LIBCPP_HIDE_FROM_ABI int try_lock(_L0& __l0, _L1& __l1) {
+_LIBCPP_HIDE_FROM_ABI int try_lock(_L0& __l0, _L1& __l1) {
   unique_lock<_L0> __u0(__l0, try_to_lock_t());
   if (__u0.owns_lock()) {
     if (__l1.try_lock()) {
@@ -335,7 +335,7 @@ _LIBCPP_NO_THREAD_SAFETY_ANALYSIS _LIBCPP_HIDE_FROM_ABI int try_lock(_L0& __l0,
 #    ifndef _LIBCPP_CXX03_LANG
 
 template <class _L0, class _L1, class _L2, class... _L3>
-_LIBCPP_NO_THREAD_SAFETY_ANALYSIS _LIBCPP_HIDE_FROM_ABI int try_lock(_L0& __l0, _L1& __l1, _L2& __l2, _L3&... __l3) {
+_LIBCPP_HIDE_FROM_ABI int try_lock(_L0& __l0, _L1& __l1, _L2& __l2, _L3&... __l3) {
   int __r = 0;
   unique_lock<_L0> __u0(__l0, try_to_lock);
   if (__u0.owns_lock()) {
@@ -350,11 +350,8 @@ _LIBCPP_NO_THREAD_SAFETY_ANALYSIS _LIBCPP_HIDE_FROM_ABI int try_lock(_L0& __l0,
 
 #    endif // _LIBCPP_CXX03_LANG
 
-// We're using unique_lock to implement the functions, which thread annotations don't support. So we have to disable
-// the analysis inside the function.
 template <class _L0, class _L1>
-_LIBCPP_NO_THREAD_SAFETY_ANALYSIS _LIBCPP_HIDE_FROM_ABI void lock(_L0& __l0, _L1& __l1)
-    _LIBCPP_ACQUIRE_CAPABILITY(__l0, __l1) {
+_LIBCPP_HIDE_FROM_ABI void lock(_L0& __l0, _L1& __l1) {
   while (true) {
     {
       unique_lock<_L0> __u0(__l0);
@@ -378,7 +375,7 @@ _LIBCPP_NO_THREAD_SAFETY_ANALYSIS _LIBCPP_HIDE_FROM_ABI void lock(_L0& __l0, _L1
 #    ifndef _LIBCPP_CXX03_LANG
 
 template <class _L0, class _L1, class _L2, class... _L3>
-_LIBCPP_NO_THREAD_SAFETY_ANALYSIS void __lock_first(int __i, _L0& __l0, _L1& __l1, _L2& __l2, _L3&... __l3) {
+void __lock_first(int __i, _L0& __l0, _L1& __l1, _L2& __l2, _L3&... __l3) {
   while (true) {
     switch (__i) {
     case 0: {
@@ -413,14 +410,8 @@ _LIBCPP_NO_THREAD_SAFETY_ANALYSIS void __lock_first(int __i, _L0& __l0, _L1& __l
   }
 }
 
-// We're using unique_lock to implement the functions, which thread annotations don't support. So we have to disable
-// the analysis inside the function.
 template <class _L0, class _L1, class _L2, class... _L3>
-_LIBCPP_NO_THREAD_SAFETY_ANALYSIS inline _LIBCPP_HIDE_FROM_ABI void lock(_L0& __l0, _L1& __l1, _L2& __l2, _L3&... __l3)
-#      if defined(_LIBCPP_CLANG_VER) && _LIBCPP_CLANG_VER >= 2101
-    _LIBCPP_ACQUIRE_CAPABILITY(__l0, __l1, __l2, __l3...)
-#      endif
-{
+inline _LIBCPP_HIDE_FROM_ABI void lock(_L0& __l0, _L1& __l1, _L2& __l2, _L3&... __l3) {
   std::__lock_first(0, __l0, __l1, __l2, __l3...);
 }
 
diff --git a/libcxx/test/extensions/clang/thread/thread.mutex/lock.verify.cpp b/libcxx/test/extensions/clang/thread/thread.mutex/lock.verify.cpp
deleted file mode 100644
index 51ffa6962ac83..0000000000000
--- a/libcxx/test/extensions/clang/thread/thread.mutex/lock.verify.cpp
+++ /dev/null
@@ -1,47 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-// UNSUPPORTED: no-threads
-
-// <mutex>
-
-// GCC doesn't have thread safety attributes
-// UNSUPPORTED: gcc
-
-// ADDITIONAL_COMPILE_FLAGS: -Wthread-safety -Wno-comment
-
-// XFAIL: FROZEN-CXX03-HEADERS-FIXME
-
-#include <mutex>
-
-#include "test_macros.h"
-
-std::mutex m0;
-std::mutex m1;
-std::mutex m2;
-std::mutex m3;
-
-void f1() {
-  std::lock(m0, m1);
-} // expected-warning {{mutex 'm0' is still held at the end of function}} \
-     expected-warning {{mutex 'm1' is still held at the end of function}}
-
-#if TEST_STD_VER >= 11 && defined(TEST_CLANG_VER) && TEST_CLANG_VER >= 2101
-void f2() {
-  std::lock(m0, m1, m2);
-} // expected-warning {{mutex 'm0' is still held at the end of function}} \
-     expected-warning {{mutex 'm1' is still held at the end of function}} \
-     expected-warning {{mutex 'm2' is still held at the end of function}}
-
-void f3() {
-  std::lock(m0, m1, m2, m3);
-} // expected-warning {{mutex 'm0' is still held at the end of function}} \
-     expected-warning {{mutex 'm1' is still held at the end of function}} \
-     expected-warning {{mutex 'm2' is still held at the end of function}} \
-     expected-warning {{mutex 'm3' is still held at the end of function}}
-#endif

@PiJoules PiJoules requested a review from ldionne September 4, 2025 20:24
@frederick-vs-ja

This comment was marked as resolved.

Copy link
Contributor

@philnik777 philnik777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand the reasoning here. -Wthread-safety inherently has false-positives, so false-positives don't seem like a reason to remove the annotations again. If anything, we should expand them. If you can't deal with false-postives, don't enable a warning that has them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants