Bug 238313 – [JSC] JSRemoteFunction thunk should materialize code-pointer

• Component: New Bugs
• Status: RESOLVED FIXED

[Yusuke Suzuki]

[JSC] JSRemoteFunction thunk should materialize code-pointer

[Yusuke Suzuki]

Created attachment 455615 [details]
Patch

[Yusuke Suzuki]

Created attachment 455619 [details]
Patch

[Mark Lam]

Comment on attachment 455619 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=455619&action=review

r=me

> Source/JavaScriptCore/ChangeLog:10
> +        Any GC can jettison CodeBlock's JITCode. It means that it is possible that JITCode is discarded because
> +        of wrapper creation for arguments of remote function. It causes occasional crashes on JSTests/stress/shadow-realm-evaluate.js
> +        test. This patch fixes it by materializing code pointer if it is already jettisoned.

I suggest rephrasing this as:

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.

> Source/JavaScriptCore/jit/ThunkGenerators.cpp:1539
> +    // Any GC can potentially jettison JIT code in JSFunction. This means that calling operationGetWrappedValueForTarget already discarded
> +    // the code. We should check and materialize it here. At this point, once we got the code. So, we can always materialize the code.

I suggest rephrasing this as:

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.

[Yusuke Suzuki]

Sounds good!

[Yusuke Suzuki]

Committed r291815 (248842@trunk): <https://commits.webkit.org/248842@trunk>

[Radar WebKit Bug Importer]

<rdar://problem/90791656>

[Chris Dumez]

Reopening to attach new patch.

[Chris Dumez]

Created attachment 455699 [details]
Patch

[Chris Dumez]

Bad webkit-patch upload.