Skip to content

[git-webkit] Link git output capture to logging level#48

Draft
JonWBedard wants to merge 1 commit into
WebKit:mainfrom
JonWBedard:eng/output-capture-to-logging-level
Draft

[git-webkit] Link git output capture to logging level#48
JonWBedard wants to merge 1 commit into
WebKit:mainfrom
JonWBedard:eng/output-capture-to-logging-level

Conversation

@JonWBedard

Copy link
Copy Markdown
Member

d6b428f

[git-webkit] Link git output capture to logging level
https://bugs.webkit.org/show_bug.cgi?id=233637

Reviewed by NOBODY (OOPS!).

WIP

* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/setup.py:
(Setup.git):

https://bugs.webkit.org/show_bug.cgi?id=233637

Reviewed by NOBODY (OOPS!).

WIP

* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/setup.py:
(Setup.git):
@JonWBedard JonWBedard self-assigned this Nov 30, 2021
@JonWBedard
JonWBedard marked this pull request as draft November 30, 2021 15:53
@aj062

aj062 commented Feb 14, 2022

Copy link
Copy Markdown
Member

rs=me

@JonWBedard JonWBedard added merging-blocked Applied to prevent a change from being merged merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged merge-queue Applied to send a pull request to merge-queue labels Mar 1, 2022
@JonWBedard JonWBedard added merge-queue Applied to send a pull request to merge-queue and removed merge-queue Applied to send a pull request to merge-queue labels Mar 14, 2022
@JonWBedard JonWBedard added Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases WebKit Nightly Build labels Mar 29, 2022
webkit-commit-queue pushed a commit that referenced this pull request May 13, 2022
…arget labels.

https://bugs.webkit.org/show_bug.cgi?id=240370

Reviewed by Saam Barati.

