REGRESSION(r244766): [GTK] 2.25.1 does not build on 32-bit ARM due to bit-packing assertion, requires -DENABLE_DARK_MODE_CSS=OFF

[Michael Catanzaro]

2.25.1 is rejected for GNOME because it does not build on aarch64:

FAILED: Source/WebCore/CMakeFiles/WebCore.dir/__/__/DerivedSources/WebCore/unified-sources/UnifiedSource-043dd90b-23.cpp.o 
/usr/bin/c++  -DBUILDING_GTK__=1 -DBUILDING_WEBKIT -DBUILDING_WITH_CMAKE=1 -DBUILDING_WebCore -DGETTEXT_PACKAGE=\"WebKit2GTK-4.0\" -DHAVE_CONFIG_H=1 -DJSC_GLIB_API_ENABLED -DSTATICALLY_LINKED_WITH_PAL=1 -DWEBKITGTK_API_VERSION_STRING=\"4.0\" -IDerivedSources/ForwardingHeaders -I. -IDerivedSources/WebCore -I../Source/WebCore -I../Source/WebCore/Modules/airplay -I../Source/WebCore/Modules/applepay -I../Source/WebCore/Modules/applepay/paymentrequest -I../Source/WebCore/Modules/applicationmanifest -I../Source/WebCore/Modules/beacon -I../Source/WebCore/Modules/cache -I../Source/WebCore/Modules/credentialmanagement -I../Source/WebCore/Modules/encryptedmedia -I../Source/WebCore/Modules/encryptedmedia/legacy -I../Source/WebCore/Modules/entriesapi -I../Source/WebCore/Modules/fetch -I../Source/WebCore/Modules/geolocation -I../Source/WebCore/Modules/indexeddb -I../Source/WebCore/Modules/indexeddb/client -I../Source/WebCore/Modules/indexeddb/server -I../Source/WebCore/Modules/indexeddb/shared -I../Source/WebCore/Modules/mediacapabilities -I../Source/WebCore/Modules/mediacontrols -I../Source/WebCore/Modules/mediarecorder -I../Source/WebCore/Modules/mediasession -I../Source/WebCore/Modules/mediasource -I../Source/WebCore/Modules/mediastream -I../Source/WebCore/Modules/mediastream/libwebrtc -I../Source/WebCore/Modules/navigatorcontentutils -I../Source/WebCore/Modules/notifications -I../Source/WebCore/Modules/paymentrequest -I../Source/WebCore/Modules/plugins -I../Source/WebCore/Modules/quota -I../Source/WebCore/Modules/speech -I../Source/WebCore/Modules/streams -I../Source/WebCore/Modules/webaudio -I../Source/WebCore/Modules/webauthn -I../Source/WebCore/Modules/webauthn/cbor -I../Source/WebCore/Modules/webauthn/fido -I../Source/WebCore/Modules/webdatabase -I../Source/WebCore/Modules/webdriver -I../Source/WebCore/Modules/webgpu -I../Source/WebCore/Modules/websockets -I../Source/WebCore/Modules/webvr -I../Source/WebCore/accessibility -I../Source/WebCore/accessibility/isolatedtree -I../Source/WebCore/animation -I../Source/WebCore/bindings -I../Source/WebCore/bindings/js -I../Source/WebCore/bridge -I../Source/WebCore/bridge/c -I../Source/WebCore/bridge/jsc -I../Source/WebCore/contentextensions -I../Source/WebCore/crypto -I../Source/WebCore/crypto/algorithms -I../Source/WebCore/crypto/keys -I../Source/WebCore/crypto/parameters -I../Source/WebCore/css -I../Source/WebCore/css/parser -I../Source/WebCore/css/typedom -I../Source/WebCore/cssjit -I../Source/WebCore/dom -I../Source/WebCore/dom/messageports -I../Source/WebCore/domjit -I../Source/WebCore/editing -I../Source/WebCore/fileapi -I../Source/WebCore/history -I../Source/WebCore/html -I../Source/WebCore/html/canvas -I../Source/WebCore/html/forms -I../Source/WebCore/html/parser -I../Source/WebCore/html/shadow -I../Source/WebCore/html/track -I../Source/WebCore/inspector -I../Source/WebCore/inspector/agents -I../Source/WebCore/inspector/agents/page -I../Source/WebCore/inspector/agents/worker -I../Source/WebCore/layout -I../Source/WebCore/layout/blockformatting -I../Source/WebCore/layout/displaytree -I../Source/WebCore/layout/floats -I../Source/WebCore/layout/inlineformatting -I../Source/WebCore/layout/inlineformatting/text -I../Source/WebCore/layout/layouttree -I../Source/WebCore/loader -I../Source/WebCore/loader/appcache -I../Source/WebCore/loader/archive -I../Source/WebCore/loader/archive/mhtml -I../Source/WebCore/loader/cache -I../Source/WebCore/loader/icon -I../Source/WebCore/mathml -I../Source/WebCore/page -I../Source/WebCore/page/animation -I../Source/WebCore/page/csp -I../Source/WebCore/page/scrolling -I../Source/WebCore/platform -I../Source/WebCore/platform/animation -I../Source/WebCore/platform/audio -I../Source/WebCore/platform/encryptedmedia -I../Source/WebCore/platform/gamepad -I../Source/WebCore/platform/graphics -I../Source/WebCore/platform/graphics/cpu/arm -I../Source/WebCore/platform/graphics/cpu/arm/filters -I../Source/WebCore/platform/graphics/displaylists -I../Source/WebCore/platform/graphics/filters -I../Source/WebCore/platform/graphics/iso -I../Source/WebCore/platform/graphics/opentype -I../Source/WebCore/platform/graphics/transforms -I../Source/WebCore/platform/mediacapabilities -I../Source/WebCore/platform/mediarecorder -I../Source/WebCore/platform/mediasession -I../Source/WebCore/platform/mediastream -I../Source/WebCore/platform/mediastream/libwebrtc -I../Source/WebCore/platform/mock -I../Source/WebCore/platform/mock/mediasource -I../Source/WebCore/platform/network -I../Source/WebCore/platform/sql -I../Source/WebCore/platform/text -I../Source/WebCore/platform/vr -I../Source/WebCore/plugins -I../Source/WebCore/rendering -I../Source/WebCore/rendering/line -I../Source/WebCore/rendering/mathml -I../Source/WebCore/rendering/shapes -I../Source/WebCore/rendering/style -I../Source/WebCore/rendering/svg -I../Source/WebCore/rendering/updating -I../Source/WebCore/replay -I../Source/WebCore/storage -I../Source/WebCore/style -I../Source/WebCore/svg -I../Source/WebCore/svg/animation -I../Source/WebCore/svg/graphics -I../Source/WebCore/svg/graphics/filters -I../Source/WebCore/svg/properties -I../Source/WebCore/websockets -I../Source/WebCore/workers -I../Source/WebCore/workers/service -I../Source/WebCore/workers/service/context -I../Source/WebCore/workers/service/server -I../Source/WebCore/worklets -I../Source/WebCore/xml -I../Source/WebCore/xml/parser -I../Source/WebCore/platform/graphics/gpu -I../Source/ThirdParty/xdgmime/src -I../Source/WebCore/platform/graphics/cairo -I../Source/WebCore/platform/graphics/freetype -I../Source/WebCore/platform/graphics/harfbuzz -I../Source/WebCore/platform/graphics/harfbuzz/ng -I../Source/WebCore/platform/graphics/gstreamer -I../Source/WebCore/platform/graphics/gstreamer/mse -I../Source/WebCore/platform/graphics/gstreamer/eme -I../Source/WebCore/platform/audio/gstreamer -I../Source/WebCore/platform/image-decoders -I../Source/WebCore/platform/image-decoders/bmp -I../Source/WebCore/platform/image-decoders/gif -I../Source/WebCore/platform/image-decoders/ico -I../Source/WebCore/platform/image-decoders/jpeg -I../Source/WebCore/platform/image-decoders/jpeg2000 -I../Source/WebCore/platform/image-decoders/png -I../Source/WebCore/platform/image-decoders/webp -I../Source/WebCore/platform/network/soup -I../Source/WebCore/platform/graphics/texmap -I../Source/WebCore/page/scrolling/nicosia -I../Source/WebCore/platform/graphics/texmap/coordinated -I../Source/WebCore/platform/graphics/nicosia -I../Source/WebCore/platform/graphics/nicosia/cairo -I../Source/WebCore/platform/graphics/nicosia/texmap -I../Source/WebCore/accessibility/atk -I../Source/WebCore/editing/atk -I../Source/WebCore/page/gtk -I../Source/WebCore/platform/generic -I../Source/WebCore/platform/gtk -I../Source/WebCore/platform/graphics/egl -I../Source/WebCore/platform/graphics/glx -I../Source/WebCore/platform/graphics/gtk -I../Source/WebCore/platform/graphics/opengl -I../Source/WebCore/platform/graphics/wayland -I../Source/WebCore/platform/graphics/x11 -I../Source/WebCore/platform/mediastream/gtk -I../Source/WebCore/platform/mediastream/gstreamer -I../Source/WebCore/platform/network/gtk -I../Source/WebCore/platform/text/gtk -I../Source/WebCore/bindings/gobject -IDerivedSources -I../Source/ThirdParty -isystem /usr/include/libxml2 -isystem /usr/include/cairo -isystem /usr/include/freetype2 -isystem /usr/include/harfbuzz -isystem /usr/include/gstreamer-1.0 -isystem /usr/include/glib-2.0 -isystem /usr/lib/arm-linux-gnueabihf/glib-2.0/include -isystem /usr/lib/arm-linux-gnueabihf/libffi-3.2.1/include -isystem /usr/include/orc-0.4 -isystem /usr/lib/arm-linux-gnueabihf/gstreamer-1.0/include -isystem /usr/include/libdrm -isystem /usr/include/openjpeg-2.3 -isystem /usr/include/libsoup-2.4 -isystem /usr/include/atk-1.0 -isystem /usr/include/enchant-2 -isystem /usr/include/gio-unix-2.0 -isystem /usr/include/libmount -isystem /usr/include/blkid -isystem /usr/include/uuid -isystem /usr/include/libsecret-1 -fdiagnostics-color=always -Wextra -Wall -Wno-expansion-to-defined -Wno-psabi -Wno-noexcept-type -Wno-maybe-uninitialized -Wwrite-strings -Wundef -Wpointer-arith -Wmissing-format-attribute -Wformat-security -Wcast-align -O2 -g -pipe -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -g1 -fno-strict-aliasing -fno-exceptions -fno-rtti -std=c++17 -O3 -DNDEBUG -fPIC -MD -MT Source/WebCore/CMakeFiles/WebCore.dir/__/__/DerivedSources/WebCore/unified-sources/UnifiedSource-043dd90b-23.cpp.o -MF Source/WebCore/CMakeFiles/WebCore.dir/__/__/DerivedSources/WebCore/unified-sources/UnifiedSource-043dd90b-23.cpp.o.d -o Source/WebCore/CMakeFiles/WebCore.dir/__/__/DerivedSources/WebCore/unified-sources/UnifiedSource-043dd90b-23.cpp.o -c DerivedSources/WebCore/unified-sources/UnifiedSource-043dd90b-23.cpp
In file included from DerivedSources/ForwardingHeaders/wtf/StdLibExtras.h:32,
                 from DerivedSources/ForwardingHeaders/wtf/FastMalloc.h:25,
                 from ../Source/WebCore/config.h:50,
                 from ../Source/WebCore/rendering/style/StyleGeneratedImage.cpp:24,
                 from DerivedSources/WebCore/unified-sources/UnifiedSource-043dd90b-23.cpp:1:
../Source/WebCore/rendering/style/StyleRareInheritedData.cpp:78:47: error: static assertion failed: StyleRareInheritedData_should_bit_pack
 COMPILE_ASSERT(sizeof(StyleRareInheritedData) <= sizeof(GreaterThanOrSameSizeAsStyleRareInheritedData), StyleRareInheritedData_should_bit_pack);
DerivedSources/ForwardingHeaders/wtf/Assertions.h:440:50: note: in definition of macro ‘COMPILE_ASSERT’
 #define COMPILE_ASSERT(exp, name) static_assert((exp), #name)
                                                  ^~~


Suspicious change to StyleRareInheritedData is:

#if ENABLE(DARK_MODE_CSS)
    StyleColorScheme colorScheme;
#endif

[Michael Catanzaro]

Same failure on 32-bit ARM.

[Michael Catanzaro]

I was able to get 2.25.1 into GNOME by using -DENABLE_DARK_MODE_CSS=OFF (not great!). We will have to remove that build flag from gnome-build-meta when this is fixed.

[Michael Catanzaro]

I'm struggling to figure out what the problem is here:

 * We know Fedora (which uses GCC 9) is not affected
 * We know our aarch64 bot (not sure which GCC it uses) is not affected
 * We know from Berto that Debian is affected
 * We know from me that GNOME (which uses GCC 8) is affected

