WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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+
Details
Formatted Diff
Diff
Patch for landing
(11.04 KB, patch)
2021-07-04 07:47 PDT
,
Robin Morisset
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch for landing
(11.13 KB, patch)
2021-07-04 11:46 PDT
,
Robin Morisset
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Robin Morisset
Comment 1
2021-07-04 00:16:42 PDT
rdar://80011612
Robin Morisset
Comment 2
2021-07-04 00:23:09 PDT
Created
attachment 432859
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug