[libcxx-commits] [libcxx] [libc++] Fix FreeBSD atomic timed wait system call (PR #180400)

[via libcxx-commits]

================
@@ -130,22 +130,20 @@ static void __platform_wake_by_address(void const* __ptr, bool __notify_one) {
 template <std::size_t _Size>
 static void __platform_wait_on_address(void const* __ptr, void const* __val, uint64_t __timeout_ns) {
   static_assert(_Size == 8, "Can only wait on 8 bytes value");
-  char buffer[_Size];
+  alignas(__cxx_contention_t) char buffer[_Size];
   std::memcpy(&buffer, const_cast<const void*>(__val), _Size);
----------------
huixie90 wrote:

> So my understanding of the C++ object model is admittedly weaker than C's aliasing model, but I believe that, although you can access any object as if it were characters/bytes to see its representation, you cannot in general take an array of characters and reinterpret it as something else, without first creating such an object with a placement new? At which point why would this not just memcpy to a __cxx_contention_t directly? That's how std::bit_cast can be implemented. This seems to be a repeated pattern among other platforms, too.
> 
> To be honest I'm also not sure why we're using __cxx_contention_t in here. I'd expect __platform_wait_on_address to deal with the supported native type(s), with a fallback for those that aren't but need to be, and it's up to the layers above to deal with the __cxx_contention_t. As far as __platform_wait_on_address is concerned there's no reason it couldn't support `_Size == 4`, such an operation exists in FreeBSD, it's just that the upper layers dealing with __cxx_contention_t don't have a use for such an operation. But that's a bigger issue.

> you cannot in general take an array of characters and reinterpret it as something else

I believe what you said is true. But `std::memcpy` implicitly start life time for _implicit-lifetime_ object if I remember correctly. And `__cxx_contention_t` is an implicitly lifetime object.

> At which point why would this not just memcpy to a `__cxx_contention_t` directly

So this function is going to be shared between the stable and unstable ABI. You are correct that right now it is going to be passed a `__cxx_contention_t` so indeed, i don't need to do this. I can even directly reinprecast the input void* to __cxx_contention_t* , if you agree with me.  It was an char array, because this code was modified along with adding unstable ABI support for Apple platform where we support two types of futex and the system api expect uint32_t and uint64_t values. We support any types that satisfy the requirement (size, alignment, unique object representation, trivially copyable)  and memcpy to char array. but you are right that here we don't need any of this because FreeBSD is stuck at the current __cxx_contention_t


https://github.com/llvm/llvm-project/pull/180400