Attachment 423105 Details for Bug 141952 – Patch

[patch] Patch

bug-141952-20210313210542.patch (text/plain), 11.23 KB, created by Alexey Shvayka on 2021-03-13 11:05:44 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Alexey Shvayka

Created: 2021-03-13 11:05:44 PST

Size: 11.23 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 274349)
>+++ JSTests/ChangeLog	(working copy)
>@@ -1,3 +1,21 @@
>+2021-03-12  Alexey Shvayka  <shvaikalesh@gmail.com>
>+
>+        GenericArguments<Type>::getOwnPropertyNames() should respect modified descriptors
>+        https://bugs.webkit.org/show_bug.cgi?id=141952
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * ChakraCore.yaml:
>+        * ChakraCore/test/Function/arguments2.baseline:
>+        * ChakraCore/test/Function/arguments2.js:
>+        Fixes the incorrect test case, rather than adding a JSC baseline,
>+        because an exception is raised if at least one test fails.
>+
>+        * ChakraCore/test/Function/arguments3.baseline-jsc: Removed.
>+        * microbenchmarks/for-in-mapped-arguments.js: Added.
>+        * stress/arguments-bizarre-behaviour-disable-enumerability.js:
>+        * stress/arguments-properties-order.js:
>+
> 2021-03-11  Tadeu Zagallo  <tzagallo@apple.com>
> 
>         AI validator patchpoint should read heap top
>Index: JSTests/ChakraCore.yaml
>===================================================================
>--- JSTests/ChakraCore.yaml	(revision 274349)
>+++ JSTests/ChakraCore.yaml	(working copy)
>@@ -572,7 +572,7 @@
> - path: ChakraCore/test/Function/arguments2.js
>   cmd: runChakra :baseline, "NoException", "arguments2.baseline", []
> - path: ChakraCore/test/Function/arguments3.js
>-  cmd: runChakra :baseline, "NoException", "arguments3.baseline-jsc", []
>+  cmd: runChakra :baseline, "NoException", "arguments3.baseline", []
> - path: ChakraCore/test/Function/arguments4.js
>   cmd: runChakra :pass, "NoException", "", []
> - path: ChakraCore/test/Function/argumentsMisc.js
>Index: JSTests/ChakraCore/test/Function/arguments2.baseline
>===================================================================
>--- JSTests/ChakraCore/test/Function/arguments2.baseline	(revision 274349)
>+++ JSTests/ChakraCore/test/Function/arguments2.baseline	(working copy)
>@@ -98,8 +98,8 @@ Test10.3.2 : [0: 13, 1: arg1, 2: arg2, 6
> PASSED : Test10.3.2 exp: [0: 13, 1: arg1, 2: arg2, 6: arg6, foo: bar] act : [0: 13, 1: arg1, 2: arg2, 6: arg6, foo: bar]
> Test10.3.1 : [0: 13, 2: string]
> PASSED : Test10.3.1 exp: [0: 13, 2: string] act : [0: 13, 2: string]
>-Test10.3.2 : [0: 13, 2: arg2, 1: arg1, 6: arg6, foo: bar]
>-PASSED : Test10.3.2 exp: [0: 13, 2: arg2, 1: arg1, 6: arg6, foo: bar] act : [0: 13, 2: arg2, 1: arg1, 6: arg6, foo: bar]
>+Test10.3.2 : [0: 13, 1: arg1, 2: arg2, 6: arg6, foo: bar]
>+PASSED : Test10.3.2 exp: [0: 13, 1: arg1, 2: arg2, 6: arg6, foo: bar] act : [0: 13, 1: arg1, 2: arg2, 6: arg6, foo: bar]
> Test11 args : sentinel
> PASSED : Test11 args exp: sentinel act : sentinel
> Test12 args : sentinel
>Index: JSTests/ChakraCore/test/Function/arguments2.js
>===================================================================
>--- JSTests/ChakraCore/test/Function/arguments2.js	(revision 274349)
>+++ JSTests/ChakraCore/test/Function/arguments2.js	(working copy)
>@@ -306,7 +306,7 @@ delete_args_test(get_args(13, 24),
> 
> delete_args_test(get_args(13, 24, "string", true),
>                  "[0: 13, 2: string]", //[1][3] deleted
>-                 "[0: 13, 2: arg2, 1: arg1, 6: arg6, foo: bar]"
>+                 "[0: 13, 1: arg1, 2: arg2, 6: arg6, foo: bar]"
> //[1] set after formal args [0][2]
>                  );
> 
>Index: JSTests/ChakraCore/test/Function/arguments3.baseline-jsc
>===================================================================
>--- JSTests/ChakraCore/test/Function/arguments3.baseline-jsc	(revision 274349)
>+++ JSTests/ChakraCore/test/Function/arguments3.baseline-jsc	(nonexistent)
>@@ -1,10 +0,0 @@
>---- toString test ---
>-[object Arguments]
>-[object Arguments]
>-
>---- getOwnPropertyNames should enumerate all properties ---
>-length,callee
>-0,length,callee
>-0,1,length,callee
>-0,1,2,3,4,length,callee
>-2,3,0,length,callee
>Index: JSTests/microbenchmarks/for-in-mapped-arguments.js
>===================================================================
>--- JSTests/microbenchmarks/for-in-mapped-arguments.js	(nonexistent)
>+++ JSTests/microbenchmarks/for-in-mapped-arguments.js	(working copy)
>@@ -0,0 +1,13 @@
>+function getArgs(a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, a14, a15, a16, a17, a18, a19, a20, a21, a22, a23, a24, a25, a26, a27, a28, a29, a30, a31, a32, a33, a34, a35, a36, a37, a38, a39, a40, a41, a42, a43, a44, a45, a46, a47, a48, a49, a50, a51, a52, a53, a54, a55, a56, a57, a58, a59, a60, a61, a62, a63, a64, a65, a66, a67, a68, a69, a70, a71, a72, a73, a74, a75, a76, a77, a78, a79, a80, a81, a82, a83, a84, a85, a86, a87, a88, a89, a90, a91, a92, a93, a94, a95, a96, a97, a98, a99) {
>+    return arguments;
>+}
>+
>+noInline(getArgs);
>+
>+(function() {
>+    var count = 0;
>+    for (var i = 0; i < 1e4; i++) {
>+        for (var _ in getArgs(0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99))
>+            count++;
>+    }
>+})();
>Index: JSTests/stress/arguments-bizarre-behaviour-disable-enumerability.js
>===================================================================
>--- JSTests/stress/arguments-bizarre-behaviour-disable-enumerability.js	(revision 274349)
>+++ JSTests/stress/arguments-bizarre-behaviour-disable-enumerability.js	(working copy)
>@@ -21,7 +21,7 @@ for (var s in result[2])
> if (array.join(",") != "")
>     throw new Error();
> 
>-if (Object.keys(result[2]).join(",") != "0")
>+if (Object.keys(result[2]).join(",") != "")
>     throw new Error();
> 
> if (Object.getOwnPropertyDescriptor(result[2], 0).enumerable === true)
>Index: JSTests/stress/arguments-properties-order.js
>===================================================================
>--- JSTests/stress/arguments-properties-order.js	(revision 274349)
>+++ JSTests/stress/arguments-properties-order.js	(working copy)
>@@ -72,8 +72,59 @@ noInline(forIn);
>     }
> })();
> 
>+(function indexDeleted() {
>+    for (let i = 0; i < 1e4; ++i) {
>+        const args = getMappedArguments(0, 1, 2, 3);
>+        delete args[1];
>+        delete args[2];
>+        args[2] = 2;
>+        args[1] = 1;
>+
>+        shouldBeArray(forIn(args), ["0", "1", "2", "3"]);
>+        shouldBeArray(Object.keys(args), ["0", "1", "2", "3"]);
>+        shouldBeArray(Reflect.ownKeys(args), ["0", "1", "2", "3", "length", "callee", Symbol.iterator]);
>+    }
>+})();
>+
>+(function indexDeletedThenAdded() {
>+    for (let i = 0; i < 1e4; ++i) {
>+        const args = getMappedArguments(0, 1, 2, 3);
>+        delete args[1];
>+        delete args[2];
>+        Object.defineProperty(args, "2", { value: 2, enumerable: false });
>+        Object.defineProperty(args, "1", { value: 1, enumerable: true });
>+
>+        shouldBeArray(forIn(args), ["0", "1", "3"]);
>+        shouldBeArray(Object.keys(args), ["0", "1", "3"]);
>+        shouldBeArray(Reflect.ownKeys(args), ["0", "1", "2", "3", "length", "callee", Symbol.iterator]);
>+    }
>+})();
>+
>+(function indexRedefinedRemainsMapped() {
>+    for (let i = 0; i < 1e4; ++i) {
>+        const args = getMappedArguments(0, 1, 2, 3);
>+        Object.defineProperty(args, "2", { enumerable: false, value: 22 });
>+        Object.defineProperty(args, "0", { enumerable: true, configurable: false });
>+
>+        shouldBeArray(forIn(args), ["0", "1", "3"]);
>+        shouldBeArray(Object.keys(args), ["0", "1", "3"]);
>+        shouldBeArray(Reflect.ownKeys(args), ["0", "1", "2", "3", "length", "callee", Symbol.iterator]);
>+    }
>+})();
>+
>+(function indexRedefinedBecomesUnmapped() {
>+    for (let i = 0; i < 1e4; ++i) {
>+        const args = getMappedArguments(0, 1, 2, 3);
>+        Object.defineProperty(args, "2", { enumerable: true, get() {} });
>+        Object.defineProperty(args, "1", { enumerable: false, writable: false });
>+
>+        shouldBeArray(forIn(args), ["0", "2", "3"]);
>+        shouldBeArray(Object.keys(args), ["0", "2", "3"]);
>+        shouldBeArray(Reflect.ownKeys(args), ["0", "1", "2", "3", "length", "callee", Symbol.iterator]);
>+    }
>+})();
>+
> // FIXME: Add more tests, covering:
> // * added symbol properties;
> // * added together index, non-index, and symbol properties;
>-// * deleted, re-added, and redefined as DontEnum index properties, both within and beyond "length";
> // * deleted, re-added, and redefined as DontEnum "length", "callee", and Symbol.iterator properties.
>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 274349)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,26 @@
>+2021-03-12  Alexey Shvayka  <shvaikalesh@gmail.com>
>+
>+        GenericArguments<Type>::getOwnPropertyNames() should respect modified descriptors
>+        https://bugs.webkit.org/show_bug.cgi?id=141952
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        r210146 fixed most of GenericArguments' methods to support modified descriptors.
>+        This patch does the same for getOwnPropertyNames() so the inbound indices, which
>+        were redefined as non-enumerable or deleted and re-added, are enumerated correctly.
>+
>+        Exception checking isn't performed because Base::getOwnPropertySlotByIndex() can't
>+        possibly throw; and even if it could, the exception would be unobservably handled
>+        after getOwnPropertyNames() exits.
>+
>+        Aligns JSC with the spec [1] and SpiderMonkey.
>+        Provided microbenchmark is neutral as the change preserves the fast path.
>+
>+        [1]: https://tc39.es/ecma262/#sec-createmappedargumentsobject (step 19.b.ii)
>+
>+        * runtime/GenericArgumentsInlines.h:
>+        (JSC::GenericArguments<Type>::getOwnPropertyNames):
>+
> 2021-03-12  Carlos Garcia Campos  <cgarcia@igalia.com>
> 
>         [GTK] Bump API version when building with libsoup3
>Index: Source/JavaScriptCore/runtime/GenericArgumentsInlines.h
>===================================================================
>--- Source/JavaScriptCore/runtime/GenericArgumentsInlines.h	(revision 274349)
>+++ Source/JavaScriptCore/runtime/GenericArgumentsInlines.h	(working copy)
>@@ -98,11 +98,23 @@ void GenericArguments<Type>::getOwnPrope
>     VM& vm = globalObject->vm();
>     Type* thisObject = jsCast<Type*>(object);
> 
>+    auto shouldEnumerateIndex = [&] (unsigned index) -> bool {
>+        if (!thisObject->isModifiedArgumentDescriptor(index) && thisObject->isMappedArgument(index))
>+            return true;
>+
>+        PropertySlot slot(thisObject, PropertySlot::InternalMethodType::VMInquiry, &vm);
>+        bool hasProperty = Base::getOwnPropertySlotByIndex(thisObject, globalObject, index, slot);
>+        slot.disallowVMEntry.reset();
>+        if (!hasProperty)
>+            return false;
>+
>+        return mode == DontEnumPropertiesMode::Include || !(slot.attributes() & PropertyAttribute::DontEnum);
>+    };
>+
>     if (array.includeStringProperties()) {
>         for (unsigned i = 0; i < thisObject->internalLength(); ++i) {
>-            if (!thisObject->isMappedArgument(i))
>-                continue;
>-            array.add(Identifier::from(vm, i));
>+            if (shouldEnumerateIndex(i))
>+                array.add(Identifier::from(vm, i));
>         }
>         thisObject->getOwnIndexedPropertyNames(globalObject, array, mode);
>     }

Flags:

ysuzuki: review+
ews-feeder: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 141952: 423105