Attachment 368305 Details for Bug 197306 – Patch

[patch] Patch

bug-197306-20190425232301.patch (text/plain), 6.75 KB, created by Yusuke Suzuki on 2019-04-25 23:23:02 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-04-25 23:23:02 PDT

Size: 6.75 KB

patch

obsolete

>Subversion Revision: 244666
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index a8ad475f4d4d35b1d6cb7d191dfbb1664a3c6a45..d3f6c642da14d8ba51cc312d0a08bf560b28c751 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-04-25  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] linkPolymorphicCall now does GC
>+        https://bugs.webkit.org/show_bug.cgi?id=197306
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Previously, we assumed that linkPolymorphicCall does not perform allocations. So we put CallVariant into a Vector<>.
>+        But now, WebAssemblyFunction's entrypoint generation can allocate JSToWasmICCallee and cause GC. Since CallLinkInfo
>+        does not hold these cells, they can be collected, and we will see dead cells in the middle of linkPolymorphicCall.
>+        We should defer GC in linkPolymorphicCall. DeferGC may cause GC and clear CallLinkInfo just before finishing
>+        linkPolymorphicCall, but it is OK since it just reverts the call.
>+
>+        * jit/Repatch.cpp:
>+        (JSC::linkPolymorphicCall):
>+
> 2019-04-25  Timothy Hatcher  <timothy@apple.com>
> 
>         Disable date and time inputs on iOSMac.
>diff --git a/Source/JavaScriptCore/jit/Repatch.cpp b/Source/JavaScriptCore/jit/Repatch.cpp
>index efca5f009549fbeb97a5793348e6f6cfc1a05c6c..a3524f22c4e65b03d0d82b90146ac0b3dca234a6 100644
>--- a/Source/JavaScriptCore/jit/Repatch.cpp
>+++ b/Source/JavaScriptCore/jit/Repatch.cpp
>@@ -968,6 +968,8 @@ void linkPolymorphicCall(
>     ASSERT(callerFrame->callee().isCell());
> 
>     VM& vm = callerFrame->vm();
>+    DeferGC deferGC(vm.heap);
>+
>     CodeBlock* callerCodeBlock = callerFrame->codeBlock();
>     bool isWebAssembly = isWebAssemblyToJSCallee(callerFrame->callee().asCell());
> 
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 6f73054b4dc9a090ea0aeb98dc29287c87bcf7a1..a5c46911988328a18f6ed60456dcb66210182033 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-04-25  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] linkPolymorphicCall now does GC
>+        https://bugs.webkit.org/show_bug.cgi?id=197306
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The test is sensitive to allocations and even adding $vm makes it non-reproducible.
>+        To add this test, we add `runWithoutBaseOption`.
>+
>+        * Scripts/run-jsc-stress-tests:
>+
> 2019-04-25  Timothy Hatcher  <timothy@apple.com>
> 
>         Disable date and time inputs on iOSMac.
>diff --git a/Tools/Scripts/run-jsc-stress-tests b/Tools/Scripts/run-jsc-stress-tests
>index 6eb29fd7f7edeef568aa02feac44c88258aaaee0..cfae9c95895fde534c01b4921e9c70195102f48e 100755
>--- a/Tools/Scripts/run-jsc-stress-tests
>+++ b/Tools/Scripts/run-jsc-stress-tests
>@@ -616,10 +616,18 @@ def runWithOutputHandler(kind, outputHandler, *options)
>     addRunCommand(kind, [pathToVM.to_s] + BASE_OPTIONS + $testSpecificRequiredOptions + options + [$benchmark.to_s], outputHandler, simpleErrorHandler)
> end
> 
>+def runWithOutputHandlerWithoutBaseOption(kind, outputHandler, *options)
>+    addRunCommand(kind, [pathToVM.to_s] + $testSpecificRequiredOptions + options + [$benchmark.to_s], outputHandler, simpleErrorHandler)
>+end
>+
> def run(kind, *options)
>     runWithOutputHandler(kind, silentOutputHandler, *options)
> end
> 
>+def runWithoutBaseOption(kind, *options)
>+    runWithOutputHandlerWithoutBaseOption(kind, silentOutputHandler, *options)
>+end
>+
> def runNoFTL(*optionalTestSpecificOptions)
>     run("no-ftl", *optionalTestSpecificOptions)
> end
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 315ecee71258ce3c3e14d46e1030b53c323ee8b4..63e0447a5419e5df71c5c8b3bad8f32e5666e28a 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-04-25  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] linkPolymorphicCall now does GC
>+        https://bugs.webkit.org/show_bug.cgi?id=197306
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/link-polymorphic-call-can-gc.js: Added.
>+        (module):
>+        (instance):
>+
> 2019-04-23  Saam Barati  <sbarati@apple.com>
> 
>         LICM incorrectly assumes it'll never insert a node which provably OSR exits
>diff --git a/JSTests/stress/link-polymorphic-call-can-gc.js b/JSTests/stress/link-polymorphic-call-can-gc.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..1bf6fe11bddc4030d3e2532092f15679cdae9994
>--- /dev/null
>+++ b/JSTests/stress/link-polymorphic-call-can-gc.js
>@@ -0,0 +1,65 @@
>+//@ skip if !$jitTests
>+//@ skip if !$isFTLPlatform
>+//@ runWithoutBaseOption("default", "--slowPathAllocsBetweenGCs=10", "--jitPolicyScale=0", "--useConcurrentJIT=0", "--validateExceptionChecks=1")
>+'use strict';
>+let o = {
>+    x0: ()=>0,
>+    x1: ()=>0,
>+    x2: ()=>0,
>+};
>+
>+function module(bytes) {
>+    let buffer = new ArrayBuffer(bytes.length);
>+    let view = new Uint8Array(buffer);
>+    for (let i = 0; i < bytes.length; ++i) {
>+        view[i] = bytes.charCodeAt(i);
>+    }
>+    return new WebAssembly.Module(buffer);
>+}
>+
>+function instance(bytes, imports = {o}) {
>+    return new WebAssembly.Instance(module(bytes), imports);
>+}
>+
>+function call(instance_, name) {
>+    return instance_.exports[name]();
>+}
>+
>+function exports(name, instance_) {
>+    return { [name]: instance_.exports };
>+}
>+
>+function run(action) {
>+    action();
>+}
>+
>+function fn1() {
>+}
>+function fn1() {
>+}
>+function fn1() {
>+}
>+function fn1() {
>+}
>+function fn1() {
>+}
>+function fn1() {
>+}
>+function fn1() {
>+}
>+function fn1() {
>+}
>+
>+try {
>+    (function f() {
>+        f();
>+    }());
>+} catch (e) {
>+}
>+
>+let $1 = instance('\0asm\x01\0\0\0\x01\x91\x80\x80\x80\0\x04`\0\0`\0\x01\x7F`\0\x01}`\x01\x7F\x01\x7F\x03\x87\x80\x80\x80\0\x06\0\x01\x01\x02\x03\x01\x05\x84\x80\x80\x80\0\x01\x01\x01\x01\x07Ã«\x80\x80\x80\0\x06\x0Fzero_everything\0\0\x12test_store_to_load\0\x01\x13test_redundant_load\0\x02\x0Ftest_dead_store\0\x03\x06malloc\0\x04\x0Fmalloc_aliasing\0\x05\n\xBD\x81\x80\x80\0\x06\x9E\x80\x80\x80\0\0A\0A\x006\x02\0A\x04A\x006\x02\0A\bA\x006\x02\0A\fA\x006\x02\0\x0B\x98\x80\x80\x80\0\0A\bA\x006\x02\0A\x05C\0\0\0\x808\x02\0A\b(\x02\0\x0B\xA2\x80\x80\x80\0\x01\x02\x7FA\b(\x02\0!\0A\x05A\x80\x80\x80\x80x6\x02\0A\b(\x02\0!\x01 \0 \x01j\x0B\x9F\x80\x80\x80\0\x01\x01}A\bA\xA3Ã\x8C\x99\x026\x02\0A\x0B*\x02\0!\0A\bA\x006\x02\0 \0\x0B\x84\x80\x80\x80\0\0A\x10\x0B\xA3\x80\x80\x80\0\x01\x02\x7FA\x04\x10\x04!\0A\x04\x10\x04!\x01 \0A*6\x02\0 \x01A+6\x02\0 \0(\x02\0\x0B');
>+
>+call($1, 'zero_everything');
>+run(() => call($1, 'zero_everything', []));
>+run(() => call(instance('\0asm\x01\0\0\0\x01\x88\x80\x80\x80\0\x02`\0\0`\0\x01}\x02\x96\x80\x80\x80\0\x01\x02$1\x0Ftest_dead_store\0\x01\x03\x82\x80\x80\x80\0\x01\0\x07\x87\x80\x80\x80\0\x01\x03run\0\x01\n\x9A\x80\x80\x80\0\x01\x94\x80\x80\x80\0\0\x02@\x10\0\xBCC#\0\0\0\xBCFE\r\0\x0F\x0B\0\x0B', exports('$1', $1)), 'run', []));
>+run(() => call($1, 'malloc_aliasing', []));

Flags:

saam: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 197306: 368305