Skip to content

Conversation

@llvmbot
Copy link
Member

@llvmbot llvmbot commented Aug 3, 2025

Backport a048aeb

Requested by: @aaronpuchert

@llvmbot
Copy link
Member Author

llvmbot commented Aug 3, 2025

@melver What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from melver August 3, 2025 21:34
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:analysis labels Aug 3, 2025
@llvmbot
Copy link
Member Author

llvmbot commented Aug 3, 2025

@llvm/pr-subscribers-clang-analysis

@llvm/pr-subscribers-clang

Author: None (llvmbot)

Changes

Backport a048aeb

Requested by: @aaronpuchert


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

2 Files Affected:

  • (modified) clang/lib/Analysis/ThreadSafety.cpp (+1-1)
  • (modified) clang/test/SemaCXX/warn-thread-safety-negative.cpp (+32)
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 80e7c8eff671a..dadb0b757a2c8 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1331,7 +1331,7 @@ void ThreadSafetyAnalyzer::addLock(FactSet &FSet,
       FSet.removeLock(FactMan, NegC);
     }
     else {
-      if (inCurrentScope(*Entry) && !Entry->asserted())
+      if (inCurrentScope(*Entry) && !Entry->asserted() && !Entry->reentrant())
         Handler.handleNegativeNotHeld(Entry->getKind(), Entry->toString(),
                                       NegC.toString(), Entry->loc());
     }
diff --git a/clang/test/SemaCXX/warn-thread-safety-negative.cpp b/clang/test/SemaCXX/warn-thread-safety-negative.cpp
index 9eabd67e4fc76..0caf6d6139e58 100644
--- a/clang/test/SemaCXX/warn-thread-safety-negative.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-negative.cpp
@@ -21,6 +21,15 @@ class LOCKABLE Mutex {
   void AssertReaderHeld() ASSERT_SHARED_LOCK();
 };
 
+class LOCKABLE REENTRANT_CAPABILITY ReentrantMutex {
+public:
+  void Lock() EXCLUSIVE_LOCK_FUNCTION();
+  void Unlock() UNLOCK_FUNCTION();
+
+  // for negative capabilities
+  const ReentrantMutex& operator!() const { return *this; }
+};
+
 class SCOPED_LOCKABLE MutexLock {
 public:
   MutexLock(Mutex *mu) EXCLUSIVE_LOCK_FUNCTION(mu);
@@ -89,6 +98,29 @@ class Foo {
   }
 };
 
+class Reentrant {
+  ReentrantMutex mu;
+
+public:
+  void acquire() {
+    mu.Lock();   // no warning -- reentrant mutex
+    mu.Unlock();
+  }
+
+  void requireNegative() EXCLUSIVE_LOCKS_REQUIRED(!mu) { // warning?
+    mu.Lock();
+    mu.Unlock();
+  }
+
+  void callRequireNegative() {
+    requireNegative(); // expected-warning{{calling function 'requireNegative' requires negative capability '!mu'}}
+  }
+
+  void callHaveNegative() EXCLUSIVE_LOCKS_REQUIRED(!mu) {
+    requireNegative();
+  }
+};
+
 }  // end namespace SimpleTest
 
 Mutex globalMutex;

@github-project-automation github-project-automation bot moved this from Needs Triage to Needs Merge in LLVM Release Status Aug 4, 2025
…lvm#150857)

The point of reentrant capabilities is that they can be acquired
multiple times, so they should probably be excluded from requiring a
negative capability on acquisition via -Wthread-safety-negative.

However, we still propagate explicit negative requirements.

(cherry picked from commit a048aeb)
@tru tru merged commit d4046ae into llvm:release/21.x Aug 5, 2025
@github-project-automation github-project-automation bot moved this from Needs Merge to Done in LLVM Release Status Aug 5, 2025
@github-actions
Copy link

github-actions bot commented Aug 5, 2025

@aaronpuchert (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:analysis clang Clang issues not falling into any other category

Projects

Development

Successfully merging this pull request may close these issues.

4 participants