[JSC] JSRemoteFunction thunk should materialize code-pointer
Created attachment 455615 [details] Patch
Created attachment 455619 [details] Patch
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.
Sounds good!
Committed r291815 (248842@trunk): <https://commits.webkit.org/248842@trunk>
<rdar://problem/90791656>
Reopening to attach new patch.
Created attachment 455699 [details] Patch
Bad webkit-patch upload.