Error: ``` FAILED: Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/__/__/DerivedSources/JavaScriptCore/unified-sources/UnifiedSource-bfc896e1-11.cpp.o /home/buildbot/buildroot/buildroot/output.rpi3/host/usr/ccache/arm-buildroot-linux-gnueabihf-g++ --sysroot=/home/buildbot/buildroot/buildroot/output.rpi3/host/usr/arm-buildroot-linux-gnueabihf/sysroot -DBUILDING_JSCONLY__ -DBUILDING_JavaScriptCore -DBUILDING_WITH_CMAKE=1 -DHAVE_CONFIG_H=1 -DJavaScriptCore_EXPORTS -DSTATICALLY_LINKED_WITH_WTF -IDerivedSources/ForwardingHeaders -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/udis86 -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 -IDerivedSources/JavaScriptCore -IDerivedSources/JavaScriptCore/inspector -IDerivedSources/JavaScriptCore/runtime -IDerivedSources/JavaScriptCore/yarr -I../../Source/ThirdParty/capstone/Source/include -IDerivedSources -I../../Source/ThirdParty -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 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -O3 -fno-strict-aliasing -fno-exceptions -fno-rtti -DNDEBUG -fPIC -ffp-contract=off -std=c++17 -MD -MT Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/__/__/DerivedSources/JavaScriptCore/unified-sources/UnifiedSource-bfc896e1-11.cpp.o -MF Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/__/__/DerivedSources/JavaScriptCore/unified-sources/UnifiedSource-bfc896e1-11.cpp.o.d -o Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/__/__/DerivedSources/JavaScriptCore/unified-sources/UnifiedSource-bfc896e1-11.cpp.o -c DerivedSources/JavaScriptCore/unified-sources/UnifiedSource-bfc896e1-11.cpp In file included from DerivedSources/JavaScriptCore/unified-sources/UnifiedSource-bfc896e1-11.cpp:5: ../../Source/JavaScriptCore/dfg/DFGOSRExit.cpp: In function ‘void JSC::DFG::reifyInlinedCallFrames(JSC::DFG::Context&, JSC::CodeBlock*, const JSC::DFG::OSRExitBase&)’: ../../Source/JavaScriptCore/dfg/DFGOSRExit.cpp:828:58: error: no matching function for call to ‘JSC::CallSiteIndex::CallSiteIndex(const JSC::Instruction*&)’ uint32_t locationBits = CallSiteIndex(instruction).bits(); ``` it is related with JIT code because WPE still builds with `-DENABLE_JIT=OFF` the fail was introduced in `BytecodeIndex should be a proper C++ class` https://trac.webkit.org/changeset/251468
Created attachment 381671 [details] patch
Created attachment 381672 [details] patch
This https://trac.webkit.org/changeset/251468/webkit#file83 is still needed. I just uploaded a patch adapting the code to the changes done in r251468.
Does this patch pass JSC tests on 32-bit targets? I think the i386 EWS bit is broken. So, we can’t rely on it to tell us.
(In reply to Mark Lam from comment #4) > Does this patch pass JSC tests on 32-bit targets? I think the i386 EWS bit > is broken. So, we can’t rely on it to tell us. I just tested in ARMv7 boards right now.
(In reply to Pablo Saavedra from comment #5) > (In reply to Mark Lam from comment #4) > > Does this patch pass JSC tests on 32-bit targets? I think the i386 EWS bit > > is broken. So, we can’t rely on it to tell us. > > I just tested in ARMv7 boards right now. Sorry, I mean, I tested build step ... not the JSC tests.
(In reply to Pablo Saavedra from comment #6) > (In reply to Pablo Saavedra from comment #5) > > (In reply to Mark Lam from comment #4) > > > Does this patch pass JSC tests on 32-bit targets? I think the i386 EWS bit > > > is broken. So, we can’t rely on it to tell us. > > > > I just tested in ARMv7 boards right now. > > > Sorry, I mean, I tested build step ... not the JSC tests. It is being a bit complicated to ensure anything because the r251425 (https://bugs.webkit.org/show_bug.cgi?id=202392#c41) was introduced so many failures the tests JSC tests
Created attachment 381729 [details] WIP fixes This one contains both compile-time fixes as well as a few runtime fixes that temporarily cover work in #203285 or bring back 32-bit specifics from r251468. This still leaves a good handful of tests failing, seemingly due to something in the DFG layer.
Created attachment 381734 [details] WIP fixes Another fix in CodeOrigin::isHashTableDeletedValue().
Comment on attachment 381672 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=381672&action=review > Source/JavaScriptCore/interpreter/CallFrame.h:55 > +#if USE(JSVALUE32_64) > + explicit CallSiteIndex(const Instruction* instruction) > + : m_bytecodeIndex(bitwise_cast<uint32_t>(instruction)) > + { } > +#endif I would like to get the refactoring that removes 32 / 64bit difference here: Use 64bit's logic and remove USE(JSVALUE32_64) around CallSiteIndex. Can you do that? Soon, we will introduce another change and that change could make this 32bit logic not working.
Created attachment 381779 [details] patch
(In reply to Yusuke Suzuki from comment #10) > Comment on attachment 381672 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=381672&action=review > > > Source/JavaScriptCore/interpreter/CallFrame.h:55 > > +#if USE(JSVALUE32_64) > > + explicit CallSiteIndex(const Instruction* instruction) > > + : m_bytecodeIndex(bitwise_cast<uint32_t>(instruction)) > > + { } > > +#endif > > I would like to get the refactoring that removes 32 / 64bit difference here: > Use 64bit's logic and remove USE(JSVALUE32_64) around CallSiteIndex. > Can you do that? > Soon, we will introduce another change and that change could make this 32bit > logic not working. We can do manual casting to BytecodeIndex where required.
Created attachment 381789 [details] Pending fixes This is a list of logic fixes that fix the remaining test failures (all but one). I'll polish this into a patch in a bit.
Created attachment 381796 [details] Patch
Comment on attachment 381796 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381796&action=review > Source/JavaScriptCore/bytecode/BytecodeIndex.h:42 > + BytecodeIndex(WTF::HashTableDeletedValueType) > + : m_offset(deletedValue().asBits()) > + { > + } This missing constructor was the main problem of the remaining issues. Without it, the BytecodeIndex(WTF::HashTableDeletedValue) construction defaulted to the uin32_t constructor. With WTF::HashTableDeletedValue being effectively 0, this ended up constructing zero-valued BytecodeIndex objects that on their own are completely valid, non-empty and non-deleted.
View in context: https://bugs.webkit.org/attachment.cgi?id=381796&action=review > Source/JavaScriptCore/bytecode/BytecodeIndex.h:42 > + BytecodeIndex(WTF::HashTableDeletedValueType) > + : m_offset(deletedValue().asBits()) > + { > + } This missing constructor was the main problem of the remaining issues. Without it, the BytecodeIndex(WTF::HashTableDeletedValue) construction defaulted to the uin32_t constructor. With WTF::HashTableDeletedValue being effectively 0, this ended up constructing zero-valued BytecodeIndex objects that on their own are completely valid, non-empty and non-deleted.
On 32-bit ARMv7, the patch puts into working order all the tests except one ChakraCore test. That failure is due to the BytecodeIndex rework, and apparently originates from the DFG layer, but it's still unclear why it occurs.
Comment on attachment 381796 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381796&action=review r=me weird that this worked on 64 bit... since I would have guessed this would break a lot of things... >> Source/JavaScriptCore/bytecode/BytecodeIndex.h:42 >> + } > > This missing constructor was the main problem of the remaining issues. Without it, the BytecodeIndex(WTF::HashTableDeletedValue) construction defaulted to the uin32_t constructor. With WTF::HashTableDeletedValue being effectively 0, this ended up constructing zero-valued BytecodeIndex objects that on their own are completely valid, non-empty and non-deleted. Ugh, we should make WTF::HashTableDeletedValueType be a enum class...
Comment on attachment 381796 [details] Patch Clearing flags on attachment: 381796 Committed r251583: <https://trac.webkit.org/changeset/251583>
All reviewed patches have been landed. Closing bug.
<rdar://problem/56608497>