| 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 | ||
|
Description
Michael Catanzaro
2023-07-26 11:03:54 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. :/
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.
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. 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! Confirmed fixed on all three architectures. Thank you! |