Changeset 274308 in webkit for trunk/Source/JavaScriptCore/jit/JIT.cpp

Timestamp:

Mar 11, 2021, 4:08:05 PM (4 years ago)

Author:

Alexey Shvayka

Message:

Align JSGlobalObject::defineOwnProperty() with the spec and other runtimes
​https://bugs.webkit.org/show_bug.cgi?id=203456

Reviewed by Robin Morisset.

JSTests:

• microbenchmarks/global-var-put-to-scope.js: Added.
• stress/eval-func-decl-in-frozen-global.js:

Object.freeze() redefines all global variables as ReadOnly, including hoisted var error.
Aligns with V8.

• stress/global-object-define-own-property-put-to-scope.js: Added.
• stress/global-object-define-own-property.js: Added.
• stress/to-this-before-arrow-function-closes-over-this-that-starts-as-lexical-environment.js:

Fix unwanted name conflict, which was an error in the original test, not an intended part of it.
Also, remove misleading comment on defineProperty and assert accessors are created on global object.
Aligns with V8.

LayoutTests/imported/w3c:

• web-platform-tests/html/browsers/the-windowproxy-exotic-object/windowproxy-define-own-property-unforgeable-same-origin-expected.txt: Added.
• web-platform-tests/html/browsers/the-windowproxy-exotic-object/windowproxy-define-own-property-unforgeable-same-origin.html: Added.

Source/JavaScriptCore:

Per spec, top-level var bindings are non-configurable properties of the global
object [1], while undefined / NaN / Infinity are also non-writable [2].

Prior to this change, redefining global var binding with accessor descriptor
failed silently (rather than throwing a TypeError); redefining with data or
generic descriptor created a structure property, which took precedence over
symbol table entry in JSGlobalObject::getOwnPropertySlot(), effectively
destroying live binding between global.foo and var foo.

This patch re-engineers JSGlobalObject::defineOwnProperty(), fixing both issues
mentioned above. If defineOwnProperty() override is removed, there is no way
a live binding can be maintained.

In a follow-up change, JSGlobalObject::getOwnPropertySlot() will be updated to
search symbol table first, aligning it with the spec [3], put(), and
defineOwnProperty(). Apart from consistency, this will bring a mild speed-up.

To accomodate global var binding reassignment right after it becomes read-only
(in the same scope), this patch introduces a watchpoint that can be fired by
JSGlobalObject::defineOwnProperty(). put_to_scope performance is neutral.

Also, this patch removes unused symbolTableGet() overload and orphaned
JSGlobalObject::defineGetter() / JSGlobalObject::defineSetter() declarations.

[1]: ​https://tc39.es/ecma262/#sec-object-environment-records-createmutablebinding-n-d
[2]: ​https://tc39.es/ecma262/#sec-value-properties-of-the-global-object
[3]: ​https://tc39.es/ecma262/#sec-global-environment-records-getbindingvalue-n-s

• dfg/DFGByteCodeParser.cpp:

(JSC::DFG::ByteCodeParser::needsDynamicLookup):
(JSC::DFG::ByteCodeParser::parseBlock):

• jit/JIT.cpp:

(JSC::JIT::emitVarReadOnlyCheck):

• jit/JIT.h:
• jit/JITPropertyAccess.cpp:

(JSC::JIT::emit_op_put_to_scope):

• jit/JITPropertyAccess32_64.cpp:

(JSC::JIT::emit_op_put_to_scope):

• llint/LowLevelInterpreter.asm:
• llint/LowLevelInterpreter32_64.asm:
• llint/LowLevelInterpreter64.asm:
• runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::JSGlobalObject):
(JSC::JSGlobalObject::defineOwnProperty):

• runtime/JSGlobalObject.h:

(JSC::JSGlobalObject::varReadOnlyWatchpoint):

• runtime/JSSymbolTableObject.h:

(JSC::symbolTableGet):

Source/WebCore:

This patch removes location special-casing, which a) incorrectly returned
false if new descriptor was the same as the current one and b) failed
silently otherwise (rather than throwing a TypeError).

However, this change introduces window / document special-casing because
they exist on the structure and as symbol table entries (for performance reasons).
Aligns WebKit with Blink and partly with Gecko.

Test: imported/w3c/web-platform-tests/html/browsers/the-windowproxy-exotic-object/windowproxy-define-own-property-unforgeable-same-origin.html

• bindings/js/JSDOMWindowCustom.cpp:

(WebCore::JSDOMWindow::defineOwnProperty):

LayoutTests:

• fast/dom/Window/Location/window-override-location-using-defineGetter-expected.txt:
• fast/dom/Window/Location/window-override-location-using-defineGetter.html:
• fast/dom/Window/Location/window-override-window-using-defineGetter-expected.txt:
• fast/dom/Window/Location/window-override-window-using-defineGetter.html:
• fast/dom/getter-on-window-object2-expected.txt:
• fast/dom/getter-on-window-object2.html:

File:

• trunk/Source/JavaScriptCore/jit/JIT.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/jit/JIT.cpp

@@ -116 +116 @@
 {
     addSlowCase(branch8(NotEqual, Address(pointerToSet, WatchpointSet::offsetOfState()), TrustedImm32(IsInvalidated)));
+}
+
+void JIT::emitVarReadOnlyCheck(ResolveType resolveType)
+{
+    if (resolveType == GlobalVar || resolveType == GlobalVarWithVarInjectionChecks)
+        addSlowCase(branch8(Equal, AbsoluteAddress(m_codeBlock->globalObject()->varReadOnlyWatchpoint()->addressOfState()), TrustedImm32(IsInvalidated)));
 }