Bug 259525
| Summary: | REGRESSION(266186@main): Broke cloop build on all architectures | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> |
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | daniel_liu4, mcatanzaro |
| Priority: | P2 | ||
| Version: | WebKit Nightly Build | ||
| Hardware: | PC | ||
| OS: | Linux | ||
Michael Catanzaro
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
$ 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
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
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
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
Confirmed fixed on all three architectures. Thank you!