Disassemblies used to look like this:

     0x10e480ff8:    ldurb    w17, [x0, #7]
     0x10e480ffc:    cmp      w17, #0
     0x10e481000:    b.hi   0x10e48103c
     0x10e481004:    stur     x0, [fp, #-72]
     ...
     0x10e481040:    movk     x3, #0xfffe, lsl #48
     0x10e481044:    b        0x10e4814f4
     0x10e481048:    nop

With this patch, it will now look like this:

       <748> 0x10e120aec:    ldurb    w17, [x0, #7]
       <752> 0x10e120af0:    cmp      w17, #0
       <756> 0x10e120af4:    b.hi     0x10e120b30 -> <816>
       <760> 0x10e120af8:    stur     x0, [fp, #-80]
       ...
       <820> 0x10e120b34:    movk     x3, #0xfffe, lsl #48
       <824> 0x10e120b38:    b        0x10e120fc8 -> <1992>
       <828> 0x10e120b3c:    nop

1. Each instruction pc is now prefixed with a pc index i.e. the offset of the
   pc address from the start of the compilation unit e.g. <756>.

2. Relative branches now show the branch target as a pc index (effectively, an
   internal label in this compilation unit) in addition to the pc address e.g.
   the "-> <816>" in:
       <756> 0x10e120af4:    b.hi     0x10e120b30 -> <816>

   Also fixed a formatting bug where the space between relative branch instructions
   and their target pc was short 2 spaces.

3. If the relative branch target is a known thunk, the disassembler will now
   print the thunk label e.g.

       <828> 0x10e12033c:    bl       0x10e0f0a00 -> <thunk: get_from_scope thunk>
      <1476> 0x10e120dc4:    cbnz     x16, 0x10e104100 -> <thunk: handleExceptionWithCallFrameRollback>
      <2368> 0x10e121140:    b        0x10e10c000 -> <thunk: DFG OSR exit generation thunk>

   Introduced a FINALIZE_THUNK macro that will be used instead of FINALIZE_CODE in
   thunk generators.  By doing so, thunk labels will automatically be registered
   with the disassembler, and will be used for the above look up.

   Thunk label registration is only done if disassembly is enabled.

4. If the branch target is neither an internal label nor a thunk, then the
   disassembler will print some useful info about it to the best of its
   knowledge e.g.

       <168> 0x10e1002e8:    b        0x10e120b60 -> <JIT PC>
       <168> 0x10e1002e8:    b        0x10e120b60 -> <LLInt PC>
       <168> 0x10e1002e8:    b        0x10e120b60 -> <unknown>

5. The disassemble() function now takes 2 additional arguments: codeStart, and
   codeEnd.  These are needed so that the disassembler can compute the pc index
   for each instruction, as well as determine if a branch target is internal to
   this compilation unit, or pointing out of it.

This feature is currently only supported for the ARM64 disassembler.

Printing of JIT operation labels (via movz + movk + indirect branch) is not yet
supported.

* assembler/LinkBuffer.cpp:
(JSC::LinkBuffer::finalizeCodeWithDisassemblyImpl):
* assembler/LinkBuffer.h:
(JSC::LinkBuffer::setIsThunk):
* b3/air/AirDisassembler.cpp:
(JSC::B3::Air::Disassembler::dump):
* dfg/DFGDisassembler.cpp:
(JSC::DFG::Disassembler::dumpDisassembly):
* dfg/DFGThunks.cpp:
(JSC::DFG::osrExitGenerationThunkGenerator):
(JSC::DFG::osrEntryThunkGenerator):
* disassembler/ARM64/A64DOpcode.cpp:
(JSC::ARM64Disassembler::A64DOpcode::appendPCRelativeOffset):
(JSC::ARM64Disassembler::A64DOpcodeConditionalBranchImmediate::format):
* disassembler/ARM64/A64DOpcode.h:
(JSC::ARM64Disassembler::A64DOpcode::A64DOpcode):
(JSC::ARM64Disassembler::A64DOpcode::appendPCRelativeOffset): Deleted.
* disassembler/ARM64Disassembler.cpp:
(JSC::tryToDisassemble):
* disassembler/CapstoneDisassembler.cpp:
(JSC::tryToDisassemble):
* disassembler/Disassembler.cpp:
(JSC::disassemble):
(JSC::disassembleAsynchronously):
(JSC::ensureThunkLabelMap):
(JSC::registerThunkLabel):
(JSC::labelForThunk):
* disassembler/Disassembler.h:
(JSC::tryToDisassemble):
* disassembler/RISCV64Disassembler.cpp:
(JSC::tryToDisassemble):
* disassembler/X86Disassembler.cpp:
(JSC::tryToDisassemble):
* ftl/FTLThunks.cpp:
(JSC::FTL::genericGenerationThunkGenerator):
(JSC::FTL::slowPathCallThunkGenerator):
* jit/JIT.cpp:
(JSC::JIT::consistencyCheckGenerator):
* jit/JITCall.cpp:
(JSC::JIT::returnFromBaselineGenerator):
* jit/JITDisassembler.cpp:
(JSC::JITDisassembler::dump):
(JSC::JITDisassembler::dumpDisassembly):
* jit/JITDisassembler.h:
* jit/JITOpcodes.cpp:
(JSC::JIT::valueIsFalseyGenerator):
(JSC::JIT::valueIsTruthyGenerator):
(JSC::JIT::op_throw_handlerGenerator):
(JSC::JIT::op_enter_handlerGenerator):
(JSC::JIT::op_check_traps_handlerGenerator):
* jit/JITPropertyAccess.cpp:
(JSC::JIT::slow_op_get_by_val_callSlowOperationThenCheckExceptionGenerator):
(JSC::JIT::slow_op_get_private_name_callSlowOperationThenCheckExceptionGenerator):
(JSC::JIT::slow_op_put_by_val_callSlowOperationThenCheckExceptionGenerator):
(JSC::JIT::slow_op_put_private_name_callSlowOperationThenCheckExceptionGenerator):
(JSC::JIT::slow_op_del_by_id_callSlowOperationThenCheckExceptionGenerator):
(JSC::JIT::slow_op_del_by_val_callSlowOperationThenCheckExceptionGenerator):
(JSC::JIT::slow_op_get_by_id_callSlowOperationThenCheckExceptionGenerator):
(JSC::JIT::slow_op_get_by_id_with_this_callSlowOperationThenCheckExceptionGenerator):
(JSC::JIT::slow_op_put_by_id_callSlowOperationThenCheckExceptionGenerator):
(JSC::JIT::generateOpResolveScopeThunk):
(JSC::JIT::slow_op_resolve_scopeGenerator):
(JSC::JIT::generateOpGetFromScopeThunk):
(JSC::JIT::slow_op_get_from_scopeGenerator):
(JSC::JIT::slow_op_put_to_scopeGenerator):
* jit/SlowPathCall.cpp:
(JSC::JITSlowPathCall::generateThunk):
* jit/SpecializedThunkJIT.h:
(JSC::SpecializedThunkJIT::finalize):
* jit/ThunkGenerator.h:
* jit/ThunkGenerators.cpp:
(JSC::handleExceptionGenerator):
(JSC::handleExceptionWithCallFrameRollbackGenerator):
(JSC::popThunkStackPreservesAndHandleExceptionGenerator):
(JSC::checkExceptionGenerator):
(JSC::throwExceptionFromCallSlowPathGenerator):
(JSC::linkCallThunkGenerator):
(JSC::linkPolymorphicCallThunkGenerator):
(JSC::virtualThunkFor):
(JSC::nativeForGenerator):
(JSC::arityFixupGenerator):
(JSC::unreachableGenerator):
(JSC::stringGetByValGenerator):
(JSC::boundFunctionCallGenerator):
(JSC::remoteFunctionCallGenerator):
* llint/LLIntThunks.cpp:
(JSC::LLInt::generateThunkWithJumpTo):
(JSC::LLInt::generateThunkWithJumpToPrologue):
(JSC::LLInt::generateThunkWithJumpToLLIntReturnPoint):
(JSC::LLInt::createJSGateThunk):
(JSC::LLInt::createWasmGateThunk):
(JSC::LLInt::createTailCallGate):
(JSC::LLInt::tagGateThunk):
(JSC::LLInt::untagGateThunk):
* yarr/YarrDisassembler.cpp:
(JSC::Yarr::YarrDisassembler::dump):
(JSC::Yarr::YarrDisassembler::dumpDisassembly):
* yarr/YarrDisassembler.h:

Canonical link: https://commits.webkit.org/250547@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294180 268f45cc-cd09-0410-ab3c-d52691b4dbfc
webkit-commit-queue pushed a commit that referenced this pull request May 17, 2022
…n constants.

https://bugs.webkit.org/show_bug.cgi?id=240443

Reviewed by Yusuke Suzuki.

Source/JavaScriptCore:

1. Added a Options::needDisassemblySupport() option.  This option is true if
   any of the JSC disassembly options are enabled.  This allows us to trivially
   check if we need to enable additional infrastructure (like those added in this
   patch) to support disassembly.

2. Refactor the Disassembler's thunkLabelMap into a more generic labelMap.  It
   can now map a pointer to a CString or a const char* label.

3. Enable JITOperationList infrastructure when ENABLE(JIT_OPERATION_DISASSEMBLY)
   is true.  This adds a name to the JITOperationAnnotation record.  Since this is
   guarded by ENABLE(JIT_OPERATION_DISASSEMBLY), we can trivially turn this part
   off later if adding this name adds too much size burden (or if any unexpected
   issues arise).  Turning this off will only disable disassembly of JIT operation
   names.  Disassembly of other constants (see below) will still be supported.

   If Options::needDisassemblySupport() is true, the JITOperationList will
   register all JIT operations (and the LLInt ones as well) with the Disassembler
   labelMap.

4. Removed the < > brackets around branch target labels except for internal pc
   index labels (e.g. <716>) and <unknown> (when we don't know what the branch
   target is).  The < > notation takes up space and doesn't add any info.  The
   branch targets now look like this:

      <32> 0x1180102c0:    b.ne     0x1180101c0 -> thunk: native Tail With Saved Tags Call trampoline
      <92> 0x11801c87c:    b        0x118008980 -> thunk: LLInt function for call jump to prologue thunk
    <3508> 0x1198e16b4:    b        0x1198bc1a0 -> JIT PC

   Internal pc index labels will still use the < > brackets:

    <3476> 0x1198e1694:    b.eq     0x1198e16a0 -> <3488>

5. Added ARM64 disassembler support to compute the value of a constant being
   loaded into a register using "MoveWide" instructions e.g. movz and movk.

   When the disassembler sees a movz or movn or mov (of the MoveWide variety),
   the disassembler initializes a m_builtConstant value.  With each subsequent
   movk instruction with the same destination register, the disassembler splices
   in the corresponding 16-bit immediate.

   After disassembling a MoveWide instruction, it will check if the next
   instruction is also a MoveWide instruction targeting the same destination
   register.  If so, the constant is still being build: hence, it does nothing and
   continues.

   If the next instruction is (a) a MoveWide instruction targeting a different
   register, (b) not a MoveWide instruction, or (c) we've reached the end of the
   LinkBuffer (i.e. there's no next instruction), then the constant building for
   the current target register is complete.

6. Added ARM64 disassembler support for looking up constants (built in (5) above)
   and printing symbolic info / names of those constants.

   With ENABLE(JIT_OPERATION_DISASSEMBLY), we now have JIT operation names e.g.

     <176> 0x118010270:    movk     x3, #0x9f6e, lsl #48 -> 0x102dc8950 operationVMHandleException
     <164> 0x1180180a4:    movk     x8, #0xe5c, lsl #48 -> 0x102db9420 operationVirtualCall

   Apart from looking up the constant in the Disassembler labelMap, it also looks
   up some commonly used constants e.g.

   a. VM pointers:

     <156> 0x11801105c:    movk     x0, #0x1, lsl #32 -> 0x139435000 vm

   b. Some VM internal pointers (more can be added later as needed):

      <24> 0x118014d18:    movk     x17, #0x1, lsl #32 -> 0x13943ee78 vm +40568: vm.topCallFrame
      <76> 0x118014d4c:    movk     x17, #0x1, lsl #32 -> 0x139441a10 vm +51728: vm.m_exception
     <196> 0x118011244:    movk     x17, #0x1, lsl #32 -> 0x1394417d0 vm +51152: vm.targetMachinePCForThrow
     <208> 0x1198e09d0:    movk     x17, #0x1, lsl #32 -> 0x104432cc0 vm +52416: vm.m_doesGC

   c. VM related pointers (only 1 for now; not VM fields, but hangs off of VM):

      <12> 0x11801938c:    movk     x1, #0x1, lsl #32 -> 0x1052cd3c8 vm scratchBuffer.m_buffer

   d. Well known PtrTags:

     <204> 0x11801124c:    movz     x16, #0x6f9b -> 0x6f9b ExceptionHandlerPtrTag ?
     <212> 0x1180150d4:    movz     x16, #0x593 -> 0x593 JSEntryPtrTag ?
     <168> 0x1180183a8:    movz     lr, #0xb389 -> 0xb389 OperationPtrTag ?

     * the ? is because we cannot be certain that the 16-bit constant is a PtrTag.
       It may just be a coincidence that the value is the same.  However, the user
       can trivially look at the surrounding disassembled code, and be able to
       tell if the value is used as a PtrTag.

   For constants that are not found in the known sets:

   e. Small 16-bit constant (prints decimal value for convenience):

     <200> 0x1198e09c8:    movz     x17, #0x2cc0 -> 11456

   f. Unknown pointers that aren't in the JIT and LLINT regions will simply print
      the pointer / constant:

      <88> 0x1198e0958:    movz     x17, #0x2088
      <92> 0x1198e095c:    movk     x17, #0x30d, lsl #16
      <96> 0x1198e0960:    movk     x17, #0x1, lsl #32 -> 0x1030d2088

      <unknown> is only used in relative branches.

7. Enhanced the Integrity::isSanePointer() check to reject pointer values that
   are less than 4G on CPU(ADDRESS64) targets for OS(DARWIN).  No sane pointer
   should be in the lowest 4G address region.

   This also helps the disassembler more quickly filter out constant values that
   aren't pointers.

   This change affects DFG's speculationFromCell(), which is the only place that
   may affect user facing performance.  However, I think the impact should be
   insignificant (since the added check is cheap).

* assembler/JITOperationList.cpp:
(JSC::JITOperationList::populatePointersInJavaScriptCore):
(JSC::llintOperations):
(JSC::JITOperationList::populatePointersInJavaScriptCoreForLLInt):
(JSC::JITOperationList::addDisassemblyLabels):
(JSC::JITOperationList::populateDisassemblyLabelsInJavaScriptCore):
(JSC::JITOperationList::populateDisassemblyLabelsInJavaScriptCoreForLLInt):
(JSC::JITOperationList::populateDisassemblyLabelsInEmbedder):
* assembler/JITOperationList.h:
(JSC::JITOperationList::populatePointersInJavaScriptCore):
(JSC::JITOperationList::populatePointersInJavaScriptCoreForLLInt):
* assembler/JITOperationValidation.h:
* assembler/LinkBuffer.cpp:
(JSC::LinkBuffer::finalizeCodeWithDisassemblyImpl):
* b3/testb3_1.cpp:
(main):
* disassembler/ARM64/A64DOpcode.cpp:
(JSC::ARM64Disassembler::A64DOpcode::appendPCRelativeOffset):
(JSC::ARM64Disassembler::MoveWideFormatTrait::rejectedResult):
(JSC::ARM64Disassembler::MoveWideFormatTrait::acceptedResult):
(JSC::ARM64Disassembler::MoveWideIsValidTrait::rejectedResult):
(JSC::ARM64Disassembler::MoveWideIsValidTrait::acceptedResult):
(JSC::ARM64Disassembler::A64DOpcodeMoveWide::handlePotentialDataPointer):
(JSC::ARM64Disassembler::A64DOpcodeMoveWide::handlePotentialPtrTag):
(JSC::ARM64Disassembler::A64DOpcodeMoveWide::parse):
(JSC::ARM64Disassembler::A64DOpcodeMoveWide::format):
(JSC::ARM64Disassembler::A64DOpcodeMoveWide::isValid):
* disassembler/ARM64/A64DOpcode.h:
(JSC::ARM64Disassembler::A64DOpcodeMoveWide::baseFormat):
(JSC::ARM64Disassembler::A64DOpcodeMoveWide::formatBuffer):
* disassembler/Disassembler.cpp:
(JSC::Disassembler::ensureLabelMap):
(JSC::registerLabel):
(JSC::labelFor):
(JSC::ensureThunkLabelMap): Deleted.
(JSC::registerThunkLabel): Deleted.
(JSC::labelForThunk): Deleted.
* disassembler/Disassembler.h:
* jsc.cpp:
(jscmain):
* runtime/Gate.h:
* runtime/JSCPtrTag.cpp:
(JSC::ptrTagName):
* runtime/JSCPtrTag.h:
* runtime/Options.cpp:
(JSC::Options::recomputeDependentOptions):
* runtime/OptionsList.h:
* runtime/VM.cpp:
(JSC::VM::isScratchBuffer):
* runtime/VM.h:
* tools/Integrity.h:
(JSC::Integrity::isSanePointer):

Source/WebCore:

* bindings/js/WebCoreJITOperations.cpp:
(WebCore::populateJITOperations):
(WebCore::populateDisassemblyLabels):
* bindings/js/WebCoreJITOperations.h:
(WebCore::populateDisassemblyLabels):
(WebCore::populateJITOperations):
* testing/js/WebCoreTestSupport.cpp:
(WebCoreTestSupport::populateJITOperations):
(WebCoreTestSupport::populateDisassemblyLabels):
* testing/js/WebCoreTestSupport.h:
(WebCoreTestSupport::populateDisassemblyLabels):
(WebCoreTestSupport::populateJITOperations):

Source/WTF:

1. Added a ENABLE(JIT_OPERATION_DISASSEMBLY) flag.
   Currently, this feature relies on an USE(APPLE_INTERNAL_SDK) for enumerating
   JIT operations similar to ENABLE(JIT_OPERATION_VALIDATION).  It is also
   restricted to ENABLE(DISASSEMBLER) && CPU(ARM64E).

* wtf/PlatformCallingConventions.h:
* wtf/PlatformEnable.h:

Canonical link: https://commits.webkit.org/250631@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294287 268f45cc-cd09-0410-ab3c-d52691b4dbfc
webkit-commit-queue pushed a commit that referenced this pull request Jun 23, 2022
https://bugs.webkit.org/show_bug.cgi?id=241856

Reviewed by Yusuke Suzuki.

1. Ruby treats numeric 0 as truthy.  However, there's a test in arm64LowerMalformedLoadStoreAddresses
   which assumes a value of 0 would be false.  As a result, we see offlineasm emit inefficient LLInt
   code like this:
    ".loc 3 821\n"        "movz x16, #0 \n"                    // LowLevelInterpreter64.asm:821
                          "add x13, x3, x16 \n"
                          "ldr x0, [x13] \n"

  ...  instead of this:
    ".loc 3 821\n"        "ldr x0, [x3] \n"                    // LowLevelInterpreter64.asm:821

   This patch fixes this.

2. offlineasm's emitARM64MoveImmediate chooses to use `movn` instead of `movz` based on whether a
   64-bit value is negative or not.  Instead, it should be making that decision based on the number of
   halfwords (16-bits) in the value that is 0xffff vs 0.  As a result, offlineasm emits code like this:
    ".loc 1 1638\n"       "movn x27, #1, lsl #48 \n"           // LowLevelInterpreter.asm:1638
                          "movk x27, #0, lsl #32 \n"
                          "movk x27, #0, lsl #16 \n"
                          "movk x27, #0 \n"

  ...  instead of this:
    ".loc 1 1638\n"       "movz x27, #65534, lsl #48 \n"       // LowLevelInterpreter.asm:1638

   This patch fixes this.

3. offlineasm is trivially assuming the range of immediate offsets for ldr/str instructions is
   [-255..4095].  However, that's only the range for byte sized load-stores.  For 32-bit, the range
   is actually [-255..16380].  For 64-bit, the range is actually [-255..32760].  As a result,
    offlineasm emits code like this:
    ".loc 1 633\n"        "movn x16, #16383 \n"                // LowLevelInterpreter.asm:633
    ".loc 1 1518\n"       "and x3, x3, x16 \n"                 // LowLevelInterpreter.asm:1518
    ".loc 1 1519\n"       "movz x16, #16088 \n"                // LowLevelInterpreter.asm:1519
                          "add x17, x3, x16 \n"
                          "ldr x3, [x17] \n"

  ...  instead of this:
    ".loc 1 633\n"        "movn x17, #16383 \n"                // LowLevelInterpreter.asm:633
    ".loc 1 1518\n"       "and x3, x3, x17 \n"                 // LowLevelInterpreter.asm:1518
    ".loc 1 1519\n"       "ldr x3, [x3, #16088] \n"            // LowLevelInterpreter.asm:1519

   This patch fixes this for 64-bit and 32-bit load-stores.  16-bit load-stores also has a wider
   range, but for now, it will continue to use the conservative range.

   This patch also introduces an `isMalformedArm64LoadAStoreAddress` so that this range check can be
   done consistently in all the places that checks for it.

4. offlineasm is eagerly emitting no-op arguments in instructions, e.g. "lsl #0", and adding 0.
   As a result, offlineasm emits code like this:
    ".loc 3 220\n"        "movz x13, #51168, lsl #0 \n"        // LowLevelInterpreter64.asm:220
                          "add x17, x1, x13, lsl #0 \n"
                          "ldr w4, [x17, #0] \n"

  ...  instead of this:
    ".loc 3 220\n"        "movz x13, #51168 \n"                // LowLevelInterpreter64.asm:220
                          "add x17, x1, x13 \n"
                          "ldr w4, [x17] \n"

   This unnecessary arguments are actually very common throughout the emitted LLIntAssembly.h.

   This patch removes these unnecessary arguments, which makes the emitted LLInt code more human
   readable due to less clutter.

This patch has passed the testapi and JSC stress tests with a Release build on an M1 Mac.

I also manually verified that the emitARM64MoveImmediate code is working properly by
hacking up LowLevelInterpreter64.asm to emit moves of constants of different values in
the ranges, and for load-store instructions of different sizes, and visually inspecting
the emitted code.

* Source/JavaScriptCore/offlineasm/arm64.rb:

Canonical link: https://commits.webkit.org/251771@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295766 268f45cc-cd09-0410-ab3c-d52691b4dbfc
webkit-early-warning-system pushed a commit to MenloDorian/Webkit that referenced this pull request Jun 23, 2022
https://bugs.webkit.org/show_bug.cgi?id=241905

Reviewed by Yusuke Suzuki.

offlineasm used to emit this LLInt code:
    ".loc 1 996\n"        "ldr x19, [x0] \n"                   // LowLevelInterpreter.asm:996
    ".loc 1 997\n"        "ldr x20, [x0, WebKit#8] \n"               // LowLevelInterpreter.asm:997
    ".loc 1 998\n"        "ldr x21, [x0, WebKit#16] \n"              // LowLevelInterpreter.asm:998
    ".loc 1 999\n"        "ldr x22, [x0, WebKit#24] \n"              // LowLevelInterpreter.asm:999
    ...
    ".loc 1 1006\n"       "ldr d8, [x0, WebKit#80] \n"               // LowLevelInterpreter.asm:1006
    ".loc 1 1007\n"       "ldr d9, [x0, WebKit#88] \n"               // LowLevelInterpreter.asm:1007
    ".loc 1 1008\n"       "ldr d10, [x0, WebKit#96] \n"              // LowLevelInterpreter.asm:1008
    ".loc 1 1009\n"       "ldr d11, [x0, WebKit#104] \n"             // LowLevelInterpreter.asm:1009
    ...

Now, it can emit this instead:
    ".loc 1 996\n"        "ldp x19, x20, [x0, #0] \n"          // LowLevelInterpreter.asm:996
    ".loc 1 997\n"        "ldp x21, x22, [x0, WebKit#16] \n"         // LowLevelInterpreter.asm:997
    ...
    ".loc 1 1001\n"       "ldp d8, d9, [x0, WebKit#80] \n"           // LowLevelInterpreter.asm:1001
    ".loc 1 1002\n"       "ldp d10, d11, [x0, WebKit#96] \n"         // LowLevelInterpreter.asm:1002
    ...

Also, there was some code that kept recomputing the base address of a sequence of load/store
instructions.  For example,
    ".loc 6 902\n"        "add x13, sp, x10, lsl WebKit#3 \n"        // WebAssembly.asm:902
                          "ldr x0, [x13, WebKit#48] \n"
                          "add x13, sp, x10, lsl WebKit#3 \n"
                          "ldr x1, [x13, WebKit#56] \n"
                          "add x13, sp, x10, lsl WebKit#3 \n"
                          "ldr x2, [x13, WebKit#64] \n"
                          "add x13, sp, x10, lsl WebKit#3 \n"
                          "ldr x3, [x13, WebKit#72] \n"
    ...

For such places, we observe that the base address is the same for every load/store instruction
in the sequence, and precompute it in the LLInt asm code to help out the offline asm.  This
allows the offlineasm to now emit this more efficient code instead:
    ".loc 6 896\n"        "add x10, sp, x10, lsl WebKit#3 \n"        // WebAssembly.asm:896
    ".loc 6 898\n"        "ldp x0, x1, [x10, WebKit#48] \n"          // WebAssembly.asm:898
                          "ldp x2, x3, [x10, WebKit#64] \n"
    ...

* Source/JavaScriptCore/llint/LowLevelInterpreter.asm:
* Source/JavaScriptCore/llint/WebAssembly.asm:
* Source/JavaScriptCore/offlineasm/arm64.rb:
* Source/JavaScriptCore/offlineasm/instructions.rb:

Canonical link: https://commits.webkit.org/251799@main
webkit-commit-queue pushed a commit to Constellation/WebKit that referenced this pull request Aug 26, 2024
https://bugs.webkit.org/show_bug.cgi?id=278617
rdar://134636872

Reviewed by Yijia Huang.

When the next op is also one character PatternCharacterClass, backtracking should not load index register
since the next op will override it anyway.

Before:
      57:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a2,0x1f0b2,0x1f0c2,0x1f0d2)]
                <4472> 0x10f3a2078:    ldur     x1, [sp, WebKit#64]
      56:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a3,0x1f0b3,0x1f0c3,0x1f0d3)]
                <4476> 0x10f3a207c:    ldur     x1, [sp, WebKit#48]
      55:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a4,0x1f0b4,0x1f0c4,0x1f0d4)]
                <4480> 0x10f3a2080:    ldur     x1, [sp, WebKit#32]
      54:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a5,0x1f0b5,0x1f0c5,0x1f0d5)]
                <4484> 0x10f3a2084:    ldur     x1, [sp, WebKit#16]
      53:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a6,0x1f0b6,0x1f0c6,0x1f0d6)]
                <4488> 0x10f3a2088:    ldur     x1, [sp]
      52:BodyAlternativeNext minimum-size:(10),checked-offset:(10)
                <4492> 0x10f3a208c:    b        0x10f3a1e78 -> <3960>

