Bug 235372 – [bmalloc][32-bits] Build error in libpas after r285853

• Status: RESOLVED
• Resolution: FIXED
• Component: bmalloc

[Pablo Saavedra]

Getting this error during compiling WPE/WebKit for an 32-bits target (ARM32)


```
tmp/work/cortexa9t2hf-neon-imx-poky-linux-gnueabi/wpewebkit/trunk+httpsAUTOINC+f8506f6a3b-r0/build/bmalloc/Headers/bmalloc/pas_utils.h: At global scope:
tmp/work/cortexa9t2hf-neon-imx-poky-linux-gnueabi/wpewebkit/trunk+httpsAUTOINC+f8506f6a3b-r0/build/bmalloc/Headers/bmalloc/pas_utils.h:633:9: error: '__uint128_t' does not name a type; did you mean '__uint8_t'?
  633 | typedef __uint128_t pas_pair;
      |         ^~~~~~~~~~~
      |         __uint8_t
tmp/work/cortexa9t2hf-neon-imx-poky-linux-gnueabi/wpewebkit/trunk+httpsAUTOINC+f8506f6a3b-r0/build/bmalloc/Headers/bmalloc/pas_utils.h:635:15: error: 'pas_pair' does not name a type
```


BENABLE_LIBPAS = 0

The error can be related with the changes done in https://trac.webkit.org/changeset/285853/webkit. In particular, with those associated to the point 8:


