Changeset 252239 in webkit


Ignore:
Timestamp:
Nov 8, 2019 8:58:49 AM (4 years ago)
Author:
mark.lam@apple.com
Message:

Add a stack overflow check in Yarr::ByteCompiler::emitDisjunction().
​https://bugs.webkit.org/show_bug.cgi?id=203936
<rdar://problem/56624724>

Reviewed by Saam Barati.

JSTests:

This issue originally manifested as incorrect-exception-assertion-in-operationRegExpExecNonGlobalOrSticky.js
failing on iOS devices due to its smaller stack. We adapted the original test
here using $vm.callWithStackSize() to reproduce the issue on x86_64 though it
has a much larger physical stack to work with. This new test will crash while
stepping on the guard page beyond the end of the stack if the fix is not applied.

  • stress/stack-overflow-in-yarr-byteCompile.js: Added.

Source/JavaScriptCore:

Basically, any functions below Yarr::ByteCompiler::compile() that recurses need
to check if it's safe to recurse before doing so. This patch adds the stack
checks in Yarr::ByteCompiler::compile() because it is the entry point to this
sub-system, and Yarr::ByteCompiler::emitDisjunction() because it is the only
function that recurses. All other functions called below compile() are either
leaf functions or have shallow stack usage. Hence, their stack needs are covered
by the DefaultReservedZone, and they do not need stack checks.

This patch also does the following:

  1. Added $vm.callWithStackSize() which can be used to call a test function near the end of the physical stack. This enables is to simulate the smaller stack size of more resource constrained devices.

$vm.callWithStackSize() uses inline asm to adjust the stack pointer and
does the callback via the JIT probe trampoline.

  1. Added the --disableOptionsFreezingForTesting to the jsc shell to make it possible to disable freezing of JSC options. $vm.callWithStackSize() relies on this to modify the VM's stack limits.
  1. Removed the inline modifier on VM::updateStackLimits() so that we can call it from $vm.callWithStackSize() as well. It is not a performance critical function and is rarely called.
  1. Added a JSDollarVMHelper class that other parts of the system can declare as a friend. This gives $vm a backdoor into the private functions and fields of classes for its debugging work. In this patch, we're only using it to access VM::updateVMStackLimits().
  • jsc.cpp:

(CommandLine::parseArguments):

  • runtime/VM.cpp:

(JSC::VM::updateStackLimits):

  • runtime/VM.h:
  • tools/JSDollarVM.cpp:

(JSC::JSDollarVMHelper::JSDollarVMHelper):
(JSC::JSDollarVMHelper::vmStackStart):
(JSC::JSDollarVMHelper::vmStackLimit):
(JSC::JSDollarVMHelper::vmSoftStackLimit):
(JSC::JSDollarVMHelper::updateVMStackLimits):
(JSC::callWithStackSizeProbeFunction):
(JSC::functionCallWithStackSize):
(JSC::JSDollarVM::finishCreation):
(IGNORE_WARNINGS_BEGIN): Deleted.

  • yarr/YarrInterpreter.cpp:

(JSC::Yarr::ByteCompiler::compile):
(JSC::Yarr::ByteCompiler::emitDisjunction):
(JSC::Yarr::ByteCompiler::isSafeToRecurse):

Source/WTF:

  1. Add a StackCheck utility class so that we don't have to keep reinventing this every time we need to add stack checking somewhere.
  2. Rename some arguments and constants in StackBounds to be more descriptive of what they actually are.
  • WTF.xcodeproj/project.pbxproj:
  • wtf/CMakeLists.txt:
  • wtf/StackBounds.h:

(WTF::StackBounds::recursionLimit const):

  • wtf/StackCheck.h: Added.

(WTF::StackCheck::StackCheck):
(WTF::StackCheck::isSafeToRecurse):

