ActiveScratchBufferScope should take the buffer as argument

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Robin Morisset]

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.

[Robin Morisset]

rdar://80011612

[Robin Morisset]

Created attachment 432859 [details]
Patch

[Mark Lam]

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]

Created attachment 432863 [details]
Patch for landing

[Robin Morisset]

Created attachment 432865 [details]
Patch for landing

[EWS]

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].