Attachment 343194 Details for Bug 186869 – Patch

[patch] Patch

bug-186869-20180620173402.patch (text/plain), 6.54 KB, created by Keith Miller on 2018-06-20 17:34:03 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Keith Miller

Created: 2018-06-20 17:34:03 PDT

Size: 6.54 KB

patch

obsolete

>Subversion Revision: 233003
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index dc723042167481801f513b9760c30db462e90197..04524506e36f2d1eb09734c2af4944854dc636ac 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2018-06-20  Keith Miller  <keith_miller@apple.com>
>+
>+        flattenDictionaryStruture needs to zero inline storage.
>+        https://bugs.webkit.org/show_bug.cgi?id=186869
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch also adds the assetion that unused property storage is
>+        zero or JSValue() to putDirectInternal. Additionally, functions
>+        have been added to $vm that flatten dictionary objects and return
>+        the inline capacity of an object.
>+
>+        * runtime/JSObjectInlines.h:
>+        (JSC::JSObject::putDirectInternal):
>+        * runtime/Structure.cpp:
>+        (JSC::Structure::flattenDictionaryStructure):
>+        * tools/JSDollarVM.cpp:
>+        (JSC::functionInlineCapacity):
>+        (JSC::functionFlattenDictionaryObject):
>+        (JSC::JSDollarVM::finishCreation):
>+
> 2018-06-19  Keith Miller  <keith_miller@apple.com>
> 
>         Fix broken assertion on 32-bit
>diff --git a/Source/JavaScriptCore/runtime/JSObjectInlines.h b/Source/JavaScriptCore/runtime/JSObjectInlines.h
>index ec331a139d6341f7bd78e4f1ece59ec2ddc924a7..409bd94a22d6d79a871d050e8ab5b04dea565d50 100644
>--- a/Source/JavaScriptCore/runtime/JSObjectInlines.h
>+++ b/Source/JavaScriptCore/runtime/JSObjectInlines.h
>@@ -381,6 +381,10 @@ ALWAYS_INLINE bool JSObject::putDirectInternal(VM& vm, PropertyName propertyName
>         Butterfly* newButterfly = allocateMoreOutOfLineStorage(vm, oldCapacity, newCapacity);
>         nukeStructureAndSetButterfly(vm, structureID, newButterfly);
>     }
>+
>+    // This assertion verifies that the concurrent GC won't read garbage if the concurrentGC
>+    // is running at the same time we put without transitioning.
>+    ASSERT(!getDirect(offset) || !JSValue::encode(getDirect(offset)));
>     putDirect(vm, offset, value);
>     setStructure(vm, newStructure);
>     slot.setNewProperty(this, offset);
>diff --git a/Source/JavaScriptCore/runtime/Structure.cpp b/Source/JavaScriptCore/runtime/Structure.cpp
>index 8e0219cbb6b1a9ebf14514e1942e34fc4a5c5830..30eeee9cc26f141bea0c893602d12e145aa159ea 100644
>--- a/Source/JavaScriptCore/runtime/Structure.cpp
>+++ b/Source/JavaScriptCore/runtime/Structure.cpp
>@@ -782,6 +782,11 @@ Structure* Structure::flattenDictionaryStructure(VM& vm, JSObject* object)
> 
>         // We need to zero our unused property space; otherwise the GC might see a
>         // stale pointer when we add properties in the future.
>+        memset(
>+            object->inlineStorageUnsafe() + inlineSize(),
>+            0,
>+            (inlineCapacity() - inlineSize()) * sizeof(EncodedJSValue));
>+
>         Butterfly* butterfly = object->butterfly();
>         memset(
>             butterfly->base(butterfly->indexingHeader()->preCapacity(this), beforeOutOfLineCapacity),
>diff --git a/Source/JavaScriptCore/tools/JSDollarVM.cpp b/Source/JavaScriptCore/tools/JSDollarVM.cpp
>index 96204c6e9329e8d1add4d7a2734e37dd00d10639..17c4654031972f1cf9a296d0d934974251b63c99 100644
>--- a/Source/JavaScriptCore/tools/JSDollarVM.cpp
>+++ b/Source/JavaScriptCore/tools/JSDollarVM.cpp
>@@ -1378,6 +1378,15 @@ static EncodedJSValue JSC_HOST_CALL functionIndexingMode(ExecState* exec)
>     return JSValue::encode(jsString(exec, stream.toString()));
> }
> 
>+static EncodedJSValue JSC_HOST_CALL functionInlineCapacity(ExecState* exec)
>+{
>+    VM& vm = exec->vm();
>+    if (auto* object = jsDynamicCast<JSObject*>(vm, exec->argument(0)))
>+        return JSValue::encode(jsNumber(object->structure(vm)->inlineCapacity()));
>+
>+    return encodedJSUndefined();
>+}
>+
> // Gets the dataLog dump of a given JS value as a string.
> // Usage: print("value = " + $vm.value(jsValue))
> static EncodedJSValue JSC_HOST_CALL functionValue(ExecState* exec)
>@@ -1647,6 +1656,15 @@ static EncodedJSValue JSC_HOST_CALL functionReturnTypeFor(ExecState* exec)
>     return JSValue::encode(JSONParse(exec, jsonString));
> }
> 
>+static EncodedJSValue JSC_HOST_CALL functionFlattenDictionaryObject(ExecState* exec)
>+{
>+    VM& vm = exec->vm();
>+    JSValue value = exec->argument(0);
>+    RELEASE_ASSERT(value.isObject() && value.getObject()->structure()->isDictionary());
>+    value.getObject()->flattenDictionaryObject(vm);
>+    return encodedJSUndefined();
>+}
>+
> static EncodedJSValue JSC_HOST_CALL functionDumpBasicBlockExecutionRanges(ExecState* exec)
> {
>     VM& vm = exec->vm();
>@@ -1824,6 +1842,7 @@ void JSDollarVM::finishCreation(VM& vm)
>     addFunction(vm, "printStack", functionPrintStack, 0);
> 
>     addFunction(vm, "indexingMode", functionIndexingMode, 1);
>+    addFunction(vm, "inlineCapacity", functionInlineCapacity, 1);
>     addFunction(vm, "value", functionValue, 1);
>     addFunction(vm, "getpid", functionGetPID, 0);
> 
>@@ -1855,6 +1874,8 @@ void JSDollarVM::finishCreation(VM& vm)
>     addFunction(vm, "findTypeForExpression", functionFindTypeForExpression, 2);
>     addFunction(vm, "returnTypeFor", functionReturnTypeFor, 1);
> 
>+    addFunction(vm, "flattenDictionaryObject", functionFlattenDictionaryObject, 1);
>+
>     addFunction(vm, "dumpBasicBlockExecutionRanges", functionDumpBasicBlockExecutionRanges , 0);
>     addFunction(vm, "hasBasicBlockExecuted", functionHasBasicBlockExecuted, 2);
>     addFunction(vm, "basicBlockExecutionCount", functionBasicBlockExecutionCount, 2);
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index b83e27e8a92c0aad7d15381f04699e518159ff8c..57f1ec4471cb78192e883f5368530afc6079eb63 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,12 @@
>+2018-06-20  Keith Miller  <keith_miller@apple.com>
>+
>+        flattenDictionaryStruture needs to zero inline storage.
>+        https://bugs.webkit.org/show_bug.cgi?id=186869
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/flatten-object-zero-unused-inline-properties.js: Added.
>+
> 2018-06-19  Saam Barati  <sbarati@apple.com>
> 
>         DirectArguments::create needs to initialize to undefined instead of the empty value
>diff --git a/JSTests/stress/flatten-object-zero-unused-inline-properties.js b/JSTests/stress/flatten-object-zero-unused-inline-properties.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..6fe5a14c4958b1bb2df691b182f09926d83aa290
>--- /dev/null
>+++ b/JSTests/stress/flatten-object-zero-unused-inline-properties.js
>@@ -0,0 +1,7 @@
>+let o = { foo: 1, bar: 2, baz: 3 };
>+if ($vm.inlineCapacity(o) <= 3)
>+    throw new Error("There should be inline capacity");
>+
>+delete o.foo;
>+$vm.flattenDictionaryObject(o);
>+o.foo = 1;

Flags:

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

Actions: View | Formatted Diff | Diff

Attachments on bug 186869: 343194