Skip to content

Refactor jitMemcpy into atomic and non-atomic versions #45969

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

justinmichaud
Copy link
Contributor

@justinmichaud justinmichaud commented May 27, 2025

cbfdbee

Refactor jitMemcpy into atomic and non-atomic versions
https://bugs.webkit.org/show_bug.cgi?id=293649

Reviewed by NOBODY (OOPS!).

On ARMv7, there are some concurrent repatching issues because of
unaligned writes to the jit region. Instead of repatching whenever it
is easy, we should be explicit about when we expect writes to the jit
region to tear or not.

We also add a new option to test this that forces tears in the non-atomic
version.

Finally, we increase the alignment on armv7 for patchable jumps.

* Source/JavaScriptCore/assembler/ARM64Assembler.h:
* Source/JavaScriptCore/assembler/ARMv7Assembler.h:
(JSC::ARMv7Assembler::revertJumpTo_movT3movtcmpT2):
(JSC::ARMv7Assembler::revertJumpTo_movT3):
(JSC::ARMv7Assembler::linkPointer):
(JSC::ARMv7Assembler::relinkCall):
(JSC::ARMv7Assembler::repatchPointer):
(JSC::ARMv7Assembler::patchableJumpSize):
(JSC::ARMv7Assembler::setInt32):
(JSC::ARMv7Assembler::setUInt7ForLoad):
(JSC::ARMv7Assembler::setPointerAtomic):
(JSC::ARMv7Assembler::setPointer):
* Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:
(JSC::AbstractMacroAssembler::padBeforePatch):
* Source/JavaScriptCore/assembler/AssemblerCommon.h:
(JSC::memcpyAtomic):
(JSC::memcpyNonAtomic):
(JSC::machineCodeCopy):
(JSC::memcpyAtomicIfPossible): Deleted.
* Source/JavaScriptCore/assembler/MacroAssemblerARM64.h:
(JSC::MacroAssemblerARM64::reemitInitialMoveWithPatch):
* Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:
(JSC::MacroAssemblerARMv7::threadSafePatchableNearCall):
(JSC::MacroAssemblerARMv7::threadSafePatchableNearTailCall):
* Source/JavaScriptCore/assembler/X86Assembler.h:
(JSC::X86Assembler::replaceWithHlt):
(JSC::X86Assembler::replaceWithJump):
(JSC::X86Assembler::revertJumpTo_movq_i64r):
(JSC::X86Assembler::revertJumpTo_movl_i32r):
(JSC::X86Assembler::revertJumpTo_cmpl_ir_force32):
(JSC::X86Assembler::revertJumpTo_cmpl_im_force32):
(JSC::X86Assembler::setPointer):
(JSC::X86Assembler::setInt32):
(JSC::X86Assembler::setInt8):
* Source/JavaScriptCore/jit/ExecutableAllocator.h:
(JSC::jitMemcpyCheckForZeros):
(JSC::jitMemcpyChecks):
(JSC::performJITMemcpyImpl):
(JSC::performJITMemcpyAtomic):
(JSC::performJITMemcpy):
* Source/JavaScriptCore/runtime/OptionsList.h:

172b2e6

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ❌ 🛠 mac ❌ 🛠 wpe ❌ 🛠 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ❌ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ❌ 🧪 api-mac ❌ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ❌ 🛠 wpe-cairo
❌ 🛠 🧪 jsc ✅ 🧪 api-ios ❌ 🧪 mac-wk2 ❌ 🛠 gtk
❌ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ❌ 🧪 gtk-wk2
✅ 🛠 vision-sim ❌ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🧪 vision-wk2 ❌ 🧪 mac-intel-wk2 ❌ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ❌ 🧪 jsc-armv7-tests
❌ 🛠 watch
✅ 🛠 watch-sim

@justinmichaud justinmichaud requested a review from a team as a code owner May 27, 2025 22:19
@justinmichaud justinmichaud self-assigned this May 27, 2025
@justinmichaud justinmichaud added the JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. label May 27, 2025
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 27, 2025
@justinmichaud justinmichaud marked this pull request as draft May 29, 2025 02:09
https://bugs.webkit.org/show_bug.cgi?id=293649

Reviewed by NOBODY (OOPS!).

On ARMv7, there are some concurrent repatching issues because of
unaligned writes to the jit region. Instead of repatching whenever it
is easy, we should be explicit about when we expect writes to the jit
region to tear or not.

We also add a new option to test this that forces tears in the non-atomic
version.

Finally, we increase the alignment on armv7 for patchable jumps.
@justinmichaud justinmichaud force-pushed the eng/Refactor-jitMemcpy-into-atomic-and-non-atomic-versions branch from f04552a to 34e2af2 Compare June 24, 2025 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. merging-blocked Applied to prevent a change from being merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants