Bug 235401 – REGRESSION(r288200): [GTK][WPE] Build broken on Ubuntu 18.04 and Debian 10

• Component: New Bugs
• Status: RESOLVED FIXED

[Diego Pino]

Build broke on Ubuntu 18.04 after r288200. Stacktrace:

/usr/lib/ccache/g++-8  -DBUILDING_GTK__=1 -DBUILDING_LLIntOffsetsExtractor -DBUILDING_WITH_CMAKE=1 -DGETTEXT_PACKAGE=\"WebKit2GTK-4.0\" -DHAVE_CONFIG_H=1 -DJSC_COMPILATION -DJSC_GLIB_API_ENABLED -DPAS_BMALLOC=1 -DSTATICALLY_LINKED_WITH_WTF -DSTATICALLY_LINKED_WITH_bmalloc -DWEBKITGTK_API_VERSION_STRING=\"4.0\" -IJavaScriptCore/Headers -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I. -I../../Source/JavaScriptCore -I../../Source/JavaScriptCore/API -I../../Source/JavaScriptCore/assembler -I../../Source/JavaScriptCore/b3 -I../../Source/JavaScriptCore/b3/air -I../../Source/JavaScriptCore/bindings -I../../Source/JavaScriptCore/builtins -I../../Source/JavaScriptCore/bytecode -I../../Source/JavaScriptCore/bytecompiler -I../../Source/JavaScriptCore/dfg -I../../Source/JavaScriptCore/disassembler -I../../Source/JavaScriptCore/disassembler/ARM64 -I../../Source/JavaScriptCore/disassembler/zydis/Zydis -I../../Source/JavaScriptCore/domjit -I../../Source/JavaScriptCore/ftl -I../../Source/JavaScriptCore/heap -I../../Source/JavaScriptCore/debugger -I../../Source/JavaScriptCore/inspector -I../../Source/JavaScriptCore/inspector/agents -I../../Source/JavaScriptCore/inspector/augmentable -I../../Source/JavaScriptCore/inspector/remote -I../../Source/JavaScriptCore/interpreter -I../../Source/JavaScriptCore/jit -I../../Source/JavaScriptCore/llint -I../../Source/JavaScriptCore/parser -I../../Source/JavaScriptCore/profiler -I../../Source/JavaScriptCore/runtime -I../../Source/JavaScriptCore/tools -I../../Source/JavaScriptCore/wasm -I../../Source/JavaScriptCore/wasm/js -I../../Source/JavaScriptCore/yarr -IJavaScriptCore/DerivedSources -IJavaScriptCore/DerivedSources/inspector -IJavaScriptCore/DerivedSources/runtime -IJavaScriptCore/DerivedSources/yarr -I../../Source/JavaScriptCore/API/glib -IJavaScriptCoreGLib/DerivedSources/jsc -IJavaScriptCoreGLib/Headers -I../../Source/JavaScriptCore/inspector/remote/glib -IWTF/Headers -Ibmalloc/Headers -fdiagnostics-color=always -Wextra -Wall -Wno-expansion-to-defined -Wno-noexcept-type -Wno-psabi -Wno-misleading-indentation -Wno-maybe-uninitialized -Wwrite-strings -Wundef -Wpointer-arith -Wmissing-format-attribute -Wformat-security -Wcast-align -Wno-tautological-compare  -fno-strict-aliasing -fno-exceptions -fno-rtti -O3 -DNDEBUG -fPIE -fvisibility=hidden   -std=c++2a -MD -MT Source/JavaScriptCore/CMakeFiles/LLIntOffsetsExtractor.dir/llint/LLIntOffsetsExtractor.cpp.o -MF Source/JavaScriptCore/CMakeFiles/LLIntOffsetsExtractor.dir/llint/LLIntOffsetsExtractor.cpp.o.d -o Source/JavaScriptCore/CMakeFiles/LLIntOffsetsExtractor.dir/llint/LLIntOffsetsExtractor.cpp.o -c ../../Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp
In file included from WTF/Headers/wtf/StdLibExtras.h:33,
                 from WTF/Headers/wtf/FastMalloc.h:26,
                 from ../../Source/JavaScriptCore/config.h:38,
                 from ../../Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:26:
../../Source/JavaScriptCore/wasm/WasmCompilationMode.h: In function ‘constexpr bool JSC::Wasm::isOSREntry(JSC::Wasm::CompilationMode)’:
WTF/Headers/wtf/Assertions.h:283:49: error: call to non-‘constexpr’ function ‘void WTFCrash()’
 #define CRASH_UNDER_CONSTEXPR_CONTEXT() WTFCrash()
                                         ~~~~~~~~^~
WTF/Headers/wtf/Assertions.h:651:9: note: in expansion of macro ‘CRASH_UNDER_CONSTEXPR_CONTEXT’
         CRASH_UNDER_CONSTEXPR_CONTEXT(); \
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Source/JavaScriptCore/wasm/WasmCompilationMode.h:53:5: note: in expansion of macro ‘RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT’
     RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT(false);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Source/JavaScriptCore/wasm/WasmCompilationMode.h: In function ‘constexpr bool JSC::Wasm::isAnyBBQ(JSC::Wasm::CompilationMode)’:
WTF/Headers/wtf/Assertions.h:283:49: error: call to non-‘constexpr’ function ‘void WTFCrash()’
 #define CRASH_UNDER_CONSTEXPR_CONTEXT() WTFCrash()
                                         ~~~~~~~~^~
WTF/Headers/wtf/Assertions.h:651:9: note: in expansion of macro ‘CRASH_UNDER_CONSTEXPR_CONTEXT’
         CRASH_UNDER_CONSTEXPR_CONTEXT(); \
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Source/JavaScriptCore/wasm/WasmCompilationMode.h:68:5: note: in expansion of macro ‘RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT’
     RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT(false);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Source/JavaScriptCore/wasm/WasmCompilationMode.h: In function ‘constexpr bool JSC::Wasm::isAnyOMG(JSC::Wasm::CompilationMode)’:
WTF/Headers/wtf/Assertions.h:283:49: error: call to non-‘constexpr’ function ‘void WTFCrash()’
 #define CRASH_UNDER_CONSTEXPR_CONTEXT() WTFCrash()
                                         ~~~~~~~~^~
WTF/Headers/wtf/Assertions.h:651:9: note: in expansion of macro ‘CRASH_UNDER_CONSTEXPR_CONTEXT’
         CRASH_UNDER_CONSTEXPR_CONTEXT(); \
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Source/JavaScriptCore/wasm/WasmCompilationMode.h:83:5: note: in expansion of macro ‘RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT’
     RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT(false);

[Diego Pino]

Created attachment 449570 [details]
Patch

[Michael Catanzaro]

Comment on attachment 449570 [details]
Patch

So RELEASE_ASSERT_UNDER_CONSTEXPR_CONTEXT doesn't actually work. Sigh.

Please use IGNORE_RETURN_TYPE_WARNINGS_BEGIN/END then, to avoid reintroducing -Wreturn-type.

[Michael Catanzaro]

Actually, maybe we can fix it.

[Michael Catanzaro]

Created attachment 449574 [details]
Patch

[Michael Catanzaro]

I think this will work, but I haven't tested it on Ubuntu 18.04.

If it doesn't work, then IGNORE_RETURN_TYPE_WARNINGS_BEGIN/END is the easy solution.

[Darin Adler]

CRASH_UNDER_CONSTEXPR_CONTEXT needs to be fixed to work on Ubuntu; if not to fix this bug, in the future.

[Michael Catanzaro]

Note it's only a problem for old compiler versions. (Normal RELEASE_ASSERT() seems to work perfectly fine under constexpr already, so there doesn't seem to be a need for the _UNDER_CONSTEXPR_CONTEXT versions anymore with newer GCC, unless Clang doesn't like it.)

[Diego Pino]

I tested Michael's patch on Ubuntu 18.04 and it builds fine.

[Diego Pino]

It seems this build error is also happening in Debian 10. Debian 10 uses GCC8.3.

https://build.webkit.org/#/builders/46

[Michael Catanzaro]

Since this is a build break, I will land it unreviewed.

[Michael Catanzaro]

Actually... a change in fundamental macros like this should be approved by one other reviewer. I will ping some reviewers.

[Adrian Perez]

Comment on attachment 449574 [details]
Patch

It looks like making the constexpr-crash version of the macros expand
to the corresponding implementation (as done in your patch) should have
been part of the fix for bug #186536 but we missed the change there =)

[EWS]

Committed r288459 (246345@main): <https://commits.webkit.org/246345@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449574 [details].

[Radar WebKit Bug Importer]

<rdar://problem/87983893>