Location:
trunk
Files:
2 added
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r252196 r252239  
     12019-11-07  Mark Lam  <mark.lam@apple.com>
     2
     3        Add a stack overflow check in Yarr::ByteCompiler::emitDisjunction().
     4        https://bugs.webkit.org/show_bug.cgi?id=203936
     5        <rdar://problem/56624724>
     6
     7        Reviewed by Saam Barati.
     8
     9        This issue originally manifested as incorrect-exception-assertion-in-operationRegExpExecNonGlobalOrSticky.js
     10        failing on iOS devices due to its smaller stack.  We adapted the original test
     11        here using $vm.callWithStackSize() to reproduce the issue on x86_64 though it
     12        has a much larger physical stack to work with.  This new test will crash while
     13        stepping on the guard page beyond the end of the stack if the fix is not applied.
     14
     15        * stress/stack-overflow-in-yarr-byteCompile.js: Added.
     16
    1172019-11-07  Tuomas Karkkainen  <tuomas.webkit@apple.com>
    218

  • trunk/Source/JavaScriptCore/ChangeLog

    r252231 r252239  
     12019-11-07  Mark Lam  <mark.lam@apple.com>
     2
     3        Add a stack overflow check in Yarr::ByteCompiler::emitDisjunction().
     4        https://bugs.webkit.org/show_bug.cgi?id=203936
     5        <rdar://problem/56624724>
     6
     7        Reviewed by Saam Barati.
     8
     9        Basically, any functions below Yarr::ByteCompiler::compile() that recurses need
     10        to check if it's safe to recurse before doing so.  This patch adds the stack
     11        checks in Yarr::ByteCompiler::compile() because it is the entry point to this
     12        sub-system, and Yarr::ByteCompiler::emitDisjunction() because it is the only
     13        function that recurses.  All other functions called below compile() are either
     14        leaf functions or have shallow stack usage.  Hence, their stack needs are covered
     15        by the DefaultReservedZone, and they do not need stack checks.
     16
     17        This patch also does the following:
     18        1. Added $vm.callWithStackSize() which can be used to call a test function near
     19           the end of the physical stack.  This enables is to simulate the smaller stack
     20           size of more resource constrained devices.
     21
     22           $vm.callWithStackSize() uses inline asm to adjust the stack pointer and
     23           does the callback via the JIT probe trampoline.
     24
     25        2. Added the --disableOptionsFreezingForTesting to the jsc shell to make it
     26           possible to disable freezing of JSC options.  $vm.callWithStackSize() relies
     27           on this to modify the VM's stack limits.
     28
     29        3. Removed the inline modifier on VM::updateStackLimits() so that we can call it
     30           from $vm.callWithStackSize() as well.  It is not a performance critical
     31           function and is rarely called.
     32
     33        4. Added a JSDollarVMHelper class that other parts of the system can declare as
     34           a friend.  This gives $vm a backdoor into the private functions and fields of
     35           classes for its debugging work.  In this patch, we're only using it to access
     36           VM::updateVMStackLimits().
     37
     38        * jsc.cpp:
     39        (CommandLine::parseArguments):
     40        * runtime/VM.cpp:
     41        (JSC::VM::updateStackLimits):
     42        * runtime/VM.h:
     43        * tools/JSDollarVM.cpp:
     44        (JSC::JSDollarVMHelper::JSDollarVMHelper):
     45        (JSC::JSDollarVMHelper::vmStackStart):
     46        (JSC::JSDollarVMHelper::vmStackLimit):
     47        (JSC::JSDollarVMHelper::vmSoftStackLimit):
     48        (JSC::JSDollarVMHelper::updateVMStackLimits):
     49        (JSC::callWithStackSizeProbeFunction):
     50        (JSC::functionCallWithStackSize):
     51        (JSC::JSDollarVM::finishCreation):
     52        (IGNORE_WARNINGS_BEGIN): Deleted.
     53        * yarr/YarrInterpreter.cpp:
     54        (JSC::Yarr::ByteCompiler::compile):
     55        (JSC::Yarr::ByteCompiler::emitDisjunction):
     56        (JSC::Yarr::ByteCompiler::isSafeToRecurse):
     57
    1582019-11-07  Tadeu Zagallo  <tzagallo@apple.com>
    259

  • trunk/Source/JavaScriptCore/jsc.cpp

    r251691 r252239  
    28922892            continue;
    28932893        }
     2894        if (!strcmp(arg, "--disableOptionsFreezingForTesting")) {
     2895            Config::disableFreezingForTesting();
     2896            continue;
     2897        }
    28942898
    28952899        static const char* timeoutMultiplierOptStr = "--timeoutMultiplier=";

  • trunk/Source/JavaScriptCore/runtime/VM.cpp

    r252177 r252239  
    882882#endif
    883883
    884 inline void VM::updateStackLimits()
     884void VM::updateStackLimits()
    885885{
    886886#if OS(WINDOWS)

  • trunk/Source/JavaScriptCore/runtime/VM.h

    r252177 r252239  
    10941094    friend class CatchScope;
    10951095    friend class ExceptionScope;
     1096    friend class JSDollarVMHelper;
    10961097    friend class ThrowScope;
    10971098    friend class VMTraps;

  • trunk/Source/JavaScriptCore/tools/JSDollarVM.cpp

    r251736 r252239  
    3232#include "DOMJITGetterSetter.h"
    3333#include "Debugger.h"
     34#include "Error.h"
    3435#include "FrameTracers.h"
    3536#include "FunctionCodeBlock.h"
    … …  
    4445#include "Options.h"
    4546#include "Parser.h"
     47#include "ProbeContext.h"
    4648#include "ShadowChicken.h"
    4749#include "Snippet.h"
    … …  
    6466
    6567IGNORE_WARNINGS_BEGIN("frame-address")
     68
     69extern "C" void ctiMasmProbeTrampoline();
     70
     71namespace JSC {
     72
     73// This class is only here as a simple way to grant JSDollarVM friend privileges
     74// to all the classes that it needs special access to.
     75class JSDollarVMHelper {
     76public:
     77    JSDollarVMHelper(VM& vm)
     78        : m_vm(vm)
     79    { }
     80
     81    void updateVMStackLimits() { return m_vm.updateStackLimits(); };
     82
     83private:
     84    VM& m_vm;
     85};
     86
     87} // namespace JSC
    6688
    6789namespace {
    … …  
    19691991}
    19701992
     1993// Calls the specified test function after adjusting the stack to have the specified
     1994// remaining size from the end of the physical stack.
     1995// Usage: $vm.callWithStackSize(funcToCall, desiredStackSize)
     1996//
     1997// This function will only work in test configurations, specifically, only if JSC
     1998// options are not frozen. For the jsc shell, the --disableOptionsFreezingForTesting
     1999// argument needs to be passed in on the command line.
     2000
     2001#if ENABLE(MASM_PROBE)
     2002static void callWithStackSizeProbeFunction(Probe::State* state)
     2003{
     2004    JSGlobalObject* globalObject = bitwise_cast<JSGlobalObject*>(state->arg);
     2005    JSFunction* function = bitwise_cast<JSFunction*>(state->probeFunction);
     2006    state->initializeStackFunction = nullptr;
     2007    state->initializeStackArg = nullptr;
     2008
     2009    DollarVMAssertScope assertScope;
     2010    VM& vm = globalObject->vm();
     2011
     2012    CallData callData;
     2013    CallType callType = getCallData(vm, function, callData);
     2014    MarkedArgumentBuffer args;
     2015    call(globalObject, function, callType, callData, jsUndefined(), args);
     2016}
     2017#endif // ENABLE(MASM_PROBE)
     2018
     2019static EncodedJSValue JSC_HOST_CALL functionCallWithStackSize(JSGlobalObject* globalObject, CallFrame* callFrame)
     2020{
     2021    DollarVMAssertScope assertScope;
     2022    VM& vm = globalObject->vm();
     2023    JSLockHolder lock(vm);
     2024    auto throwScope = DECLARE_THROW_SCOPE(vm);
     2025
     2026#if OS(DARWIN) && CPU(X86_64)
     2027    constexpr bool isSupportedByPlatform = true;
     2028#else
     2029    constexpr bool isSupportedByPlatform = false;
     2030#endif
     2031
     2032    if (!isSupportedByPlatform)
     2033        return throwVMError(globalObject, throwScope, "Not supported for this platform");
     2034
     2035#if ENABLE(MASM_PROBE)
     2036    if (g_jscConfig.isPermanentlyFrozen || !g_jscConfig.disabledFreezingForTesting)
     2037        return throwVMError(globalObject, throwScope, "Options are frozen");
     2038
     2039    if (callFrame->argumentCount() < 2)
     2040        return throwVMError(globalObject, throwScope, "Invalid number of arguments");
     2041    JSValue arg0 = callFrame->argument(0);
     2042    JSValue arg1 = callFrame->argument(1);
     2043    if (!arg0.isFunction(vm))
     2044        return throwVMError(globalObject, throwScope, "arg0 should be a function");
     2045    if (!arg1.isNumber())
     2046        return throwVMError(globalObject, throwScope, "arg1 should be a number");
     2047
     2048    JSFunction* function = jsCast<JSFunction*>(arg0.toObject(globalObject));
     2049    size_t desiredStackSize = arg1.asNumber();
     2050
     2051    const StackBounds& bounds = Thread::current().stack();
     2052    uint8_t* currentStackPosition = bitwise_cast<uint8_t*>(currentStackPointer());
     2053    uint8_t* end = bitwise_cast<uint8_t*>(bounds.end());
     2054    uint8_t* desiredStart = end + desiredStackSize;
     2055    if (desiredStart >= currentStackPosition)
     2056        return throwVMError(globalObject, throwScope, "Unable to setup desired stack size");
     2057
     2058    JSDollarVMHelper helper(vm);
     2059
     2060    unsigned originalMaxPerThreadStackUsage = Options::maxPerThreadStackUsage();
     2061    void* originalVMSoftStackLimit = vm.softStackLimit();
     2062    void* originalVMStackLimit = vm.stackLimit();
     2063
     2064    // This is a hack to make the VM think it's stack limits are near the end
     2065    // of the physical stack.
     2066    uint8_t* vmStackStart = bitwise_cast<uint8_t*>(vm.stackPointerAtVMEntry());
     2067    uint8_t* vmStackEnd = vmStackStart - originalMaxPerThreadStackUsage;
     2068    ptrdiff_t sizeDiff = vmStackEnd - end;
     2069    RELEASE_ASSERT(sizeDiff >= 0);
     2070    RELEASE_ASSERT(sizeDiff < UINT_MAX);
     2071
     2072    Options::maxPerThreadStackUsage() = originalMaxPerThreadStackUsage + sizeDiff;
     2073    helper.updateVMStackLimits();
     2074
     2075#if OS(DARWIN) && CPU(X86_64)
     2076    __asm__ volatile (
     2077        "subq %[sizeDiff], %%rsp" "\n"
     2078        "pushq %%rax" "\n"
     2079        "pushq %%rcx" "\n"
     2080        "pushq %%rdx" "\n"
     2081        "pushq %%rbx" "\n"
     2082        "callq *%%rax" "\n"
     2083        "addq %[sizeDiff], %%rsp" "\n"
     2084        :
     2085        : "a" (ctiMasmProbeTrampoline)
     2086        , "c" (callWithStackSizeProbeFunction)
     2087        , "d" (function)
     2088        , "b" (globalObject)
     2089        , [sizeDiff] "rm" (sizeDiff)
     2090        : "memory"
     2091    );
     2092#else
     2093    UNUSED_PARAM(function);
     2094    UNUSED_PARAM(callWithStackSizeProbeFunction);
     2095#endif // OS(DARWIN) && CPU(X86_64)
     2096
     2097    Options::maxPerThreadStackUsage() = originalMaxPerThreadStackUsage;
     2098    helper.updateVMStackLimits();
     2099    RELEASE_ASSERT(vm.softStackLimit() == originalVMSoftStackLimit);
     2100    RELEASE_ASSERT(vm.stackLimit() == originalVMStackLimit);
     2101
     2102    return encodedJSUndefined();
     2103
     2104#else // not ENABLE(MASM_PROBE)
     2105    UNUSED_PARAM(callFrame);
     2106    return throwVMError(globalObject, throwScope, "Not supported for this platform");
     2107#endif // ENABLE(MASM_PROBE)
     2108}
     2109
    19712110// Creates a new global object.
    19722111// Usage: $vm.createGlobalObject()
    … …  
    26302769    addFunction(vm, "haveABadTime", functionHaveABadTime, 1);
    26312770    addFunction(vm, "isHavingABadTime", functionIsHavingABadTime, 1);
     2771
     2772    addFunction(vm, "callWithStackSize", functionCallWithStackSize, 2);
    26322773
    26332774    addFunction(vm, "createGlobalObject", functionCreateGlobalObject, 0);

  • trunk/Source/JavaScriptCore/yarr/YarrInterpreter.cpp

    r248846 r252239  
    3535#include <wtf/CheckedArithmetic.h>
    3636#include <wtf/DataLog.h>
     37#include <wtf/StackCheck.h>
    3738#include <wtf/text/CString.h>
    3839#include <wtf/text/WTFString.h>
    … …  
    16951696    std::unique_ptr<BytecodePattern> compile(BumpPointerAllocator* allocator, ConcurrentJSLock* lock, ErrorCode& errorCode)
    16961697    {
     1698        if (UNLIKELY(!isSafeToRecurse())) {
     1699            errorCode = ErrorCode::TooManyDisjunctions;
     1700            return nullptr;
     1701        }
     1702
    16971703        regexBegin(m_pattern.m_numSubpatterns, m_pattern.m_body->m_callFrameSize, m_pattern.m_body->m_alternatives[0]->onceThrough());
    16981704        if (auto error = emitDisjunction(m_pattern.m_body, 0, 0)) {
    … …  
    20292035    Optional<ErrorCode> WARN_UNUSED_RETURN emitDisjunction(PatternDisjunction* disjunction, Checked<unsigned, RecordOverflow> inputCountAlreadyChecked, unsigned parenthesesInputCountAlreadyChecked)
    20302036    {
     2037        if (UNLIKELY(!isSafeToRecurse()))
     2038            return ErrorCode::TooManyDisjunctions;
     2039
    20312040        for (unsigned alt = 0; alt < disjunction->m_alternatives.size(); ++alt) {
    20322041            auto currentCountAlreadyChecked = inputCountAlreadyChecked;
    … …  
    24062415
    24072416private:
     2417    inline bool isSafeToRecurse() { return m_stackCheck.isSafeToRecurse(); }
     2418
    24082419    YarrPattern& m_pattern;
    24092420    std::unique_ptr<ByteDisjunction> m_bodyDisjunction;
     2421    StackCheck m_stackCheck;
    24102422    unsigned m_currentAlternativeIndex { 0 };
    24112423    Vector<ParenthesesStackEntry> m_parenthesesStack;

  • trunk/Source/WTF/ChangeLog

    r252177 r252239  
     12019-11-07  Mark Lam  <mark.lam@apple.com>
     2
     3        Add a stack overflow check in Yarr::ByteCompiler::emitDisjunction().
     4        https://bugs.webkit.org/show_bug.cgi?id=203936
     5        <rdar://problem/56624724>
     6
     7        Reviewed by Saam Barati.
     8
     9        1. Add a StackCheck utility class so that we don't have to keep reinventing this
     10           every time we need to add stack checking somewhere.
     11        2. Rename some arguments and constants in StackBounds to be more descriptive of
     12           what they actually are.
     13
     14        * WTF.xcodeproj/project.pbxproj:
     15        * wtf/CMakeLists.txt:
     16        * wtf/StackBounds.h:
     17        (WTF::StackBounds::recursionLimit const):
     18        * wtf/StackCheck.h: Added.
     19        (WTF::StackCheck::StackCheck):
     20        (WTF::StackCheck::isSafeToRecurse):
     21
    1222019-11-06  Mark Lam  <mark.lam@apple.com>
    223

  • trunk/Source/WTF/WTF.xcodeproj/project.pbxproj

    r252068 r252239  
    695695                FE05FAE61FDB214300093230 /* DumbPtrTraits.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DumbPtrTraits.h; sourceTree = "<group>"; };
    696696                FE05FAFE1FE5007500093230 /* WTFAssertions.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WTFAssertions.cpp; sourceTree = "<group>"; };
     697                FE1D6D87237401CD007A5C26 /* StackCheck.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = StackCheck.h; sourceTree = "<group>"; };
    697698                FE1E2C392240C05400F6B729 /* PtrTag.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PtrTag.cpp; sourceTree = "<group>"; };
    698699                FE1E2C41224187C600F6B729 /* PlatformRegisters.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PlatformRegisters.cpp; sourceTree = "<group>"; };
    … …  
    11531154                                A8A4730E151A825B004123FF /* StackBounds.cpp */,
    11541155                                A8A4730F151A825B004123FF /* StackBounds.h */,
     1156                                FE1D6D87237401CD007A5C26 /* StackCheck.h */,
    11551157                                FEEA4DF8216D7BE400AC0602 /* StackPointer.cpp */,
    11561158                                FEEA4DF7216D608C00AC0602 /* StackPointer.h */,

  • trunk/Source/WTF/wtf/CMakeLists.txt

    r252068 r252239  
    216216    Spectrum.h
    217217    StackBounds.h
     218    StackCheck.h
    218219    StackPointer.h
    219220    StackShot.h

  • trunk/Source/WTF/wtf/StackBounds.h

    r252177 r252239  
    3535class StackBounds {
    3636    WTF_MAKE_FAST_ALLOCATED;
     37public:
    3738
    3839    // This 64k number was picked because a sampling of stack usage differences
    … …  
    4142    // conservative availability value that is not too large but comfortably
    4243    // exceeds 27k with some buffer for error.
    43     static constexpr size_t s_defaultAvailabilityDelta = 64 * 1024;
     44    static constexpr size_t DefaultReservedZone = 64 * 1024;
    4445
    45 public:
    4646    static constexpr StackBounds emptyBounds() { return StackBounds(); }
    4747
    … …  
    8383    }
    8484
    85     void* recursionLimit(size_t minAvailableDelta = s_defaultAvailabilityDelta) const
     85    void* recursionLimit(size_t minReservedZone = DefaultReservedZone) const
    8686    {
    8787        checkConsistency();
    88         return static_cast<char*>(m_bound) + minAvailableDelta;
     88        return static_cast<char*>(m_bound) + minReservedZone;
    8989    }
    9090
Note: See TracChangeset for help on using the changeset viewer.