aix: guard STATIC_ASSERT for glibc work around

[richardlau]

Refs: nodejs/node#20129 (comment)

On 64-bit AIX sizeof(uv_sem_t) is 4 bytes which is not large enough to
store a pointer. AIX doesn't use glibc so the work around introduced by
#1795 doesn't apply, so guard the
STATIC_ASSERT with #ifdef __GLIBC__.

cc @addaleax @cjihrig

[richardlau]

Until #1807 and nodejs/build#1245 are resolved the libuv CI doesn't actually build/test 64-bit AIX. I guess we could cherry pick this over to nodejs/node#20129 and test on the Node.js CI?

[gireeshpunathil]

If I understand it correctly, the wrapper structure uv_semaphore_t is stored always in uv_sem_t pointer and never in its pointed value - in which case the assertion itself can go off - for the all platforms?

[addaleax]

Looking at the changes that were made above, I think we also need to check for __MVS__ now, where we always seem to use this pattern

[bnoordhuis]

I'd guard it on #if platform_needs_custom_semaphore. Can you drop the empty lines around the STATIC_ASSERT?

[addaleax]

@gireeshpunathil Practically speaking, right now the assertion would go off only on 64-bit platforms where uv_sem_t is an 4-byte integer type, which happens to be the case for 32-bit AIX.

[gireeshpunathil]

@addaleax - sorry, but I didn't follow you - the premise of the assertion is to make sure that the user's pointer (actual sem *) is capable of holding your new pointer right? so it is always pointer-to-pointer only, (never pointer-to-value) that means we don't need it, in any platforms, 32 or 64? Or Am I missing anything?

[addaleax]

the premise of the assertion is to make sure that the user's pointer (actual sem *) is capable of holding your new pointer right

If I’m understanding you correctly, no, that’s not the case. The static assertion is correct the way it is – we want to check that the uv_sem_t itself has place for an uv_semaphore_t pointer, because we store it there (and generally, we can't store it in the uv_sem_t* because that's not a modifiable parameter of uv_sem_init() from the libuv point of view)

[gireeshpunathil]

got it, thanks!

[gireeshpunathil]

LGTM

[richardlau]

Empty lines around the STATIC_ASSERT have been dropped. Guard is now #if defined(__GLIBC__) || platform_needs_custom_semaphore as platform_needs_custom_semaphore is only #defined if !defined(__GLIBC__).

[bnoordhuis]

LGTM. CI: https://ci.nodejs.org/job/libuv-test-commit/798/

[cjihrig]

Another PR to verify AIX - https://ci.nodejs.org/view/libuv/job/libuv-test-commit/801/

Landed in 56220e5. Thanks!