Attachment 373560 Details for Bug 199541 – Patch

[patch] Patch

199541.patch (text/plain), 6.09 KB, created by Michael Saboff on 2019-07-05 20:12:17 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Michael Saboff

Created: 2019-07-05 20:12:17 PDT

Size: 6.09 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 247190)
>+++ JSTests/ChangeLog	(working copy)
>@@ -1,3 +1,17 @@
>+2019-07-05  Michael Saboff  <msaboff@apple.com>
>+
>+        switch(String) needs to check for exceptions when resolving the string
>+        https://bugs.webkit.org/show_bug.cgi?id=199541
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        New tests.
>+
>+        * stress/switch-string-oom.js: Added.
>+        (test):
>+        (testLowerTiers):
>+        (testFTL):
>+
> 2019-07-05  Mark Lam  <mark.lam@apple.com>
> 
>         ArgumentsEliminationPhase::eliminateCandidatesThatInterfere() should not decrement nodeIndex pass zero.
>Index: JSTests/stress/switch-string-oom.js
>===================================================================
>--- JSTests/stress/switch-string-oom.js	(nonexistent)
>+++ JSTests/stress/switch-string-oom.js	(working copy)
>@@ -0,0 +1,52 @@
>+//@ requireOptions("--jitPolicyScale=0", "--useConcurrentJIT=0")
>+// This tests that when a switch(String) converts the String argument, it properly handles OOM
>+
>+function test(createOOMString)
>+{
>+    var str = String.fromCharCode(365);
>+    if (createOOMString)
>+        str = str.padEnd(2147483644, '123');
>+
>+    switch (str) {
>+    case "one":
>+        throw "Case \"one\", dhouldn't get here";
>+        break;
>+    case "two": 
>+        throw "Case \"two\", shouldn't get here";
>+        break;
>+    case "three":
>+        throw "Case \"three\", shouldn't get here";
>+        break;
>+    default:
>+        if (createOOMString)
>+            throw "Default case, shouldn't get here";
>+        break;
>+    }
>+}
>+
>+function testLowerTiers()
>+{
>+    for (let i = 0; i < 200; i++) {
>+        try {
>+            test(true);
>+        } catch(e) {
>+            if (e != "Error: Out of memory")
>+                throw "Unexpecte error: \"" + e + "\"";
>+        }
>+    }
>+}
>+
>+function testFTL()
>+{
>+    for (let i = 0; i < 1000; i++) {
>+        try {
>+            test(i >= 50);
>+        } catch(e) {
>+            if (e != "Error: Out of memory")
>+                throw "Unexpecte error: \"" + e + "\"";
>+        }
>+    }
>+}
>+
>+testLowerTiers();
>+testFTL();
>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 247190)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,17 @@
>+2019-07-05  Michael Saboff  <msaboff@apple.com>
>+
>+        switch(String) needs to check for exceptions when resolving the string
>+        https://bugs.webkit.org/show_bug.cgi?id=199541
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added exception checks for resolved Strings in switch processing for all tiers.
>+
>+        * dfg/DFGOperations.cpp:
>+        * jit/JITOperations.cpp:
>+        * llint/LLIntSlowPaths.cpp:
>+        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
>+
> 2019-07-05  Mark Lam  <mark.lam@apple.com>
> 
>         ArgumentsEliminationPhase::eliminateCandidatesThatInterfere() should not decrement nodeIndex pass zero.
>Index: Source/JavaScriptCore/dfg/DFGOperations.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOperations.cpp	(revision 247190)
>+++ Source/JavaScriptCore/dfg/DFGOperations.cpp	(working copy)
>@@ -2462,8 +2462,13 @@ char* JIT_OPERATION operationSwitchStrin
> {
>     VM& vm = exec->vm();
>     NativeCallFrameTracer tracer(&vm, exec);
>+    auto throwScope = DECLARE_THROW_SCOPE(vm);
>+
>+    StringImpl* strImpl = string->value(exec).impl();
> 
>-    return exec->codeBlock()->stringSwitchJumpTable(tableIndex).ctiForValue(string->value(exec).impl()).executableAddress<char*>();
>+    RETURN_IF_EXCEPTION(throwScope, nullptr);
>+
>+    return exec->codeBlock()->stringSwitchJumpTable(tableIndex).ctiForValue(strImpl).executableAddress<char*>();
> }
> 
> int32_t JIT_OPERATION operationSwitchStringAndGetBranchOffset(ExecState* exec, size_t tableIndex, JSString* string)
>@@ -2471,7 +2476,13 @@ int32_t JIT_OPERATION operationSwitchStr
>     VM& vm = exec->vm();
>     NativeCallFrameTracer tracer(&vm, exec);
> 
>-    return exec->codeBlock()->stringSwitchJumpTable(tableIndex).offsetForValue(string->value(exec).impl(), std::numeric_limits<int32_t>::min());
>+    auto throwScope = DECLARE_THROW_SCOPE(vm);
>+
>+    StringImpl* strImpl = string->value(exec).impl();
>+
>+    RETURN_IF_EXCEPTION(throwScope, 0);
>+
>+    return exec->codeBlock()->stringSwitchJumpTable(tableIndex).offsetForValue(strImpl, std::numeric_limits<int32_t>::min());
> }
> 
> uintptr_t JIT_OPERATION operationCompareStringImplLess(StringImpl* a, StringImpl* b)
>Index: Source/JavaScriptCore/jit/JITOperations.cpp
>===================================================================
>--- Source/JavaScriptCore/jit/JITOperations.cpp	(revision 247190)
>+++ Source/JavaScriptCore/jit/JITOperations.cpp	(working copy)
>@@ -2317,7 +2317,12 @@ char* JIT_OPERATION operationSwitchStrin
>     StringJumpTable& jumpTable = codeBlock->stringSwitchJumpTable(tableIndex);
> 
>     if (key.isString()) {
>+        auto throwScope = DECLARE_THROW_SCOPE(vm);
>+
>         StringImpl* value = asString(key)->value(exec).impl();
>+
>+        RETURN_IF_EXCEPTION(throwScope, nullptr);
>+
>         result = jumpTable.ctiForValue(value).executableAddress();
>     } else
>         result = jumpTable.ctiDefault.executableAddress();
>Index: Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
>===================================================================
>--- Source/JavaScriptCore/llint/LLIntSlowPaths.cpp	(revision 247190)
>+++ Source/JavaScriptCore/llint/LLIntSlowPaths.cpp	(working copy)
>@@ -1317,8 +1317,13 @@ LLINT_SLOW_PATH_DECL(slow_path_switch_st
>     if (!scrutinee.isString())
>         JUMP_TO(defaultOffset);
>     else {
>+        StringImpl* scrutineeStringImpl = asString(scrutinee)->value(exec).impl();
>+
>+        LLINT_CHECK_EXCEPTION();
>+
>         CodeBlock* codeBlock = exec->codeBlock();
>-        JUMP_TO(codeBlock->stringSwitchJumpTable(bytecode.m_tableIndex).offsetForValue(asString(scrutinee)->value(exec).impl(), defaultOffset));
>+
>+        JUMP_TO(codeBlock->stringSwitchJumpTable(bytecode.m_tableIndex).offsetForValue(scrutineeStringImpl, defaultOffset));
>     }
>     LLINT_END();
> }

Flags:

mark.lam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 199541: 373560