[libcxx-commits] [libcxx] 42a024f - [libc++] Add basic runtime assertions to <semaphore>

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Wed Jul 19 14:30:17 PDT 2023


Author: Edoardo Sanguineti
Date: 2023-07-19T17:29:59-04:00
New Revision: 42a024fad9fca3fbe37afd55681e84dc60dd1abe

URL: https://github.com/llvm/llvm-project/commit/42a024fad9fca3fbe37afd55681e84dc60dd1abe
DIFF: https://github.com/llvm/llvm-project/commit/42a024fad9fca3fbe37afd55681e84dc60dd1abe.diff

LOG: [libc++] Add basic runtime assertions to <semaphore>

Adding assertions will aid users that have bugs or logic mistakes in
their code to receive error messages when debugging.

Differential Revision: https://reviews.llvm.org/D155399

Added: 
    libcxx/test/libcxx/thread/thread.semaphore/assert.ctor.pass.cpp
    libcxx/test/libcxx/thread/thread.semaphore/assert.release.pass.cpp
    libcxx/test/std/thread/thread.semaphore/ctor.verify.cpp

Modified: 
    libcxx/include/semaphore

Removed: 
    


################################################################################
diff  --git a/libcxx/include/semaphore b/libcxx/include/semaphore
index cfbc2477a8bafc..a4cc51f8b15dda 100644
--- a/libcxx/include/semaphore
+++ b/libcxx/include/semaphore
@@ -81,6 +81,8 @@ functions. It avoids contention against users' own use of those facilities.
 
 */
 
+#define _LIBCPP_SEMAPHORE_MAX (numeric_limits<ptr
diff _t>::max())
+
 class __atomic_semaphore_base
 {
     __atomic_base<ptr
diff _t> __a_;
@@ -93,9 +95,14 @@ public:
     _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY
     void release(ptr
diff _t __update = 1)
     {
-        if(0 < __a_.fetch_add(__update, memory_order_release))
-            ;
-        else if(__update > 1)
+        auto __old = __a_.fetch_add(__update, memory_order_release);
+        _LIBCPP_ASSERT_UNCATEGORIZED(__update <= _LIBCPP_SEMAPHORE_MAX - __old, "update is greater than the expected value");
+
+        if (__old > 0)
+        {
+            // Nothing to do
+        }
+        else if (__update > 1)
             __a_.notify_all();
         else
             __a_.notify_one();
@@ -131,20 +138,30 @@ public:
     }
 };
 
-#define _LIBCPP_SEMAPHORE_MAX (numeric_limits<ptr
diff _t>::max())
-
 template<ptr
diff _t __least_max_value = _LIBCPP_SEMAPHORE_MAX>
 class counting_semaphore
 {
     __atomic_semaphore_base __semaphore_;
 
 public:
+    static_assert(__least_max_value >= 0, "The least maximum value must be a positive number");
+
     static constexpr ptr
diff _t max() noexcept {
         return __least_max_value;
     }
 
     _LIBCPP_INLINE_VISIBILITY
-    constexpr explicit counting_semaphore(ptr
diff _t __count) : __semaphore_(__count) { }
+    constexpr explicit counting_semaphore(ptr
diff _t __count) : __semaphore_(__count)
+    {
+        _LIBCPP_ASSERT_UNCATEGORIZED(
+            __count >= 0,
+            "counting_semaphore::counting_semaphore(ptr
diff _t): counting_semaphore cannot be "
+            "initialized with a negative value");
+        _LIBCPP_ASSERT_UNCATEGORIZED(
+            __count <= max(),
+            "counting_semaphore::counting_semaphore(ptr
diff _t): counting_semaphore cannot be "
+            "initialized with a value greater than max()");
+    }
     ~counting_semaphore() = default;
 
     counting_semaphore(const counting_semaphore&) = delete;
@@ -153,6 +170,7 @@ public:
     _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY
     void release(ptr
diff _t __update = 1)
     {
+        _LIBCPP_ASSERT_UNCATEGORIZED(__update >= 0, "counting_semaphore:release called with a negative value");
         __semaphore_.release(__update);
     }
     _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY

diff  --git a/libcxx/test/libcxx/thread/thread.semaphore/assert.ctor.pass.cpp b/libcxx/test/libcxx/thread/thread.semaphore/assert.ctor.pass.cpp
new file mode 100644
index 00000000000000..291481f32eb20e
--- /dev/null
+++ b/libcxx/test/libcxx/thread/thread.semaphore/assert.ctor.pass.cpp
@@ -0,0 +1,37 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: !libcpp-has-hardened-mode && !libcpp-has-debug-mode
+
+// XFAIL: availability-verbose_abort-missing
+
+// REQUIRES: has-unix-headers
+
+// <semaphore>
+
+// constexpr explicit counting_semaphore(ptr
diff _t __count);
+
+// Make sure that constructing counting_semaphore with a negative value triggers an assertion
+
+#include <semaphore>
+
+#include "check_assertion.h"
+
+int main(int, char**) {
+  {
+    TEST_LIBCPP_ASSERT_FAILURE(
+        [] { std::counting_semaphore<> s(-1); }(),
+        "counting_semaphore::counting_semaphore(ptr
diff _t): counting_semaphore cannot be "
+        "initialized with a negative value");
+  }
+  // We can't check the precondition for max() because there's no value
+  // that would violate the precondition (in our implementation)
+
+  return 0;
+}

diff  --git a/libcxx/test/libcxx/thread/thread.semaphore/assert.release.pass.cpp b/libcxx/test/libcxx/thread/thread.semaphore/assert.release.pass.cpp
new file mode 100644
index 00000000000000..be8375129326e7
--- /dev/null
+++ b/libcxx/test/libcxx/thread/thread.semaphore/assert.release.pass.cpp
@@ -0,0 +1,41 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: !libcpp-has-hardened-mode && !libcpp-has-debug-mode
+
+// XFAIL: availability-verbose_abort-missing
+
+// REQUIRES: has-unix-headers
+
+// <semaphore>
+
+// void release(ptr
diff _t __update = 1);
+
+// Make sure that calling release with a negative value triggers or with a value
+// greater than expected triggers an assertion
+
+#include <semaphore>
+
+#include "check_assertion.h"
+
+int main(int, char**) {
+  {
+    std::counting_semaphore<> s(2);
+    TEST_LIBCPP_ASSERT_FAILURE(s.release(-1), "counting_semaphore:release called with a negative value");
+  }
+
+  {
+    // Call release with an arbitrary larger than expected value
+    std::counting_semaphore<> s(2);
+    TEST_LIBCPP_ASSERT_FAILURE(
+        s.release(std::counting_semaphore<>::max()), "update is greater than the expected value");
+  }
+
+  return 0;
+}

diff  --git a/libcxx/test/std/thread/thread.semaphore/ctor.verify.cpp b/libcxx/test/std/thread/thread.semaphore/ctor.verify.cpp
new file mode 100644
index 00000000000000..ae98aab759a321
--- /dev/null
+++ b/libcxx/test/std/thread/thread.semaphore/ctor.verify.cpp
@@ -0,0 +1,19 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// <semaphore>
+
+#include <semaphore>
+
+void not_positive() {
+  std::counting_semaphore<-1> s(2); // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed{{.*}}The least maximum value must be a positive number}}
+  (void)s;
+}


        


More information about the libcxx-commits mailing list