We should use different stack limits for stack checks from JS and host code.
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Jul 2016 00:19:15 +0000 (00:19 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Jul 2016 00:19:15 +0000 (00:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=159442
<rdar://problem/26889188>

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

We have 2 stack reservedZoneSizes:
1. Options::softReservedZoneSize()
2. Options::reservedZoneSize()

Respectively, there are used to define 2 stack limits based on these reserved
zone sizes:
1. VM::m_softStackLimit
2. VM::m_stackLimit

Options::reservedZoneSize() is the amount of the stack space that JSC guarantees
to the VM and client host code for it's use.  Host code that has well known
stack usage characteristics (i.e. doesn't call arbitrary code) may do stack
checks against the VM::m_stackLimit limit (which is computed using
Options::reservedZoneSize()).

Options::softReservedZoneSize() is a more conservative amount of reserved stack
space.  This is used to compute the VM::m_softStackLimit limit.  Any code that
is difficult to have its stack usage characterized (i.e. may call arbitrary code)
may need more stack space for its work.  Hence, these should do stack checks
against the VM::m_softStackLimit limit.

JS code and host code that may call into JS code falls into the category of code
that may call arbitrary code.  Hence, they should do stack checks against the
VM::m_softStackLimit limit.

Accordingly, the VM now provides 2 recursion check functions:

1. VM::isSafeToRecurseSoft() will do a stack check against VM::m_softStackLimit.
   In addition, for C Loop builds, VM::isSafeToRecurseSoft() will also
   check the CLoopStack against VM::m_cloopStackLimit.

2. VM::isSafeToRecurse() will do a stack check against VM::m_stackLimit.

Also added a promise-infinite-recursion-should-not-crash.js test.

* bytecompiler/BytecodeGenerator.h:
(JSC::BytecodeGenerator::emitNodeInTailPosition):
(JSC::BytecodeGenerator::emitNodeInConditionContext):
* interpreter/CLoopStack.cpp:
(JSC::CLoopStack::grow):
* interpreter/CLoopStack.h:
(JSC::CLoopStack::size):
* interpreter/CLoopStackInlines.h:
(JSC::CLoopStack::ensureCapacityFor):
(JSC::CLoopStack::isSafeToRecurse):
(JSC::CLoopStack::topOfFrameFor):
* interpreter/CachedCall.h:
(JSC::CachedCall::CachedCall):
* interpreter/Interpreter.cpp:
(JSC::Interpreter::execute):
(JSC::Interpreter::executeCall):
(JSC::Interpreter::executeConstruct):
* llint/LLIntSlowPaths.cpp:
(JSC::LLInt::LLINT_SLOW_PATH_DECL):
* parser/Parser.cpp:
* runtime/Options.h:
* runtime/ProxyObject.cpp:
(JSC::performProxyGet):
(JSC::ProxyObject::performInternalMethodGetOwnProperty):
(JSC::ProxyObject::performHasProperty):
(JSC::ProxyObject::getOwnPropertySlotCommon):
(JSC::ProxyObject::performPut):
(JSC::performProxyCall):
(JSC::performProxyConstruct):
(JSC::ProxyObject::performDelete):
(JSC::ProxyObject::performPreventExtensions):
(JSC::ProxyObject::performIsExtensible):
(JSC::ProxyObject::performDefineOwnProperty):
(JSC::ProxyObject::performGetOwnPropertyNames):
(JSC::ProxyObject::performSetPrototype):
(JSC::ProxyObject::performGetPrototype):
* runtime/RegExp.cpp:
(JSC::RegExp::finishCreation):
(JSC::RegExp::compile):
(JSC::RegExp::compileMatchOnly):
* runtime/StringRecursionChecker.h:
(JSC::StringRecursionChecker::performCheck):
* runtime/VM.cpp:
(JSC::VM::setStackPointerAtVMEntry):
(JSC::VM::updateSoftReservedZoneSize):
(JSC::preCommitStackMemory):
(JSC::VM::updateStackLimits):
(JSC::VM::updateStackLimit): Deleted.
* runtime/VM.h:
(JSC::VM::stackLimit):
(JSC::VM::softStackLimit):
(JSC::VM::addressOfSoftStackLimit):
(JSC::VM::setCLoopStackLimit):
(JSC::VM::isSafeToRecurse):
(JSC::VM::lastStackTop):
(JSC::VM::setException):
* runtime/VMInlines.h:
(JSC::VM::ensureStackCapacityFor):
(JSC::VM::isSafeToRecurseSoft):
(JSC::VM::shouldTriggerTermination):
* tests/stress/promise-infinite-recursion-should-not-crash.js: Added.
(testPromise):
(promiseFunc):
* yarr/YarrPattern.cpp:

Tools:

In http://trac.webkit.org/r203067, we limited the amount of stack that tests will
run with to keep stack overflow tests sane.  Turns out, we also need to teach the
LayoutTestRelay to pass env vars over to the iOS simulator.  This is needed in
order to keep the js/regress-139548.html test happy with this patch.

Also fixed up run_webkit_tests.py to explicitly pass an int size value for the
JSC_maxPerThreadStackUsage option.  Otherwise, it will pass a float value.

* LayoutTestRelay/LayoutTestRelay/LTRelayController.m:
(-[LTRelayController _environmentVariables]):
* Scripts/webkitpy/layout_tests/run_webkit_tests.py:
(main):

LayoutTests:

* js/regress-141098-expected.txt:
* js/script-tests/regress-141098.js:
(testEval):
(probeAndRecurse):
- Gave all the test constants names.
- Tweaked the constants to allow the test to run in the least amount of time, and
  also to behave consistently across all test configurations.
- Re-enable eager tests now that the test should finish quickly.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@203142 268f45cc-cd09-0410-ab3c-d52691b4dbfc

24 files changed:
LayoutTests/ChangeLog
LayoutTests/js/regress-141098-expected.txt
LayoutTests/js/script-tests/regress-141098.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
Source/JavaScriptCore/interpreter/CLoopStack.cpp
Source/JavaScriptCore/interpreter/CLoopStack.h
Source/JavaScriptCore/interpreter/CLoopStackInlines.h
Source/JavaScriptCore/interpreter/CachedCall.h
Source/JavaScriptCore/interpreter/Interpreter.cpp
Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
Source/JavaScriptCore/parser/Parser.cpp
Source/JavaScriptCore/runtime/Options.h
Source/JavaScriptCore/runtime/ProxyObject.cpp
Source/JavaScriptCore/runtime/RegExp.cpp
Source/JavaScriptCore/runtime/StringRecursionChecker.h
Source/JavaScriptCore/runtime/VM.cpp
Source/JavaScriptCore/runtime/VM.h
Source/JavaScriptCore/runtime/VMInlines.h
Source/JavaScriptCore/tests/stress/promise-infinite-recursion-should-not-crash.js [new file with mode: 0644]
Source/JavaScriptCore/yarr/YarrPattern.cpp
Tools/ChangeLog
Tools/LayoutTestRelay/LayoutTestRelay/LTRelayController.m
Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py

index 7aeff10..c664763 100644 (file)
@@ -1,3 +1,20 @@
+2016-07-12  Mark Lam  <mark.lam@apple.com>
+
+        We should use different stack limits for stack checks from JS and host code.
+        https://bugs.webkit.org/show_bug.cgi?id=159442
+        <rdar://problem/26889188>
+
+        Reviewed by Geoffrey Garen.
+
+        * js/regress-141098-expected.txt:
+        * js/script-tests/regress-141098.js:
+        (testEval):
+        (probeAndRecurse):
+        - Gave all the test constants names.
+        - Tweaked the constants to allow the test to run in the least amount of time, and
+          also to behave consistently across all test configurations.
+        - Re-enable eager tests now that the test should finish quickly.
+
 2016-07-12  Dean Jackson  <dino@apple.com>
 
         REGRESSION (202694): Audio and Video playback controls: Cannot find a position slider to adjust playback position using VO.
index 59ab26b..72fedba 100644 (file)
@@ -4,7 +4,9 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 
 PASS Initial setup
+Starting 1st probeAndRecurse
 PASS Exception: RangeError: Maximum call stack size exceeded.
+Starting 2nd probeAndRecurse
 PASS Exception: RangeError: Maximum call stack size exceeded.
 PASS successfullyParsed is true
 
index fab200d..3264902 100644 (file)
@@ -1,21 +1,36 @@
-//@ noEagerNoNoLLIntTestsRunLayoutTest
+//@ noNoLLIntRunLayoutTest
 
 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.");
 
 // The tiering up to test higher levels of optimization will only test the DFG
 // if run in run-jsc-stress-tests with the eager settings.
 
+// countStart, countIncrement, and numberOfFramesToBackoffFromStackOverflowPoint were
+// empirically determined to be the values that will cause StackOverflowErrors to be
+// thrown where the tests expects them. If stack frame layouts change sufficiently (e.g.
+// due to JIT changes) such that this test starts failing (due to text output
+// differences), then these values will need to be rebased.
+// Under no circumstance should this test ever crash.
+let countStart = 2;
+let countIncrement = 8;
+let numberOfFramesToBackoffFromStackOverflowPoint = 50;
+
+// backoffEverything is chosen to be -1 because a negative number will never be
+// decremented to 0, and hence, will cause probeAndRecurse() to return out of every
+// frame instead of retrying the eval test.
+let backoffEverything = -1;
+
 var lastEvalString = "";
 
 function testEval(maxIterations)
 {
     var result;
-    var count = 1;
+    var count = countStart;
 
     if (!maxIterations)
         var result = eval(lastEvalString);
     else {
-        for (var iter = 0; iter < maxIterations; count *= 4, iter++) {
+        for (var iter = 0; iter < maxIterations; count *= countIncrement, iter++) {
             var evalString = "\"dummy\".valueOf(";
 
             for (var i = 0; i < count; i++) {
@@ -35,26 +50,29 @@ function testEval(maxIterations)
     return result;
 }
 
-function probeAndRecurse(depth, reuseEvalString)
+function probeAndRecurse(reuseEvalString)
 {
-    var result;
+    var framesToBackOffFromStackOverflowPoint;
 
     // Probe stack depth
     try {
-        result = probeAndRecurse(depth+1, reuseEvalString);
+        remainingFramesToBackOff = probeAndRecurse(reuseEvalString);
 
-        if (!result) {
+        if (!remainingFramesToBackOff) {
+            // We've backed off a number of frames. Now retry the eval test to see if we
+            // still overflow.
             try {
                 testEval(1);
             } catch (e) {
-                return -49;
+                // Yikes. We still overflow. Back off some more.
+                return numberOfFramesToBackoffFromStackOverflowPoint;
             }
         } else
-            return result + 1
+            return remainingFramesToBackOff - 1
     } catch (e) {
         // We exceeded stack space, now return up the stack until we can execute a simple eval.
         // Then run an eval test to exceed stack.
-        return -49;
+        return numberOfFramesToBackoffFromStackOverflowPoint;
     }
 
     try {
@@ -63,15 +81,16 @@ function probeAndRecurse(depth, reuseEvalString)
         testPassed("Exception: " + e);
     }
 
-    return 1;
+    return backoffEverything;
 }
 
-// Because this test intentionlly exhausts the stack, we call testPassed() to ensure
+// Because this test intentionally exhausts the stack, we call testPassed() to ensure
 // everything we need in that path has been compiled and is available.  Otherwise we
 // might properly handle an out of stack, but run out of stack calling testPassed().
 testPassed("Initial setup");
 
-var depth = probeAndRecurse(0, false);
+debug("Starting 1st probeAndRecurse");
+probeAndRecurse(false);
 
 // Tier up the eval'ed code.
 // When run with run-jsc-stress-tests and it's agressive options, this low of a count will
@@ -79,4 +98,5 @@ var depth = probeAndRecurse(0, false);
 for (var i = 0; i < 200; i++)
     testEval(0);
 
-probeAndRecurse(0, true);
+debug("Starting 2nd probeAndRecurse");
+probeAndRecurse(true);
index 31eee8d..c950b3c 100644 (file)
@@ -1,3 +1,111 @@
+2016-07-12  Mark Lam  <mark.lam@apple.com>
+
+        We should use different stack limits for stack checks from JS and host code.
+        https://bugs.webkit.org/show_bug.cgi?id=159442
+        <rdar://problem/26889188>
+
+        Reviewed by Geoffrey Garen.
+
+        We have 2 stack reservedZoneSizes:
+        1. Options::softReservedZoneSize()
+        2. Options::reservedZoneSize()
+
+        Respectively, there are used to define 2 stack limits based on these reserved
+        zone sizes:
+        1. VM::m_softStackLimit
+        2. VM::m_stackLimit
+
+        Options::reservedZoneSize() is the amount of the stack space that JSC guarantees
+        to the VM and client host code for it's use.  Host code that has well known
+        stack usage characteristics (i.e. doesn't call arbitrary code) may do stack
+        checks against the VM::m_stackLimit limit (which is computed using
+        Options::reservedZoneSize()).
+
+        Options::softReservedZoneSize() is a more conservative amount of reserved stack
+        space.  This is used to compute the VM::m_softStackLimit limit.  Any code that
+        is difficult to have its stack usage characterized (i.e. may call arbitrary code)
+        may need more stack space for its work.  Hence, these should do stack checks
+        against the VM::m_softStackLimit limit.
+
+        JS code and host code that may call into JS code falls into the category of code
+        that may call arbitrary code.  Hence, they should do stack checks against the
+        VM::m_softStackLimit limit.
+
+        Accordingly, the VM now provides 2 recursion check functions:
+
+        1. VM::isSafeToRecurseSoft() will do a stack check against VM::m_softStackLimit.
+           In addition, for C Loop builds, VM::isSafeToRecurseSoft() will also
+           check the CLoopStack against VM::m_cloopStackLimit.
+
+        2. VM::isSafeToRecurse() will do a stack check against VM::m_stackLimit.
+
+        Also added a promise-infinite-recursion-should-not-crash.js test.
+
+        * bytecompiler/BytecodeGenerator.h:
+        (JSC::BytecodeGenerator::emitNodeInTailPosition):
+        (JSC::BytecodeGenerator::emitNodeInConditionContext):
+        * interpreter/CLoopStack.cpp:
+        (JSC::CLoopStack::grow):
+        * interpreter/CLoopStack.h:
+        (JSC::CLoopStack::size):
+        * interpreter/CLoopStackInlines.h:
+        (JSC::CLoopStack::ensureCapacityFor):
+        (JSC::CLoopStack::isSafeToRecurse):
+        (JSC::CLoopStack::topOfFrameFor):
+        * interpreter/CachedCall.h:
+        (JSC::CachedCall::CachedCall):
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::execute):
+        (JSC::Interpreter::executeCall):
+        (JSC::Interpreter::executeConstruct):
+        * llint/LLIntSlowPaths.cpp:
+        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
+        * parser/Parser.cpp:
+        * runtime/Options.h:
+        * runtime/ProxyObject.cpp:
+        (JSC::performProxyGet):
+        (JSC::ProxyObject::performInternalMethodGetOwnProperty):
+        (JSC::ProxyObject::performHasProperty):
+        (JSC::ProxyObject::getOwnPropertySlotCommon):
+        (JSC::ProxyObject::performPut):
+        (JSC::performProxyCall):
+        (JSC::performProxyConstruct):
+        (JSC::ProxyObject::performDelete):
+        (JSC::ProxyObject::performPreventExtensions):
+        (JSC::ProxyObject::performIsExtensible):
+        (JSC::ProxyObject::performDefineOwnProperty):
+        (JSC::ProxyObject::performGetOwnPropertyNames):
+        (JSC::ProxyObject::performSetPrototype):
+        (JSC::ProxyObject::performGetPrototype):
+        * runtime/RegExp.cpp:
+        (JSC::RegExp::finishCreation):
+        (JSC::RegExp::compile):
+        (JSC::RegExp::compileMatchOnly):
+        * runtime/StringRecursionChecker.h:
+        (JSC::StringRecursionChecker::performCheck):
+        * runtime/VM.cpp:
+        (JSC::VM::setStackPointerAtVMEntry):
+        (JSC::VM::updateSoftReservedZoneSize):
+        (JSC::preCommitStackMemory):
+        (JSC::VM::updateStackLimits):
+        (JSC::VM::updateStackLimit): Deleted.
+        * runtime/VM.h:
+        (JSC::VM::stackLimit):
+        (JSC::VM::softStackLimit):
+        (JSC::VM::addressOfSoftStackLimit):
+        (JSC::VM::setCLoopStackLimit):
+        (JSC::VM::isSafeToRecurse):
+        (JSC::VM::lastStackTop):
+        (JSC::VM::setException):
+        * runtime/VMInlines.h:
+        (JSC::VM::ensureStackCapacityFor):
+        (JSC::VM::isSafeToRecurseSoft):
+        (JSC::VM::shouldTriggerTermination):
+        * tests/stress/promise-infinite-recursion-should-not-crash.js: Added.
+        (testPromise):
+        (promiseFunc):
+        * yarr/YarrPattern.cpp:
+
 2016-07-12  Per Arne Vollan  <pvollan@apple.com>
 
         [Win] Fix for build error when trying to version stamp dll.
index 52b31c1..5faf04f 100644 (file)
@@ -384,7 +384,7 @@ namespace JSC {
         {
             // Node::emitCode assumes that dst, if provided, is either a local or a referenced temporary.
             ASSERT(!dst || dst == ignoredResult() || !dst->isTemporary() || dst->refCount());
-            if (!m_vm->isSafeToRecurse()) {
+            if (UNLIKELY(!m_vm->isSafeToRecurse())) {
                 emitThrowExpressionTooDeepException();
                 return;
             }
@@ -411,7 +411,7 @@ namespace JSC {
         {
             // Node::emitCode assumes that dst, if provided, is either a local or a referenced temporary.
             ASSERT(!dst || dst == ignoredResult() || !dst->isTemporary() || dst->refCount());
-            if (!m_vm->isSafeToRecurse())
+            if (UNLIKELY(!m_vm->isSafeToRecurse()))
                 return emitThrowExpressionTooDeepException();
             return n->emitBytecode(*this, dst);
         }
@@ -428,7 +428,7 @@ namespace JSC {
 
         void emitNodeInConditionContext(ExpressionNode* n, Label* trueTarget, Label* falseTarget, FallThroughMode fallThroughMode)
         {
-            if (!m_vm->isSafeToRecurse()) {
+            if (UNLIKELY(!m_vm->isSafeToRecurse())) {
                 emitThrowExpressionTooDeepException();
                 return;
             }
index ba46516..9c80722 100644 (file)
@@ -88,7 +88,7 @@ bool CLoopStack::grow(Register* newTopOfStack)
     }
 
     // Compute the chunk size of additional memory to commit, and see if we
-    // have it is still within our budget. If not, we'll fail to grow and
+    // have it still within our budget. If not, we'll fail to grow and
     // return false.
     ptrdiff_t delta = reinterpret_cast<char*>(m_commitTop) - reinterpret_cast<char*>(newTopOfStackWithReservedZone);
     delta = WTF::roundUpToMultipleOf(commitSize(), delta);
index 5c0b69a..20fdbb1 100644 (file)
@@ -68,6 +68,7 @@ namespace JSC {
         size_t size() const { return highAddress() - lowAddress(); }
 
         void setSoftReservedZoneSize(size_t);
+        bool isSafeToRecurse() const;
 
         inline Register* topOfStack();
 
index 44e385b..65e9968 100644 (file)
@@ -42,6 +42,12 @@ inline bool CLoopStack::ensureCapacityFor(Register* newTopOfStack)
     return grow(newTopOfStack);
 }
 
+bool CLoopStack::isSafeToRecurse() const
+{
+    void* reservationLimit = reinterpret_cast<int8_t*>(reservationTop() + m_reservedZoneSizeInRegisters);
+    return !m_topCallFrame || (m_topCallFrame->topOfFrame() > reservationLimit);
+}
+
 inline Register* CLoopStack::topOfFrameFor(CallFrame* frame)
 {
     if (UNLIKELY(!frame))
index 30cad2a..593a58d 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009, 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2009, 2013, 2016 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -33,6 +33,7 @@
 #include "Interpreter.h"
 #include "ProtoCallFrame.h"
 #include "VMEntryScope.h"
+#include "VMInlines.h"
 
 namespace JSC {
     class CachedCall {
@@ -44,7 +45,7 @@ namespace JSC {
             , m_entryScope(callFrame->vm(), function->scope()->globalObject())
         {
             ASSERT(!function->isHostFunctionNonInline());
-            if (callFrame->vm().isSafeToRecurse()) {
+            if (UNLIKELY(callFrame->vm().isSafeToRecurseSoft())) {
                 m_arguments.resize(argumentCount);
                 m_closure = m_interpreter->prepareForRepeatCall(function->jsExecutable(), callFrame, &m_protoCallFrame, function, argumentCount + 1, function->scope(), m_arguments.data());
             } else
index c1d1de3..2b4c7c9 100644 (file)
@@ -828,7 +828,7 @@ JSValue Interpreter::execute(ProgramExecutable* program, CallFrame* callFrame, J
     if (vm.isCollectorBusy())
         return jsNull();
 
-    if (!vm.isSafeToRecurse())
+    if (UNLIKELY(!vm.isSafeToRecurseSoft()))
         return checkedReturn(throwStackOverflowError(callFrame));
 
     // First check if the "program" is actually just a JSON object. If so,
@@ -988,7 +988,7 @@ JSValue Interpreter::executeCall(CallFrame* callFrame, JSObject* function, CallT
     }
 
     VMEntryScope entryScope(vm, globalObject);
-    if (!vm.isSafeToRecurse())
+    if (UNLIKELY(!vm.isSafeToRecurseSoft()))
         return checkedReturn(throwStackOverflowError(callFrame));
 
     if (isJSCall) {
@@ -1050,7 +1050,7 @@ JSObject* Interpreter::executeConstruct(CallFrame* callFrame, JSObject* construc
     }
 
     VMEntryScope entryScope(vm, globalObject);
-    if (!vm.isSafeToRecurse())
+    if (UNLIKELY(!vm.isSafeToRecurseSoft()))
         return checkedReturn(throwStackOverflowError(callFrame));
 
     if (isJSConstruct) {
@@ -1147,7 +1147,7 @@ JSValue Interpreter::execute(EvalExecutable* eval, CallFrame* callFrame, JSValue
         return jsNull();
 
     VMEntryScope entryScope(vm, scope->globalObject());
-    if (!vm.isSafeToRecurse())
+    if (UNLIKELY(!vm.isSafeToRecurseSoft()))
         return checkedReturn(throwStackOverflowError(callFrame));        
 
     unsigned numVariables = eval->numVariables();
@@ -1248,7 +1248,7 @@ JSValue Interpreter::execute(ModuleProgramExecutable* executable, CallFrame* cal
         return jsNull();
 
     VMEntryScope entryScope(vm, scope->globalObject());
-    if (!vm.isSafeToRecurse())
+    if (UNLIKELY(!vm.isSafeToRecurseSoft()))
         return checkedReturn(throwStackOverflowError(callFrame));
 
     JSObject* compileError = executable->prepareForExecution(callFrame, nullptr, scope, CodeForCall);
index 65a1713..45f265c 100644 (file)
@@ -482,10 +482,9 @@ LLINT_SLOW_PATH_DECL(stack_check)
     dataLogF("Num callee registers = %u.\n", exec->codeBlock()->m_numCalleeLocals);
     dataLogF("Num vars = %u.\n", exec->codeBlock()->m_numVars);
 
-#if ENABLE(JIT)
-    dataLogF("Current end is at %p.\n", exec->vm().softStackLimit());
-#else
-    dataLogF("Current end is at %p.\n", exec->vm().cloopStackLimit());
+    dataLogF("Current OS stack end is at %p.\n", exec->vm().softStackLimit());
+#if !ENABLE(JIT)
+    dataLogF("Current C Loop stack end is at %p.\n", exec->vm().cloopStackLimit());
 #endif
 
 #endif
index 3ad0d2f..dccab2c 100644 (file)
@@ -55,7 +55,7 @@
 #define consumeOrFail(tokenType, ...) do { if (!consume(tokenType)) { handleErrorToken(); internalFailWithMessage(true, __VA_ARGS__); } } while (0)
 #define consumeOrFailWithFlags(tokenType, flags, ...) do { if (!consume(tokenType, flags)) { handleErrorToken(); internalFailWithMessage(true, __VA_ARGS__); } } while (0)
 #define matchOrFail(tokenType, ...) do { if (!match(tokenType)) { handleErrorToken(); internalFailWithMessage(true, __VA_ARGS__); } } while (0)
-#define failIfStackOverflow() do { if (!canRecurse()) failWithStackOverflow(); } while (0)
+#define failIfStackOverflow() do { if (UNLIKELY(!canRecurse())) failWithStackOverflow(); } while (0)
 #define semanticFail(...) do { internalFailWithMessage(false, __VA_ARGS__); } while (0)
 #define semanticFailIfTrue(cond, ...) do { if (cond) internalFailWithMessage(false, __VA_ARGS__); } while (0)
 #define semanticFailIfFalse(cond, ...) do { if (!(cond)) internalFailWithMessage(false, __VA_ARGS__); } while (0)
index 70365f0..38ce7ea 100644 (file)
@@ -113,8 +113,8 @@ typedef const char* optionString;
     v(bool, reportMustSucceedExecutableAllocations, false, Normal, nullptr) \
     \
     v(unsigned, maxPerThreadStackUsage, 4 * MB, Normal, "Max allowed stack usage by the VM") \
-    v(unsigned, softReservedZoneSize, 128 * KB, Normal, "The amount of stack JSC usually reserves for host code.") \
-    v(unsigned, reservedZoneSize, 64 * KB, Normal, "This is the amount of stack JSC guarantees for client and VM code.") \
+    v(unsigned, softReservedZoneSize, 128 * KB, Normal, "A buffer greater than reservedZoneSize that reserves space for stringifying exceptions.") \
+    v(unsigned, reservedZoneSize, 64 * KB, Normal, "The amount of stack space we guarantee to our clients (and to interal VM code that does not call out to clients).") \
     \
     v(bool, crashIfCantAllocateJITMemory, false, Normal, nullptr) \
     v(unsigned, jitMemoryReservationSize, 0, Normal, "Set this number to change the executable allocation size in ExecutableAllocatorFixedVMPool. (In bytes.)") \
index 6f45321..6ee486c 100644 (file)
@@ -33,6 +33,7 @@
 #include "ObjectConstructor.h"
 #include "SlotVisitorInlines.h"
 #include "StructureInlines.h"
+#include "VMInlines.h"
 
 // Note that this file is compile with -fno-optimize-sibling-calls because we rely on the machine stack
 // growing larger for throwing OOM errors for when we have an effectively cyclic prototype chain.
@@ -98,7 +99,7 @@ static const char* s_proxyAlreadyRevokedErrorMessage = "Proxy has already been r
 static JSValue performProxyGet(ExecState* exec, ProxyObject* proxyObject, JSValue receiver, PropertyName propertyName)
 {
     VM& vm = exec->vm();
-    if (UNLIKELY(!vm.isSafeToRecurse())) {
+    if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
         throwStackOverflowError(exec);
         return JSValue();
     }
@@ -167,7 +168,7 @@ bool ProxyObject::performGet(ExecState* exec, PropertyName propertyName, Propert
 bool ProxyObject::performInternalMethodGetOwnProperty(ExecState* exec, PropertyName propertyName, PropertySlot& slot)
 {
     VM& vm = exec->vm();
-    if (UNLIKELY(!vm.isSafeToRecurse())) {
+    if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
         throwStackOverflowError(exec);
         return false;
     }
@@ -273,7 +274,7 @@ bool ProxyObject::performInternalMethodGetOwnProperty(ExecState* exec, PropertyN
 bool ProxyObject::performHasProperty(ExecState* exec, PropertyName propertyName, PropertySlot& slot)
 {
     VM& vm = exec->vm();
-    if (UNLIKELY(!vm.isSafeToRecurse())) {
+    if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
         throwStackOverflowError(exec);
         return false;
     }
@@ -338,7 +339,7 @@ bool ProxyObject::performHasProperty(ExecState* exec, PropertyName propertyName,
 
 bool ProxyObject::getOwnPropertySlotCommon(ExecState* exec, PropertyName propertyName, PropertySlot& slot)
 {
-    if (UNLIKELY(!exec->vm().isSafeToRecurse())) {
+    if (UNLIKELY(!exec->vm().isSafeToRecurseSoft())) {
         throwStackOverflowError(exec);
         return false;
     }
@@ -376,7 +377,7 @@ template <typename PerformDefaultPutFunction>
 bool ProxyObject::performPut(ExecState* exec, JSValue putValue, JSValue thisValue, PropertyName propertyName, PerformDefaultPutFunction performDefaultPut)
 {
     VM& vm = exec->vm();
-    if (UNLIKELY(!vm.isSafeToRecurse())) {
+    if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
         throwStackOverflowError(exec);
         return false;
     }
@@ -468,7 +469,7 @@ bool ProxyObject::putByIndex(JSCell* cell, ExecState* exec, unsigned propertyNam
 static EncodedJSValue JSC_HOST_CALL performProxyCall(ExecState* exec)
 {
     VM& vm = exec->vm();
-    if (UNLIKELY(!vm.isSafeToRecurse())) {
+    if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
         throwStackOverflowError(exec);
         return JSValue::encode(JSValue());
     }
@@ -517,7 +518,7 @@ CallType ProxyObject::getCallData(JSCell* cell, CallData& callData)
 static EncodedJSValue JSC_HOST_CALL performProxyConstruct(ExecState* exec)
 {
     VM& vm = exec->vm();
-    if (UNLIKELY(!vm.isSafeToRecurse())) {
+    if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
         throwStackOverflowError(exec);
         return JSValue::encode(JSValue());
     }
@@ -572,7 +573,7 @@ template <typename DefaultDeleteFunction>
 bool ProxyObject::performDelete(ExecState* exec, PropertyName propertyName, DefaultDeleteFunction performDefaultDelete)
 {
     VM& vm = exec->vm();
-    if (UNLIKELY(!vm.isSafeToRecurse())) {
+    if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
         throwStackOverflowError(exec);
         return false;
     }
@@ -648,7 +649,7 @@ bool ProxyObject::deletePropertyByIndex(JSCell* cell, ExecState* exec, unsigned
 bool ProxyObject::performPreventExtensions(ExecState* exec)
 {
     VM& vm = exec->vm();
-    if (UNLIKELY(!vm.isSafeToRecurse())) {
+    if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
         throwStackOverflowError(exec);
         return false;
     }
@@ -700,7 +701,7 @@ bool ProxyObject::preventExtensions(JSObject* object, ExecState* exec)
 bool ProxyObject::performIsExtensible(ExecState* exec)
 {
     VM& vm = exec->vm();
-    if (UNLIKELY(!vm.isSafeToRecurse())) {
+    if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
         throwStackOverflowError(exec);
         return false;
     }
@@ -758,7 +759,7 @@ bool ProxyObject::isExtensible(JSObject* object, ExecState* exec)
 bool ProxyObject::performDefineOwnProperty(ExecState* exec, PropertyName propertyName, const PropertyDescriptor& descriptor, bool shouldThrow)
 {
     VM& vm = exec->vm();
-    if (UNLIKELY(!vm.isSafeToRecurse())) {
+    if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
         throwStackOverflowError(exec);
         return false;
     }
@@ -855,7 +856,7 @@ bool ProxyObject::defineOwnProperty(JSObject* object, ExecState* exec, PropertyN
 void ProxyObject::performGetOwnPropertyNames(ExecState* exec, PropertyNameArray& trapResult, EnumerationMode enumerationMode)
 {
     VM& vm = exec->vm();
-    if (UNLIKELY(!vm.isSafeToRecurse())) {
+    if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
         throwStackOverflowError(exec);
         return;
     }
@@ -1004,7 +1005,7 @@ bool ProxyObject::performSetPrototype(ExecState* exec, JSValue prototype, bool s
     ASSERT(prototype.isObject() || prototype.isNull());
 
     VM& vm = exec->vm();
-    if (UNLIKELY(!vm.isSafeToRecurse())) {
+    if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
         throwStackOverflowError(exec);
         return false;
     }
@@ -1068,7 +1069,7 @@ bool ProxyObject::setPrototype(JSObject* object, ExecState* exec, JSValue protot
 JSValue ProxyObject::performGetPrototype(ExecState* exec)
 {
     VM& vm = exec->vm();
-    if (UNLIKELY(!vm.isSafeToRecurse())) {
+    if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
         throwStackOverflowError(exec);
         return JSValue();
     }
index 6ff7772..c87751f 100644 (file)
@@ -222,7 +222,7 @@ RegExp::RegExp(VM& vm, const String& patternString, RegExpFlags flags)
 void RegExp::finishCreation(VM& vm)
 {
     Base::finishCreation(vm);
-    Yarr::YarrPattern pattern(m_patternString, m_flags, &m_constructionError, vm.softStackLimit());
+    Yarr::YarrPattern pattern(m_patternString, m_flags, &m_constructionError, vm.stackLimit());
     if (m_constructionError)
         m_state = ParseError;
     else
@@ -264,7 +264,7 @@ void RegExp::compile(VM* vm, Yarr::YarrCharSize charSize)
 {
     ConcurrentJITLocker locker(m_lock);
     
-    Yarr::YarrPattern pattern(m_patternString, m_flags, &m_constructionError, vm->softStackLimit());
+    Yarr::YarrPattern pattern(m_patternString, m_flags, &m_constructionError, vm->stackLimit());
     if (m_constructionError) {
         RELEASE_ASSERT_NOT_REACHED();
 #if COMPILER_QUIRK(CONSIDERS_UNREACHABLE_CODE)
@@ -317,7 +317,7 @@ void RegExp::compileMatchOnly(VM* vm, Yarr::YarrCharSize charSize)
 {
     ConcurrentJITLocker locker(m_lock);
     
-    Yarr::YarrPattern pattern(m_patternString, m_flags, &m_constructionError, vm->softStackLimit());
+    Yarr::YarrPattern pattern(m_patternString, m_flags, &m_constructionError, vm->stackLimit());
     if (m_constructionError) {
         RELEASE_ASSERT_NOT_REACHED();
 #if COMPILER_QUIRK(CONSIDERS_UNREACHABLE_CODE)
index 0f1990e..cca658d 100644 (file)
@@ -1,5 +1,5 @@
 /*
- *  Copyright (C) 2011 Apple Inc. All rights reserved.
+ *  Copyright (C) 2011, 2016 Apple Inc. All rights reserved.
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Lesser General Public
@@ -21,6 +21,7 @@
 #define StringRecursionChecker_h
 
 #include "Interpreter.h"
+#include "VMInlines.h"
 #include <wtf/StackStats.h>
 #include <wtf/WTFThreadData.h>
 
@@ -50,7 +51,7 @@ private:
 inline JSValue StringRecursionChecker::performCheck()
 {
     VM& vm = m_exec->vm();
-    if (!vm.isSafeToRecurse())
+    if (UNLIKELY(!vm.isSafeToRecurseSoft()))
         return throwStackOverflowError();
 
     bool alreadyVisited = false;
index b842ec3..1bddcee 100644 (file)
@@ -610,7 +610,7 @@ JSObject* VM::throwException(ExecState* exec, JSObject* error)
 void VM::setStackPointerAtVMEntry(void* sp)
 {
     m_stackPointerAtVMEntry = sp;
-    updateStackLimit();
+    updateStackLimits();
 }
 
 size_t VM::updateSoftReservedZoneSize(size_t softReservedZoneSize)
@@ -621,7 +621,7 @@ size_t VM::updateSoftReservedZoneSize(size_t softReservedZoneSize)
     interpreter->cloopStack().setSoftReservedZoneSize(softReservedZoneSize);
 #endif
 
-    updateStackLimit();
+    updateStackLimits();
 
     return oldSoftReservedZoneSize;
 }
@@ -651,21 +651,32 @@ static void preCommitStackMemory(void* stackLimit)
 }
 #endif
 
-inline void VM::updateStackLimit()
+inline void VM::updateStackLimits()
 {
 #if PLATFORM(WIN)
     void* lastSoftStackLimit = m_softStackLimit;
 #endif
 
+    size_t reservedZoneSize = Options::reservedZoneSize();
     if (m_stackPointerAtVMEntry) {
         ASSERT(wtfThreadData().stack().isGrowingDownward());
         char* startOfStack = reinterpret_cast<char*>(m_stackPointerAtVMEntry);
         m_softStackLimit = wtfThreadData().stack().recursionLimit(startOfStack, Options::maxPerThreadStackUsage(), m_currentSoftReservedZoneSize);
+        m_stackLimit = wtfThreadData().stack().recursionLimit(startOfStack, Options::maxPerThreadStackUsage(), reservedZoneSize);
     } else {
         m_softStackLimit = wtfThreadData().stack().recursionLimit(m_currentSoftReservedZoneSize);
+        m_stackLimit = wtfThreadData().stack().recursionLimit(reservedZoneSize);
     }
 
 #if PLATFORM(WIN)
+    // We only need to precommit stack memory dictated by the VM::m_softStackLimit limit.
+    // This is because VM::m_softStackLimit applies to stack usage by LLINT asm or JIT
+    // generated code which can allocate stack space that the C++ compiler does not know
+    // about. As such, we have to precommit that stack memory manually.
+    //
+    // In contrast, we do not need to worry about VM::m_stackLimit because that limit is
+    // used exclusively by C++ code, and the C++ compiler will automatically commit the
+    // needed stack pages.
     if (lastSoftStackLimit != m_softStackLimit)
         preCommitStackMemory(m_softStackLimit);
 #endif
index d398593..f0de151 100644 (file)
@@ -465,6 +465,7 @@ public:
     static size_t committedStackByteCount();
     inline bool ensureStackCapacityFor(Register* newTopOfStack);
 
+    void* stackLimit() { return m_stackLimit; }
     void* softStackLimit() { return m_softStackLimit; }
     void** addressOfSoftStackLimit() { return &m_softStackLimit; }
 #if !ENABLE(JIT)
@@ -472,12 +473,10 @@ public:
     void setCLoopStackLimit(void* limit) { m_cloopStackLimit = limit; }
 #endif
 
-    bool isSafeToRecurse(size_t neededStackInBytes = 0) const
+    inline bool isSafeToRecurseSoft() const;
+    bool isSafeToRecurse() const
     {
-        ASSERT(wtfThreadData().stack().isGrowingDownward());
-        int8_t* curr = reinterpret_cast<int8_t*>(&curr);
-        int8_t* limit = reinterpret_cast<int8_t*>(m_softStackLimit);
-        return curr >= limit && static_cast<size_t>(curr - limit) >= neededStackInBytes;
+        return isSafeToRecurse(m_stackLimit);
     }
 
     void* lastStackTop() { return m_lastStackTop; }
@@ -626,7 +625,14 @@ private:
     static VM*& sharedInstanceInternal();
     void createNativeThunk();
 
-    void updateStackLimit();
+    void updateStackLimits();
+
+    bool isSafeToRecurse(void* stackLimit) const
+    {
+        ASSERT(wtfThreadData().stack().isGrowingDownward());
+        void* curr = reinterpret_cast<void*>(&curr);
+        return curr >= stackLimit;
+    }
 
     void setException(Exception* exception)
     {
@@ -649,6 +655,7 @@ private:
 
     void* m_stackPointerAtVMEntry;
     size_t m_currentSoftReservedZoneSize;
+    void* m_stackLimit { nullptr };
     void* m_softStackLimit { nullptr };
 #if !ENABLE(JIT)
     void* m_cloopStackLimit { nullptr };
index 4b3bad5..d12250d 100644 (file)
@@ -47,6 +47,15 @@ bool VM::ensureStackCapacityFor(Register* newTopOfStack)
     
 }
 
+bool VM::isSafeToRecurseSoft() const
+{
+    bool safe = isSafeToRecurse(m_softStackLimit);
+#if !ENABLE(JIT)
+    safe = safe && interpreter->cloopStack().isSafeToRecurse();
+#endif
+    return safe;
+}
+
 bool VM::shouldTriggerTermination(ExecState* exec)
 {
     if (!watchdog())
diff --git a/Source/JavaScriptCore/tests/stress/promise-infinite-recursion-should-not-crash.js b/Source/JavaScriptCore/tests/stress/promise-infinite-recursion-should-not-crash.js
new file mode 100644 (file)
index 0000000..cb261f9
--- /dev/null
@@ -0,0 +1,14 @@
+//@ defaultNoSamplingProfilerRun
+
+// This should not crash
+var boundFunc;
+
+function testPromise(func) {
+    var p1 = new Promise(func);
+}
+function promiseFunc(resolve, reject) {
+    boundFunc();
+}
+
+boundFunc = testPromise.bind(null, promiseFunc);
+boundFunc();
index 68b8aad..34ab9a8 100644 (file)
@@ -580,7 +580,7 @@ public:
 
     bool setupAlternativeOffsets(PatternAlternative* alternative, unsigned currentCallFrameSize, unsigned initialInputPosition, unsigned& newCallFrameSize) WARN_UNUSED_RETURN
     {
-        if (!isSafeToRecurse())
+        if (UNLIKELY(!isSafeToRecurse()))
             return false;
 
         alternative->m_hasFixedSize = true;
@@ -682,7 +682,7 @@ public:
 
     bool setupDisjunctionOffsets(PatternDisjunction* disjunction, unsigned initialCallFrameSize, unsigned initialInputPosition, unsigned& callFrameSize) WARN_UNUSED_RETURN
     {
-        if (!isSafeToRecurse())
+        if (UNLIKELY(!isSafeToRecurse()))
             return false;
 
         if ((disjunction != m_pattern.m_body) && (disjunction->m_alternatives.size() > 1))
index 72b97d8..c367a16 100644 (file)
@@ -1,3 +1,24 @@
+2016-07-12  Mark Lam  <mark.lam@apple.com>
+
+        We should use different stack limits for stack checks from JS and host code.
+        https://bugs.webkit.org/show_bug.cgi?id=159442
+        <rdar://problem/26889188>
+
+        Reviewed by Geoffrey Garen.
+
+        In http://trac.webkit.org/r203067, we limited the amount of stack that tests will
+        run with to keep stack overflow tests sane.  Turns out, we also need to teach the
+        LayoutTestRelay to pass env vars over to the iOS simulator.  This is needed in
+        order to keep the js/regress-139548.html test happy with this patch.
+
+        Also fixed up run_webkit_tests.py to explicitly pass an int size value for the
+        JSC_maxPerThreadStackUsage option.  Otherwise, it will pass a float value.
+
+        * LayoutTestRelay/LayoutTestRelay/LTRelayController.m:
+        (-[LTRelayController _environmentVariables]):
+        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
+        (main):
+
 2016-07-12  Filip Pizlo  <fpizlo@apple.com>
 
         platformUserPreferredLanguages on Mac should not try to put the region into the language
index 4e8cc5f..942c7dc 100644 (file)
@@ -28,6 +28,7 @@
 #import "CoreSimulatorSPI.h"
 #import "LTPipeRelay.h"
 #import <AppKit/AppKit.h>
+#import <crt_externs.h>
 
 @interface LTRelayController ()
 @property (readonly, strong) dispatch_source_t standardInputDispatchSource;
             }
         }
 
+        for (char** envp = *_NSGetEnviron(); *envp; envp++) {
+            const char* env = *envp;
+            if (!strncmp("JSC_", env, 4) || !strncmp("__XPC_JSC_", env, 10)) {
+                const char* equal = strchr(env, '=');
+                if (!equal) {
+                    NSLog(@"Missing '=' in env var '%s'", env);
+                    continue;
+                }
+
+                static const size_t maxKeyLength = 256;
+                size_t keyLength = equal - env;
+                if (keyLength >= maxKeyLength) {
+                    NSLog(@"Env var '%s' is too long", env);
+                    continue;
+                }
+
+                char key[maxKeyLength];
+                strncpy(key, env, keyLength);
+                key[keyLength] = '\0';
+                const char* value = equal + 1;
+
+                NSString *nsKey = [NSString stringWithUTF8String:key];
+                NSString *nsValue = [NSString stringWithUTF8String:value];
+                [dictionary setObject:nsValue forKey:nsKey];
+            }
+        }
+
         environmentVariables = [dictionary copy];
     });
 
index d9f8900..ef256d6 100755 (executable)
@@ -75,7 +75,7 @@ def main(argv, stdout, stderr):
 
     try:
         # Force all tests to use a smaller stack so that stack overflow tests can run faster.
-        stackSizeInBytes = 1.5 * 1024 * 1024
+        stackSizeInBytes = int(1.5 * 1024 * 1024)
         options.additional_env_var.append('JSC_maxPerThreadStackUsage=' + str(stackSizeInBytes))
         options.additional_env_var.append('__XPC_JSC_maxPerThreadStackUsage=' + str(stackSizeInBytes))
         run_details = run(port, options, args, stderr)