[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)

Aaron Puchert via cfe-commits cfe-commits at lists.llvm.org
Sun Jun 30 16:32:08 PDT 2024


================
@@ -1954,6 +1954,125 @@ struct TestTryLock {
 } // end namespace TrylockTest
 
 
+// Regression test for trylock attributes with an enumerator success argument.
+// Prior versions of the analysis silently failed to evaluate success arguments
+// that were neither `CXXBoolLiteralExpr` nor `IntegerLiteral` and assumed the
+// value was false.
+namespace TrylockSuccessEnumFalseNegative {
+
+enum TrylockResult { Failure = 0, Success = 1 };
+
+class LOCKABLE Mutex {
+public:
+  TrylockResult TryLock() EXCLUSIVE_TRYLOCK_FUNCTION(Success);
+  void Unlock() EXCLUSIVE_UNLOCK_FUNCTION();
+};
+
+class TrylockTest {
+  Mutex mu_;
+  int a_ GUARDED_BY(mu_) = 0;
+
+  void test_bool_expression() {
+    if (!mu_.TryLock()) { // expected-note {{mutex acquired here}}
+      a_++;  // expected-warning {{writing variable 'a_' requires holding mutex 'mu_' exclusively}}
+      mu_.Unlock();  // expected-warning {{releasing mutex 'mu_' that was not held}}
+    }
+  }  // expected-warning {{mutex 'mu_' is not held on every path through here}}
+};
+} // namespace TrylockSuccessEnumFalseNegative
+
+// This test demonstrates that the analysis does not distinguish between
+// different non-zero enumerators.
+namespace TrylockFalseNegativeWhenEnumHasMultipleFailures {
+
+enum TrylockResult { Failure1 = 0, Failure2, Success };
+
+class LOCKABLE Mutex {
+public:
+  TrylockResult TryLock() EXCLUSIVE_TRYLOCK_FUNCTION(Success);
+  void Unlock() EXCLUSIVE_UNLOCK_FUNCTION();
+};
+
+class TrylockTest {
+  Mutex mu_;
+  int a_ GUARDED_BY(mu_) = 0;
+
+  void test_eq_success() {
+    if (mu_.TryLock() == Success) {
+      a_++;
+      mu_.Unlock();
+    }
+  }
+
+  void test_bool_expression() {
+    // This branch checks whether the trylock function returned a non-zero
+    // value. This satisfies the analysis, but fails to account for `Failure2`.
+    // From the analysis's perspective, `Failure2` and `Success` are equivalent!
+    if (mu_.TryLock()) {
+      a_++;
+      mu_.Unlock();
+    }
+  }
+};
+} // namespace TrylockSuccessEnumMultipleFailureModesFalseNegative
+
+
+// This test demonstrates that the analysis does not detect when all enumerators
+// are positive, and thus incapable of representing a failure.
----------------
aaronpuchert wrote:

positive → non-zero or nonzero.

https://github.com/llvm/llvm-project/pull/95290


More information about the cfe-commits mailing list