After:
      57:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a2,0x1f0b2,0x1f0c2,0x1f0d2)]
      56:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a3,0x1f0b3,0x1f0c3,0x1f0d3)]
      55:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a4,0x1f0b4,0x1f0c4,0x1f0d4)]
      54:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a5,0x1f0b5,0x1f0c5,0x1f0d5)]
      53:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a6,0x1f0b6,0x1f0c6,0x1f0d6)]
                <3656> 0x11e840e48:    ldur     x1, [sp]
      52:BodyAlternativeNext minimum-size:(10),checked-offset:(10)
                <3660> 0x11e840e4c:    b        0x11e840ca8 -> <3240>

* Source/JavaScriptCore/yarr/YarrJIT.cpp:

Canonical link: https://commits.webkit.org/282746@main
webkit-commit-queue pushed a commit to msaboff/WebKit that referenced this pull request Feb 21, 2025
https://bugs.webkit.org/show_bug.cgi?id=288102
rdar://145222010

Reviewed by Yusuke Suzuki.

Added the notion of a string list to a parsed RegExp that is in the form of
  /^(?:break|case|which|do|for)/ with an optional trailing $.
Such a RegExp will not backtrack and therefore we can streamline the code we emit for such a pattern.

This change involves recognizing beginning of string anchored alternations of strings while parsing and
then treating the generation of JIT code differently for these patterns.  This includes changing how
conditional branching works, specifically that instead of the "fall through on match" for each term,
to a "jump on match" for the whole alternation.

The current code generated for the "case" elternative is:
   8:Term PatternCharacter checked-offset:(3) 'c'
               <156> 0x11381430c:    add      w1, w1, WebKit#2
               <160> 0x113814310:    cmp      w1, w2
               <164> 0x113814314:    b.hi     0x113814444 -> <468>
  10:Term PatternCharacter checked-offset:(4) 'c'
               <168> 0x113814318:    sub      x17, x0, WebKit#4
               <172> 0x11381431c:    ldr      w17, [x17, x1]
               <176> 0x113814320:    movz     w16, #0x6163
               <180> 0x113814324:    movk     w16, #0x6573, lsl WebKit#16 -> 0x65736163
               <184> 0x113814328:    cmp      w17, w16
               <188> 0x11381432c:    b.ne     0x113814444 -> <468>
  11:Term PatternCharacter checked-offset:(4) 'a' already handled
  12:Term PatternCharacter checked-offset:(4) 's' already handled
  13:Term PatternCharacter checked-offset:(4) 'e' already handled
  14:NestedAlternativeNext minimum-size:(5),checked-offset:(5)
               <192> 0x113814330:    movz     x16, #0x4444
               <196> 0x113814334:    movk     x16, #0x1381, lsl WebKit#16
               <200> 0x113814338:    movk     x16, #0x8001, lsl WebKit#32
               <204> 0x11381433c:    movk     x16, #0xc973, lsl WebKit#48 -> 0x113814444 JIT PC
               <208> 0x113814340:    stur     x16, [sp, WebKit#8]
               <212> 0x113814344:    b        0x113814404 -> <404>
With some additional backtracking code:
   9:NestedAlternativeNext minimum-size:(4),checked-offset:(4)
               <468> 0x113814444:    sub      w1, w1, WebKit#2
               <472> 0x113814448:    b        0x113814348 -> <216>

With this change, the processing of "case" becomes:
   9:StringListAlternativeNext minimum-size:(4),checked-offset:(4)
               <132> 0x12a8285c4:    sub      w1, w1, WebKit#1
               <136> 0x12a8285c8:    cmp      w1, w2
               <140> 0x12a8285cc:    b.hi     0x12a8285e8 -> <168>
  10:Term PatternCharacter checked-offset:(4) 'c'
               <144> 0x12a8285d0:    sub      x17, x0, WebKit#4
               <148> 0x12a8285d4:    ldr      w17, [x17, x1]
               <152> 0x12a8285d8:    movz     w16, #0x6163
               <156> 0x12a8285dc:    movk     w16, #0x6573, lsl WebKit#16 -> 0x65736163
               <160> 0x12a8285e0:    cmp      w17, w16
               <164> 0x12a8285e4:    b.eq     0x12a82866c -> <300>
  11:Term PatternCharacter checked-offset:(4) 'a' already handled
  12:Term PatternCharacter checked-offset:(4) 's' already handled
  13:Term PatternCharacter checked-offset:(4) 'e' already handled
  14:StringListAlternativeNext minimum-size:(5),checked-offset:(5)
With no backtracking code.

We are able to eliminate one branch and the saving of the continuation PC for backtracking.
The code size to process these string list RegExp is reduces.  For the example RegExp above,
the prior version created 1940 bytes (485 instructions) of code while the code created with this
1392 bytes (345 instructions) of code, a nearly 30% reduction in code.

This change is a ~18% progression on the new regexp-keyword-parsing microbenchmark:

                                 Baseline               YarrStringList

regexp-keyword-parsing      136.7065+-0.9807     ^    116.0161+-1.1791        ^ definitely 1.1783x faster

<geometric>                 136.7065+-0.9807     ^    116.0161+-1.1791        ^ definitely 1.1783x faster

* JSTests/microbenchmarks/regexp-keyword-parsing.js: Added.
(arrayToString):
(objectToString):
(dumpValue):
(compareArray):
(compareGroups):
(testRegExp):
(testRegExpSyntaxError):
(let.re.break.case.catch.continue.debugger.default.else.finally.if):
(let.re1.break.case.catch.continue.debugger.default.else.finally.if):
* JSTests/stress/regexp-parsing-tokens.js: Added.
(arrayToString):
(objectToString):
(dumpValue):
(compareArray):
(compareGroups):
(testRegExp):
(testRegExpSyntaxError):
* Source/JavaScriptCore/yarr/YarrJIT.cpp:
* Source/JavaScriptCore/yarr/YarrPattern.cpp:
(JSC::Yarr::YarrPatternConstructor::atomParenthesesEnd):
(JSC::Yarr::YarrPatternConstructor::checkForTerminalParentheses):
(JSC::Yarr::PatternAlternative::dump):
(JSC::Yarr::PatternTerm::dump):
* Source/JavaScriptCore/yarr/YarrPattern.h:
(JSC::Yarr::PatternTerm::PatternTerm):
(JSC::Yarr::PatternAlternative::PatternAlternative):

Canonical link: https://commits.webkit.org/290791@main
webkit-commit-queue pushed a commit to msaboff/WebKit that referenced this pull request Feb 24, 2025
https://bugs.webkit.org/show_bug.cgi?id=288102
rdar://145222010

Reviewed by Yusuke Suzuki.

Added the notion of a string list to a parsed RegExp that is in the form of
  /^(?:break|case|which|do|for)/ with an optional trailing $.
Such a RegExp will not backtrack and therefore we can streamline the code we emit for such a pattern.

This change involves recognizing beginning of string anchored alternations of strings while parsing and
then treating the generation of JIT code differently for these patterns.  This includes changing how
conditional branching works, specifically that instead of the "fall through on match" for each term,
to a "jump on match" for the whole alternation.

Fixed a bug in the original version where we weren't properly checking the nested alternatives to see
if they only contain fixed single count PatternCharacter terms.

The current code generated for the "case" elternative is:
   8:Term PatternCharacter checked-offset:(3) 'c'
               <156> 0x11381430c:    add      w1, w1, WebKit#2
               <160> 0x113814310:    cmp      w1, w2
               <164> 0x113814314:    b.hi     0x113814444 -> <468>
  10:Term PatternCharacter checked-offset:(4) 'c'
               <168> 0x113814318:    sub      x17, x0, WebKit#4
               <172> 0x11381431c:    ldr      w17, [x17, x1]
               <176> 0x113814320:    movz     w16, #0x6163
               <180> 0x113814324:    movk     w16, #0x6573, lsl WebKit#16 -> 0x65736163
               <184> 0x113814328:    cmp      w17, w16
               <188> 0x11381432c:    b.ne     0x113814444 -> <468>
  11:Term PatternCharacter checked-offset:(4) 'a' already handled
  12:Term PatternCharacter checked-offset:(4) 's' already handled
  13:Term PatternCharacter checked-offset:(4) 'e' already handled
  14:NestedAlternativeNext minimum-size:(5),checked-offset:(5)
               <192> 0x113814330:    movz     x16, #0x4444
               <196> 0x113814334:    movk     x16, #0x1381, lsl WebKit#16
               <200> 0x113814338:    movk     x16, #0x8001, lsl WebKit#32
               <204> 0x11381433c:    movk     x16, #0xc973, lsl WebKit#48 -> 0x113814444 JIT PC
               <208> 0x113814340:    stur     x16, [sp, WebKit#8]
               <212> 0x113814344:    b        0x113814404 -> <404>
With some additional backtracking code:
   9:NestedAlternativeNext minimum-size:(4),checked-offset:(4)
               <468> 0x113814444:    sub      w1, w1, WebKit#2
               <472> 0x113814448:    b        0x113814348 -> <216>

With this change, the processing of "case" becomes:
   9:StringListAlternativeNext minimum-size:(4),checked-offset:(4)
               <132> 0x12a8285c4:    sub      w1, w1, WebKit#1
               <136> 0x12a8285c8:    cmp      w1, w2
               <140> 0x12a8285cc:    b.hi     0x12a8285e8 -> <168>
  10:Term PatternCharacter checked-offset:(4) 'c'
               <144> 0x12a8285d0:    sub      x17, x0, WebKit#4
               <148> 0x12a8285d4:    ldr      w17, [x17, x1]
               <152> 0x12a8285d8:    movz     w16, #0x6163
               <156> 0x12a8285dc:    movk     w16, #0x6573, lsl WebKit#16 -> 0x65736163
               <160> 0x12a8285e0:    cmp      w17, w16
               <164> 0x12a8285e4:    b.eq     0x12a82866c -> <300>
  11:Term PatternCharacter checked-offset:(4) 'a' already handled
  12:Term PatternCharacter checked-offset:(4) 's' already handled
  13:Term PatternCharacter checked-offset:(4) 'e' already handled
  14:StringListAlternativeNext minimum-size:(5),checked-offset:(5)
With no backtracking code.

We are able to eliminate one branch and the saving of the continuation PC for backtracking.
The code size to process these string list RegExp is reduces.  For the example RegExp above,
the prior version created 1940 bytes (485 instructions) of code while the code created with this
1392 bytes (345 instructions) of code, a nearly 30% reduction in code.

This change is a ~18% progression on the new regexp-keyword-parsing microbenchmark:

                                 Baseline               YarrStringList

regexp-keyword-parsing      136.7065+-0.9807     ^    116.0161+-1.1791        ^ definitely 1.1783x faster

<geometric>                 136.7065+-0.9807     ^    116.0161+-1.1791        ^ definitely 1.1783x faster

* JSTests/microbenchmarks/regexp-keyword-parsing.js: Added.
(arrayToString):
(objectToString):
(dumpValue):
(compareArray):
(compareGroups):
(testRegExp):
(testRegExpSyntaxError):
(let.re.break.case.catch.continue.debugger.default.else.finally.if):
(let.re1.break.case.catch.continue.debugger.default.else.finally.if):
* JSTests/stress/regexp-parsing-tokens.js: Added.
(arrayToString):
(objectToString):
(dumpValue):
(compareArray):
(compareGroups):
(testRegExp):
(testRegExpSyntaxError):
* Source/JavaScriptCore/yarr/YarrJIT.cpp:
* Source/JavaScriptCore/yarr/YarrPattern.cpp:
(JSC::Yarr::YarrPatternConstructor::atomParenthesesEnd):
(JSC::Yarr::YarrPatternConstructor::checkForTerminalParentheses):
(JSC::Yarr::PatternAlternative::dump):
(JSC::Yarr::PatternTerm::dump):
* Source/JavaScriptCore/yarr/YarrPattern.h:
(JSC::Yarr::PatternTerm::PatternTerm):
(JSC::Yarr::PatternAlternative::PatternAlternative):

Canonical link: https://commits.webkit.org/290982@main
Wowfunhappy referenced this pull request in Wowfunhappy/WebKit Jun 16, 2026
The platform path was disabled by a short-circuit (sending plain C++
RequestData) plus an encode-exception swallow, masking FOUR stacked
10.9 incompatibilities, now each root-caused and fixed:

1. (lost source) CoreIPCSecureCoding.mm was in no Sources list, so its
   wrapper ctor resolved to a libpolyfill no-op stub -> uninitialized
   m_secureCoding. Add it to SourcesCocoa.txt.
2. (encode) transformURLs wraps the request's NSURL in
   WKSecureCodingURLWrapper (an NSURL subclass), which 10.9's
   -[NSURLRequest encodeWithCoder:] (CFNetwork archive-list) cannot
   store in its URL slot -> null URL. Disable wrapping for NSURLRequest
   via the existing setTransformURLs: knob (10.9-only path).
3. (decode) The unarchiver never set requiresSecureCoding=YES. Upstream
   uses initForReadingFromData:error: (10.13+, secure by default); the
   10.9 fallback initForReadingWithData: defaults NO, so the secure
   archive decoded via the non-secure -[NSURLRequest initWithCoder:] ->
   _CFURLRequestCreateFromArchiveList -> SIGSEGV. Set it explicitly,
   mirroring the encode side (also restores upstream's security invariant).
4. (decode) Guard _enableStrictSecureDecodingMode (10.13+ SPI) with
   respondsToSelector, like the neighboring setDecodingFailurePolicy:.

Removes the ResourceRequestCocoa.mm short-circuits (#51, now byte-
identical to upstream) and the encode-exception swallow (#48). Verified:
URL round-trips, complex sites load correct content, cross-origin POST
bodies intact, WebContent stable across navigation.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants