Attachment 368401 Details for Bug 197311 – Patch

[patch] Patch

bug-197311-20190427184654.patch (text/plain), 4.41 KB, created by Tadeu Zagallo on 2019-04-27 09:46:56 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2019-04-27 09:46:56 PDT

Size: 4.41 KB

patch

obsolete

>Subversion Revision: 244685
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index f6c51221c502f51a3314a45ab4f0c82606e7ddc0..b73f9002017552e9fa3e6891f523a699959c4f48 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,21 @@
>+2019-04-26  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        tryCachePutByID should not crash if target offset changes
>+        https://bugs.webkit.org/show_bug.cgi?id=197311
>+        <rdar://problem/48033612>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When tryCachePutID is called with a setter, it asserts that after performing
>+        the put, if the property remains at the same depth in prototype chain, it
>+        should also be at the same offset. However, that assertion does not hold if
>+        the setter deletes properties of the target prototype object and causes the
>+        structure to be flattened after the deletion. Instead of asserting, we just
>+        use the updated offset instead.
>+
>+        * jit/Repatch.cpp:
>+        (JSC::tryCachePutByID):
>+
> 2019-04-25  Fujii Hironori  <Hironori.Fujii@sony.com>
> 
>         Unreviewed, rolling out r244669.
>diff --git a/Source/JavaScriptCore/jit/Repatch.cpp b/Source/JavaScriptCore/jit/Repatch.cpp
>index efca5f009549fbeb97a5793348e6f6cfc1a05c6c..022a37f656b91392a7229119001d17da6302042a 100644
>--- a/Source/JavaScriptCore/jit/Repatch.cpp
>+++ b/Source/JavaScriptCore/jit/Repatch.cpp
>@@ -576,9 +576,10 @@ static InlineCacheAction tryCachePutByID(ExecState* exec, JSValue baseValue, Str
>                         if (!conditionSet.isValid())
>                             return GiveUpOnCache;
> 
>-                        PropertyOffset conditionSetOffset = conditionSet.slotBaseCondition().offset();
>-                        if (UNLIKELY(offset != conditionSetOffset))
>-                            CRASH_WITH_INFO(offset, conditionSetOffset, slot.base()->type(), baseCell->type(), conditionSet.size());
>+                        if (!(conditionSet.slotBaseCondition().attributes() & PropertyAttribute::Accessor))
>+                            return GiveUpOnCache;
>+
>+                        offset = conditionSet.slotBaseCondition().offset();
>                     }
> 
>                 }
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 315ecee71258ce3c3e14d46e1030b53c323ee8b4..3ba199447e4d2b523840f7d5cd3a182f7d97abb6 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-04-26  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        tryCachePutByID should not crash if target offset changes
>+        https://bugs.webkit.org/show_bug.cgi?id=197311
>+        <rdar://problem/48033612>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/cache-put-by-id-different-offset.js: Added.
>+        (Foo):
>+        (set x):
>+
> 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/cache-put-by-id-different-attributes.js b/JSTests/stress/cache-put-by-id-different-attributes.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..b38fc78a1449d71fd13cf43973098507fbcd393e
>--- /dev/null
>+++ b/JSTests/stress/cache-put-by-id-different-attributes.js
>@@ -0,0 +1,23 @@
>+//@ runNoLLInt
>+
>+function Foo() { }
>+
>+Foo.prototype.x = 0;
>+
>+Object.defineProperty(Foo.prototype, 'y', {
>+    set(x) {
>+        if (Foo.prototype.x++ === 9) {
>+            Object.defineProperty(Foo.prototype, 'y', {
>+                value: 13,
>+                writable: true,
>+            });
>+            if (typeof $vm !== 'undefined')
>+                $vm.flattenDictionaryObject(Foo.prototype);
>+        }
>+    },
>+    configurable: true,
>+});
>+
>+let foo = new Foo();
>+for (let i = 0; i < 11; i++)
>+    foo.y = 42;
>diff --git a/JSTests/stress/cache-put-by-id-different-offset.js b/JSTests/stress/cache-put-by-id-different-offset.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..7f29713e7c2853cac7cd4a43ef6757ec8ae39bfc
>--- /dev/null
>+++ b/JSTests/stress/cache-put-by-id-different-offset.js
>@@ -0,0 +1,19 @@
>+//@ runNoLLInt
>+
>+function Foo() { }
>+
>+Foo.prototype.x = 0;
>+
>+Object.defineProperty(Foo.prototype, 'y', {
>+    set(x) {
>+        if (Foo.prototype.x++ === 1) {
>+            delete Foo.prototype.x;
>+            if (typeof $vm !== 'undefined')
>+                $vm.flattenDictionaryObject(Foo.prototype);
>+        }
>+    }
>+});
>+
>+let foo = new Foo();
>+while (typeof foo.x === 'number')
>+    foo.y = 42;

Actions: View | Formatted Diff | Diff

Attachments on bug 197311: 368318 | 368401 | 369117 | 369129