Bug 164351 – JSC is crashing on release mode when running exit-from-setter.js when compiled with MSVC

• Status: NEW
• Component: JavaScriptCore

[Christopher Reid]

The RSI register is getting corrupted when exiting from the jsc vm. In MSVC this register is a nonvolatile register and used to store the scope address.

Here's what's going on:

Generated JIT code for Access stub for foo#ECI8tQ:[00000248F1BCC310->00000248F1B878A0, BaselineFunctionCall, 52 (NeverInline)] bc#24 with return point CodePtr(00000248B17F1859): Setter:(Generated, structure = 00000248F1B9C460:[Object, {_f:0, f:1}, NonArray, Proto:00000248F1B4C0A0, Leaf], offset = 1, callLinkInfo = 00000248F1B6F9C0):
    Code at [00000248B17F1EA0, 00000248B17F1F40):
		  ...
          0xb17f1eb0: sub $0x10, %rsp
          0xb17f1eb4: mov %rsi, (%rsp)
          0xb17f1eb8: mov %rdi, 0x8(%rsp)
          0xb17f1ebd: mov $0x18, 0x24(%rbp)
          0xb17f1ec4: mov 0x18(%r8), %r8
          0xb17f1ec8: test %r8, %r8
          0xb17f1ecb: jz 0x248b17f1f20
          0xb17f1ed1: sub $0x30, %rsp
          ...
>         0xb17f1f20: lea -0xa0(%rbp), %rsp
          0xb17f1f27: mov (%rsp), %rsi
          0xb17f1f2b: mov 0x8(%rsp), %rdi
          0xb17f1f30: add $0x10, %rsp
          ...

The generated access stub always assumes that RSP is set up to be a constant offset from RBP. This code works fine when run by baseline JIT but not when called by DFG JIT.

Generated Baseline JIT code for foo#ECI8tQ:[00000248F1BCC310->00000248F1B878A0, BaselineFunctionCall, 52 (NeverInline)], instruction count = 52
   Source: function foo(o_, v_) { var o = o_.f; var v = v_.f; o.f = v; o.f = v + 1; }
   Code at [00000248B17F15E0, 00000248B17F1E8B):
              0xb17f15e0: nop
              0xb17f15e1: push %rbp
              0xb17f15e2: mov %rsp, %rbp
              ...
>             0xb17f161d: lea -0x90(%rbp), %rdx
              ...
              0xb17f1637: mov %rdx, %rsp
              ...

Generated DFG JIT code for foo#ECI8tQ:[00000248F1BCCCD0->00000248F1BCC310->00000248F1B878A0, DFGFunctionCall, 52 (NeverInline)], instruction count = 52:
    Optimized with execution counter = 2550.045166/2547.000000, 3
    Code at [00000248B17F2FE0, 00000248B17F3561):
              0xb17f2fe0: push %rbp
              0xb17f2fe1: mov %rsp, %rbp
              ...
>             0xb17f300c: lea -0xb0(%rbp), %rsp
              ...

When the access stub is called by DFG JIT code, the RSP offset from RBP is different causing it to restore the wrong values in to RSI.

[Christopher Reid]

I have tried a fix moving rsp if needed instead of using an offset of rbp, but that seems to be causing issues I'm assuming because rsp is not restored in all cases.

I don't know enough about the javascript core to know exactly what is off but I was wondering if that offset used in the DFG JIT code is correct. I was also wondering if the access stub is supposed to be regenerated with the correct offset in the DFG jit case.

[Christopher Reid]

Created attachment 293731 [details]
WIP patch

Here's the patch I was trying to remove the frame pointer and just modifying the stack pointer but it seems like the stack pointer isn't preserved as it's causing some new tests to fail.

[Christopher Reid]

Created attachment 295747 [details]
Patch

[Filip Pizlo]

Comment on attachment 295747 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=295747&action=review

Please investigate why the stack pointer was not set correctly in the first place.

> Source/JavaScriptCore/ChangeLog:13
> +        What appears to have been going on was that an inline multiplication is attempted to be done in
> +        DFG jit which overflows and returns to baseline jit where the access stub is later called.
> +        When the access stub is called, the stack pointer hasn't been set up properly so that when it
> +        is restored with lea -0xXX(%rbp), %rsp, the stack pointer becomes misaligned and

An intended invariant of JSC is that the stack pointer is set correctly at every baseline JIT bytecode instruction boundary.  This implies that the bug here is that the DFG JIT is not setting the stack pointer to the appropriate height.

> Source/JavaScriptCore/ChangeLog:17
> +        There doesn't seem to be a place where the stack pointer is supposed to be set up before this
> +        call. This change sets up the stack pointer before saves based on what is expected in
> +        PolymorphicAccess.cpp:1104

There is such a place.  What you are describing sounds like a fundamental OSR exit bug, and your fix is a very weak workaround at best.  Many more things depend on the stack pointer being set right, not just PolymorphicAccess.