commit cc83e6240649bfe828f932d1d7fa96f090d706b0
Author: Yusuke Suzuki <[e-mail]>
Date:   Tue Nov 16 07:02:45 2021 +0000

    [libpas] Build and enable libpas on 64bit JSCOnly
    https://bugs.webkit.org/show_bug.cgi?id=233097
    
    Reviewed by Filip Pizlo.
    
    This patch makes libpas built on 64bit Linux. And now enabling it on JSCOnly port.
    libpas is built in C and tailored to the current clang. It made building libpas on
    GCC hard since GCC is stricter on C languages.
    
    1. C does not handle `const` variables as constants. So libpas's config is not strictly constant
       in the C spec, and GCC actually rejects it. To make it built correctly, we need to build them
       in C++. In this patch, when building libpas via CMake, we build some libpas C files as C++.
    2. GCC C++ does not allow designated initializer for array. We work-around this problem by adding
       cpp_initialization_t constexpr constructors only when building these headers in C++ compiler.
    3. Atomic operations are using clang extension, so they cannot be built with GCC. This patch adds
       GCC version of these implementations.
    4. Add PAS_OS(DARWIN) ifdefs for Darwin specific code (e.g. malloc handling).
    5. Add explicit cast / PAS_UNUSED_PARAM since GCC emits warnings
    6. Use (unsigned long long) explicit cast for printing uint64_t via `%llx` / `%llu` since uint64_t
       definition is `unsigned long int` in GCC (while clang's one is `unsigned long long`).
    7. Reorder designated initializer for structures since not following to the definition ordering
       becomes compile error in GCC.
[THIS]    8. Use __uint128_t for pas_pair in GCC to avoid compile error.

[Pablo Saavedra]

Created attachment 449507 [details]
patch

[Pablo Saavedra]

128-bit integer type is only available for 64-bit targets. In >=GCC 4.6you can use __SIZEOF_INT128__ to detect it.

I just uploaded a functional patch that fix the build error by checking and falling back to an 64 bits long type the 128-bit is not available.

[Yusuke Suzuki]

Comment on attachment 449507 [details]
patch

libpas does not support 32bit environment. So the right fix is ensuring that all built libpas C files are guarded by LIBPAS_ENABLED so that we do not build this file.
Can you paste which file is including pas_utils.h?

[Yusuke Suzuki]

I wonder if this is broken due to bug 230841

[Yusuke Suzuki]

And I also wonder if this fixed this issue. https://bugs.webkit.org/show_bug.cgi?id=235275

[Pablo Saavedra]

(In reply to Yusuke Suzuki from comment #3)
> Comment on attachment 449507 [details]
> patch
> 
> libpas does not support 32bit environment. So the right fix is ensuring that
> all built libpas C files are guarded by LIBPAS_ENABLED so that we do not
> build this file.
> Can you paste which file is including pas_utils.h?

The failure comes from here :

```
In file included from /workspace/sources/wpewebkit/Source/WebCore/platform/graphics/HEVCUtilities.cpp:38:
/tmp/work/cortexa9t2hf-neon-imx-poky-linux-gnueabi/wpewebkit/trunk+https999-r0/wpewebkit-trunk+https999/bmalloc/Headers/bmalloc/pas_utils.h: In function 'bool pas_compare_and_swap_uintptr_weak(uintptr_t*, uintptr_t, uintptr_t)':
/tmp/work/cortexa9t2hf-neon-imx-poky-linux-gnueabi/wpewebkit/trunk+https999-r0/wpewebkit-trunk+https999/bmalloc/Headers/bmalloc/pas_utils.h:557:45: warning: cast from 'uintptr_t*' {aka 'unsigned int*'} to 'uint64_t*' {aka 'long long unsigned int*'} increases required alignment of target type [-Wcast-align]
  557 |     return pas_compare_and_swap_uint64_weak((uint64_t*)ptr, (uint64_t)old_value, (uint64_t)new_value);
      |                                             ^~~~~~~~~~~~~~
/tmp/work/cortexa9t2hf-neon-imx-poky-linux-gnueabi/wpewebkit/trunk+https999-r0/wpewebkit-trunk+https999/bmalloc/Headers/bmalloc/pas_utils.h: In function 'uintptr_t pas_compare_and_swap_uintptr_strong(uintptr_t*, uintptr_t, uintptr_t)':
/tmp/work/cortexa9t2hf-neon-imx-poky-linux-gnueabi/wpewebkit/trunk+https999-r0/wpewebkit-trunk+https999/bmalloc/Headers/bmalloc/pas_utils.h:562:58: warning: cast from 'uintptr_t*' {aka 'unsigned int*'} to 'uint64_t*' {aka 'long long unsigned int*'} increases required alignment of target type [-Wcast-align]
  562 |     return (uintptr_t)pas_compare_and_swap_uint64_strong((uint64_t*)ptr, (uint64_t)old_value, (uint64_t)new_value);
      |                                                          ^~~~~~~~~~~~~~
/tmp/work/cortexa9t2hf-neon-imx-poky-linux-gnueabi/wpewebkit/trunk+https999-r0/wpewebkit-trunk+https999/bmalloc/Headers/bmalloc/pas_utils.h: At global scope:
/tmp/work/cortexa9t2hf-neon-imx-poky-linux-gnueabi/wpewebkit/trunk+https999-r0/wpewebkit-trunk+https999/bmalloc/Headers/bmalloc/pas_utils.h:633:9: error: '__uint128_t' does not name a type; did you mean '__uint8_t'?
  633 | typedef __uint128_t pas_pair;
      |         ^~~~~~~~~~~
      |         __uint8_t
```


It took me time because the non unified-build was broken. 



A tentative patch it could be something like this:

```
--- a/Source/WebCore/platform/graphics/HEVCUtilities.cpp
+++ b/Source/WebCore/platform/graphics/HEVCUtilities.cpp
@@ -34,7 +34,7 @@
 #include <wtf/SortedArrayMap.h>
 #include <wtf/text/StringToIntegerConversion.h>
 
-#if __has_include(<bmalloc/pas_utils.h>)
+#if LIBPAS_ENABLED
 #include <bmalloc/pas_utils.h>
 #endif
 
@@ -42,7 +42,7 @@ namespace WebCore {
 
 static inline uint32_t reverseBits(uint32_t value)
 {
-#if __has_include(<bmalloc/pas_utils.h>)
+#if LIBPAS_ENABLED
     return pas_reverse(value);
 #else
     // From pas_reverse():
```

[Pablo Saavedra]

Created attachment 449565 [details]
patch

[Pablo Saavedra]

(In reply to Yusuke Suzuki from comment #5)
> And I also wonder if this fixed this issue.
> https://bugs.webkit.org/show_bug.cgi?id=235275

Yes, based on the previous comment that change should be fix the  issue.

[Pablo Saavedra]

(In reply to Pablo Saavedra from comment #8)
> (In reply to Yusuke Suzuki from comment #5)
> > And I also wonder if this fixed this issue.
> > https://bugs.webkit.org/show_bug.cgi?id=235275
> 
> Yes, based on the previous comment that change should be fix the  issue.

I will test it in local just for a double-check.

[Pablo Saavedra]

Fixed. Thanks for your help ysuzuki.

[Radar WebKit Bug Importer]

<rdar://problem/87844683>