[webkit-changes] [WebKit/WebKit] 14f1a4: [JSC] Implement CanDeclareGlobalFunction abstract ...

[Commit Queue]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 14f1a47bd7264df4cf795d6ddb88f6e321e8701f
      https://github.com/WebKit/WebKit/commit/14f1a47bd7264df4cf795d6ddb88f6e321e8701f
  Author: Alexey Shvayka <ashvayka at apple.com>
  Date:   2023-09-05 (Tue, 05 Sep 2023)

  Changed paths:
    A JSTests/stress/can-declare-global-function-invoked-before-any-binding-is-created-eval.js
    A JSTests/stress/can-declare-global-function-invoked-before-any-binding-is-created-global.js
    A JSTests/stress/can-declare-global-function-invoked-before-any-func-decl-is-hoisted-eval.js
    A JSTests/stress/can-declare-global-function-invoked-before-can-declare-var-eval.js
    A JSTests/stress/can-declare-global-function-invoked-before-can-declare-var-global.js
    A JSTests/stress/can-declare-global-function-invoked-var-by-the-same-name-eval.js
    A JSTests/stress/can-declare-global-function-invoked-var-by-the-same-name-global.js
    A JSTests/stress/can-declare-global-var-invoked-before-any-binding-is-created-eval.js
    A JSTests/stress/can-declare-global-var-invoked-before-any-binding-is-created-global.js
    A JSTests/stress/can-declare-global-var-invoked-before-any-func-decl-is-hoisted-eval.js
    A JSTests/stress/can-declare-global-var-invoked-during-func-decl-hoisting-eval.js
    A JSTests/stress/can-declare-global-var-invoked-from-jsonp-fast-path.js
    A JSTests/stress/create-global-function-binding-updates-descriptor-eval.js
    A JSTests/stress/create-global-function-binding-updates-descriptor-global.js
    A JSTests/stress/eval-func-decl-block-does-not-invoke-setter.js
    M JSTests/test262/expectations.yaml
    A LayoutTests/js/dom/function-declarations-shadowing-expected.txt
    A LayoutTests/js/dom/function-declarations-shadowing.html
    M LayoutTests/js/dom/var-declarations-shadowing-expected.txt
    M LayoutTests/js/dom/var-declarations-shadowing.html
    M Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
    M Source/JavaScriptCore/interpreter/Interpreter.cpp
    M Source/JavaScriptCore/runtime/ExceptionHelpers.cpp
    M Source/JavaScriptCore/runtime/ExceptionHelpers.h
    M Source/JavaScriptCore/runtime/JSGlobalObject.cpp
    M Source/JavaScriptCore/runtime/JSGlobalObject.h
    M Source/JavaScriptCore/runtime/JSGlobalObjectInlines.h
    M Source/JavaScriptCore/runtime/ProgramExecutable.cpp

  Log Message:
  -----------
  [JSC] Implement CanDeclareGlobalFunction abstract operation and friends
https://bugs.webkit.org/show_bug.cgi?id=260487
<rdar://problem/114215396>

Reviewed by Yusuke Suzuki.

For both global code and sloppy mode eval() in global scope, this change adds CanDeclareGlobalFunction [1]
that checks if a function can be declared while conforming to invariants of essential internal methods [2],
and also introduces CreateGlobalFunctionBinding [3] that creates a binding with best possible descriptor,
attempting to make the property both writable and enumerable.

For global code, the presence of these helpers is observable when attempting to shadow a non-configurable
global propeperty, and in browser environments with multiple <script> elements: if first script defines
non-configurable property X, the second one containing `function X() {}` should throw a TypeError.
In Bun, they can be observed via vm.runInThisContext() API.

Just like before the change, global code functions are defined on the symbol table unless there is a
non-configurable structure property, ensuring the best possible performance. CanDeclareGlobalFunction
and CreateGlobalFunctionBinding substitute the usage of non-idiomatic VM::DeletePropertyModeScope hack,
which used to ensure there is no property by the same name both on the structure and on the symbol table.
This change makes its removal one step closer.

Functions declared via sloppy mode eval() in global scope are unconditionally defined on the structure
because JSC relies on symbol table entries being immutable. This also preserves a bit weird global object
property enumeration order that is different from both V8 and SpiderMonkey.

For sloppy mode eval() in global scope, this change fixes a) binding creation for hoisted block-level
function declarations (Annex B) not to invoke [[Set]] on global object, and b) CanDeclareGlobalVar [3]
implementation to check only own properties, rather than performing [[Prototype]] lookup.

For global code, this change introduces CanDeclareGlobalVar checks that throw if the global object
is both non-extensible and doesn't have the given property, which is observable only in non-browser
environments where extension of global object can actually be prevented.

Performance analysis for declaration in global code:
  * `function` before: structure lookup, symbol table lookup [x2], add symbol table entry;
  * `function` after: structure lookup [x2], symbol table lookup [x2], add symbol table entry;
  * `var` before: structure lookup, symbol table lookup, symbol table lookup, add symbol table entry;
  * `var` after: structure lookup, symbol table lookup, add symbol table entry.

While this change adds extra structure lookup for `function`, it removes a few virtual calls, as well
as calling into JSLocalDOMWindow::getOwnPropertySlot().

As demonstrated by added tests, merging CanDeclareGlobalX with CreateGlobalXBinding is impossible since
no binding should be created, which is observable after catching an error, even if a single CanDeclareGlobalX
check fails. However, follow-up optimizations to reduce structure / symbol table lookups are possible.

All added / modified tests are were proven to align JSC with the spec, SpiderMonkey, and V8 (for the most part).

[1] https://tc39.es/ecma262/#sec-candeclareglobalfunction
[2] https://tc39.es/ecma262/#sec-invariants-of-the-essential-internal-methods
[3] https://tc39.es/ecma262/#sec-createglobalfunctionbinding
[4] https://tc39.es/ecma262/#sec-candeclareglobalvar

* JSTests/stress/can-declare-global-function-invoked-before-any-binding-is-created-eval.js: Added.
* JSTests/stress/can-declare-global-function-invoked-before-any-binding-is-created-global.js: Added.
* JSTests/stress/can-declare-global-function-invoked-before-any-func-decl-is-hoisted-eval.js: Added.
* JSTests/stress/can-declare-global-function-invoked-before-can-declare-var-eval.js: Added.
* JSTests/stress/can-declare-global-function-invoked-before-can-declare-var-global.js: Added.
* JSTests/stress/can-declare-global-function-invoked-var-by-the-same-name-eval.js: Added.
* JSTests/stress/can-declare-global-function-invoked-var-by-the-same-name-global.js: Added.
* JSTests/stress/can-declare-global-var-invoked-before-any-binding-is-created-eval.js: Added.
* JSTests/stress/can-declare-global-var-invoked-before-any-binding-is-created-global.js: Added.
* JSTests/stress/can-declare-global-var-invoked-before-any-func-decl-is-hoisted-eval.js: Added.
* JSTests/stress/can-declare-global-var-invoked-during-func-decl-hoisting-eval.js: Added.
* JSTests/stress/can-declare-global-var-invoked-from-jsonp-fast-path.js: Added.
* JSTests/stress/create-global-function-binding-updates-descriptor-eval.js: Added.
* JSTests/stress/create-global-function-binding-updates-descriptor-global.js: Added.
* JSTests/stress/eval-func-decl-block-does-not-invoke-setter.js: Added.
* JSTests/test262/expectations.yaml: Mark 21 tests as passing.
* LayoutTests/js/dom/var-declarations-shadowing-expected.txt:
* LayoutTests/js/dom/var-declarations-shadowing.html:
* LayoutTests/js/dom/function-declarations-shadowing-expected.txt: Added.
* LayoutTests/js/dom/function-declarations-shadowing.html: Added.
* Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
* Source/JavaScriptCore/interpreter/Interpreter.cpp:
(JSC::Interpreter::executeProgram):
(JSC::Interpreter::executeEval):
* Source/JavaScriptCore/runtime/ExceptionHelpers.cpp:
(JSC::createErrorForInvalidGlobalFunctionDeclaration):
(JSC::createErrorForInvalidGlobalVarDeclaration):
* Source/JavaScriptCore/runtime/ExceptionHelpers.h:
* Source/JavaScriptCore/runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::canDeclareGlobalFunction):
(JSC::JSGlobalObject::createGlobalFunctionBinding):
(JSC::JSGlobalObject::addSymbolTableEntry):
(JSC::JSGlobalObject::addGlobalVar): Deleted.
(JSC::JSGlobalObject::addFunction): Deleted.
* Source/JavaScriptCore/runtime/JSGlobalObject.h:
* Source/JavaScriptCore/runtime/JSGlobalObjectInlines.h:
(JSC::JSGlobalObject::canDeclareGlobalVar):
(JSC::JSGlobalObject::createGlobalVarBinding):
(JSC::JSGlobalObject::addVar): Deleted.
* Source/JavaScriptCore/runtime/ProgramExecutable.cpp:
(JSC::ProgramExecutable::initializeGlobalProperties):

Canonical link: https://commits.webkit.org/267655@main