RESOLVED FIXED259525
REGRESSION(266186@main): Broke cloop build on all architectures 
https://bugs.webkit.org/show_bug.cgi?id=259525
Summary REGRESSION(266186@main): Broke cloop build on all architectures
Michael Catanzaro
Reported 2023-07-26 11:03:54 PDT
It seems 266186@main broke the cloop build on at least x86_64, ppc64le, and s390x. The generated LLIntAssembly.h is trying to use a bunch of declarations that don't exist: In file included from /home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:433: /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h: In static member function ‘static JSC::JSValue JSC::LLInt::CLoop::execute(JSC::OpcodeID, void*, JSC::VM*, JSC::ProtoCallFrame*, bool)’: /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40233:36: error: ‘ipint_unreachable_validate’ was not declared in this scope 40233 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_unreachable_validate) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40233:1: error: ‘OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL’ was not declared in this scope; did you mean ‘OFFLINE_ASM_GLOBAL_LABEL’? 40233 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_unreachable_validate) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | OFFLINE_ASM_GLOBAL_LABEL /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40238:36: error: ‘ipint_nop_validate’ was not declared in this scope 40238 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_nop_validate) | ^~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40243:36: error: ‘ipint_block_validate’ was not declared in this scope 40243 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_block_validate) | ^~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40248:36: error: ‘ipint_loop_validate’ was not declared in this scope 40248 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_loop_validate) | ^~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40253:36: error: ‘ipint_if_validate’ was not declared in this scope 40253 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_if_validate) | ^~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40258:36: error: ‘ipint_else_validate’ was not declared in this scope 40258 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_else_validate) | ^~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40268:36: error: ‘ipint_end_validate’ was not declared in this scope 40268 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_end_validate) | ^~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40273:36: error: ‘ipint_br_validate’ was not declared in this scope 40273 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_br_validate) | ^~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40278:36: error: ‘ipint_br_if_validate’ was not declared in this scope 40278 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_br_if_validate) | ^~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40283:36: error: ‘ipint_br_table_validate’ was not declared in this scope 40283 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_br_table_validate) | ^~~~~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40288:36: error: ‘ipint_return_validate’ was not declared in this scope 40288 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_return_validate) | ^~~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40293:36: error: ‘ipint_call_validate’ was not declared in this scope 40293 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_call_validate) | ^~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40298:36: error: ‘ipint_call_indirect_validate’ was not declared in this scope 40298 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_call_indirect_validate) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40311:36: error: ‘ipint_drop_validate’ was not declared in this scope 40311 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_drop_validate) | ^~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40316:36: error: ‘ipint_select_validate’ was not declared in this scope 40316 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_select_validate) | ^~~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40321:36: error: ‘ipint_select_t_validate’ was not declared in this scope 40321 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_select_t_validate) | ^~~~~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40329:36: error: ‘ipint_local_get_validate’ was not declared in this scope 40329 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_local_get_validate) | ^~~~~~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40334:36: error: ‘ipint_local_set_validate’ was not declared in this scope 40334 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_local_set_validate) | ^~~~~~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40339:36: error: ‘ipint_local_tee_validate’ was not declared in this scope 40339 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_local_tee_validate) | ^~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated due to -fmax-errors=20. I see that InPlaceInterpreter.h attempts to define these symbols only on aarch64 and x86_64, but the above build failure is actually on x86_64. (ppc64le and s390x fail in the same way.) I need to investigate more to figure out what's going wrong, but my guess is there are at least two different problems here: * Broken on x86_64 due to missing #include? It's failing when building LowLevelInterpreter.cpp which notably does not #include InPlaceInterpreter.h * Broken on other architecturers due to improper conditional compilation? Looks like the code attempts to use the declarations on more architectures than they are declared on.
Attachments
Add attachment proposed patch, testcase, etc.
Michael Catanzaro
Comment 1 2023-07-26 11:10:33 PDT
$ git diff diff --git a/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp b/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp index f6da744880f3..a5873b0b58ca 100644 --- a/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp +++ b/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp @@ -34,6 +34,7 @@ #include "CLoopStackInlines.h" #include "CodeBlock.h" #include "CommonSlowPaths.h" +#include "InPlaceInterpreter.h" #include "Interpreter.h" #include "LLIntCLoop.h" #include "LLIntData.h" This fixes the above failures on x86_64, but I'm sure it will still be broken on other architectures, so more is needed. However, there is another build failure that occurs next: In file included from /home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:434: /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h: In static member function ‘static JSC::JSValue JSC::LLInt::CLoop::execute(JSC::OpcodeID, void*, JSC::VM*, JSC::ProtoCallFrame*, bool)’: /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/JavaScriptCore/DerivedSources/LLIntAssembly.h:40233:1: error: ‘OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL’ was not declared in this scope; did you mean ‘OFFLINE_ASM_GLOBAL_LABEL’? 40233 | OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(ipint_unreachable_validate) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | OFFLINE_ASM_GLOBAL_LABEL This macro is new in 266186@main and is only defined for ARM thumb2 CPUs, but it's used unconditionally. :/
Michael Catanzaro
Comment 2 2023-07-26 11:59:51 PDT
I was able to get past that with this hack: diff --git a/Source/JavaScriptCore/offlineasm/asm.rb b/Source/JavaScriptCore/offlineasm/asm.rb index 34afcedc8449..4a38c695592f 100644 --- a/Source/JavaScriptCore/offlineasm/asm.rb +++ b/Source/JavaScriptCore/offlineasm/asm.rb @@ -239,11 +239,7 @@ class Assembler @internalComment = $enableLabelCountComments ? "Global Label #{@numGlobalLabels}" : nil if isGlobal if !$emitWinAsm - if isAligned - @outp.puts(formatDump("OFFLINE_ASM_GLOBAL_LABEL(#{labelName})", lastComment)) - else - @outp.puts(formatDump("OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL(#{labelName})", lastComment)) - end + @outp.puts(formatDump("OFFLINE_ASM_GLOBAL_LABEL(#{labelName})", lastComment)) else putsProc(labelName, lastComment) end which is of course not an acceptable solution, but I don't know the right way to fix it. I'm skeptical that this was only tested on ARM thumb2, but that's definitely the only place where OFFLINE_ASM_UNALIGNED_GLOBAL_LABEL is defined. Anyway, next problem is it doesn't link: /usr/bin/ld: Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/__/__/JavaScriptCore/DerivedSources/unified-sources/UnifiedSource-6e4525b9-1.cpp.o: in function `JSC::IPInt::initialize()': /home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/llint/InPlaceInterpreter.cpp:53: undefined reference to `ipint_unreachable_validate' /usr/bin/ld: /home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/llint/InPlaceInterpreter.cpp:53: undefined reference to `ipint_unreachable_validate' /usr/bin/ld: /home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/llint/InPlaceInterpreter.cpp:53: undefined reference to `ipint_unreachable_validate' /usr/bin/ld: /home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/llint/InPlaceInterpreter.cpp:53: undefined reference to `ipint_nop_validate' /usr/bin/ld: /home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/llint/InPlaceInterpreter.cpp:53: undefined reference to `ipint_unreachable_validate' There are a few hundred more undefined references; I just copied the top few. I don't know where there are supposed to be defined. Normally at this point I would do a revert to get the build working again, since this doesn't look straightforward to fix, but there are two more follow-up commits that depend on the original change, and it looks like Daniel is actively working on this, so I'll wait a couple days to give Daniel a chance to look first.
daniel_liu4
Comment 3 2023-07-26 14:41:29 PDT
Hi Michael, thanks for bringing this up! I didn't realize that this completely broke the cloop build, so my apologies for that. I'm currently testing on an ARM device, but compiling with `Tools/Scripts/build-jsc --cloop --debug WK_VALIDATE_DEPENDENCIES=YES ARCHS=x86_64`, I've been able to make some changes that should hopefully fix the build issues. I'll wrap it into my current in-place interpreter PR and see if that hopefully fixes it.
daniel_liu4
Comment 4 2023-07-27 18:33:53 PDT
Hi Michael! It looks like the cloop build works (based on build.webkit.org) as of [8e237a17b93c448615544a93160b779c8220f68d](https://github.com/WebKit/WebKit/commit/8e237a17b93c448615544a93160b779c8220f68d)! If you're still running into the same issue, please let me know!
Michael Catanzaro
Comment 5 2023-07-28 07:51:02 PDT
Confirmed fixed on all three architectures. Thank you!
Note You need to log in before you can comment on or make changes to this bug.