RESOLVED FIXED 227670
ActiveScratchBufferScope should take the buffer as argument
https://bugs.webkit.org/show_bug.cgi?id=227670
Summary ActiveScratchBufferScope should take the buffer as argument
Robin Morisset
Reported 2021-07-04 00:15:52 PDT
https://bugs.webkit.org/show_bug.cgi?id=227013 created ActiveScratchBufferScope. It is used by operations that can cause the GC to run, to mark as roots the contents of the scratch buffer that is live during that time (if any). The bug is that it simply asks the VM for a scratch buffer of the right size, but this will always return the last scratch buffer, and not necessarily the one that the operations is actually using. A fairly simple fix is to pass it directly the scratch buffer, since the operation normally can get it easily enough. In most cases the operation has access to the m_buffer field of the ScratchBuffer, but getting a pointer to the entire structure from that is fairly simple.
Attachments
Patch (11.52 KB, patch)
2021-07-04 00:23 PDT, Robin Morisset
mark.lam: review+
Patch for landing (11.04 KB, patch)
2021-07-04 07:47 PDT, Robin Morisset
ews-feeder: commit-queue-
Patch for landing (11.13 KB, patch)
2021-07-04 11:46 PDT, Robin Morisset
no flags
Robin Morisset
Comment 1 2021-07-04 00:16:42 PDT
Robin Morisset
Comment 2 2021-07-04 00:23:09 PDT
Mark Lam
Comment 3 2021-07-04 00:44:27 PDT
Comment on attachment 432859 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432859&action=review r=me with suggested improvement, and if EWS bots are happy. > Source/JavaScriptCore/dfg/DFGThunks.cpp:94 > - // Set up one argument. > + // Set up two arguments. > jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0); > + jit.move(bufferGPR, GPRInfo::argumentGPR1); Let's just replace this with: jit.setupArguments<decltype(operationCompileOSRExit)>(GPRInfo::callFrameRegister, bufferGPR); This has the benefit of ensuring that if bufferGPR happens to be GPRInfo::argumentGPR0, that it will do the right thing to shuffle the value out before overwriting it. And you can also remove the comment now. > Source/JavaScriptCore/runtime/JSGlobalObject.cpp:1916 > - > + Please undo since there are no other relevant changes in this file.
Robin Morisset
Comment 4 2021-07-04 07:47:37 PDT
Created attachment 432863 [details] Patch for landing
Robin Morisset
Comment 5 2021-07-04 11:46:22 PDT
Created attachment 432865 [details] Patch for landing
EWS
Comment 6 2021-07-04 16:55:55 PDT
Committed r279560 (239391@main): <https://commits.webkit.org/239391@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 432865 [details].
Note You need to log in before you can comment on or make changes to this bug.