[libcxx-commits] [libcxx] 1122cbf - [libc++] add basic runtime assertions to <barrier>

Mark de Wever via libcxx-commits libcxx-commits at lists.llvm.org
Sat Jul 15 03:50:33 PDT 2023


Author: Edoardo Sanguineti
Date: 2023-07-15T12:50:28+02:00
New Revision: 1122cbf403548c3bd1a76689b4f6fff948ef336e

URL: https://github.com/llvm/llvm-project/commit/1122cbf403548c3bd1a76689b4f6fff948ef336e
DIFF: https://github.com/llvm/llvm-project/commit/1122cbf403548c3bd1a76689b4f6fff948ef336e.diff

LOG: [libc++] add basic runtime assertions to <barrier>

Adding assertions will aid users that have bugs in their code to receive better error messages.

Reviewed By: #libc, ldionne

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

Added: 
    libcxx/test/libcxx/thread/thread.barrier/assert.arrive.pass.cpp
    libcxx/test/libcxx/thread/thread.barrier/assert.ctor.pass.cpp

Modified: 
    libcxx/include/barrier

Removed: 
    


################################################################################
diff  --git a/libcxx/include/barrier b/libcxx/include/barrier
index 64e48775ea0930..e0d63fa59ffda7 100644
--- a/libcxx/include/barrier
+++ b/libcxx/include/barrier
@@ -138,6 +138,9 @@ public:
     [[__nodiscard__]] _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY
     arrival_token arrive(ptr
diff _t __update)
     {
+        _LIBCPP_ASSERT_UNCATEGORIZED(
+            __update <= __expected_, "update is greater than the expected count for the current barrier phase");
+
         auto const __old_phase = __phase_.load(memory_order_relaxed);
         for(; __update; --__update)
             if(__arrive_barrier_algorithm_base(__base_.get(), __old_phase)) {
@@ -205,7 +208,11 @@ public:
         auto const __old_phase = __phase.load(memory_order_relaxed);
         auto const __result = __arrived.fetch_sub(update, memory_order_acq_rel) - update;
         auto const new_expected = __expected.load(memory_order_relaxed);
-        if(0 == __result) {
+
+        _LIBCPP_ASSERT_UNCATEGORIZED(
+            update <= new_expected, "update is greater than the expected count for the current barrier phase");
+
+        if (0 == __result) {
             __completion();
             __arrived.store(new_expected, memory_order_relaxed);
             __phase.store(!__old_phase, memory_order_release);
@@ -261,7 +268,11 @@ public:
     {
         auto const __inc = __arrived_unit * update;
         auto const __old = __phase_arrived_expected.fetch_add(__inc, memory_order_acq_rel);
-        if((__old ^ (__old + __inc)) & __phase_bit) {
+
+        _LIBCPP_ASSERT_UNCATEGORIZED(
+            update <= __old, "update is greater than the expected count for the current barrier phase");
+
+        if ((__old ^ (__old + __inc)) & __phase_bit) {
             __phase_arrived_expected.fetch_add((__old & __expected_mask) << 32, memory_order_relaxed);
             __phase_arrived_expected.notify_all();
         }
@@ -300,6 +311,13 @@ public:
     _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY
     explicit barrier(ptr
diff _t __count, _CompletionF __completion = _CompletionF())
         : __b_(__count, _VSTD::move(__completion)) {
+        _LIBCPP_ASSERT_UNCATEGORIZED(
+            __count >= 0,
+            "barrier::barrier(ptr
diff _t, CompletionFunction): barrier cannot be initialized with a negative value");
+        _LIBCPP_ASSERT_UNCATEGORIZED(
+            __count <= max(),
+            "barrier::barrier(ptr
diff _t, CompletionFunction): barrier cannot be initialized with "
+            "a value greater than max()");
     }
 
     barrier(barrier const&) = delete;
@@ -308,6 +326,7 @@ public:
     [[__nodiscard__]] _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY
     arrival_token arrive(ptr
diff _t __update = 1)
     {
+        _LIBCPP_ASSERT_UNCATEGORIZED(__update > 0, "barrier:arrive must be called with a value greater than 0");
         return __b_.arrive(__update);
     }
     _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY

diff  --git a/libcxx/test/libcxx/thread/thread.barrier/assert.arrive.pass.cpp b/libcxx/test/libcxx/thread/thread.barrier/assert.arrive.pass.cpp
new file mode 100644
index 00000000000000..e36f8bc15a8aaa
--- /dev/null
+++ b/libcxx/test/libcxx/thread/thread.barrier/assert.arrive.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
+
+// REQUIRES: availability-synchronization_library-missing
+// XFAIL: target={{.+}}-apple-macosx10.{{13|15}}
+
+// REQUIRES: has-unix-headers
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_ASSERTIONS=1
+
+// <barrier>
+
+// class barrier;
+
+// void arrive(ptr
diff _t __update = 1);
+
+// Make sure that calling arrive with a negative value or with a value greater than 'expected' triggers an assertion
+
+#include <barrier>
+
+#include "check_assertion.h"
+
+int main(int, char**) {
+  {
+    std::barrier<> b(5);
+    TEST_LIBCPP_ASSERT_FAILURE(b.arrive(0), "barrier:arrive must be called with a value greater than 0");
+  }
+
+  {
+    std::barrier<> b(5);
+    TEST_LIBCPP_ASSERT_FAILURE(b.arrive(10), "update is greater than the expected count for the current barrier phase");
+  }
+
+  return 0;
+}

diff  --git a/libcxx/test/libcxx/thread/thread.barrier/assert.ctor.pass.cpp b/libcxx/test/libcxx/thread/thread.barrier/assert.ctor.pass.cpp
new file mode 100644
index 00000000000000..10641ebf0c14fb
--- /dev/null
+++ b/libcxx/test/libcxx/thread/thread.barrier/assert.ctor.pass.cpp
@@ -0,0 +1,49 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+
+// REQUIRES: availability-synchronization_library-missing
+// XFAIL: target={{.+}}-apple-macosx10.{{13|15}}
+
+// REQUIRES: has-unix-headers
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_ASSERTIONS=1
+
+// <barrier>
+
+// class barrier;
+
+// barrier(ptr
diff _t __count, _CompletionF __completion = _CompletionF());
+
+// Make sure that constructing barrier with a negative value triggers an assertion
+
+#include <barrier>
+
+#include "check_assertion.h"
+
+int main(int, char**) {
+  {
+    TEST_LIBCPP_ASSERT_FAILURE(
+        [] { std::barrier<> b(-1); }(),
+        "barrier::barrier(ptr
diff _t, CompletionFunction): barrier cannot be initialized with a negative value");
+  }
+
+  {
+    TEST_LIBCPP_ASSERT_FAILURE(
+        [] {
+          auto completion = []() {};
+          std::barrier<decltype(completion)> b(-1, completion);
+        }(),
+        "barrier::barrier(ptr
diff _t, CompletionFunction): barrier 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;
+}


        


More information about the libcxx-commits mailing list