I've tested a modified buildroot configured to build 2.25.1 with both GCC 7 and GCC 8, with good results. Stumped. We can't plausibly debug this without access to an affected build machine.

[Alberto Garcia]

(In reply to Michael Catanzaro from comment #3)
>  * We know from Berto that Debian is affected

Debian can build 2.25.1 for aarch64:

https://buildd.debian.org/status/package.php?p=webkit2gtk&suite=experimental

[Michael Catanzaro]

Fedora reports having trouble only on 32-bit ARM. Sounds like it's the same for Debian.

Reviewing our build logs, I see I've been wrong about this the whole time. This error only occurs on 32-bit ARM. For aarch64, our original build failure is mysterious, with no error message whatsoever that I can see. There is probably an error buried in there somewhere, impossible to see past all the spam from bug #198014, but it's not a normal GCC error since the string "error:" doesn't appear anywhere in the logs.

So I wasted a bunch of time building for aarch64 yesterday. Will try again with 32-bit ARM now.

[Michael Catanzaro]

CCing some people who have edited these structs recently.

I stole from https://stackoverflow.com/a/43434880/1120203:

#define COMPILE_TIME_SIZEOF(t)      template<int s> struct SIZEOF_ ## t ## _IS; \
                                    struct foo { \
                                        int a,b; \
                                    }; \
                                    SIZEOF_ ## t ## _IS<sizeof(t)> SIZEOF_ ## t ## _IS;

COMPILE_TIME_SIZEOF(StyleRareInheritedData);
COMPILE_TIME_SIZEOF(GreaterThanOrSameSizeAsStyleRareInheritedData);

With this, I learned:

 * The size of both structs on x86_64 is currently 272
 * The size of StyleRareInheritedData on ARM is currently 224
 * The size of GreaterThanOrSameSizeAsStyleRareInheritedData on ARM is currently 216. Bad!

Before r244766 "[GTK] Support prefers-color-scheme media query", the size of both structs on ARM was 216, so the dark mode CSS is to "blame," even though that change itself looks good; the problem had just been latent before.

The very first thing I tried was moving void* customPropertyDataRefs[1] in GreaterThanOrSameSizeAsStyleRareInheritedData from the bottom of the struct to up between TextDecorationThickness thickness and unsigned bitfields[4], which successfully increased the size of GreaterThanOrSameSizeAsStyleRareInheritedData to 224. Of course, this sort of defeats the goal of ensuring that StyleRareInheritedData remains small, but the logic behind the desired packing is quite unclear. Obviously it must be possible to reduce the size of StyleRareInheritedData down to 216 bytes by moving everything around, but this would reduce the readability of the struct, and since this isn't code I'm familiar with, I'd rather not touch that myself, so my patch will just increase the size of GreaterThanOrSameSizeAsStyleRareInheritedData to keep things simple.

After this change, both structs are 224 bytes on ARM. StyleRareInheritedData remains unchanged at 272 bytes on x86_64. But GreaterThanOrSameSizeAsStyleRareInheritedData increases to 280 bytes on x86_64. Not ideal, but again, I really don't understand how to keep the size down without reordering everything to be exactly the same order as GreaterThanOrSameSizeAsStyleRareInheritedData, and even then the order of the members of GreaterThanOrSameSizeAsStyleRareInheritedData seems pretty arbitrary. So other reviewers' opinions would be great here.

I *suspect* this change will allow us to remove this obsolete member from GreaterThanOrSameSizeAsStyleRareInheritedData:

#if PLATFORM(IOS_FAMILY)
    Color compositionColor; // FIXME: this has gone.
#endif

which is probably currently needed to prevent iOS from encountering this build failure. But of course, no way to know without first trying it on EWS.

[Michael Catanzaro]

Created attachment 371591 [details]
Patch

[Michael Catanzaro]

Ping reviewers

I'll push unreviewed if nobody wants to review it, since this is an important build fix. But a review would be great.

[Michael Catanzaro]

Comment on attachment 371591 [details]
Patch

OK then, thanks Timothy!

[WebKit Commit Bot]

Comment on attachment 371591 [details]
Patch

Clearing flags on attachment: 371591

Committed r246286: <https://trac.webkit.org/changeset/246286>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.