llvm-project [libc++] Sync the filesystem clock_gettime handling with chrono for OpenBSD

Copy the same OS checks for clock_gettime from chrono to filesystem and tweak the comment as Hurd is in the same situation.

May 19 '24 02:05 brad0

@llvm/pr-subscribers-libcxx

Author: Brad Smith (brad0)

Changes

Copy the same OS checks for clock_gettime from chrono to filesystem and tweak the comment as Hurd is in the same situation.

Full diff: https://github.com/llvm/llvm-project/pull/92675.diff

2 Files Affected:

(modified) libcxx/src/chrono.cpp (+2-2)
(modified) libcxx/src/filesystem/filesystem_clock.cpp (+3-1)

diff --git a/libcxx/src/chrono.cpp b/libcxx/src/chrono.cpp
index e7d6dfbc22924..e8ebb67284bbb 100644
--- a/libcxx/src/chrono.cpp
+++ b/libcxx/src/chrono.cpp
@@ -31,8 +31,8 @@
 #  include <sys/time.h> // for gettimeofday and timeval
 #endif
 
-// 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.
+// GNU Hurd and OpenBSD do not implement a fully conformant suite of POSIX timers, but
+// they do have clock_gettime and CLOCK_MONOTONIC which are 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/libcxx/src/filesystem/filesystem_clock.cpp b/libcxx/src/filesystem/filesystem_clock.cpp
index e13b2853e367c..a120be14978a3 100644
--- a/libcxx/src/filesystem/filesystem_clock.cpp
+++ b/libcxx/src/filesystem/filesystem_clock.cpp
@@ -29,7 +29,9 @@
 #  include <sys/time.h> // for gettimeofday and timeval
 #endif
 
-#if defined(__APPLE__) || defined(__gnu_hurd__) || (defined(_POSIX_TIMERS) && _POSIX_TIMERS > 0)
+// GNU Hurd and OpenBSD do not implement a fully conformant suite of POSIX timers, but
+// they do have clock_gettime and CLOCK_MONOTONIC which are all we need.
+#if defined(__APPLE__) || defined(__gnu_hurd__) || defined(__OpenBSD__) || (defined(_POSIX_TIMERS) && _POSIX_TIMERS > 0)
 #  define _LIBCPP_HAS_CLOCK_GETTIME
 #endif

May 19 '24 02:05 llvmbot

In general we like to have a CI runner to test platform specific behaviour. Do you intent to add a GNU Hurd runner? (This change is small and innocent, but without CI tests there is no guarantee there won't be regressions.)

Can you make the commit message a bit more descriptive. Something along the lines of [libc++] Add GNU Hurd clock_gettime support seems better describing the goal of the change.

May 20 '24 15:05 mordante

Gentle ping -- I'll close this as inactive if there is no plan to officially support this platform.

Jun 25 '24 15:06 ldionne

Gentle ping -- I'll close this as inactive if there is no plan to officially support this platform.

I'd like to have official support for the platform. But looking at the current code and support policy this will consistently be an issue for us. Our current Clang / libc++ is 16.

Jun 27 '24 02:06 brad0

@brad0 I'll need to forward-port some patches just to get the bare minimum of simple executables able to run. Maybe that is good enough even if OpenBSD is still officially hanging back on LLVM 16 for a while longer?

OpenBSD is a faster-moving ABI with the security experiments (and I would not want that to change!), but I think it should be possible to have less outsanding patches against upstream without compromising that ABI agility or incurring painful upstreaming drudgery.

Jun 27 '24 15:06 Ericson2314

What kind of downstream changes do you folks have? We may be able to accommodate some of them or at least make them easier to maintain by introducing a few "extension points" in the library. This is a common situation and we're quite used to finding ways to make it work both for the upstream project and for downstream needs.

Jun 27 '24 21:06 ldionne

@ldionne I am learning as I go, as my cross compiled binaries do not yet work :).

First on my docket is that LLD needs some linker script modifications so that special sections like .openbsd.mutable and .openbsd.syscalls end of being linked correctly.

The change that currently does this just uses #ifdef __OpenBSD__ but that is not right for cross compilation, so this will need to be fixed up.

To be clear, I haven't seen any critical downstream modification in libc++ itself yet, but I would consider LLD / Clang / LLVM changes blockers insofar is if binaries don't run, then the libc++ testsuite cannot run --- unless you would consider a "does it at least build" smoke test a decent first step.

Jun 27 '24 21:06 Ericson2314

What kind of downstream changes do you folks have? We may be able to accommodate some of them or at least make them easier to maintain by introducing a few "extension points" in the library. This is a common situation and we're quite used to finding ways to make it work both for the upstream project and for downstream needs.

For libc++. Nothing.

Various local patches for Clang, LLVM and LLD though.

Jun 28 '24 03:06 brad0

@Ericson2314 @brad0 I think that to begin with, we could start with only compiling the test suite. We could at least see how far we're able to go in that direction. I really want to avoid having "phantom" platforms in the code base (where we have 0 visibility into what's working and what's not working).

Jul 04 '24 18:07 ldionne

I can provide support for adding a new OpenBSD build bot if you want. The documentation for adding a new CI job using BuildKite is here: https://libcxx.llvm.org/AddingNewCIJobs.html#addingnewcijobs

It's also possible to set up a custom Github Actions runner if you prefer that, either way is fine by us.

Jul 04 '24 18:07 ldionne

I can provide support for adding a new OpenBSD build bot if you want. The documentation for adding a new CI job using BuildKite is here: https://libcxx.llvm.org/AddingNewCIJobs.html#addingnewcijobs

There is not a Buildkite binary for OpenBSD. But it built Ok. I'll see about creating a package.

I'll be able to try setting something up around the end of the month.

Jul 06 '24 11:07 brad0

Ack. Please let me know when/whether I can provide support. I will set up a reminder to ping this at the end of the month.

I would aim to have this done in time for the LLVM 20 release. In terms of process, if we are not able to set this up in time for LLVM 20, I would remove patches that intend to provide support for OpenBSD until we are able to actually provide support for it, to avoid an accumulation of half-supported things in the codebase.

Jul 09 '24 20:07 ldionne

I'll close this for the time being but let's reopen it once we have what we need CI-wise.

Jul 09 '24 20:07 ldionne

Scheduled ping. Any interest for setting up CI? Should I start cleaning up OpenBSD support from the upstream tree?

Jul 31 '24 15:07 ldionne

Scheduled ping. Any interest for setting up CI? Should I start cleaning up OpenBSD support from the upstream tree?

I have the system. I just got the OS installed and it's ready to go.

Aug 01 '24 06:08 brad0

Can you reach out to me on Discord @ldionne, I can help you set up the machine w/ BuildKite tokens and so on.

Aug 01 '24 16:08 ldionne

Can you reach out to me on Discord @ldionne, I can help you set up the machine w/ BuildKite tokens and so on.

@ldionne I have. You don't respond.

Aug 24 '24 00:08 brad0

Sorry, new message requests on Discord end up in a weird place, I missed it but I see it now.

Aug 26 '24 14:08 ldionne