[libcxx][libcxxabi] Fix build for OpenBSD #92186
Conversation
llvmbot
added
libc++
|
@llvm/pr-subscribers-libcxxabi @llvm/pr-subscribers-libcxx Author: John Ericson (Ericson2314) Changes
Full diff: https://github.com/llvm/llvm-project/pull/92186.diff 3 Files Affected:
diff --git a/libcxx/src/atomic.cpp b/libcxx/src/atomic.cpp
index 2b67685c8a0a1..e0a5be86daab9 100644
--- a/libcxx/src/atomic.cpp
+++ b/libcxx/src/atomic.cpp
@@ -25,16 +25,26 @@
# if !defined(SYS_futex) && defined(SYS_futex_time64)
# define SYS_futex SYS_futex_time64
# endif
+# define _LIBCPP_FUTEX(...) syscall(SYS_futex, __VA_ARGS__)
#elif defined(__FreeBSD__)
# include <sys/types.h>
# include <sys/umtx.h>
+# define _LIBCPP_FUTEX(...) syscall(SYS_futex, __VA_ARGS__)
+
+#elif defined(__OpenBSD__)
+
+// OpenBSD has no indirect syscalls
+# define _LIBCPP_FUTEX(...) futex(__VA_ARGS__)
+
#else // <- Add other operating systems here
// Baseline needs no new headers
+# define _LIBCPP_FUTEX(...) syscall(SYS_futex, __VA_ARGS__)
+
#endif
_LIBCPP_BEGIN_NAMESPACE_STD
@@ -44,11 +54,11 @@ _LIBCPP_BEGIN_NAMESPACE_STD
static void
__libcpp_platform_wait_on_address(__cxx_atomic_contention_t const volatile* __ptr, __cxx_contention_t __val) {
static constexpr timespec __timeout = {2, 0};
- syscall(SYS_futex, __ptr, FUTEX_WAIT_PRIVATE, __val, &__timeout, 0, 0);
+ _LIBCPP_FUTEX(__ptr, FUTEX_WAIT_PRIVATE, __val, &__timeout, 0, 0);
}
static void __libcpp_platform_wake_by_address(__cxx_atomic_contention_t const volatile* __ptr, bool __notify_one) {
- syscall(SYS_futex, __ptr, FUTEX_WAKE_PRIVATE, __notify_one ? 1 : INT_MAX, 0, 0, 0);
+ _LIBCPP_FUTEX(__ptr, FUTEX_WAKE_PRIVATE, __notify_one ? 1 : INT_MAX, 0, 0, 0);
}
#elif defined(__APPLE__) && defined(_LIBCPP_USE_ULOCK)
diff --git a/libcxx/src/chrono.cpp b/libcxx/src/chrono.cpp
index c5e827c0cb59f..e7d6dfbc22924 100644
--- a/libcxx/src/chrono.cpp
+++ b/libcxx/src/chrono.cpp
@@ -31,7 +31,9 @@
# include <sys/time.h> // for gettimeofday and timeval
#endif
-#if defined(__APPLE__) || defined(__gnu_hurd__) || (defined(_POSIX_TIMERS) && _POSIX_TIMERS > 0)
+// OpenBSD does not have a fully conformant suite of POSIX timers, but
+// it does have clock_gettime and CLOCK_MONOTONIC which is all we need.
+#if defined(__APPLE__) || defined(__gnu_hurd__) || defined(__OpenBSD__) || (defined(_POSIX_TIMERS) && _POSIX_TIMERS > 0)
# define _LIBCPP_HAS_CLOCK_GETTIME
#endif
diff --git a/libcxxabi/src/cxa_guard_impl.h b/libcxxabi/src/cxa_guard_impl.h
index 7b140d3c36045..320501cb85938 100644
--- a/libcxxabi/src/cxa_guard_impl.h
+++ b/libcxxabi/src/cxa_guard_impl.h
@@ -47,6 +47,9 @@
#include "__cxxabi_config.h"
#include "include/atomic_support.h" // from libc++
#if defined(__has_include)
+# if __has_include(<sys/futex.h>)
+# include <sys/futex.h>
+# endif
# if __has_include(<sys/syscall.h>)
# include <sys/syscall.h>
# endif
@@ -411,7 +414,18 @@ struct InitByteGlobalMutex {
// Futex Implementation
//===----------------------------------------------------------------------===//
-#if defined(SYS_futex)
+#if defined(__OpenBSD__)
+void PlatformFutexWait(int* addr, int expect) {
+ constexpr int WAIT = 0;
+ futex(reinterpret_cast<volatile uint32_t*>(addr), WAIT, expect, NULL, NULL);
+ __tsan_acquire(addr);
+}
+void PlatformFutexWake(int* addr) {
+ constexpr int WAKE = 1;
+ __tsan_release(addr);
+ futex(reinterpret_cast<volatile uint32_t*>(addr), WAKE, INT_MAX, NULL, NULL);
+}
+#elif defined(SYS_futex)
void PlatformFutexWait(int* addr, int expect) {
constexpr int WAIT = 0;
syscall(SYS_futex, addr, WAIT, expect, 0);
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Ericson2314
force-pushed
the
libcxx-openbsd
branch
from
5579f64 to
bfc5001
Compare
- No indirect syscalls on OpenBSD. Instead there is a `futex` function which issues a direct syscall. - monotonic clock is avaialable despite the full POSIX suite of timers not being available in its entirety. See https://lists.boost.org/boost-bugs/2015/07/41690.php and boostorg/log@c98b1f4 for a description of an analogous problem an fix for Boost.
Ericson2314
force-pushed
the
libcxx-openbsd
branch
from
bfc5001 to
24ef89b
Compare
|
/cherry-pick af7467c |
- No indirect syscalls on OpenBSD. Instead there is a `futex` function which issues a direct syscall. - Monotonic clock is available despite the full POSIX suite of timers not being available in its entirety. See https://lists.boost.org/boost-bugs/2015/07/41690.php and boostorg/log@c98b1f4 for a description of an analogous problem and fix for Boost. (cherry picked from commit af7467c)
|
/pull-request #92601 |
|
@Ericson2314 I was looking around regarding this and I also noticed libcxx/src/filesystem/filesystem_clock.cpp and CLOCK_GETTIME. |
- No indirect syscalls on OpenBSD. Instead there is a `futex` function which issues a direct syscall. - Monotonic clock is available despite the full POSIX suite of timers not being available in its entirety. See https://lists.boost.org/boost-bugs/2015/07/41690.php and boostorg/log@c98b1f4 for a description of an analogous problem and fix for Boost. (cherry picked from commit af7467c)
|
Folks, please don't merge without getting an approval from the libc++/libc++abi reviewers in the future. This is the policy we follow in libc++. I've been OOO for the past 10 days so please let me know if somehow this followed a different approval path and I just missed it, but on the surface it looks like this change didn't go through our normal approval process. About the patch itself: Is there a reason why this is a problem now but wasn't a problem before? Also, we claim to support FreeBSD but we don't claim to support OpenBSD at all, and in order to do that we'd need to have a CI tester running per our policy. |
|
Sorry @ldionne. I didn't have so much of an idea of who is or isn't a reviewer, so didn't know anything was proceeding out of the ordinarily.
I imagine no one tried to build it on OpenBSD before.
FWIW I have just been cross compiling with Nix so far. I could contribute that but I am not sure whether it would count for building on OpenBSD. |
This shows what approvals are necessary before landing a patch. The review group must have approved (that way no need to know which individuals must have approved).
Ok, so then this patch is adding support for a new platform. We have a policy for doing that explained in https://libcxx.llvm.org/#platform-and-compiler-support. As part of our policy, we require CI to be present for all platforms that we officially support. Since this is not the case right now and it seems that more patches are coming down the pipeline to support OpenBSD, I would like us to add such support before we proceed with any other OpenBSD support patch. If we can't fulfill our support policy, I would ask to revert this patch. It's unfortunate that this was cherry-picked to the LLVM 18 release since this is definitely not something I'd consider a bugfix, but we can live with that. |
OpenBSD very recently made a change removing syscall(2) otherwise it's built fine for many many years. |
It's our system compiler / C++ standard library.. |
It's not a new platform per se. OpenBSD made a local change that broke things. We've been building and using libc++ for many years.
There are no other patches coming down the pipeline. There is nothing else at all. The one local patch we had for libcxxabi was commited as part of this patch. |
|
I think I saw #92675 which is also for OpenBSD. Generally speaking, what I'm saying is that we don't officially support platforms for which we have no tests, and OpenBSD currently falls in that category. I think it's great that OpenBSD uses libc++ as its system library and I would like the OpenBSD folks to work on getting us official test coverage on that platform so we can support it as a first class citizen. |
No indirect syscalls on OpenBSD. Instead there is a
futexfunction which issues a direct syscall.monotonic clock is avaialable despite the full POSIX suite of timers not being available in its entirety.
See https://lists.boost.org/boost-bugs/2015/07/41690.php and boostorg/log@c98b1f4 for a description of an analogous problem and fix for Boost.