Title: [180060] trunk
Revision
180060
Author
msab...@apple.com
Date
2015-02-13 10:57:57 -0800 (Fri, 13 Feb 2015)

Log Message

Google doc spreadsheet reproducibly crashes when sorting
https://bugs.webkit.org/show_bug.cgi?id=141098

Reviewed by Oliver Hunt.

Source/_javascript_Core:

Moved the stack check to before the callee registers are allocated in the
prologue() by movving it from the functionInitialization() macro.  This
way we can check the stack before moving the stack pointer, avoiding a
crash during a "call" instruction.  Before this change, we weren't even
checking the stack for program and eval execution.

Made a couple of supporting changes.

* llint/LLIntSlowPaths.cpp:
(JSC::LLInt::llint_stack_check): We can't just go up one frame as we
may be processing an exception to an entry frame.

* llint/LowLevelInterpreter.asm:

* llint/LowLevelInterpreter32_64.asm:
* llint/LowLevelInterpreter64.asm:
(llint_throw_from_slow_path_trampoline): Changed method to get the vm
from the code block to not use the codeBlock, since we may need to
continue from an exception in a native function.

LayoutTests:

New test.

* js/regress-141098-expected.txt: Added.
* js/regress-141098.html: Added.
* js/script-tests/regress-141098.js: Added.
(probeAndRecurse):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (180059 => 180060)


--- trunk/LayoutTests/ChangeLog	2015-02-13 18:46:04 UTC (rev 180059)
+++ trunk/LayoutTests/ChangeLog	2015-02-13 18:57:57 UTC (rev 180060)
@@ -1,3 +1,17 @@
+2015-02-13  Michael Saboff  <msab...@apple.com>
+
+        Google doc spreadsheet reproducibly crashes when sorting
+        https://bugs.webkit.org/show_bug.cgi?id=141098
+
+        Reviewed by Oliver Hunt.
+
+        New test.
+
+        * js/regress-141098-expected.txt: Added.
+        * js/regress-141098.html: Added.
+        * js/script-tests/regress-141098.js: Added.
+        (probeAndRecurse):
+
 2015-02-13  ChangSeok Oh  <changseok...@collabora.com>
 
         Div having contentEditable and display:grid cannot be edited if it is empty.

Added: trunk/LayoutTests/js/regress-141098-expected.txt (0 => 180060)


--- trunk/LayoutTests/js/regress-141098-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/js/regress-141098-expected.txt	2015-02-13 18:57:57 UTC (rev 180060)
@@ -0,0 +1,9 @@
+Regression test for https://webkit.org/b/141098. Make sure eval() properly handles running out of stack space. This test should run without crashing.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/js/regress-141098.html (0 => 180060)


--- trunk/LayoutTests/js/regress-141098.html	                        (rev 0)
+++ trunk/LayoutTests/js/regress-141098.html	2015-02-13 18:57:57 UTC (rev 180060)
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+</body>
+</html>

Added: trunk/LayoutTests/js/script-tests/regress-141098.js (0 => 180060)


--- trunk/LayoutTests/js/script-tests/regress-141098.js	                        (rev 0)
+++ trunk/LayoutTests/js/script-tests/regress-141098.js	2015-02-13 18:57:57 UTC (rev 180060)
@@ -0,0 +1,46 @@
+description("Regression test for https://webkit.org/b/141098. Make sure eval() properly handles running out of stack space. This test should run without crashing.");
+
+function probeAndRecurse(depth)
+{
+    var result;
+
+    // Probe stack depth
+    try {
+        result = probeAndRecurse(depth+1);
+        if (result < 0)
+            return result + 1;
+        else if (result > 0)
+            return result;
+    } catch (e) {
+        // Go up a many frames and then create an _expression_ to eval that will consume the stack using
+        // callee registers.
+        return -60;
+    }
+
+    try {
+        var count = 1;
+
+        for (var i = 0; i < 40; count *= 10, i++) {
+            evalStringPrefix = "{ var first = " + count + "; ";
+            var evalStringBody = "";
+
+            for (var varIndex = 0; varIndex < count; varIndex++)
+                evalStringBody += "var s" + varIndex + " = " + varIndex + ";";
+
+            evalStringBody += "var value = [";
+            for (var varIndex = 0; varIndex < count; varIndex++) {
+                if (varIndex > 0)
+                    evalStringBody += ", ";
+                evalStringBody += "s" + varIndex;
+            }
+            evalStringBody +=  "]; ";
+
+           var evalResult = eval("{" + evalStringBody + "}");
+        }
+    } catch (e) {
+    }
+
+    return 1;
+}
+
+probeAndRecurse(0);

Modified: trunk/Source/_javascript_Core/ChangeLog (180059 => 180060)


--- trunk/Source/_javascript_Core/ChangeLog	2015-02-13 18:46:04 UTC (rev 180059)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-02-13 18:57:57 UTC (rev 180060)
@@ -1,3 +1,30 @@
+2015-02-13  Michael Saboff  <msab...@apple.com>
+
+        Google doc spreadsheet reproducibly crashes when sorting
+        https://bugs.webkit.org/show_bug.cgi?id=141098
+
+        Reviewed by Oliver Hunt.
+
+        Moved the stack check to before the callee registers are allocated in the
+        prologue() by movving it from the functionInitialization() macro.  This
+        way we can check the stack before moving the stack pointer, avoiding a
+        crash during a "call" instruction.  Before this change, we weren't even
+        checking the stack for program and eval execution.
+
+        Made a couple of supporting changes.
+
+        * llint/LLIntSlowPaths.cpp:
+        (JSC::LLInt::llint_stack_check): We can't just go up one frame as we
+        may be processing an exception to an entry frame.
+
+        * llint/LowLevelInterpreter.asm:
+
+        * llint/LowLevelInterpreter32_64.asm:
+        * llint/LowLevelInterpreter64.asm:
+        (llint_throw_from_slow_path_trampoline): Changed method to get the vm
+        from the code block to not use the codeBlock, since we may need to
+        continue from an exception in a native function.
+
 2015-02-12  Benjamin Poulain  <benja...@webkit.org>
 
         Simplify the initialization of BytecodeGenerator a bit

Modified: trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp (180059 => 180060)


--- trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp	2015-02-13 18:46:04 UTC (rev 180059)
+++ trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp	2015-02-13 18:57:57 UTC (rev 180060)
@@ -490,7 +490,7 @@
         LLINT_RETURN_TWO(pc, 0);
 #endif
 
-    exec = exec->callerFrame();
+    exec = exec->callerFrame(vm.topVMEntryFrame);
     vm.topCallFrame = exec;
     ErrorHandlingScope errorScope(vm);
     CommonSlowPaths::interpreterThrowInCaller(exec, createStackOverflowError(exec));

Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter.asm (180059 => 180060)


--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter.asm	2015-02-13 18:46:04 UTC (rev 180059)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter.asm	2015-02-13 18:57:57 UTC (rev 180060)
@@ -445,21 +445,21 @@
     subp entryFramePointer, VMEntryTotalFrameSize, resultReg
 end
 
-macro moveStackPointerForCodeBlock(codeBlock, scratch)
-    loadi CodeBlock::m_numCalleeRegisters[codeBlock], scratch
-    lshiftp 3, scratch
-    addp maxFrameExtentForSlowPathCall, scratch
-    if ARMv7
-        subp cfr, scratch, scratch
-        move scratch, sp
-    else
-        subp cfr, scratch, sp
-    end
+macro getFrameRegisterSizeForCodeBlock(codeBlock, size)
+    loadi CodeBlock::m_numCalleeRegisters[codeBlock], size
+    lshiftp 3, size
+    addp maxFrameExtentForSlowPathCall, size
 end
 
 macro restoreStackPointerAfterCall()
     loadp CodeBlock[cfr], t2
-    moveStackPointerForCodeBlock(t2, t4)
+    getFrameRegisterSizeForCodeBlock(t2, t4)
+    if ARMv7
+        subp cfr, t4, t4
+        move t4, sp
+    else
+        subp cfr, t4, sp
+    end
 end
 
 macro traceExecution()
@@ -606,8 +606,6 @@
     end
 
     codeBlockSetter(t1)
-    
-    moveStackPointerForCodeBlock(t1, t2)
 
     # Set up the PC.
     if JSVALUE64
@@ -616,6 +614,29 @@
     else
         loadp CodeBlock::m_instructions[t1], PC
     end
+
+    # Get new sp in t0 and check stack height.
+    getFrameRegisterSizeForCodeBlock(t1, t0)
+    subp cfr, t0, t0
+    loadp CodeBlock::m_vm[t1], t2
+    bpbeq VM::m_jsStackLimit[t2], t0, .stackHeightOK
+
+    # Stack height check failed - need to call a slow_path.
+    subp maxFrameExtentForSlowPathCall, sp # Set up temporary stack pointer for call
+    callSlowPath(_llint_stack_check)
+    bpeq t1, 0, .stackHeightOKGetCodeBlock
+    move t1, cfr
+    dispatch(0) # Go to exception handler in PC
+
+.stackHeightOKGetCodeBlock:
+    # Stack check slow path returned that the stack was ok.
+    # Since they were clobbered, need to get CodeBlock and new sp
+    codeBlockSetter(t1)
+    getFrameRegisterSizeForCodeBlock(t1, t0)
+    subp cfr, t0, t0
+
+.stackHeightOK:
+    move t0, sp
 end
 
 # Expects that CodeBlock is in t1, which is what prologue() leaves behind.
@@ -650,20 +671,6 @@
     end
     baddpnz -8, t0, .argumentProfileLoop
 .argumentProfileDone:
-        
-    # Check stack height.
-    loadi CodeBlock::m_numCalleeRegisters[t1], t0
-    loadp CodeBlock::m_vm[t1], t2
-    lshiftp 3, t0
-    addi maxFrameExtentForSlowPathCall, t0
-    subp cfr, t0, t0
-    bpbeq VM::m_jsStackLimit[t2], t0, .stackHeightOK
-
-    # Stack height check failed - need to call a slow_path.
-    callSlowPath(_llint_stack_check)
-    bpeq t1, 0, .stackHeightOK
-    move t1, cfr
-.stackHeightOK:
 end
 
 macro allocateJSObject(allocator, structure, result, scratch1, slowCase)

Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm (180059 => 180060)


--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm	2015-02-13 18:46:04 UTC (rev 180059)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm	2015-02-13 18:57:57 UTC (rev 180060)
@@ -2028,8 +2028,9 @@
     # When throwing from the interpreter (i.e. throwing from LLIntSlowPaths), so
     # the throw target is not necessarily interpreted code, we come to here.
     # This essentially emulates the JIT's throwing protocol.
-    loadp CodeBlock[cfr], t1
-    loadp CodeBlock::m_vm[t1], t1
+    loadp Callee[cfr], t1
+    andp MarkedBlockMask, t1
+    loadp MarkedBlock::m_weakSet + WeakSet::m_vm[t1], t1
     jmp VM::targetMachinePCForThrow[t1]
 
 

Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm (180059 => 180060)


--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm	2015-02-13 18:46:04 UTC (rev 180059)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm	2015-02-13 18:57:57 UTC (rev 180060)
@@ -1886,8 +1886,9 @@
     # When throwing from the interpreter (i.e. throwing from LLIntSlowPaths), so
     # the throw target is not necessarily interpreted code, we come to here.
     # This essentially emulates the JIT's throwing protocol.
-    loadp CodeBlock[cfr], t1
-    loadp CodeBlock::m_vm[t1], t1
+    loadp Callee[cfr], t1
+    andp MarkedBlockMask, t1
+    loadp MarkedBlock::m_weakSet + WeakSet::m_vm[t1], t1
     jmp VM::targetMachinePCForThrow[t1]
 
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to