JSC backports for 2.46 - #1703
Open
justinmichaud wants to merge 22 commits into
Open
Conversation
https://bugs.webkit.org/show_bug.cgi?id=288081 Reviewed by Yusuke Suzuki and Justin Michaud. Properly allocate a scratch FPR instead of a GPR pair for a scratch float value. * Source/JavaScriptCore/wasm/WasmBBQJIT32_64.cpp: (JSC::Wasm::BBQJITImpl::BBQJIT::addF64Copysign): Canonical link: https://commits.webkit.org/291216@main
https://bugs.webkit.org/show_bug.cgi?id=288574 Reviewed by Michael Catanzaro. This change was tested with a JSCOnly build in an armhf webkit-container-sdk container on an ARM64 Thelio Astra workstation. * Source/cmake/WebKitCommon.cmake: * Tools/Scripts/webkitdirs.pm: (generateBuildSystemFromCMakeProject): Canonical link: https://commits.webkit.org/291253@main
https://bugs.webkit.org/show_bug.cgi?id=282490 Reviewed by Yusuke Suzuki and Justin Michaud. Contrary to the -O1 and -O2 register allocators, AirAllocateRegistersAndStackAndGenerateCode.cpp calls lowerStackArgs() first and then does register allocation and code generation in a single pass. In that file, callFrameAddr uses the extendedOffsetAddrRegister() as a scratch register in order to access FP/SP offsets that are too wide for an immediate offset. However, extendedOffsetAddrRegister() may be live, since it's also used by lowerStackArgs(). This trivially crashes testCallFunctionWithHellaArguments3 on ARMv7, but a slightly modified version of this test will also crash on ARM64. Specifically, this commit modifies testCallFunctionWithHellaArguments3 to use a number of constant values that are not representable as immediates on ARM64 (otherwise they become Imm Air::Args), resulting in a crashing test at O0. This doesn't affect the other register allocators, as they spill registers by means of Arg::stack() and only later lowerStackArgs() to handle these. We fix this by naively spilling (and immediately reloading) extendedOffsetAddrRegister() whenever we need to use it to access a wide offset in callFrameAddr(). With the fix in place, this patch switches ARMv7 to use the same code paths as ARM64 for handling Air::Args with a wide offset. * Source/JavaScriptCore/b3/B3Common.cpp: (JSC::B3::extendedOffsetAddrRegister): * Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp: (JSC::B3::Air::GenerateAndAllocateRegisters::callFrameAddr): (JSC::B3::Air::GenerateAndAllocateRegisters::withCallFrameAddr): (JSC::B3::Air::GenerateAndAllocateRegisters::flush): (JSC::B3::Air::GenerateAndAllocateRegisters::alloc): (JSC::B3::Air::GenerateAndAllocateRegisters::prepareForGeneration): (JSC::B3::Air::GenerateAndAllocateRegisters::generate): (JSC::B3::Air::callFrameAddr): Deleted. * Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.h: * Source/JavaScriptCore/b3/air/AirCode.cpp: (JSC::B3::Air::Code::Code): * Source/JavaScriptCore/b3/air/AirLowerStackArgs.cpp: (JSC::B3::Air::lowerStackArgs): * Source/JavaScriptCore/b3/testb3_5.cpp: (JSC_DEFINE_NOEXCEPT_JIT_OPERATION): (testCallFunctionWithHellaArguments3): Canonical link: https://commits.webkit.org/291111@main
https://bugs.webkit.org/show_bug.cgi?id=289157 Reviewed by Yusuke Suzuki. Avoids integer overflow when multiplying by ramSize. * Source/JavaScriptCore/heap/CompleteSubspace.cpp: (JSC::CompleteSubspace::tryAllocateSlow): Canonical link: https://commits.webkit.org/291887@main
https://bugs.webkit.org/show_bug.cgi?id=293629 Reviewed by Yusuke Suzuki. It is super difficult to track alignment when branch compaction might shift instructions by 2 bytes. This makes it easy to tear writes to the jit region, so let's just skip it for now in those cases. * Source/JavaScriptCore/assembler/ARMv7Assembler.h: (JSC::ARMv7Assembler::computeJumpType): Canonical link: https://commits.webkit.org/295468@main
https://bugs.webkit.org/show_bug.cgi?id=295498 Reviewed by Justin Michaud. WASM BBQJIT on 32-bit uses wasmScratchGPR when loading F32/F64 constants, causing segmentation faults in JetStream3's tfjs-wasm. The issue occurs when emitCheckAndPrepareAndMaterializePointerApply stores a memory address in wasmScratchGPR, then passes it to a functor. If the functor loads a floating-point constant, it overwrites wasmScratchGPR, corrupting the address and causing null pointer access. This patch fixes by using MacroAssembler's move32ToFloat and move64ToDouble directly with immediate values, eliminating the use of wasmScratchGPR. * Source/JavaScriptCore/wasm/WasmBBQJIT32_64.cpp: (JSC::Wasm::BBQJITImpl::BBQJIT::emitMoveConst): Canonical link: https://commits.webkit.org/297169@main
https://bugs.webkit.org/show_bug.cgi?id=295724 Reviewed by Sosuke Suzuki. On ARMv7, we cannot store atomically to unaligned addresses. On other platforms, this patch should be a no-op, but it aligns some patchable places for ARMv7. A follow-up patch will change the way we do patching. Canonical link: https://commits.webkit.org/297343@main
https://bugs.webkit.org/show_bug.cgi?id=296221 Reviewed by Sosuke Suzuki. This should be a no-op on other platforms, but it makes sure extra padding won't confuse the repatching logic. Canonical link: https://commits.webkit.org/297678@main
https://bugs.webkit.org/show_bug.cgi?id=298046 Reviewed by Justin Michaud. The program import-mutable-global.js generates the following in armv7: 1. if shift is zero jump to end 2. if shift is less than 32 do: 2.1 spill r0, r1 to use as scratch 2.2 do the shift ... 4. end however, if we take shift is zero and we jump to end, BBQ still thinks we did the spill and reloads r0, r1, loading garbage. This PR fixes the issue by doing the spill as soon as we enter the function, so we always reload the right values. * Source/JavaScriptCore/wasm/WasmBBQJIT32_64.cpp: (JSC::Wasm::BBQJITImpl::BBQJIT::shiftI64Helper): Canonical link: https://commits.webkit.org/299328@main
https://bugs.webkit.org/show_bug.cgi?id=300217 Reviewed by Adrian Perez de Castro. Add "-mthumb" flag to GCC to enable compiling thumb2 instruction set if supported. * Source/cmake/OptionsCommon.cmake: Canonical link: https://commits.webkit.org/301124@main
https://bugs.webkit.org/show_bug.cgi?id=300509 Reviewed by Justin Michaud. In shiftI64Helper, we would check if the shift amount was zero and just jump to the end, without moving the lhs to the result. No new tests, but this was found when analysing the crash in JetStream3's tfjs-wasm.js. 32-bit JSC still crashes when running that test but this gets us a little bit further. * Source/JavaScriptCore/wasm/WasmBBQJIT32_64.cpp: (JSC::Wasm::BBQJITImpl::BBQJIT::shiftI64Helper): Canonical link: https://commits.webkit.org/301475@main
https://bugs.webkit.org/show_bug.cgi?id=301931 Reviewed by Justin Michaud. When running JS3 I noticed the following code: [ 0x22d] I64Or 0xf1b953fa: orr.w r1, r2, r4 0xf1b953fe: orr.w r0, r1, r3 Here, lhs is (r1, r2), rhs is (r3, r4) and result is (r0, r1), so when we write to resultHi (r1), we override the value in lhs, then read it again in the second orr, leading to a wrong result. This patch implements i64or in the ARMv7 backend and fixes this problem by calculating the low or to a scratch, calculating hi to the destination, then moving the low or from the scratch to the destination. * Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h: (JSC::MacroAssemblerARMv7::or64): * Source/JavaScriptCore/wasm/WasmBBQJIT32_64.cpp: (JSC::Wasm::BBQJITImpl::BBQJIT::addI64Or): Canonical link: https://commits.webkit.org/302862@main
… is also a destination https://bugs.webkit.org/show_bug.cgi?id=302469 Reviewed by Justin Michaud. This patch expands a ldrd into a pair of ldr if any of the destination registers is also the register holding the memory address. Before: 0xf1502462: ldrd r0, r1, [r0] After: 0xf1502462: ldr r1, [r0, WebPlatformForEmbedded#4] 0xf1502464: ldr r0, [r0] * Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h: (JSC::MacroAssemblerARMv7::loadPair32): Canonical link: https://commits.webkit.org/303043@main
…n loading cells. https://bugs.webkit.org/show_bug.cgi?id=305427 Reviewed by Yusuke Suzuki. Patch by @emutavchi downstream: WebPlatformForEmbedded#1593. We find ourselves in a state where canLoad says yes, because our JSValue can be loaded via FPR, but we choose to load it via GPR instead because we do not have two gprs free. This inconsistency causes an assertion failure, and incorrect tail call shuffling. This can only happen if loadsIntoFPR is true, but loadsIntoGPR is false. In addition, everything DisplacedInJSStack must either have loadsIntoFPR or loadsIntoGPR, so no other corner cases need to be considered. Canonical link: https://commits.webkit.org/305572@main
https://bugs.webkit.org/show_bug.cgi?id=288554 rdar://144072285 Reviewed by Yusuke Suzuki. CodeBlock's StubInfo can keep watchpoints alive which do not ref data they hold, meaning that when the CodeBlock is jettisoned, we may have dangling pointers to data that was freed during a GC. * Source/JavaScriptCore/bytecode/CodeBlock.cpp: (JSC::CodeBlock::jettison): Originally-landed-as: 289651.190@safari-7621-branch (15053072f223). rdar://148056646 Canonical link: https://commits.webkit.org/293230@main
https://bugs.webkit.org/show_bug.cgi?id=287567 rdar://144076957 Reviewed by Yijia Huang and Yusuke Suzuki. GetByIdModeMetadata does not reset its structure ID upon transitioning to ArrayLengthMode, meaning that this could get held over across another reset. This could later cause access to a freed (or reallocated) structure ID. This patch also cleans up the code in `setProtoLoadMode` slightly to clear up the specific writes being performed. This does not apply to other modes, since they set their structure ID explicitly. Since the mutator thread is the thread changing modes, it will only invalidate the structure ID after the call to clearToDefaultModeWithoutCache finishes, meaning that the structure ID already is cleared. * Source/JavaScriptCore/bytecode/GetByIdMetadata.h: (JSC::GetByIdModeMetadata::clearToDefaultModeWithoutCache): (JSC::GetByIdModeMetadata::setUnsetMode): (JSC::GetByIdModeMetadata::setArrayLengthMode): (JSC::GetByIdModeMetadata::setProtoLoadMode): * Source/JavaScriptCore/bytecode/GetByStatus.cpp: (JSC::GetByStatus::computeFromLLInt): Originally-landed-as: 289651.165@safari-7621-branch (a93595279e2b). rdar://148058163 Canonical link: https://commits.webkit.org/293257@main
https://bugs.webkit.org/show_bug.cgi?id=294420 rdar://153246940 Reviewed by Keith Miller. Let's make CodeBlock jettisoning a bit safer. Regardless of the type of CodeBlock, when it is getting jettisoning, ensuring unlinking it from CallLinkInfo is fine. * Source/JavaScriptCore/bytecode/CodeBlock.cpp: (JSC::CodeBlock::jettison): Canonical link: https://commits.webkit.org/296207@main
…g on 32 bits. https://bugs.webkit.org/show_bug.cgi?id=293720 Reviewed by Yusuke Suzuki. On 32-bit armv7/linux we run into an issue where the environment strings located at the bottom of the stack, as well as some random bits inside the libc portion of the stack are interpreted as cells pointing into the heap, keeping actually dead objects alive. Yusuke previously attempted to fix this by excluding environ and before, but there are additional fake roots only a few hundred bytes below that. This patch excludes the caller of main entirely. * Source/JavaScriptCore/jsc.cpp: (jscmain): * Source/JavaScriptCore/runtime/JSLock.cpp: (JSC::JSLock::didAcquireLock): * Source/WTF/wtf/StackBounds.cpp: (WTF::StackBounds::setBottomOfMainThreadMain): (WTF::StackBounds::currentThreadStackBoundsInternal): * Source/WTF/wtf/StackBounds.h: (WTF::StackBounds::checkConsistency const): * Source/WebKit/WebProcess/gtk/WebProcessMainGtk.cpp: (WebKit::WebProcessMain): * Source/WebKit/WebProcess/wpe/WebProcessMainWPE.cpp: (WebKit::WebProcessMain):
… pointer https://bugs.webkit.org/show_bug.cgi?id=289017 rdar://145852436 Reviewed by Mark Lam. On environment having 32bit pointer or non-little-endian, mode needs to be set. * Source/JavaScriptCore/bytecode/GetByIdMetadata.h: (JSC::GetByIdModeMetadata::setProtoLoadMode): Originally-landed-as: 289651.211@safari-7621-branch (38a19eb3b7c0). rdar://148052940 Canonical link: https://commits.webkit.org/293387@main
https://bugs.webkit.org/show_bug.cgi?id=288083 Cherry-pick: resolved conflict by keeping existing logic, but changing register. Reviewed by Yusuke Suzuki and Justin Michaud. On armv7 certain uses of branchPtr cause it to clobber its own arguments, leading to bogus assembly. Catch this pitfall at least in debug builds and fix instances of this bug. * Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h: (JSC::MacroAssemblerARMv7::branch32): * Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp: (JSC::InlineCacheCompiler::compile): * Source/JavaScriptCore/wasm/WasmBBQJIT.cpp: (JSC::Wasm::BBQJITImpl::BBQJIT::addLoopOSREntrypoint): Canonical link: https://commits.webkit.org/292608@main
https://bugs.webkit.org/show_bug.cgi?id=294597 Reviewed by Justin Michaud. Cherry-pick: skipped the no-jit trampolines since they won't work anyway on armv7. ARMv7 can't rely on BUILTIN_FRAME_ADDRESS (it's broken on GCC). Properly initialize temporaryCallFrame in WebAssembly.asm. This should really be `or (not USE_BUILTIN_FRAME_ADDRESS)`, but we need to keep the number of configuration values down to avoid a combinatorial explosion in the size of LLIntAssembly.h. When we have a fix for GCC, we can just drop the reliance on temporaryCallFrame, but in the meantime this avoids flaky tests in EWS. * Source/JavaScriptCore/llint/WebAssembly.asm: Canonical link: https://commits.webkit.org/298095@main
…types in hot functions. https://bugs.webkit.org/show_bug.cgi?id=293113 rdar://151464447 Reviewed by Daniel Liu and Keith Miller. 274558@main changed rtt field of WasmToWasmImportableFunction from CompactPtr to normal pointer, but it didn't change OMG code generator's loadCompactPtr. So we are materializing a pointer incorrectly. This patch removes loadCompactPtr and use normal Load for OMG. * JSTests/wasm/stress/resources/wasm-gc-ref-cast.wasm: Added. * JSTests/wasm/stress/resources/wasm-gc-ref-cast.wat: Added. * JSTests/wasm/stress/wasm-gc-ref-cast.mjs: Added. * Source/JavaScriptCore/wasm/WasmFormat.h: * Source/JavaScriptCore/wasm/WasmOMGIRGenerator.cpp: (JSC::Wasm::OMGIRGenerator::emitLoadRTTFromFuncref): * Source/JavaScriptCore/wasm/WasmOMGIRGenerator32_64.cpp: (JSC::Wasm::OMGIRGenerator::emitLoadRTTFromFuncref): Canonical link: https://commits.webkit.org/295902@main
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
These are direct backports from ToT or 2.38 that can be applied directly to the tree with very little risk.
d23d085