impl shadow realm as js builtins#8
Conversation
| JSC_TO_STRING_TAG_WITHOUT_TRANSITION(); | ||
| } | ||
|
|
||
| JSC_DEFINE_HOST_FUNCTION(importInRealm, (JSGlobalObject* globalObject, CallFrame* callFrame)) |
There was a problem hiding this comment.
mostly copied and adapted from
WebKit/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp
Lines 813 to 835 in fca70ae
|
|
||
| auto sourceOrigin = callFrame->callerSourceOrigin(vm); | ||
| auto* specifier = callFrame->uncheckedArgument(1).toString(globalObject); | ||
| RETURN_IF_EXCEPTION(scope, { }); |
There was a problem hiding this comment.
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"); |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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
| 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; | ||
| } |
There was a problem hiding this comment.
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.
| if (bindingStr in m) { | ||
| return @wrap(m[bindingStr]); |
There was a problem hiding this comment.
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. | ||
| */ | ||
|
|
There was a problem hiding this comment.
my (idiomatic) JS is horrible so in general be thorough with this file
|
sorry; this was meant to be a PR for my personal fork |
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.
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
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
…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
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
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
add WeakMapImpl.h and WeakMapImplInlines.h to build
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
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):
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
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
…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
…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
…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
accidentally opened this; was meant to be a PR for my personal fork