Attachment 343072 Details for Bug 186812 – Patch

[patch] Patch

bug-186812-20180619121537.patch (text/plain), 7.90 KB, created by Keith Miller on 2018-06-19 12:15:38 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Keith Miller

Created: 2018-06-19 12:15:38 PDT

Size: 7.90 KB

patch

obsolete

>Subversion Revision: 232953
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 13a25dbabfc1f12eed06dc5adf850f088a584f4f..c0d47bdeef601a06acb8cd074e901a9c0dd666f4 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,22 @@
>+2018-06-19  Keith Miller  <keith_miller@apple.com>
>+
>+        constructArray variants should take the slow path for subclasses of Array
>+        https://bugs.webkit.org/show_bug.cgi?id=186812
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch fixes a crash in the ObjectInitializationScope where we would
>+        allocate a new structure for an indexing type change while initializing
>+        a subclass of Array. Since the new array hasn't been fully initialized
>+        if the GC ran it would see garbage and we might crash.
>+
>+        * runtime/JSArray.cpp:
>+        (JSC::constructArray):
>+        (JSC::constructArrayNegativeIndexed):
>+        * runtime/JSArray.h:
>+        (JSC::constructArray): Deleted.
>+        (JSC::constructArrayNegativeIndexed): Deleted.
>+
> 2018-06-18  Keith Miller  <keith_miller@apple.com>
> 
>         Unreviewed, fix the build...
>diff --git a/Source/JavaScriptCore/runtime/JSArray.cpp b/Source/JavaScriptCore/runtime/JSArray.cpp
>index 0392d2a2c88b98aabe7995e79d422b5893f6fe37..eeb60d89acc6c3426461d2b257abff523695af4d 100644
>--- a/Source/JavaScriptCore/runtime/JSArray.cpp
>+++ b/Source/JavaScriptCore/runtime/JSArray.cpp
>@@ -1336,4 +1336,78 @@ bool JSArray::isIteratorProtocolFastAndNonObservable()
>     return true;
> }
> 
>+JSArray* constructArray(ExecState* exec, Structure* arrayStructure, const ArgList& values)
>+{
>+    VM& vm = exec->vm();
>+    unsigned length = values.size();
>+    ObjectInitializationScope scope(vm);
>+
>+    // FIXME: We only need this for subclasses of Array because we might need to allocate a new structure to change
>+    // indexing types while initializing. If this triggered a GC then we might scan our currently uninitialized
>+    // array and crash. https://bugs.webkit.org/show_bug.cgi?id=186811
>+    JSArray* array;
>+    if (arrayStructure->globalObject()->isOriginalArrayStructure(arrayStructure))
>+        array = JSArray::tryCreateUninitializedRestricted(scope, arrayStructure, length);
>+    else
>+        array = JSArray::create(vm, arrayStructure, length);
>+
>+    // FIXME: we should probably throw an out of memory error here, but
>+    // when making this change we should check that all clients of this
>+    // function will correctly handle an exception being thrown from here.
>+    // https://bugs.webkit.org/show_bug.cgi?id=169786
>+    RELEASE_ASSERT(array);
>+
>+    for (unsigned i = 0; i < length; ++i)
>+        array->initializeIndex(scope, i, values.at(i));
>+    return array;
>+}
>+
>+JSArray* constructArray(ExecState* exec, Structure* arrayStructure, const JSValue* values, unsigned length)
>+{
>+    VM& vm = exec->vm();
>+    ObjectInitializationScope scope(vm);
>+    // FIXME: We only need this for subclasses of Array because we might need to allocate a new structure to change
>+    // indexing types while initializing. If this triggered a GC then we might scan our currently uninitialized
>+    // array and crash. https://bugs.webkit.org/show_bug.cgi?id=186811
>+    JSArray* array;
>+    if (arrayStructure->globalObject()->isOriginalArrayStructure(arrayStructure))
>+        array = JSArray::tryCreateUninitializedRestricted(scope, arrayStructure, length);
>+    else
>+        array = JSArray::create(vm, arrayStructure, length);
>+
>+    // FIXME: we should probably throw an out of memory error here, but
>+    // when making this change we should check that all clients of this
>+    // function will correctly handle an exception being thrown from here.
>+    // https://bugs.webkit.org/show_bug.cgi?id=169786
>+    RELEASE_ASSERT(array);
>+
>+    for (unsigned i = 0; i < length; ++i)
>+        array->initializeIndex(scope, i, values[i]);
>+    return array;
>+}
>+
>+JSArray* constructArrayNegativeIndexed(ExecState* exec, Structure* arrayStructure, const JSValue* values, unsigned length)
>+{
>+    VM& vm = exec->vm();
>+    ObjectInitializationScope scope(vm);
>+    // FIXME: We only need this for subclasses of Array because we might need to allocate a new structure to change
>+    // indexing types while initializing. If this triggered a GC then we might scan our currently uninitialized
>+    // array and crash. https://bugs.webkit.org/show_bug.cgi?id=186811
>+    JSArray* array;
>+    if (arrayStructure->globalObject()->isOriginalArrayStructure(arrayStructure))
>+        array = JSArray::tryCreateUninitializedRestricted(scope, arrayStructure, length);
>+    else
>+        array = JSArray::create(vm, arrayStructure, length);
>+
>+    // FIXME: we should probably throw an out of memory error here, but
>+    // when making this change we should check that all clients of this
>+    // function will correctly handle an exception being thrown from here.
>+    // https://bugs.webkit.org/show_bug.cgi?id=169786
>+    RELEASE_ASSERT(array);
>+
>+    for (int i = 0; i < static_cast<int>(length); ++i)
>+        array->initializeIndex(scope, i, values[-i]);
>+    return array;
>+}
>+
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/runtime/JSArray.h b/Source/JavaScriptCore/runtime/JSArray.h
>index 602f2a0a0729a1d1a5d00bd962bb3ca2c4e2959c..dfc49ca8af6962778cb82ebd3280c1365a512619 100644
>--- a/Source/JavaScriptCore/runtime/JSArray.h
>+++ b/Source/JavaScriptCore/runtime/JSArray.h
>@@ -303,56 +303,8 @@ inline bool isJSArray(JSCell* cell)
> 
> inline bool isJSArray(JSValue v) { return v.isCell() && isJSArray(v.asCell()); }
> 
>-inline JSArray* constructArray(ExecState* exec, Structure* arrayStructure, const ArgList& values)
>-{
>-    VM& vm = exec->vm();
>-    unsigned length = values.size();
>-    ObjectInitializationScope scope(vm);
>-    JSArray* array = JSArray::tryCreateUninitializedRestricted(scope, arrayStructure, length);
>-
>-    // FIXME: we should probably throw an out of memory error here, but
>-    // when making this change we should check that all clients of this
>-    // function will correctly handle an exception being thrown from here.
>-    // https://bugs.webkit.org/show_bug.cgi?id=169786
>-    RELEASE_ASSERT(array);
>-
>-    for (unsigned i = 0; i < length; ++i)
>-        array->initializeIndex(scope, i, values.at(i));
>-    return array;
>-}
>-    
>-inline JSArray* constructArray(ExecState* exec, Structure* arrayStructure, const JSValue* values, unsigned length)
>-{
>-    VM& vm = exec->vm();
>-    ObjectInitializationScope scope(vm);
>-    JSArray* array = JSArray::tryCreateUninitializedRestricted(scope, arrayStructure, length);
>-
>-    // FIXME: we should probably throw an out of memory error here, but
>-    // when making this change we should check that all clients of this
>-    // function will correctly handle an exception being thrown from here.
>-    // https://bugs.webkit.org/show_bug.cgi?id=169786
>-    RELEASE_ASSERT(array);
>-
>-    for (unsigned i = 0; i < length; ++i)
>-        array->initializeIndex(scope, i, values[i]);
>-    return array;
>-}
>-
>-inline JSArray* constructArrayNegativeIndexed(ExecState* exec, Structure* arrayStructure, const JSValue* values, unsigned length)
>-{
>-    VM& vm = exec->vm();
>-    ObjectInitializationScope scope(vm);
>-    JSArray* array = JSArray::tryCreateUninitializedRestricted(scope, arrayStructure, length);
>-
>-    // FIXME: we should probably throw an out of memory error here, but
>-    // when making this change we should check that all clients of this
>-    // function will correctly handle an exception being thrown from here.
>-    // https://bugs.webkit.org/show_bug.cgi?id=169786
>-    RELEASE_ASSERT(array);
>-
>-    for (int i = 0; i < static_cast<int>(length); ++i)
>-        array->initializeIndex(scope, i, values[-i]);
>-    return array;
>-}
>+JSArray* constructArray(ExecState*, Structure*, const ArgList& values);
>+JSArray* constructArray(ExecState*, Structure*, const JSValue* values, unsigned length);
>+JSArray* constructArrayNegativeIndexed(ExecState*, Structure*, const JSValue* values, unsigned length);
> 
> } // namespace JSC

Actions: View | Formatted Diff | Diff

Attachments on bug 186812: 343072 | 343083 | 343087