Title: [192352] trunk/Source/_javascript_Core
Revision
192352
Author
mark....@apple.com
Date
2015-11-11 22:59:13 -0800 (Wed, 11 Nov 2015)

Log Message

Crash in sorting-benchmark.js on ARM64 with full op_sub FTL coverage.
https://bugs.webkit.org/show_bug.cgi?id=150936

Reviewed by Michael Saboff.

The OSR entry thunk does not currently restore the callee saved registers that the baseline
JIT saves but the DFG does not.  As a result, sorting-benchmark.js was crashing with the
following scenario:

    1. There exists 2 functions: benchmark() and bottom_up_merge_sort().
       Let's call them A() and B() respectively for brevity.
    2. A() is FTL compiled.
    3. B() is FTL compiled.
    4. FTL A() calls FTL B().  FTL B() trashes register x26.
    5. FTL B() goes through an OSR exit, and deopts to baseline.  The OSR exit off-ramp
       puts x26's original value in the baseline B()'s stash location for x26 in its stack
       frame.  It expects baseline B() to restore x26 when it returns.
    6. Baseline B() gets DFG optimized, and we OSR enter into DFG B().
       The OSR entry thunk does nothing to restore x26's original value.  Hence, x26 contains
       whatever value FTL B() left in it.
    7. DFG B() returns to FTL A().
       x26 is not one of the callee saved registers used by DFG B().  Hence, it does nothing
       to restore it.
    8. FTL A() tries to use x26 and crashes, because x26 contains FTL B()'s value for it, and
       not FTL A()'s.

This patch fixes this issue by having the OSR entry thunk restore all the callee saved
registers before running the DFG code.

No new test needed because this issue will be covered by sorting-benchmark.js as soon as
https://bugs.webkit.org/show_bug.cgi?id=150712 lands.

* dfg/DFGThunks.cpp:
(JSC::DFG::osrEntryThunkGenerator):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (192351 => 192352)


--- trunk/Source/_javascript_Core/ChangeLog	2015-11-12 06:33:53 UTC (rev 192351)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-11-12 06:59:13 UTC (rev 192352)
@@ -1,3 +1,40 @@
+2015-11-11  Mark Lam  <mark....@apple.com>
+
+        Crash in sorting-benchmark.js on ARM64 with full op_sub FTL coverage.
+        https://bugs.webkit.org/show_bug.cgi?id=150936
+
+        Reviewed by Michael Saboff.
+
+        The OSR entry thunk does not currently restore the callee saved registers that the baseline
+        JIT saves but the DFG does not.  As a result, sorting-benchmark.js was crashing with the
+        following scenario:
+
+            1. There exists 2 functions: benchmark() and bottom_up_merge_sort().
+               Let's call them A() and B() respectively for brevity.
+            2. A() is FTL compiled.
+            3. B() is FTL compiled.
+            4. FTL A() calls FTL B().  FTL B() trashes register x26.
+            5. FTL B() goes through an OSR exit, and deopts to baseline.  The OSR exit off-ramp
+               puts x26's original value in the baseline B()'s stash location for x26 in its stack
+               frame.  It expects baseline B() to restore x26 when it returns.
+            6. Baseline B() gets DFG optimized, and we OSR enter into DFG B().
+               The OSR entry thunk does nothing to restore x26's original value.  Hence, x26 contains
+               whatever value FTL B() left in it.
+            7. DFG B() returns to FTL A().
+               x26 is not one of the callee saved registers used by DFG B().  Hence, it does nothing
+               to restore it.
+            8. FTL A() tries to use x26 and crashes, because x26 contains FTL B()'s value for it, and
+               not FTL A()'s.
+
+        This patch fixes this issue by having the OSR entry thunk restore all the callee saved
+        registers before running the DFG code.
+
+        No new test needed because this issue will be covered by sorting-benchmark.js as soon as
+        https://bugs.webkit.org/show_bug.cgi?id=150712 lands.
+
+        * dfg/DFGThunks.cpp:
+        (JSC::DFG::osrEntryThunkGenerator):
+
 2015-11-11  Benjamin Poulain  <bpoul...@apple.com>
 
         [JSC] Add a comment explaining the opcode suffixes on x86

Modified: trunk/Source/_javascript_Core/dfg/DFGThunks.cpp (192351 => 192352)


--- trunk/Source/_javascript_Core/dfg/DFGThunks.cpp	2015-11-12 06:33:53 UTC (rev 192351)
+++ trunk/Source/_javascript_Core/dfg/DFGThunks.cpp	2015-11-12 06:59:13 UTC (rev 192352)
@@ -97,8 +97,8 @@
 
 MacroAssemblerCodeRef osrEntryThunkGenerator(VM* vm)
 {
-    MacroAssembler jit;
-    
+    AssemblyHelpers jit(vm, nullptr);
+
     // We get passed the address of a scratch buffer. The first 8-byte slot of the buffer
     // is the frame size. The second 8-byte slot is the pointer to where we are supposed to
     // jump. The remaining bytes are the new call frame header followed by the locals.
@@ -128,7 +128,12 @@
     jit.loadPtr(MacroAssembler::Address(GPRInfo::regT0, offsetOfTargetPC), GPRInfo::regT1);
     MacroAssembler::Jump ok = jit.branchPtr(MacroAssembler::Above, GPRInfo::regT1, MacroAssembler::TrustedImmPtr(bitwise_cast<void*>(static_cast<intptr_t>(1000))));
     jit.abortWithReason(DFGUnreasonableOSREntryJumpDestination);
+
     ok.link(&jit);
+    RegisterSet usedRegisters(RegisterSet::stubUnavailableRegisters(), GPRInfo::regT1);
+    jit.restoreCalleeSavesFromVMCalleeSavesBuffer(usedRegisters);
+    jit.emitMaterializeTagCheckRegisters();
+
     jit.jump(GPRInfo::regT1);
     
     LinkBuffer patchBuffer(*vm, jit, GLOBAL_THUNK_ID);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to