Changeset 291815 in webkit

Timestamp:

Mar 24, 2022, 2:41:42 PM (4 years ago)

Author:

ysuzuki@apple.com

Message:

[JSC] JSRemoteFunction thunk should materialize code-pointer
​https://bugs.webkit.org/show_bug.cgi?id=238313

Reviewed by Mark Lam.

When invoking a JSRemoteFunction, we must first wrap the arguments passed to it.
The wrapping operation may trigger a GC, and GC can jettison JIT code. As a result,
even though we know that the target JSFunction has JIT code that we want to execute,
the JIT code may be jettisoned (while wrapping the arguments for it) before we get
to the call. This resulted in occasional crashes on the JSTests/stress/shadow-realm-evaluate.js test.

This patch fixes this by doing a null check on the JIT code just before calling it,
and if null (i.e. the JIT code has been jettisoned), re-materializing the JIT code
first before making the call.

• jit/JITOperations.cpp:

(JSC::JSC_DEFINE_JIT_OPERATION):

• jit/JITOperations.h:
• jit/ThunkGenerators.cpp:

(JSC::remoteFunctionCallGenerator):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• jit/JITOperations.cpp (modified) (1 diff)
• jit/JITOperations.h (modified) (1 diff)
• jit/ThunkGenerators.cpp (modified) (4 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2022-03-24  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] JSRemoteFunction thunk should materialize code-pointer
+        https://bugs.webkit.org/show_bug.cgi?id=238313
+
+        Reviewed by Mark Lam.
+
+        When invoking a JSRemoteFunction, we must first wrap the arguments passed to it.
+        The wrapping operation may trigger a GC, and GC can jettison JIT code. As a result,
+        even though we know that the target JSFunction has JIT code that we want to execute,
+        the JIT code may be jettisoned (while wrapping the arguments for it) before we get
+        to the call. This resulted in occasional crashes on the JSTests/stress/shadow-realm-evaluate.js test.
+
+        This patch fixes this by doing a null check on the JIT code just before calling it,
+        and if null (i.e. the JIT code has been jettisoned), re-materializing the JIT code
+        first before making the call.
+
+        * jit/JITOperations.cpp:
+        (JSC::JSC_DEFINE_JIT_OPERATION):
+        * jit/JITOperations.h:
+        * jit/ThunkGenerators.cpp:
+        (JSC::remoteFunctionCallGenerator):
+
 2022-03-23  Geza Lore  <glore@igalia.com>
 

trunk/Source/JavaScriptCore/jit/JITOperations.cpp

@@ -157 +157 @@
 
     RELEASE_AND_RETURN(scope, JSValue::encode(getWrappedValue(globalObject, globalObject, JSValue::decode(encodedValue))));
+}
+
+JSC_DEFINE_JIT_OPERATION(operationMaterializeRemoteFunctionTargetCode, void*, (JSRemoteFunction* callee))
+{
+    JSGlobalObject* globalObject = callee->globalObject();
+    VM& vm = globalObject->vm();
+
+    CallFrame* callFrame = DECLARE_CALL_FRAME(vm);
+    JITOperationPrologueCallFrameTracer tracer(vm, callFrame);
+    auto throwScope = DECLARE_THROW_SCOPE(vm);
+
+    ASSERT(isRemoteFunction(vm, callee));
+
+    auto* targetFunction = jsCast<JSFunction*>(callee->targetFunction()); // We call this function only when JSRemoteFunction's target is JSFunction.
+    ExecutableBase* executable = targetFunction->executable();
+
+    // Force the executable to cache its arity entrypoint.
+    {
+        DeferTraps deferTraps(vm); // We can't jettison any code until after we link the call.
+        if (!executable->isHostFunction()) {
+            JSScope* scope = targetFunction->scopeUnchecked();
+            FunctionExecutable* functionExecutable = static_cast<FunctionExecutable*>(executable);
+            CodeBlock* codeBlockSlot = nullptr;
+            functionExecutable->prepareForExecution<FunctionExecutable>(vm, targetFunction, scope, CodeForCall, codeBlockSlot);
+            RETURN_IF_EXCEPTION(throwScope, nullptr);
+        }
+        return executable->entrypointFor(CodeForCall, MustCheckArity).executableAddress();
+    }
 }
 

trunk/Source/JavaScriptCore/jit/JITOperations.h

@@ -162 +162 @@
 JSC_DECLARE_JIT_OPERATION(operationGetWrappedValueForCaller, EncodedJSValue, (JSRemoteFunction*, EncodedJSValue));
 JSC_DECLARE_JIT_OPERATION(operationGetWrappedValueForTarget, EncodedJSValue, (JSRemoteFunction*, EncodedJSValue));
+JSC_DECLARE_JIT_OPERATION(operationMaterializeRemoteFunctionTargetCode, void*, (JSRemoteFunction*));
 JSC_DECLARE_JIT_OPERATION(operationThrowRemoteFunctionException, EncodedJSValue, (JSRemoteFunction*));
 

trunk/Source/JavaScriptCore/jit/ThunkGenerators.cpp

@@ -1503 +1503 @@
         jit.storePtr(GPRInfo::regT1, jit.addressFor(loopIndex));
 
+        jit.setupArguments<decltype(operationGetWrappedValueForTarget)>(GPRInfo::regT0, valueRegs);
         jit.prepareCallOperation(vm);
-        jit.setupArguments<decltype(operationGetWrappedValueForTarget)>(GPRInfo::regT0, valueRegs);
         jit.move(CCallHelpers::TrustedImmPtr(tagCFunction<OperationPtrTag>(operationGetWrappedValueForTarget)), GPRInfo::nonArgGPR0);
         emitPointerValidation(jit, GPRInfo::nonArgGPR0, OperationPtrTag);
@@ -1525 +1525 @@
     jit.storeCell(GPRInfo::regT2, CCallHelpers::calleeFrameSlot(CallFrameSlot::callee));
 
-    jit.loadPtr(CCallHelpers::Address(GPRInfo::regT2, JSFunction::offsetOfExecutableOrRareData()), GPRInfo::regT0);
-    auto hasExecutable = jit.branchTestPtr(CCallHelpers::Zero, GPRInfo::regT0, CCallHelpers::TrustedImm32(JSFunction::rareDataTag));
-    jit.loadPtr(CCallHelpers::Address(GPRInfo::regT0, FunctionRareData::offsetOfExecutable() - JSFunction::rareDataTag), GPRInfo::regT0);
+    jit.loadPtr(CCallHelpers::Address(GPRInfo::regT2, JSFunction::offsetOfExecutableOrRareData()), GPRInfo::regT1);
+    auto hasExecutable = jit.branchTestPtr(CCallHelpers::Zero, GPRInfo::regT1, CCallHelpers::TrustedImm32(JSFunction::rareDataTag));
+    jit.loadPtr(CCallHelpers::Address(GPRInfo::regT1, FunctionRareData::offsetOfExecutable() - JSFunction::rareDataTag), GPRInfo::regT1);
     hasExecutable.link(&jit);
 
     jit.loadPtr(
         CCallHelpers::Address(
-            GPRInfo::regT0, ExecutableBase::offsetOfJITCodeWithArityCheckFor(CodeForCall)),
-        GPRInfo::regT0);
-
+            GPRInfo::regT1, ExecutableBase::offsetOfJITCodeWithArityCheckFor(CodeForCall)),
+        GPRInfo::regT1);
+    auto codeExists = jit.branchTestPtr(CCallHelpers::NonZero, GPRInfo::regT1);
+
+    // The calls to operationGetWrappedValueForTarget above may GC, and any GC can potentially jettison the JIT code in the target JSFunction.
+    // If we find that the JIT code is null (i.e. has been jettisoned), then we need to re-materialize it for the call below. Note that we know
+    // that operationMaterializeRemoteFunctionTargetCode should be able to re-materialize the JIT code (except for any OOME) because we only
+    // went down this code path after we found a non-null JIT code (in the noCode check) above i.e. it should be possible to materialize the JIT code.
+    jit.setupArguments<decltype(operationMaterializeRemoteFunctionTargetCode)>(GPRInfo::regT0);
+    jit.prepareCallOperation(vm);
+    jit.move(CCallHelpers::TrustedImmPtr(tagCFunction<OperationPtrTag>(operationMaterializeRemoteFunctionTargetCode)), GPRInfo::nonArgGPR0);
+    emitPointerValidation(jit, GPRInfo::nonArgGPR0, OperationPtrTag);
+    jit.call(GPRInfo::nonArgGPR0, OperationPtrTag);
+    exceptionChecks.append(jit.emitJumpIfException(vm));
+    jit.move(GPRInfo::returnValueGPR, GPRInfo::regT1);
+
+    codeExists.link(&jit);
     // Based on the check above, we should be good with this. On ARM64, emitPointerValidation will do this.
 #if ASSERT_ENABLED && !CPU(ARM64E)
     {
-        CCallHelpers::Jump checkNotNull = jit.branchTestPtr(CCallHelpers::NonZero, GPRInfo::regT0);
+        CCallHelpers::Jump checkNotNull = jit.branchTestPtr(CCallHelpers::NonZero, GPRInfo::regT1);
         jit.abortWithReason(TGInvalidPointer);
         checkNotNull.link(&jit);
@@ -1544 +1558 @@
 #endif
 
-    emitPointerValidation(jit, GPRInfo::regT0, JSEntryPtrTag);
-    jit.call(GPRInfo::regT0, JSEntryPtrTag);
+    emitPointerValidation(jit, GPRInfo::regT1, JSEntryPtrTag);
+    jit.call(GPRInfo::regT1, JSEntryPtrTag);
 
     // Wrap return value
@@ -1558 +1572 @@
     resultIsPrimitive.append(jit.branchIfNotObject(resultRegs.payloadGPR()));
 
-    jit.move(CCallHelpers::TrustedImmPtr(tagCFunction<OperationPtrTag>(operationGetWrappedValueForCaller)), GPRInfo::nonArgGPR0);
-    emitPointerValidation(jit, GPRInfo::nonArgGPR0, OperationPtrTag);
-
     jit.loadCell(CCallHelpers::addressFor(CallFrameSlot::callee), GPRInfo::regT2);
     jit.setupArguments<decltype(operationGetWrappedValueForCaller)>(GPRInfo::regT2, resultRegs);
     jit.prepareCallOperation(vm);
+    jit.move(CCallHelpers::TrustedImmPtr(tagCFunction<OperationPtrTag>(operationGetWrappedValueForCaller)), GPRInfo::nonArgGPR0);
+    emitPointerValidation(jit, GPRInfo::nonArgGPR0, OperationPtrTag);
     jit.call(GPRInfo::nonArgGPR0, OperationPtrTag);
     exceptionChecks.append(jit.emitJumpIfException(vm));