Skip to content

impl shadow realm as js builtins#8

Closed
philomates wants to merge 1 commit into
WebKit:mainfrom
philomates:js-shadow-realms
Closed

impl shadow realm as js builtins#8
philomates wants to merge 1 commit into
WebKit:mainfrom
philomates:js-shadow-realms

Conversation

@philomates

@philomates philomates commented Sep 9, 2021

Copy link
Copy Markdown
Contributor

accidentally opened this; was meant to be a PR for my personal fork

JSC_TO_STRING_TAG_WITHOUT_TRANSITION();
}

JSC_DEFINE_HOST_FUNCTION(importInRealm, (JSGlobalObject* globalObject, CallFrame* callFrame))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostly copied and adapted from

JSC_DEFINE_HOST_FUNCTION(globalFuncImportModule, (JSGlobalObject* globalObject, CallFrame* callFrame))
{
VM& vm = globalObject->vm();
auto* promise = JSPromise::create(vm, globalObject->promiseStructure());
auto scope = DECLARE_THROW_SCOPE(vm);
auto sourceOrigin = callFrame->callerSourceOrigin(vm);
RELEASE_ASSERT(callFrame->argumentCount() == 1);
auto* specifier = callFrame->uncheckedArgument(0).toString(globalObject);
RETURN_IF_EXCEPTION(scope, JSValue::encode(promise->rejectWithCaughtException(globalObject, scope)));
// We always specify parameters as undefined. Once dynamic import() starts accepting fetching parameters,
// we should retrieve this from the arguments.
JSValue parameters = jsUndefined();
auto* internalPromise = globalObject->moduleLoader()->importModule(globalObject, specifier, parameters, sourceOrigin);
RETURN_IF_EXCEPTION(scope, JSValue::encode(promise->rejectWithCaughtException(globalObject, scope)));
scope.release();
promise->resolve(globalObject, internalPromise);
return JSValue::encode(promise);
}


auto sourceOrigin = callFrame->callerSourceOrigin(vm);
auto* specifier = callFrame->uncheckedArgument(1).toString(globalObject);
RETURN_IF_EXCEPTION(scope, { });

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note the difference from https://github.com/WebKit/WebKit/blob/main/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp#L824
In the case here, according to the test262 suite, we want to throw outside the promise if toString has issues. I think the difference here achieves that (at least I had to make the change to get things passing)

JSValue thisValue = callFrame->uncheckedArgument(0);
ShadowRealmObject* thisRealm = jsDynamicCast<ShadowRealmObject*>(vm, thisValue);
if (UNLIKELY(!thisRealm))
return throwVMTypeError(globalObject, scope, "'this' should be a ShadowRealm");

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when should one use throwVMTypeError instead of throwTypeError?

RETURN_IF_EXCEPTION(scope, { });

JSGlobalObject* realmGlobalObject = thisRealm->globalObject();
auto* internalPromise = realmGlobalObject->moduleLoader()->importModule(realmGlobalObject, specifier, jsUndefined(), sourceOrigin);

@philomates philomates Sep 9, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this creates an internal promise somehow associated with the realm global object, but we later do resolve on the original global object. I can't see anything clearly problematic with this, but felt worth bringing attention to this

Comment on lines +2573 to +2578
JSGlobalObject* JSGlobalObject::create(VM& vm, Structure* structure, const GlobalObjectMethodTable* methodTable)
{
JSGlobalObject* globalObject = new (NotNull, allocateCell<JSGlobalObject>(vm.heap)) JSGlobalObject(vm, structure, methodTable);
globalObject->finishCreation(vm);
return globalObject;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was the best way I could think of creating a new global object with a custom method table. We grab the method table from the original global object in https://github.com/WebKit/WebKit/pull/8/files#diff-44ebd0942590c3af4f75c8833b1b8f2dd9d649fd9e7dcc7a9e1410df5711c22fR33, which might be defined in jsc.cpp or elsewhere in webkit when run in the browser context.

Comment on lines +58 to +59
if (bindingStr in m) {
return @wrap(m[bindingStr]);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a better way to do this prop check/lookup?

* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my (idiomatic) JS is horrible so in general be thorough with this file

@philomates

Copy link
Copy Markdown
Contributor Author

sorry; this was meant to be a PR for my personal fork

@philomates philomates closed this Sep 9, 2021
xanlpz referenced this pull request in xanlpz/WebKit Oct 26, 2021
https://bugs.webkit.org/show_bug.cgi?id=232303

Reviewed by NOBODY (OOPS!).

This patch does two things:

Add the .size and .type directives to every llint "function"
(global, llint opcode, 'glue'). This allows a debugger to tell you
in what logical function you are inside the giant chunk of code
that is the llint interpreter. So instead of something like this:

(gdb) x/5i $pc
  => 0xf5f8af60 <wasmLLIntPCRangeStart+3856>:  b.n     0xf5f8af6c <wasmLLIntPCRangeStart+3868>
     0xf5f8af62 <wasmLLIntPCRangeStart+3858>:  ldr     r2, [r7, #8]
     0xf5f8af64 <wasmLLIntPCRangeStart+3860>:  ldr     r2, [r2, WebKit#28]
     0xf5f8af66 <wasmLLIntPCRangeStart+3862>:  subs    r0, #16
     0xf5f8af68 <wasmLLIntPCRangeStart+3864>:  ldr.w   r0, [r2, r0, lsl #3]

you get something like this:

(gdb) x/5i $pc
  => 0xf5f8c770 <wasm_f32_add+12>:      bge.n   0xf5f8c77c <wasm_f32_add+24>
     0xf5f8c772 <wasm_f32_add+14>:      add.w   r6, r7, r9, lsl #3
     0xf5f8c776 <wasm_f32_add+18>:      vldr    d0, [r6]
     0xf5f8c77a <wasm_f32_add+22>:      b.n     0xf5f8c78c <wasm_f32_add+40>
     0xf5f8c77c <wasm_f32_add+24>:      ldr     r2, [r7, #8]

The other change adds a local symbol (in addition to an internal
label) to all the "glue" labels. That allows wasm opcodes to be
seen by the debugger (and the user to break on them), among other
things.

* CMakeLists.txt: tell offlineasm we use the ELF binary format on Linux.
* llint/LowLevelInterpreter.cpp: emit a non-local label for "glue" labels.
* offlineasm/asm.rb: emit the .size and .type directives for every
llint "function" on ELF systems.
webkit-commit-queue pushed a commit that referenced this pull request Oct 26, 2021
https://bugs.webkit.org/show_bug.cgi?id=232303

Patch by Xan López <[email protected]> on 2021-10-26
Reviewed by Mark Lam.

This patch does two things:

Add the .size and .type directives to every llint "function"
(global, llint opcode, 'glue'). This allows a debugger to tell you
in what logical function you are inside the giant chunk of code
that is the llint interpreter. So instead of something like this:

(gdb) x/5i $pc
  => 0xf5f8af60 <wasmLLIntPCRangeStart+3856>:  b.n     0xf5f8af6c <wasmLLIntPCRangeStart+3868>
     0xf5f8af62 <wasmLLIntPCRangeStart+3858>:  ldr     r2, [r7, #8]
     0xf5f8af64 <wasmLLIntPCRangeStart+3860>:  ldr     r2, [r2, #28]
     0xf5f8af66 <wasmLLIntPCRangeStart+3862>:  subs    r0, #16
     0xf5f8af68 <wasmLLIntPCRangeStart+3864>:  ldr.w   r0, [r2, r0, lsl #3]

you get something like this:

(gdb) x/5i $pc
  => 0xf5f8c770 <wasm_f32_add+12>:      bge.n   0xf5f8c77c <wasm_f32_add+24>
     0xf5f8c772 <wasm_f32_add+14>:      add.w   r6, r7, r9, lsl #3
     0xf5f8c776 <wasm_f32_add+18>:      vldr    d0, [r6]
     0xf5f8c77a <wasm_f32_add+22>:      b.n     0xf5f8c78c <wasm_f32_add+40>
     0xf5f8c77c <wasm_f32_add+24>:      ldr     r2, [r7, #8]

The other change adds a local symbol (in addition to an internal
label) to all the "glue" labels. That allows wasm opcodes to be
seen by the debugger (and the user to break on them), among other
things.

* CMakeLists.txt: tell offlineasm we use the ELF binary format on Linux.
* llint/LowLevelInterpreter.cpp: emit a non-local label for "glue" labels.
* offlineasm/asm.rb: emit the .size and .type directives for every
llint "function" on ELF systems.

Canonical link: https://commits.webkit.org/243545@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@284868 268f45cc-cd09-0410-ab3c-d52691b4dbfc
webkit-early-warning-system pushed a commit to MenloDorian/Webkit that referenced this pull request Jun 23, 2022
https://bugs.webkit.org/show_bug.cgi?id=241905

Reviewed by Yusuke Suzuki.

offlineasm used to emit this LLInt code:
    ".loc 1 996\n"        "ldr x19, [x0] \n"                   // LowLevelInterpreter.asm:996
    ".loc 1 997\n"        "ldr x20, [x0, WebKit#8] \n"               // LowLevelInterpreter.asm:997
    ".loc 1 998\n"        "ldr x21, [x0, WebKit#16] \n"              // LowLevelInterpreter.asm:998
    ".loc 1 999\n"        "ldr x22, [x0, WebKit#24] \n"              // LowLevelInterpreter.asm:999
    ...
    ".loc 1 1006\n"       "ldr d8, [x0, WebKit#80] \n"               // LowLevelInterpreter.asm:1006
    ".loc 1 1007\n"       "ldr d9, [x0, WebKit#88] \n"               // LowLevelInterpreter.asm:1007
    ".loc 1 1008\n"       "ldr d10, [x0, WebKit#96] \n"              // LowLevelInterpreter.asm:1008
    ".loc 1 1009\n"       "ldr d11, [x0, WebKit#104] \n"             // LowLevelInterpreter.asm:1009
    ...

Now, it can emit this instead:
    ".loc 1 996\n"        "ldp x19, x20, [x0, #0] \n"          // LowLevelInterpreter.asm:996
    ".loc 1 997\n"        "ldp x21, x22, [x0, WebKit#16] \n"         // LowLevelInterpreter.asm:997
    ...
    ".loc 1 1001\n"       "ldp d8, d9, [x0, WebKit#80] \n"           // LowLevelInterpreter.asm:1001
    ".loc 1 1002\n"       "ldp d10, d11, [x0, WebKit#96] \n"         // LowLevelInterpreter.asm:1002
    ...

Also, there was some code that kept recomputing the base address of a sequence of load/store
instructions.  For example,
    ".loc 6 902\n"        "add x13, sp, x10, lsl WebKit#3 \n"        // WebAssembly.asm:902
                          "ldr x0, [x13, WebKit#48] \n"
                          "add x13, sp, x10, lsl WebKit#3 \n"
                          "ldr x1, [x13, WebKit#56] \n"
                          "add x13, sp, x10, lsl WebKit#3 \n"
                          "ldr x2, [x13, WebKit#64] \n"
                          "add x13, sp, x10, lsl WebKit#3 \n"
                          "ldr x3, [x13, WebKit#72] \n"
    ...

For such places, we observe that the base address is the same for every load/store instruction
in the sequence, and precompute it in the LLInt asm code to help out the offline asm.  This
allows the offlineasm to now emit this more efficient code instead:
    ".loc 6 896\n"        "add x10, sp, x10, lsl WebKit#3 \n"        // WebAssembly.asm:896
    ".loc 6 898\n"        "ldp x0, x1, [x10, WebKit#48] \n"          // WebAssembly.asm:898
                          "ldp x2, x3, [x10, WebKit#64] \n"
    ...

* Source/JavaScriptCore/llint/LowLevelInterpreter.asm:
* Source/JavaScriptCore/llint/WebAssembly.asm:
* Source/JavaScriptCore/offlineasm/arm64.rb:
* Source/JavaScriptCore/offlineasm/instructions.rb:

Canonical link: https://commits.webkit.org/251799@main
webkit-early-warning-system pushed a commit to hyjorc1/WebKit that referenced this pull request Nov 19, 2022
…a rejected promise

https://bugs.webkit.org/show_bug.cgi?id=247785
rdar://102325201

Reviewed by Yusuke Suzuki.

Rest parameter should be caught in async function. So, running this
JavaScript program should print "caught".
```
async function f(...[[]]) { }
f().catch(e => print("caught"));
```

V8 (used console.log)
```
$ node input.js
caught
```

GraalJS
```
$ js input.js
caught
```

https://tc39.es/ecma262/#sec-async-function-definitions
...
AsyncFunctionDeclaration[Yield, Await, Default] :
    async [no LineTerminator here] function BindingIdentifier[?Yield, ?Await] ( FormalParameters[~Yield, +Await] ) { AsyncFunctionBody }
    [+Default] async [no LineTerminator here] function ( FormalParameters[~Yield, +Await] ) { AsyncFunctionBody }

AsyncFunctionExpression :
    async [no LineTerminator here] function BindingIdentifier[~Yield, +Await]opt ( FormalParameters[~Yield, +Await] ) { AsyncFunctionBody }
...

According to the spec, it indicates `FormalParameters` is used for Async
Function, where `FormalParameters` can be converted to `FunctionRestParameter`.

https://tc39.es/ecma262/#sec-parameter-lists
...
FormalParameters[Yield, Await] :
    [empty]
    FunctionRestParameter[?Yield, ?Await]
    FormalParameterList[?Yield, ?Await]
    FormalParameterList[?Yield, ?Await] ,
    FormalParameterList[?Yield, ?Await] , FunctionRestParameter[?Yield, ?Await]
...

And based on RS: EvaluateAsyncFunctionBody, it will invoke the promise.reject
callback function with abrupt value ([[value]] of non-normal completion record).

https://tc39.es/ecma262/#sec-runtime-semantics-evaluateasyncfunctionbody
...
2. Let declResult be Completion(FunctionDeclarationInstantiation(functionObject, argumentsList)).
3. If declResult is an abrupt completion, then
    a. Perform ! Call(promiseCapability.[[Reject]], undefined, « declResult.[[Value]] »).
...

In that case, any non-normal results of evaluating rest parameters should be
caught and passed to the reject callback function.

To resolve this problem, we should allow the emitted RestParameterNode be wrapped
by the catch handler for promise. However, we should remove `m_restParameter` and
emit rest parameter byte code in `initializeDefaultParameterValuesAndSetupFunctionScopeStack`
if we can prove that change has no side effect. In that case, we can only use one
exception handler.

Current fix is to add another exception handler. And move the handler byte codes to
the bottom of code block in order to make other byte codes as much compact as possible.

Input:
```
async function f(arg0, ...[[]]) { }
f();
```

Dumped Byte Codes:
```
...

bb#2
Predecessors: [ WebKit#1 ]
[  20] mov                dst:loc9, src:<JSValue()>(const0)
...

bb#3
Predecessors: [ WebKit#2 ]
[  29] get_rest_length    dst:loc11, numParametersToSkip:1
...

bb#12
Predecessors: [ WebKit#8 WebKit#9 WebKit#10 ]
[ 138] new_func_exp       dst:loc10, scope:loc4, functionDecl:0
...

bb#13
Predecessors: [ ]
[ 170] catch              exception:loc10, thrownValue:loc8
[ 174] jmp                targetLabel:8(->182)
Successors: [ WebKit#15 ]

bb#14
Predecessors: [ WebKit#7 WebKit#11 ]
[ 176] catch              exception:loc10, thrownValue:loc8
[ 180] jmp                targetLabel:2(->182)
Successors: [ WebKit#15 ]

bb#15
Predecessors: [ WebKit#13 WebKit#14 ]
[ 182] mov                dst:loc12, src:Undefined(const1)
...

Exception Handlers:
	 1: { start: [  20] end: [  29] target: [ 170] } synthesized catch
	 2: { start: [  29] end: [ 138] target: [ 176] } synthesized catch
```

* JSTests/stress/catch-rest-parameter.js: Added.
(throwError):
(shouldThrow):
(async f):
(throwError.async f):
(throwError.async let):
(async let):
(x.async f):
(x):
(async shouldThrow):
* Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
(JSC::BytecodeGenerator::initializeDefaultParameterValuesAndSetupFunctionScopeStack):
* Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:

Canonical link: https://commits.webkit.org/256864@main
cdumez pushed a commit to cdumez/WebKit that referenced this pull request Jan 15, 2023
    Fix TypeError in parameter destructuring in async function should be a rejected promise
    https://bugs.webkit.org/show_bug.cgi?id=247785
    rdar://102325201

    Reviewed by Yusuke Suzuki.

    Rest parameter should be caught in async function. So, running this
    JavaScript program should print "caught".
    ```
    async function f(...[[]]) { }
    f().catch(e => print("caught"));
    ```

    V8 (used console.log)
    ```
    $ node input.js
    caught
    ```

    GraalJS
    ```
    $ js input.js
    caught
    ```

    https://tc39.es/ecma262/#sec-async-function-definitions
    ...
    AsyncFunctionDeclaration[Yield, Await, Default] :
        async [no LineTerminator here] function BindingIdentifier[?Yield, ?Await] ( FormalParameters[~Yield, +Await] ) { AsyncFunctionBody }
        [+Default] async [no LineTerminator here] function ( FormalParameters[~Yield, +Await] ) { AsyncFunctionBody }

    AsyncFunctionExpression :
        async [no LineTerminator here] function BindingIdentifier[~Yield, +Await]opt ( FormalParameters[~Yield, +Await] ) { AsyncFunctionBody }
    ...

    According to the spec, it indicates `FormalParameters` is used for Async
    Function, where `FormalParameters` can be converted to `FunctionRestParameter`.

    https://tc39.es/ecma262/#sec-parameter-lists
    ...
    FormalParameters[Yield, Await] :
        [empty]
        FunctionRestParameter[?Yield, ?Await]
        FormalParameterList[?Yield, ?Await]
        FormalParameterList[?Yield, ?Await] ,
        FormalParameterList[?Yield, ?Await] , FunctionRestParameter[?Yield, ?Await]
    ...

    And based on RS: EvaluateAsyncFunctionBody, it will invoke the promise.reject
    callback function with abrupt value ([[value]] of non-normal completion record).

    https://tc39.es/ecma262/#sec-runtime-semantics-evaluateasyncfunctionbody
    ...
    2. Let declResult be Completion(FunctionDeclarationInstantiation(functionObject, argumentsList)).
    3. If declResult is an abrupt completion, then
        a. Perform ! Call(promiseCapability.[[Reject]], undefined, « declResult.[[Value]] »).
    ...

    In that case, any non-normal results of evaluating rest parameters should be
    caught and passed to the reject callback function.

    To resolve this problem, we should allow the emitted RestParameterNode be wrapped
    by the catch handler for promise. However, we should remove `m_restParameter` and
    emit rest parameter byte code in `initializeDefaultParameterValuesAndSetupFunctionScopeStack`
    if we can prove that change has no side effect. In that case, we can only use one
    exception handler.

    Current fix is to add another exception handler. And move the handler byte codes to
    the bottom of code block in order to make other byte codes as much compact as possible.

    Input:
    ```
    async function f(arg0, ...[[]]) { }
    f();
    ```

    Dumped Byte Codes:
    ```
    ...

    bb#2
    Predecessors: [ WebKit#1 ]
    [  20] mov                dst:loc9, src:<JSValue()>(const0)
    ...

    bb#3
    Predecessors: [ WebKit#2 ]
    [  29] get_rest_length    dst:loc11, numParametersToSkip:1
    ...

    bb#12
    Predecessors: [ WebKit#8 WebKit#9 WebKit#10 ]
    [ 138] new_func_exp       dst:loc10, scope:loc4, functionDecl:0
    ...

    bb#13
    Predecessors: [ ]
    [ 170] catch              exception:loc10, thrownValue:loc8
    [ 174] jmp                targetLabel:8(->182)
    Successors: [ WebKit#15 ]

    bb#14
    Predecessors: [ WebKit#7 WebKit#11 ]
    [ 176] catch              exception:loc10, thrownValue:loc8
    [ 180] jmp                targetLabel:2(->182)
    Successors: [ WebKit#15 ]

    bb#15
    Predecessors: [ WebKit#13 WebKit#14 ]
    [ 182] mov                dst:loc12, src:Undefined(const1)
    ...

    Exception Handlers:
    	 1: { start: [  20] end: [  29] target: [ 170] } synthesized catch
    	 2: { start: [  29] end: [ 138] target: [ 176] } synthesized catch
    ```

    * JSTests/stress/catch-rest-parameter.js: Added.
    (throwError):
    (shouldThrow):
    (async f):
    (throwError.async f):
    (throwError.async let):
    (async let):
    (x.async f):
    (x):
    (async shouldThrow):
    * Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:
    (JSC::BytecodeGenerator::BytecodeGenerator):
    (JSC::BytecodeGenerator::initializeDefaultParameterValuesAndSetupFunctionScopeStack):
    * Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:

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

Canonical link: https://commits.webkit.org/252432.994@safari-7614-branch
webkit-commit-queue pushed a commit to Constellation/WebKit that referenced this pull request Jan 24, 2023
https://bugs.webkit.org/show_bug.cgi?id=251063
rdar://104585575

Reviewed by Mark Lam and Justin Michaud.

This patch enhances CallFrame::dump to support wasm frames in btjs stacktrace.
The example is as follows.

    frame #0: 0x00000001035fca78 JavaScriptCore`JSC::functionBreakpoint(globalObject=0x000000012f410068, callFrame=0x000000016fdfa9d0) at JSDollarVM.cpp:2273:9 [opt]
    frame WebKit#1: 0x000000010ec44204 0x10eccc5dc
    frame WebKit#2: 0x000000010eccc5dc callback#Dwaxn6 [Baseline bc#50](Undefined)
    frame WebKit#3: 0x000000010ec4ca84 wasm-stub [WasmToJS](Wasm::Instance: 0x10d29da40)
    frame WebKit#4: 0x000000010ed0c060 <?>.wasm-function[1] [OMG](Wasm::Instance: 0x10d29da40)
    frame WebKit#5: 0x000000010ed100d0 jsToWasm#CWTx6k [FTL bc#22](Cell[JSModuleEnvironment]: 0x12f524540, Cell[WebAssemblyFunction]: 0x10d06a3a8, 1, 2, 3)
    frame WebKit#6: 0x000000010ec881b0 #D5ymZE [Baseline bc#733](Undefined, Cell[Generator]: 0x12f55c180, 1, Cell[Object]: 0x12f69dfc0, 0, Cell[JSLexicalEnvironment]: 0x12f52cee0)
    frame WebKit#7: 0x000000010ec3c008 asyncFunctionResume#A4ayYg [LLInt bc#49](Undefined, Cell[Generator]: 0x12f55c180, Cell[Object]: 0x12f69dfc0, 0)
    frame WebKit#8: 0x000000010ec3c008 promiseReactionJobWithoutPromise#D0yDF1 [LLInt bc#25](Undefined, Cell[Function]: 0x12f44f3c0, Cell[Object]: 0x12f69dfc0, Cell[Generator]: 0x12f55c180)
    frame WebKit#9: 0x000000010ec80ec0 promiseReactionJob#EdShZz [Baseline bc#74](Undefined, Undefined, Cell[Function]: 0x12f44f3c0, Cell[Object]: 0x12f69dfc0, Cell[Generator]: 0x12f55c180)
    frame WebKit#10: 0x000000010ec3c728
    frame WebKit#11: 0x0000000103137560 JavaScriptCore`JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) [inlined] JSC::JITCode::execute(this=<unavailable>, vm=<unavailable>, protoCallFrame=<unavailable>) at JITCodeInlines.h:42:38 [opt]
    frame WebKit#12: 0x0000000103137524 JavaScriptCore`JSC::Interpreter::executeCall(this=<unavailable>, lexicalGlobalObject=<unavailable>, function=<unavailable>, callData=<unavailable>, thisValue=<unavailable>, args=<unavailable>) at Interpreter.cpp:1093:27 [opt]
    frame WebKit#13: 0x000000010349d6d0 JavaScriptCore`JSC::runJSMicrotask(globalObject=0x000000012f410068, identifier=(m_identifier = 81), job=JSValue @ x22, argument0=JSValue @ x26, argument1=JSValue @ x25, argument2=<unavailable>, argument3=<unavailable>) at JSMicrotask.cpp:98:9 [opt]
    frame WebKit#14: 0x00000001039dfc54 JavaScriptCore`JSC::VM::drainMicrotasks() (.cold.1) at VM.cpp:0:9 [opt]
    frame WebKit#15: 0x00000001035e58a4 JavaScriptCore`JSC::VM::drainMicrotasks() [inlined] JSC::MicrotaskQueue::dequeue(this=<unavailable>) at VM.cpp:0:9 [opt]
    frame WebKit#16: 0x00000001035e5894 JavaScriptCore`JSC::VM::drainMicrotasks(this=0x000000012f000000) at VM.cpp:1255:46 [opt]
    ...

* Source/JavaScriptCore/interpreter/CallFrame.cpp:
(JSC::CallFrame::dump const):

Canonical link: https://commits.webkit.org/259262@main
CGQAQ pushed a commit to CGQAQ/WebKit that referenced this pull request Aug 25, 2023
add WeakMapImpl.h and WeakMapImplInlines.h to build
webkit-commit-queue pushed a commit to robert-jenner/WebKit that referenced this pull request Dec 18, 2024
https://bugs.webkit.org/show_bug.cgi?id=281902
rdar://136486349

Reviewed by Mike Wyrzykowski.

Metal: Ensure potentially infinite loops have defined behavior

The MSL compiler would omit infinite loops and assume number domains
based on the omission logic. This would induce incorrect number domains
in case the infinite loops would be invokable. Infinite loops are
undefined in C++ and thus in MSL. It is the job of the programmer
to ensure undefined behavior cannot happen.

Consider GLSL loop like:
uniform float i;
...
    if (i != 0.5) for(;;) { }
    gl_FragColor = vec4(i);

Historically this would emit MSL loop in spirit of:
    if (i != 0.5) {
      bool c = true;
      while (c) { }
    }
    ANGLE_fragmentOut.gl_FragColor = metal::float4(i, i, i, i);

Since This could cause the MSL compiler to optimize the function to
equivalent of:
    ANGLE_fragmentOut.gl_FragColor = metal::float4(0.5, 0.5, 0.5, 0.5);

Presumably this loop omission would happen at the clang frontend part.

Before, was worked around by emitting asm statements to the MSL:
    bool c = true;
    while (c) {
        __asm__("");
    }

The asm injection would would work for this particular source pattern,
presumably because injecting the asm would avoid the loop omission at
the clang frontend part.

The MSL/C++ code is still UB, though. The asm statement does not cause
anything that C++ would consider as "forward progress" of the loop. The
success was just due to how the backend worked. The bitcode produced
would be similar to:

4:
    tail call void asm sideeffect "", ""() WebKit#6, !srcloc !28
    br label %4, !llvm.loop !29

Here, the compiler can be seen to simply fail to detect a loop that
does not make forward progress.

Considering GLSL of form:
uniform int f;
...
    for (;;) { if (f <= 1) break; }

With asm injection to the loop, this would produce:

5:
    tail call void asm sideeffect "", ""() WebKit#8, !srcloc !29
    %6 = load i32, i32 addrspace(2)* %4, align 4, !tbaa !30
    %7 = icmp slt i32 %6, 2
    br i1 %7, label %8, label %5
8:

This code is still assumed to make progress. The backend optimizer is
free to assume that the condition holds, since the load to break the
loop is from constant address space. I.e. uniform f does not change its
value during the loop.

Instead of injecting asm, inject a read of unused volatile variable.
The volatile variable access is defined in C++ as forward progress.
This means infinite loop containing such read is considered defined.
To simplify the implementation and to avoid volatile writes, the read
is to a dummy variable instead of the loop condition bool.

The tests here do not pass completely for MSL backend. In case the
compiler would omit the infinite loop (unpatched code), they would fail
with demonstration of how the values behave. After fixing, the loops
cause timeout but Metal backend does not have implementation to report
context loss. Also, the ReadPixels is just for demostration purposes of the
unpatched code.

* Source/ThirdParty/ANGLE/src/compiler/translator/msl/EmitMetal.cpp:
(GenMetalTraverser::GenMetalTraverser):
(GenMetalTraverser::emitLoopBody):
(GenMetalTraverser::emitForwardProgressStore):
(GenMetalTraverser::emitForwardProgressSignal):
(GenMetalTraverser::visitForLoop):
(GenMetalTraverser::visitWhileLoop):
(GenMetalTraverser::visitDoWhileLoop):
* Source/ThirdParty/ANGLE/src/tests/angle_end2end_tests.gni:
* Source/ThirdParty/ANGLE/src/tests/gl_tests/TimeoutDrawTest.cpp: Added.
(angle::TimeoutDrawTest::TimeoutDrawTest):
(angle::TEST_P):

Originally-landed-as: 283286.350@safari-7620-branch (b82d94e). rdar://141318430
Canonical link: https://commits.webkit.org/288020@main
philn added a commit to philn/WebKit that referenced this pull request Feb 6, 2025
Reviewed by NOBODY (OOPS!).

This was spotted by ASan, the real and imaginary AudioFloatArrays end-up using aligned_alloc() for
their storage, which expects a power-of-two size. Using fftSize / 2 + 1 makes the size
non-power-of-two, the full fftSize being power-of-two (we now have an ASSERT for this).

==1733723==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fc75c438bca bp 0x7ffed612c860 sp 0x7ffed612c028 T0)
==1733723==The signal is caused by a WRITE memory access.
==1733723==Hint: address points to the zero page.
    #0 0x7fc75c438bca in __memset_avx2_unaligned_erms (/lib64/libc.so.6+0x16dbca) (BuildId: 77c77fee058b19c6f001cf2cb0371ce3b8341211)
    WebKit#1 0x2fc4ce in __asan_memset (/var/home/phil/WebKit/local-build-gtk/WebKitBuild/GTK/Release/bin/WebKitWebProcess+0x2fc4ce) (BuildId: e2b0aff0c8fcab48026d63cf711cd70d0aeceb79)
    WebKit#2 0x7fc77047599e in WebCore::FFTFrame::FFTFrame(unsigned int) UnifiedSource-3c72abbe-20.cpp
    WebKit#3 0x7fc76d41c08b in WebCore::PeriodicWave::createBandLimitedTables(float const*, float const*, unsigned int, WebCore::ShouldDisableNormalization) UnifiedSource-f8afad56-52.cpp
    WebKit#4 0x7fc76d41f371 in WebCore::PeriodicWave::generateBasicWaveform(WebCore::PeriodicWave::Type) UnifiedSource-f8afad56-52.cpp
    WebKit#5 0x7fc76d41e633 in WebCore::PeriodicWave::createSine(float) UnifiedSource-f8afad56-52.cpp
    WebKit#6 0x7fc76d3c2352 in WebCore::BaseAudioContext::periodicWave(WebCore::OscillatorType) UnifiedSource-f8afad56-49.cpp
    WebKit#7 0x7fc76d410dbf in WebCore::OscillatorNode::setTypeForBindings(WebCore::OscillatorType) UnifiedSource-f8afad56-52.cpp
    WebKit#8 0x7fc76d4102f4 in WebCore::OscillatorNode::create(WebCore::BaseAudioContext&, WebCore::OscillatorOptions const&) UnifiedSource-f8afad56-52.cpp
    WebKit#9 0x7fc76d3bc210 in WebCore::BaseAudioContext::createOscillator() UnifiedSource-f8afad56-49.cpp
    WebKit#10 0x7fc76acbc743 in WebCore::jsBaseAudioContextPrototypeFunction_createOscillator(JSC::JSGlobalObject*, JSC::CallFrame*) UnifiedSource-3a52ce78-11.cpp
    WebKit#11 0x7fc705c10037  (<unknown module>)

* Source/WebCore/platform/audio/gstreamer/FFTFrameGStreamer.cpp:
(WebCore::FFTFrame::FFTFrame):
webkit-commit-queue pushed a commit to kmiller68/WebKit that referenced this pull request May 15, 2025
https://bugs.webkit.org/show_bug.cgi?id=292725
rdar://150797746

Reviewed by Justin Michaud and Daniel Liu.

This patch hardens how IPInt dispatches opcodes for better CFI protection. Since IPInt does an offset based
dispatch the previous dispatch was something like:

```
macro dispatchToNextIPIntInstruction()
    // x7 is set to _ipInt_instruction_base on entry to IPInt entry/call return.
    loadb [PC], x0
    emit "add x0, x7, x0, lsl WebKit#8"
    emit "br x0"
end

align 256
_ipint_dispatch_base:
.first_wasm_bytecode:
// stuff
dispatchToNextIPIntInstruction()

align 256
.second_wasm_bytecode:
// stuff
dispatchToNextIPIntInstruction()
...
align 256
.255_wasm_bytecode:
// stuff
dispatchToNextIPIntInstruction()
```
In the old system no matter what value [PC] points to there are 256 offsets reachable, all of which are
either a valid opcode or a slab of `brk`s.

However, this still means if an attacker is able to return to the IPInt interpreter on some path that
doesn't reset x7 then they'll be able to implement a JOP attack. One example would be to build a fake
object with a vtable pointing to one of the "C function" labels (e.g. _first_wasm_bytecode_validate,
which is used for offset validation).

After this change dispatch is now:

```
macro dispatchToNextIPIntInstruction()
    loadb something, x0
    pcrtoaddr _ipint_dispatch_base, x7
    emit "add x0, x7, x0, lsl WebKit#8"
    emit "br x0"
end
```

which makes it clearly impossible to get a PAC bypass in IPInt dispatch. Since there's no longer
a semi-pinned register with the base pointer this patch removes all that associated code.

Additionally, this change adds a names for all the dispatch starts to make the
code a bit easier to read.

Lastly, the SIMD prefix opcode was missing a security guard so this patch adds that too.

Canonical link: https://commits.webkit.org/294973@main
webkit-commit-queue pushed a commit to Constellation/WebKit that referenced this pull request Jun 12, 2025
https://bugs.webkit.org/show_bug.cgi?id=294344
rdar://153095450

Reviewed by Keith Miller.

Relanding Keith's change with some fixes for x64. In x64, we cannot
directly get label address, so instead of doing it, we store them into
JSCConfig and load it.

This patch hardens how IPInt dispatches opcodes for better CFI protection. Since IPInt does an offset based
dispatch the previous dispatch was something like:

```
macro dispatchToNextIPIntInstruction()
    // x7 is set to _ipInt_instruction_base on entry to IPInt entry/call return.
    loadb [PC], x0
    emit "add x0, x7, x0, lsl WebKit#8"
    emit "br x0"
end

align 256
_ipint_dispatch_base:
.first_wasm_bytecode:
// stuff
dispatchToNextIPIntInstruction()

align 256
.second_wasm_bytecode:
// stuff
dispatchToNextIPIntInstruction()
...
align 256
.255_wasm_bytecode:
// stuff
dispatchToNextIPIntInstruction()
```
In the old system no matter what value [PC] points to there are 256 offsets reachable, all of which are
either a valid opcode or a slab of `brk`s.

However, this still means if an attacker is able to return to the IPInt interpreter on some path that
doesn't reset x7 then they'll be able to implement a JOP attack. One example would be to build a fake
object with a vtable pointing to one of the "C function" labels (e.g. _first_wasm_bytecode_validate,
which is used for offset validation).

After this change dispatch is now:

```
macro dispatchToNextIPIntInstruction()
    loadb something, x0
    pcrtoaddr _ipint_dispatch_base, x7
    emit "add x0, x7, x0, lsl WebKit#8"
    emit "br x0"
end
```

which makes it clearly impossible to get a PAC bypass in IPInt dispatch. Since there's no longer
a semi-pinned register with the base pointer this patch removes all that associated code.

Additionally, this change adds a names for all the dispatch starts to make the
code a bit easier to read.

Lastly, the SIMD prefix opcode was missing a security guard so this patch adds that too.

* Source/JavaScriptCore/llint/InPlaceInterpreter.asm:
* Source/JavaScriptCore/llint/InPlaceInterpreter.cpp:
(JSC::IPInt::initialize):
* Source/JavaScriptCore/llint/InPlaceInterpreter.h:
* Source/JavaScriptCore/llint/InPlaceInterpreter32_64.asm:
* Source/JavaScriptCore/llint/InPlaceInterpreter64.asm:
* Source/JavaScriptCore/llint/LowLevelInterpreter.asm:
* Source/JavaScriptCore/runtime/JSCConfig.h:

Canonical link: https://commits.webkit.org/296121@main
webkit-commit-queue pushed a commit to hyjorc1/WebKit that referenced this pull request Jan 17, 2026
…n to fix deadlock

https://bugs.webkit.org/show_bug.cgi?id=305674
rdar://168319182

Reviewed by Yusuke Suzuki.

The WASM debugger can deadlock when Thread WebKit#8 suspends Thread WebKit#4 via
thread_suspend() while Thread WebKit#4 is in the middle of ref counting operations.
The suspended thread may hold a WordLock (used for thread-safe ref counting),
and when the suspending thread tries to create a RefPtr copy, it blocks
waiting for the same lock.

The deadlock sequence:
1. Thread WebKit#4 acquires WordLock in strongDeref() destructor
2. Thread WebKit#8 suspends Thread WebKit#4 via thread_suspend() Mach kernel trap
3. Thread WebKit#4 is frozen mid-unlock, WordLock never released
4. Thread WebKit#8's lambda calls vm.apiLock().ownerThread(), creating RefPtr copy
5. RefPtr copy triggers strongRef() which tries to acquire same WordLock
6. Deadlock: Thread WebKit#4 suspended holding lock, Thread WebKit#8 blocked waiting

The fix adds ownerThreadUID() methods that return the thread UID directly
without creating temporary RefPtr objects, avoiding all ref counting
operations and the associated lock contention.

Canonical link: https://commits.webkit.org/305766@main
webkit-commit-queue pushed a commit that referenced this pull request Jul 11, 2026
…ems and crossSizeForFlexLines

https://bugs.webkit.org/show_bug.cgi?id=318494

Reviewed by Antti Koivisto.

Cross sizing was fused into one per-line function (returning the line's extent,
its baseline groups, and the next offset at once) wrapped by
computeCrossSizeForFlexLines. FlexLayout::layout splits it instead: #7
hypotheticalCrossSizeForFlexItems returns each item's hypothetical cross size,
and #8 crossSizeForFlexLines turns those into each line's cross size. Match that
shape -- both now return plain Vector<LayoutUnit> lists, so an FFC body ports in.

crossSizeForFlexLines keeps legacy's line-size arithmetic (the running
first/last-baseline maxes with a shared descent), so this preserves behavior
rather than adopting FFC's group max. The row-flow container height growth moves
to the line-state assembly (once per line by the line's cross size, which equals
the old per-item maximum), and the baseline sharing groups are built in
performBaselineAlignment where they are used -- safe because baseline items are
never align-self:stretch, so nothing relayouts them between sizing and alignment.
LineState no longer carries the groups.

performFlexLayout now runs its sizing and alignment phases as
performContentSizing() and performContentAlignment() lambdas, matching
FlexLayout::layout. layoutFlexLines, computeCrossSizeForFlexLines, and the
FlexLineResult struct are gone.

No change in behavior.

* Source/WebCore/rendering/RenderFlexibleBox.h:
* Source/WebCore/rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::LineState::LineState):
(WebCore::RenderFlexibleBox::performFlexLayout):
(WebCore::RenderFlexibleBox::hypotheticalCrossSizeForFlexItems):
(WebCore::RenderFlexibleBox::crossSizeForFlexLines):
(WebCore::RenderFlexibleBox::handleCrossAxisAlignmentForFlexItems):
(WebCore::RenderFlexibleBox::performBaselineAlignment):
(WebCore::RenderFlexibleBox::computeCrossSizeForFlexLines): Deleted.
(WebCore::RenderFlexibleBox::layoutFlexLines): Deleted.

Canonical link: https://commits.webkit.org/316954@main
webkit-commit-queue pushed a commit that referenced this pull request Jul 12, 2026
…xItem and marginBoxAscentForFlexItem instead of reading the renderer

https://bugs.webkit.org/show_bug.cgi?id=318518

Reviewed by Antti Koivisto.

Following 318517, which captured each flex item's used cross size into
computeCrossSizeForFlexItems's list, route the remaining post-stretch cross-axis
alignment reads through that list. availableAlignmentSpaceForFlexItem and
marginBoxAscentForFlexItem now take the used cross size as an argument rather than
reading crossAxisExtentForFlexItem (the renderer's border box) themselves.

Their post-stretch callers (handleCrossAxisAlignmentForFlexItems and
performBaselineAlignment) pass crossSizes[itemIndex] -- the value captured in #11,
which nothing relays out before these phases, so it is exactly what they read from
the renderer before. The two callers that run outside that window pass
crossAxisExtentForFlexItem directly, unchanged: crossSizeForFlexLines (#8, before
the stretch pass, and only for baseline items, which are never stretched) and the
out-of-flow static-position path, whose item is not in the list. So the used
cross size now flows through the algorithm's list for the whole post-stretch
alignment path; the only renderer cross-size reads left are #8 and the external
baseline queries (firstLineBaseline / baselinePosition).

No change in behavior.

* Source/WebCore/rendering/RenderFlexibleBox.h:
* Source/WebCore/rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::availableAlignmentSpaceForFlexItem):
(WebCore::RenderFlexibleBox::marginBoxAscentForFlexItem):
(WebCore::RenderFlexibleBox::staticCrossAxisPositionForPositionedFlexItem):
(WebCore::RenderFlexibleBox::crossSizeForFlexLines):
(WebCore::RenderFlexibleBox::handleCrossAxisAlignmentForFlexItems):
(WebCore::RenderFlexibleBox::performBaselineAlignment):

Canonical link: https://commits.webkit.org/316980@main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant