Fix libcxx build on 32bit architectures with 64bit time_t defaults e.g. riscv32

[ldionne]

Summary

RISCV glibc has decided to use 64bit time_t from get go unlike
other 32bit architecture, therefore, aliasing NR_futex to
NR_futex_time64 helps avoid the below errors on rv32

[raj.khem]

Fix formatting errors

[ldionne]

libcxx/src/atomic.cpp

18

I'm not sure I understand -- libc++ doesn't use __NR_futex anywhere. Can you explain how this fix works?

[raj.khem]

libcxx/src/atomic.cpp

18

libc++ does use SYS_futex, which it expects from system C library, in glibc (/usr/include/bits/syscall.h defines it in terms of of NR_futex) rv32 is using 64bit time_t from get go unlike other 32bit architectures in glibc, therefore it wont have NR_futex defined but just NR_futex_time64, this aliases it to NR_futex so that SYS_futex is then defined for rv32

[luismarques]

I think this makes sense, but I'll let the #libc reviewers actually review/approve this.

libcxx/src/atomic.cpp

18

I suggest adding a comment here describing this situation.

[apazos]

Can you clarify on the errors you are talking about? Are you building LLVM libc++with glibc?
I have been building LLVM libc++ with MUSl and I do not see a build issue.

[luismarques]

I suppose that an alternative to this implementation might be to tweak the place of use of __NR_futex, like is done in [1]. Arguably that's cleaner. Or, in the place where you're currently checking for riscv32, check for the case where __NR_futex_ isn't defined but __NR_futex_time64 is, and #define the former in terms of the latter.

[1]: https://sourceware.org/legacy-ml/libc-alpha/2020-01/msg00686.html

[raj.khem]

this revision is not riscv32 specific. Let me know if this is ok

[raj.khem]

In D85095#2279476, @apazos wrote:

Can you clarify on the errors you are talking about? Are you building LLVM libc++with glibc?
I have been building LLVM libc++ with MUSl and I do not see a build issue.

the errors are not musl specific, but when building for riscv32 target on glibc

[raj.khem]

Add comments in source

[raj.khem]

it should have been after including syscall.h

[ldionne]

I don't understand why this has its place in libc++. Shouldn't this be in glibc instead?

[raj.khem]

In D85095#2400738, @ldionne wrote:

I don't understand why this has its place in libc++. Shouldn't this be in glibc instead?

SYS_futex is not a universal syscall name for all architectures as explained in the comment, and libc++ is assuming that incorrectly. hope that helps

[ldionne]

Rebase onto main and trigger CI.

[ldionne]

@luismarques Does this still look good to you? If so, I'll commit once CI is green.

[luismarques]

In D85095#2540371, @ldionne wrote:

@luismarques Does this still look good to you? If so, I'll commit once CI is green.

LGTM.
I think the current target-independent implementation of the patch really is the way to go, so this now looks even better. Thanks!

[raj.khem]

Can we get this on clang 12 release branch as well ?

[ldionne]

In D85095#2542488, @raj.khem wrote:

Can we get this on clang 12 release branch as well ?

See https://reviews.llvm.org/D96062.

libcxx/src/atomic.cpp

 #ifndef _LIBCPP_HAS_NO_THREADS
 
 #include <climits>
 #include <atomic>
 #include <functional>
 
 #ifdef __linux__
 
 #include <unistd.h>
 #include <linux/futex.h>
 #include <sys/syscall.h>
 
+// libc++ uses SYS_futex, which it expects from system C library.
+// in glibc (/usr/include/bits/syscall.h defines it in terms of of NR_futex)
+// rv32 is using 64bit time_t from get go unlike other 32bit architectures
+// in glibc, therefore it wont have NR_futex defined but just NR_futex_time64
+// this aliases it to NR_futex so that SYS_futex is then defined for rv32
+
+#if !defined(SYS_futex) && defined(SYS_futex_time64)
+#define SYS_futex SYS_futex_time64
+#endif
+
 #else // <- Add other operating systems here
 
 // Baseline needs no new headers
 
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD