[libc][semaphore] Add internal unnamed semaphore implementation#190851
[libc][semaphore] Add internal unnamed semaphore implementation#190851michaelrj-google merged 3 commits intollvm:mainfrom
Conversation
|
@llvm/pr-subscribers-libc Author: Pengxiang Huang (Pengxiang-Huang) ChangesImplements the first part for #190847 Add unnamed semaphore lifetime support, particularly:
Full diff: https://github.com/llvm/llvm-project/pull/190851.diff 21 Files Affected:
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index 90a0dade5435a..72fc463aac362 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -1233,6 +1233,11 @@ if(LLVM_LIBC_FULL_BUILD)
libc.src.pthread.pthread_spin_lock
libc.src.pthread.pthread_spin_trylock
libc.src.pthread.pthread_spin_unlock
+
+ # semaphore.h entrypoints
+ libc.src.semaphore.sem_destroy
+ libc.src.semaphore.sem_getvalue
+ libc.src.semaphore.sem_init
libc.src.pthread.pthread_self
libc.src.pthread.pthread_setname_np
libc.src.pthread.pthread_setspecific
diff --git a/libc/hdr/types/CMakeLists.txt b/libc/hdr/types/CMakeLists.txt
index 7e1a8974fa486..0862f0f9efecf 100644
--- a/libc/hdr/types/CMakeLists.txt
+++ b/libc/hdr/types/CMakeLists.txt
@@ -282,6 +282,14 @@ add_proxy_header_library(
libc.include.llvm-libc-types.pid_t
)
+add_proxy_header_library(
+ sem_t
+ HDRS
+ sem_t.h
+ FULL_BUILD_DEPENDS
+ libc.include.llvm-libc-types.sem_t
+)
+
add_proxy_header_library(
pthread_barrier_t
HDRS
diff --git a/libc/hdr/types/sem_t.h b/libc/hdr/types/sem_t.h
new file mode 100644
index 0000000000000..e057bca6b98df
--- /dev/null
+++ b/libc/hdr/types/sem_t.h
@@ -0,0 +1,22 @@
+//===-- Definition of sem_t type -----------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_HDR_TYPES_SEM_T_H
+#define LLVM_LIBC_HDR_TYPES_SEM_T_H
+
+#ifdef LIBC_FULL_BUILD
+
+#include "include/llvm-libc-types/sem_t.h"
+
+#else // Overlay mode
+
+#error "Cannot overlay sem_t"
+
+#endif // LLVM_LIBC_FULL_BUILD
+
+#endif // LLVM_LIBC_HDR_TYPES_SEM_T_H
diff --git a/libc/include/CMakeLists.txt b/libc/include/CMakeLists.txt
index 3375e4e21e338..aa289a4f6fa88 100644
--- a/libc/include/CMakeLists.txt
+++ b/libc/include/CMakeLists.txt
@@ -471,6 +471,15 @@ add_header_macro(
.llvm-libc-types.struct_timespec
)
+add_header_macro(
+ semaphore
+ ../libc/include/semaphore.yaml
+ semaphore.h
+ DEPENDS
+ .llvm-libc-types.sem_t
+ .llvm_libc_common_h
+)
+
add_header_macro(
spawn
../libc/include/spawn.yaml
diff --git a/libc/include/limits.yaml b/libc/include/limits.yaml
index 1f0956128fb00..f7576d89a5d80 100644
--- a/libc/include/limits.yaml
+++ b/libc/include/limits.yaml
@@ -52,6 +52,8 @@ macros:
macro_header: limits-macros.h
- macro_name: ULLONG_MAX
macro_header: limits-macros.h
+ - macro_name: SEM_VALUE_MAX
+ macro_header: limits-macros.h
- macro_name: SCHAR_MIN
macro_header: limits-macros.h
- macro_name: UCHAR_MIN
diff --git a/libc/include/llvm-libc-macros/limits-macros.h b/libc/include/llvm-libc-macros/limits-macros.h
index 79bbbe401eb02..0f999b2bedd19 100644
--- a/libc/include/llvm-libc-macros/limits-macros.h
+++ b/libc/include/llvm-libc-macros/limits-macros.h
@@ -173,6 +173,10 @@
#define ULLONG_MAX (LLONG_MAX * 2ULL + 1ULL)
#endif // ULLONG_MAX
+#ifndef SEM_VALUE_MAX
+#define SEM_VALUE_MAX INT_MAX
+#endif // SEM_VALUE_MAX
+
// *_MIN macros
#ifndef SCHAR_MIN
diff --git a/libc/include/llvm-libc-types/CMakeLists.txt b/libc/include/llvm-libc-types/CMakeLists.txt
index 0d6bc0982b847..ec4719a5e57a1 100644
--- a/libc/include/llvm-libc-types/CMakeLists.txt
+++ b/libc/include/llvm-libc-types/CMakeLists.txt
@@ -84,6 +84,7 @@ add_header(pthread_rwlockattr_t HDR pthread_rwlockattr_t.h)
add_header(pthread_spinlock_t HDR pthread_spinlock_t.h DEPENDS .pid_t)
add_header(pthread_t HDR pthread_t.h DEPENDS .__thread_type)
add_header(rlim_t HDR rlim_t.h)
+add_header(sem_t HDR sem_t.h DEPENDS .__futex_word)
if(LIBC_TYPES_TIME_T_IS_32_BIT)
add_header(time_t HDR time_t_32.h DEST_HDR time_t.h)
else()
diff --git a/libc/include/llvm-libc-types/sem_t.h b/libc/include/llvm-libc-types/sem_t.h
new file mode 100644
index 0000000000000..b9596f4e65707
--- /dev/null
+++ b/libc/include/llvm-libc-types/sem_t.h
@@ -0,0 +1,20 @@
+//===-- Definition of sem_t type -----------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_TYPES_SEM_T_H
+#define LLVM_LIBC_TYPES_SEM_T_H
+
+#include "__futex_word.h"
+
+typedef struct {
+ __futex_word __value; // current semaphore count
+ unsigned int __canary; // used for sanity check
+ unsigned char __reserved[8]; // for future usage, total fixed size 16 bytes
+} sem_t;
+
+#endif // LLVM_LIBC_TYPES_SEM_T_H
diff --git a/libc/include/semaphore.yaml b/libc/include/semaphore.yaml
new file mode 100644
index 0000000000000..a56e2eca1c390
--- /dev/null
+++ b/libc/include/semaphore.yaml
@@ -0,0 +1,21 @@
+header: semaphore.h
+standards:
+ - posix
+types:
+ - type_name: sem_t
+functions:
+ - name: sem_destroy
+ return_type: int
+ arguments:
+ - type: sem_t *
+ - name: sem_getvalue
+ return_type: int
+ arguments:
+ - type: sem_t *__restrict
+ - type: int *__restrict
+ - name: sem_init
+ return_type: int
+ arguments:
+ - type: sem_t *
+ - type: int
+ - type: unsigned int
diff --git a/libc/src/CMakeLists.txt b/libc/src/CMakeLists.txt
index 8a0acccaed708..e4b0de3d5c75d 100644
--- a/libc/src/CMakeLists.txt
+++ b/libc/src/CMakeLists.txt
@@ -25,6 +25,7 @@ if(${LIBC_TARGET_OS} STREQUAL "linux")
add_subdirectory(poll)
add_subdirectory(pthread)
add_subdirectory(sched)
+ add_subdirectory(semaphore)
add_subdirectory(sys)
add_subdirectory(termios)
endif()
diff --git a/libc/src/semaphore/CMakeLists.txt b/libc/src/semaphore/CMakeLists.txt
new file mode 100644
index 0000000000000..83572bc0464c3
--- /dev/null
+++ b/libc/src/semaphore/CMakeLists.txt
@@ -0,0 +1,49 @@
+add_header_library(
+ posix_semaphore
+ HDRS
+ posix_semaphore.h
+ DEPENDS
+ libc.hdr.types.sem_t
+ libc.src.__support.CPP.atomic
+ libc.src.__support.threads.linux.futex_word_type
+)
+
+add_entrypoint_object(
+ sem_init
+ SRCS
+ sem_init.cpp
+ HDRS
+ sem_init.h
+ DEPENDS
+ .posix_semaphore
+ libc.hdr.errno_macros
+ libc.hdr.limits_macros
+ libc.include.semaphore
+ libc.src.errno.errno
+)
+
+add_entrypoint_object(
+ sem_destroy
+ SRCS
+ sem_destroy.cpp
+ HDRS
+ sem_destroy.h
+ DEPENDS
+ .posix_semaphore
+ libc.hdr.errno_macros
+ libc.include.semaphore
+ libc.src.errno.errno
+)
+
+add_entrypoint_object(
+ sem_getvalue
+ SRCS
+ sem_getvalue.cpp
+ HDRS
+ sem_getvalue.h
+ DEPENDS
+ .posix_semaphore
+ libc.hdr.errno_macros
+ libc.include.semaphore
+ libc.src.errno.errno
+)
diff --git a/libc/src/semaphore/posix_semaphore.h b/libc/src/semaphore/posix_semaphore.h
new file mode 100644
index 0000000000000..50bcba63f5a7d
--- /dev/null
+++ b/libc/src/semaphore/posix_semaphore.h
@@ -0,0 +1,72 @@
+//===-- Shared helpers for POSIX semaphores -------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_SEMAPHORE_POSIX_SEMAPHORE_H
+#define LLVM_LIBC_SRC_SEMAPHORE_POSIX_SEMAPHORE_H
+
+#include "hdr/types/sem_t.h"
+#include "src/__support/CPP/atomic.h"
+#include "src/__support/common.h"
+#include "src/__support/threads/linux/futex_word.h"
+
+namespace LIBC_NAMESPACE_DECL {
+namespace sem_utils {
+
+// 0x53 = S, 0x45 = E, 0x4D = M, 0x31 = 1
+// canary value: SEM1 in hex
+LIBC_INLINE_VAR constexpr unsigned int SEM_CANARY = 0x53454D31U;
+
+static_assert(sizeof(__futex_word) == sizeof(FutexWordType));
+static_assert(alignof(__futex_word) >= alignof(FutexWordType));
+
+// TODO:
+// 1. Add named semaphore support: sem_open, sem_close, sem_unlink
+// 2. Add the posting and waiting operations: sem_post, sem_wait,
+// sem_trywait, sem_timedwait, sem_clockwait.
+
+LIBC_INLINE FutexWordType *value_ptr(sem_t *sem) {
+ return &sem->__value.__word;
+}
+
+// get the atomic reference for sem->__value
+LIBC_INLINE cpp::AtomicRef<FutexWordType> value(sem_t *sem) {
+ return cpp::AtomicRef<FutexWordType>(*value_ptr(sem));
+}
+
+LIBC_INLINE void initialize(sem_t *sem, unsigned int initial_value) {
+ // used in sem_init
+ // init happens before the semaphore is published to any threads
+ // initialize a initialized semaphore is undefined
+ // so RELAXED ordering is enough
+ value(sem).store(initial_value, cpp::MemoryOrder::RELAXED);
+ sem->__canary = SEM_CANARY;
+ for (unsigned char &byte : sem->__reserved)
+ byte = 0;
+}
+
+LIBC_INLINE bool is_valid(const sem_t *sem) {
+ // sanity check for a given semaphore pointer
+ return sem != nullptr && sem->__canary == SEM_CANARY;
+}
+
+LIBC_INLINE void invalidate(sem_t *sem) {
+ // used in sem_destroy
+ // invalidate is safe only when no threads is using
+ // blocked by destroyed semaphore is undefined
+ // use a destroyed semaphore is undefined
+ // RELAXED ordering is enough
+ value(sem).store(0, cpp::MemoryOrder::RELAXED);
+ sem->__canary = 0;
+ for (unsigned char &byte : sem->__reserved)
+ byte = 0;
+}
+
+} // namespace sem_utils
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_SEMAPHORE_POSIX_SEMAPHORE_H
diff --git a/libc/src/semaphore/sem_destroy.cpp b/libc/src/semaphore/sem_destroy.cpp
new file mode 100644
index 0000000000000..c64e18a88e639
--- /dev/null
+++ b/libc/src/semaphore/sem_destroy.cpp
@@ -0,0 +1,29 @@
+//===-- Implementation of sem_destroy ------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/semaphore/sem_destroy.h"
+
+#include "src/semaphore/posix_semaphore.h"
+
+#include "hdr/errno_macros.h"
+#include "src/__support/common.h"
+#include "src/__support/libc_errno.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int, sem_destroy, (sem_t * sem)) {
+ if (!sem_utils::is_valid(sem)) {
+ libc_errno = EINVAL;
+ return -1;
+ }
+
+ sem_utils::invalidate(sem);
+ return 0;
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/semaphore/sem_destroy.h b/libc/src/semaphore/sem_destroy.h
new file mode 100644
index 0000000000000..b664cf97eecf0
--- /dev/null
+++ b/libc/src/semaphore/sem_destroy.h
@@ -0,0 +1,21 @@
+//===-- Implementation header for sem_destroy ------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_SEMAPHORE_SEM_DESTROY_H
+#define LLVM_LIBC_SRC_SEMAPHORE_SEM_DESTROY_H
+
+#include "hdr/types/sem_t.h"
+#include "src/__support/macros/config.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+int sem_destroy(sem_t *sem);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_SEMAPHORE_SEM_DESTROY_H
diff --git a/libc/src/semaphore/sem_getvalue.cpp b/libc/src/semaphore/sem_getvalue.cpp
new file mode 100644
index 0000000000000..ea84c1e8a6241
--- /dev/null
+++ b/libc/src/semaphore/sem_getvalue.cpp
@@ -0,0 +1,33 @@
+//===-- Implementation of sem_getvalue -----------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/semaphore/sem_getvalue.h"
+
+#include "src/semaphore/posix_semaphore.h"
+
+#include "hdr/errno_macros.h"
+#include "src/__support/common.h"
+#include "src/__support/libc_errno.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int, sem_getvalue,
+ (sem_t *__restrict sem, int *__restrict sval)) {
+ if (!sem_utils::is_valid(sem) || sval == nullptr) {
+ libc_errno = EINVAL;
+ return -1;
+ }
+
+ // get value is informational but not a synchronization op
+ // RELAXED ordering is enough
+ *sval =
+ static_cast<int>(sem_utils::value(sem).load(cpp::MemoryOrder::RELAXED));
+ return 0;
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/semaphore/sem_getvalue.h b/libc/src/semaphore/sem_getvalue.h
new file mode 100644
index 0000000000000..4ba49491746b1
--- /dev/null
+++ b/libc/src/semaphore/sem_getvalue.h
@@ -0,0 +1,21 @@
+//===-- Implementation header for sem_getvalue -----------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_SEMAPHORE_SEM_GETVALUE_H
+#define LLVM_LIBC_SRC_SEMAPHORE_SEM_GETVALUE_H
+
+#include "hdr/types/sem_t.h"
+#include "src/__support/macros/config.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+int sem_getvalue(sem_t *__restrict sem, int *__restrict sval);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_SEMAPHORE_SEM_GETVALUE_H
diff --git a/libc/src/semaphore/sem_init.cpp b/libc/src/semaphore/sem_init.cpp
new file mode 100644
index 0000000000000..a3f6e452b6e6c
--- /dev/null
+++ b/libc/src/semaphore/sem_init.cpp
@@ -0,0 +1,32 @@
+//===-- Implementation of sem_init ---------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/semaphore/sem_init.h"
+
+#include "src/semaphore/posix_semaphore.h"
+
+#include "hdr/errno_macros.h"
+#include "hdr/limits_macros.h"
+#include "src/__support/common.h"
+#include "src/__support/libc_errno.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int, sem_init,
+ (sem_t * sem, int pshared, unsigned int value)) {
+ if (sem == nullptr || value > SEM_VALUE_MAX) {
+ libc_errno = EINVAL;
+ return -1;
+ }
+
+ (void)pshared;
+ sem_utils::initialize(sem, value);
+ return 0;
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/semaphore/sem_init.h b/libc/src/semaphore/sem_init.h
new file mode 100644
index 0000000000000..8a3114ddfa10f
--- /dev/null
+++ b/libc/src/semaphore/sem_init.h
@@ -0,0 +1,21 @@
+//===-- Implementation header for sem_init ----------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_SEMAPHORE_SEM_INIT_H
+#define LLVM_LIBC_SRC_SEMAPHORE_SEM_INIT_H
+
+#include "hdr/types/sem_t.h"
+#include "src/__support/macros/config.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+int sem_init(sem_t *sem, int pshared, unsigned int value);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_SEMAPHORE_SEM_INIT_H
diff --git a/libc/test/src/CMakeLists.txt b/libc/test/src/CMakeLists.txt
index 5e61c739c066a..236bee337d525 100644
--- a/libc/test/src/CMakeLists.txt
+++ b/libc/test/src/CMakeLists.txt
@@ -106,4 +106,5 @@ add_subdirectory(spawn)
if(${LIBC_TARGET_OS} STREQUAL "linux")
add_subdirectory(pthread)
+ add_subdirectory(semaphore)
endif()
diff --git a/libc/test/src/semaphore/CMakeLists.txt b/libc/test/src/semaphore/CMakeLists.txt
new file mode 100644
index 0000000000000..259c11fe55f07
--- /dev/null
+++ b/libc/test/src/semaphore/CMakeLists.txt
@@ -0,0 +1,19 @@
+add_custom_target(libc_semaphore_unittests)
+
+add_libc_unittest(
+ semaphore_test
+ SUITE
+ libc_semaphore_unittests
+ SRCS
+ semaphore_test.cpp
+ DEPENDS
+ libc.hdr.errno_macros
+ libc.include.limits
+ libc.include.semaphore
+ libc.src.errno.errno
+ libc.src.semaphore.sem_destroy
+ libc.src.semaphore.sem_getvalue
+ libc.src.semaphore.sem_init
+ libc.test.UnitTest.ErrnoCheckingTest
+ libc.test.UnitTest.ErrnoSetterMatcher
+)
diff --git a/libc/test/src/semaphore/semaphore_test.cpp b/libc/test/src/semaphore/semaphore_test.cpp
new file mode 100644
index 0000000000000..1e67bb046ad5f
--- /dev/null
+++ b/libc/test/src/semaphore/semaphore_test.cpp
@@ -0,0 +1,51 @@
+//===-- Unittests for POSIX semaphores -----------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "hdr/errno_macros.h"
+#include "src/semaphore/sem_destroy.h"
+#include "src/semaphore/sem_getvalue.h"
+#include "src/semaphore/sem_init.h"
+#include "test/UnitTest/ErrnoCheckingTest.h"
+#include "test/UnitTest/ErrnoSetterMatcher.h"
+#include "test/UnitTest/Test.h"
+
+#include <limits.h>
+#include <semaphore.h>
+
+using namespace LIBC_NAMESPACE::testing::ErrnoSetterMatcher;
+
+namespace {
+
+using LlvmLibcSemaphoreTest = LIBC_NAMESPACE::testing::ErrnoCheckingTest;
+
+} // namespace
+
+TEST_F(LlvmLibcSemaphoreTest, PublicSurface) {
+ sem_t sem = {};
+ (void)sem;
+ ASSERT_GE(SEM_VALUE_MAX, 32767);
+}
+
+TEST_F(LlvmLibcSemaphoreTest, UnnamedLifecycle) {
+ sem_t sem;
+ int value = -1;
+
+ ASSERT_THAT(LIBC_NAMESPACE::sem_init(&sem, 0, 3), Succeeds());
+ ASSERT_THAT(LIBC_NAMESPACE::sem_getvalue(&sem, &value), Succeeds());
+ ASSERT_EQ(value, 3);
+
+ ASSERT_THAT(LIBC_NAMESPACE::sem_destroy(&sem), Succeeds());
+ ASSERT_THAT(LIBC_NAMESPACE::sem_getvalue(&sem, &value), Fails(EINVAL));
+}
+
+TEST_F(LlvmLibcSemaphoreTest, UnnamedInitRejectsTooLargeValue) {
+ sem_t sem;
+ ASSERT_THAT(LIBC_NAMESPACE::sem_init(
+ &sem, 0, static_cast<unsigned int>(SEM_VALUE_MAX) + 1U),
+ Fails(EINVAL));
+}
|
There was a problem hiding this comment.
LGTM overall with Just one small change to make the code more CPP-styled:
Can you change LIBC_INLINE void init(unsigned int initial_value) to
LIBC_INLINE constexpr Semaphore(unsigned int value) : value(value), canary(SEM_CANARY) {}Then update reinterpret_cast<Semaphore *>(sem)->init(value); to
#include "src/__support/CPP/new.h"
new (sem) Semaphore {value}
michaelrj-google
left a comment
There was a problem hiding this comment.
Given that this PR only adds part of the semaphore mechanism I would prefer to avoid adding the incomplete entrypoints. Instead I would recommend changing the test to use the semaphore class directly. That would also means we don't need to define the public sem_t class, we can instead add that once the internal semaphore class has all the necessary functionality.
Having semaphores would be helpful and I appreciate you working on it. This way should make it easier to add in pieces without needing to edit the entrypoints repeatedly.
libc/src/semaphore/posix_semaphore.h
Outdated
| // invalidate is safe only when no threads is using | ||
| // blocked by destroyed semaphore is undefined | ||
| // use a destroyed semaphore is undefined | ||
| // RELAXED ordering is enough |
There was a problem hiding this comment.
This phrasing is confusing. Could you clarify?
libc/src/semaphore/sem_destroy.cpp
Outdated
| libc_errno = EINVAL; | ||
| return -1; | ||
| } | ||
|
|
There was a problem hiding this comment.
add a TODO to check for processes blocked on the semaphore
There was a problem hiding this comment.
I think posix specify this case is undefined:
The effect of destroying a semaphore upon which other threads are currently blocked is undefined.
|
I updated the plan and will put the |
michaelrj-google
left a comment
There was a problem hiding this comment.
LGTM, this seems like a good starting point. Thanks for working on this!
|
@Pengxiang-Huang do you need me to merge this PR for you? |
|
@michaelrj-google Yes. I appreciate it! |
Implements the first part for #190847
Add internal unnamed semaphore lifetime support, particularly:
sem_init: https://pubs.opengroup.org/onlinepubs/9799919799/functions/sem_init.html#sem_destroy: https://pubs.opengroup.org/onlinepubs/9799919799/functions/sem_destroy.html#sem_getvalue: https://pubs.opengroup.org/onlinepubs/9799919799/functions/sem_getvalue.html#