Title: [279587] branches/safari-612.1.22.0-branch/Source/_javascript_Core
Revision
279587
Author
rubent...@apple.com
Date
2021-07-06 10:05:10 -0700 (Tue, 06 Jul 2021)

Log Message

Cherry-pick r279560. rdar://problem/80212171

    ActiveScratchBufferScope should take the buffer as argument
    https://bugs.webkit.org/show_bug.cgi?id=227670
    rdar://80011612

    Reviewed by Mark Lam.

    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 operation 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 (I added ScratchBuffer::fromData() to do so).

    * dfg/DFGOSRExit.cpp:
    (JSC::DFG::JSC_DEFINE_JIT_OPERATION):
    * dfg/DFGOSRExit.h:
    * dfg/DFGOperations.cpp:
    (JSC::DFG::JSC_DEFINE_JIT_OPERATION):
    * dfg/DFGSpeculativeJIT.cpp:
    (JSC::DFG::SpeculativeJIT::compileNewArray):
    * dfg/DFGThunks.cpp:
    (JSC::DFG::osrExitGenerationThunkGenerator):
    * runtime/JSGlobalObject.cpp:
    (JSC::JSGlobalObject::haveABadTime):
    * runtime/VM.h:
    (JSC::ScratchBuffer::fromData):
    * runtime/VMInlines.h:
    (JSC::ActiveScratchBufferScope::ActiveScratchBufferScope):
    (JSC::ActiveScratchBufferScope::~ActiveScratchBufferScope):

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@279560 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Diff

Modified: branches/safari-612.1.22.0-branch/Source/_javascript_Core/ChangeLog (279586 => 279587)


--- branches/safari-612.1.22.0-branch/Source/_javascript_Core/ChangeLog	2021-07-06 17:04:34 UTC (rev 279586)
+++ branches/safari-612.1.22.0-branch/Source/_javascript_Core/ChangeLog	2021-07-06 17:05:10 UTC (rev 279587)
@@ -1,3 +1,72 @@
+2021-07-06  Ruben Turcios  <rubent...@apple.com>
+
+        Cherry-pick r279560. rdar://problem/80212171
+
+    ActiveScratchBufferScope should take the buffer as argument
+    https://bugs.webkit.org/show_bug.cgi?id=227670
+    rdar://80011612
+    
+    Reviewed by Mark Lam.
+    
+    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 operation 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 (I added ScratchBuffer::fromData() to do so).
+    
+    * dfg/DFGOSRExit.cpp:
+    (JSC::DFG::JSC_DEFINE_JIT_OPERATION):
+    * dfg/DFGOSRExit.h:
+    * dfg/DFGOperations.cpp:
+    (JSC::DFG::JSC_DEFINE_JIT_OPERATION):
+    * dfg/DFGSpeculativeJIT.cpp:
+    (JSC::DFG::SpeculativeJIT::compileNewArray):
+    * dfg/DFGThunks.cpp:
+    (JSC::DFG::osrExitGenerationThunkGenerator):
+    * runtime/JSGlobalObject.cpp:
+    (JSC::JSGlobalObject::haveABadTime):
+    * runtime/VM.h:
+    (JSC::ScratchBuffer::fromData):
+    * runtime/VMInlines.h:
+    (JSC::ActiveScratchBufferScope::ActiveScratchBufferScope):
+    (JSC::ActiveScratchBufferScope::~ActiveScratchBufferScope):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@279560 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2021-07-04  Robin Morisset  <rmoris...@apple.com>
+
+            ActiveScratchBufferScope should take the buffer as argument
+            https://bugs.webkit.org/show_bug.cgi?id=227670
+            rdar://80011612
+
+            Reviewed by Mark Lam.
+
+            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 operation 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 (I added ScratchBuffer::fromData() to do so).
+
+            * dfg/DFGOSRExit.cpp:
+            (JSC::DFG::JSC_DEFINE_JIT_OPERATION):
+            * dfg/DFGOSRExit.h:
+            * dfg/DFGOperations.cpp:
+            (JSC::DFG::JSC_DEFINE_JIT_OPERATION):
+            * dfg/DFGSpeculativeJIT.cpp:
+            (JSC::DFG::SpeculativeJIT::compileNewArray):
+            * dfg/DFGThunks.cpp:
+            (JSC::DFG::osrExitGenerationThunkGenerator):
+            * runtime/JSGlobalObject.cpp:
+            (JSC::JSGlobalObject::haveABadTime):
+            * runtime/VM.h:
+            (JSC::ScratchBuffer::fromData):
+            * runtime/VMInlines.h:
+            (JSC::ActiveScratchBufferScope::ActiveScratchBufferScope):
+            (JSC::ActiveScratchBufferScope::~ActiveScratchBufferScope):
+
 2021-06-30  Alan Coon  <alanc...@apple.com>
 
         Revert r279249. rdar://problem/79987808

Modified: branches/safari-612.1.22.0-branch/Source/_javascript_Core/dfg/DFGOSRExit.cpp (279586 => 279587)


--- branches/safari-612.1.22.0-branch/Source/_javascript_Core/dfg/DFGOSRExit.cpp	2021-07-06 17:04:34 UTC (rev 279586)
+++ branches/safari-612.1.22.0-branch/Source/_javascript_Core/dfg/DFGOSRExit.cpp	2021-07-06 17:05:10 UTC (rev 279587)
@@ -141,11 +141,11 @@
     }
 }
 
-JSC_DEFINE_JIT_OPERATION(operationCompileOSRExit, void, (CallFrame* callFrame))
+JSC_DEFINE_JIT_OPERATION(operationCompileOSRExit, void, (CallFrame* callFrame, void* bufferToPreserve))
 {
     VM& vm = callFrame->deprecatedVM();
     auto scope = DECLARE_THROW_SCOPE(vm);
-    ActiveScratchBufferScope activeScratchBufferScope(vm, GPRInfo::numberOfRegisters + FPRInfo::numberOfRegisters);
+    ActiveScratchBufferScope activeScratchBufferScope(ScratchBuffer::fromData(bufferToPreserve), GPRInfo::numberOfRegisters + FPRInfo::numberOfRegisters);
 
     if constexpr (validateDFGDoesGC) {
         // We're about to exit optimized code. So, there's no longer any optimized
@@ -930,7 +930,7 @@
 {
     VM& vm = callFrame->deprecatedVM();
     NativeCallFrameTracer tracer(vm, callFrame);
-    ActiveScratchBufferScope activeScratchBufferScope(vm, GPRInfo::numberOfRegisters + FPRInfo::numberOfRegisters);
+    ActiveScratchBufferScope activeScratchBufferScope(ScratchBuffer::fromData(scratch), GPRInfo::numberOfRegisters + FPRInfo::numberOfRegisters);
 
     SpeculationFailureDebugInfo* debugInfo = static_cast<SpeculationFailureDebugInfo*>(debugInfoRaw);
     CodeBlock* codeBlock = debugInfo->codeBlock;

Modified: branches/safari-612.1.22.0-branch/Source/_javascript_Core/dfg/DFGOSRExit.h (279586 => 279587)


--- branches/safari-612.1.22.0-branch/Source/_javascript_Core/dfg/DFGOSRExit.h	2021-07-06 17:04:34 UTC (rev 279586)
+++ branches/safari-612.1.22.0-branch/Source/_javascript_Core/dfg/DFGOSRExit.h	2021-07-06 17:05:10 UTC (rev 279587)
@@ -138,7 +138,7 @@
     Profiler::OSRExit* profilerExit { nullptr };
 };
 
-JSC_DECLARE_JIT_OPERATION(operationCompileOSRExit, void, (CallFrame*));
+JSC_DECLARE_JIT_OPERATION(operationCompileOSRExit, void, (CallFrame*, void*));
 JSC_DECLARE_JIT_OPERATION(operationDebugPrintSpeculationFailure, void, (CallFrame*, void*, void*));
 JSC_DECLARE_JIT_OPERATION(operationMaterializeOSRExitSideState, void, (VM*, const OSRExitBase*, EncodedJSValue*));
 
@@ -149,7 +149,7 @@
 struct OSRExit : public OSRExitBase {
     OSRExit(ExitKind, JSValueSource, MethodOfGettingAValueProfile, SpeculativeJIT*, unsigned streamIndex, unsigned recoveryIndex = UINT_MAX);
 
-    friend void JIT_OPERATION_ATTRIBUTES operationCompileOSRExit(CallFrame*);
+    friend void JIT_OPERATION_ATTRIBUTES operationCompileOSRExit(CallFrame*, void*);
 
     CodeLocationLabel<JSInternalPtrTag> m_patchableJumpLocation;
     MacroAssemblerCodeRef<OSRExitPtrTag> m_code;

Modified: branches/safari-612.1.22.0-branch/Source/_javascript_Core/dfg/DFGOperations.cpp (279586 => 279587)


--- branches/safari-612.1.22.0-branch/Source/_javascript_Core/dfg/DFGOperations.cpp	2021-07-06 17:04:34 UTC (rev 279586)
+++ branches/safari-612.1.22.0-branch/Source/_javascript_Core/dfg/DFGOperations.cpp	2021-07-06 17:05:10 UTC (rev 279587)
@@ -1039,9 +1039,10 @@
     VM& vm = globalObject->vm();
     CallFrame* callFrame = DECLARE_CALL_FRAME(vm);
     JITOperationPrologueCallFrameTracer tracer(vm, callFrame);
-    ActiveScratchBufferScope activeScratchBufferScope(vm, elementCount);
+    ActiveScratchBufferScope activeScratchBufferScope(ScratchBuffer::fromData(buffer), elementCount);
     auto scope = DECLARE_THROW_SCOPE(vm);
 
+
     // We assume that multiple JSArray::push calls with ArrayWithInt32/ArrayWithContiguous do not cause JS traps.
     // If it can cause any JS interactions, we can call the caller JS function of this function and overwrite the
     // content of ScratchBuffer. If the IndexingType is now ArrayWithInt32/ArrayWithContiguous, we can ensure
@@ -1734,7 +1735,7 @@
     VM& vm = globalObject->vm();
     CallFrame* callFrame = DECLARE_CALL_FRAME(vm);
     JITOperationPrologueCallFrameTracer tracer(vm, callFrame);
-    ActiveScratchBufferScope activeScratchBufferScope(vm, size);
+    ActiveScratchBufferScope activeScratchBufferScope(ScratchBuffer::fromData(buffer), size);
 
     return bitwise_cast<char*>(constructArray(globalObject, arrayStructure, static_cast<JSValue*>(buffer), size));
 }
@@ -3042,7 +3043,7 @@
     VM& vm = globalObject->vm();
     CallFrame* callFrame = DECLARE_CALL_FRAME(vm);
     JITOperationPrologueCallFrameTracer tracer(vm, callFrame);
-    ActiveScratchBufferScope activeScratchBufferScope(vm, numItems);
+    ActiveScratchBufferScope activeScratchBufferScope(ScratchBuffer::fromData(buffer), numItems);
     auto scope = DECLARE_THROW_SCOPE(vm);
 
     EncodedJSValue* values = static_cast<EncodedJSValue*>(buffer);

Modified: branches/safari-612.1.22.0-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (279586 => 279587)


--- branches/safari-612.1.22.0-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2021-07-06 17:04:34 UTC (rev 279586)
+++ branches/safari-612.1.22.0-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2021-07-06 17:05:10 UTC (rev 279587)
@@ -9238,13 +9238,14 @@
     flushRegisters();
 
     GPRFlushedCallResult result(this);
+    GPRReg resultGPR = result.gpr();
 
     callOperation(
-        operationNewArray, result.gpr(), TrustedImmPtr::weakPointer(m_graph, globalObject), m_jit.graph().registerStructure(globalObject->arrayStructureForIndexingTypeDuringAllocation(node->indexingType())),
+        operationNewArray, resultGPR, TrustedImmPtr::weakPointer(m_graph, globalObject), m_jit.graph().registerStructure(globalObject->arrayStructureForIndexingTypeDuringAllocation(node->indexingType())),
         static_cast<void*>(buffer), size_t(node->numChildren()));
     m_jit.exceptionCheck();
 
-    cellResult(result.gpr(), node, UseChildrenCalledExplicitly);
+    cellResult(resultGPR, node, UseChildrenCalledExplicitly);
 }
 
 void SpeculativeJIT::compileNewArrayWithSpread(Node* node)

Modified: branches/safari-612.1.22.0-branch/Source/_javascript_Core/dfg/DFGThunks.cpp (279586 => 279587)


--- branches/safari-612.1.22.0-branch/Source/_javascript_Core/dfg/DFGThunks.cpp	2021-07-06 17:04:34 UTC (rev 279586)
+++ branches/safari-612.1.22.0-branch/Source/_javascript_Core/dfg/DFGThunks.cpp	2021-07-06 17:05:10 UTC (rev 279587)
@@ -89,8 +89,8 @@
     }
     storeSpooler.finalizeFPR();
 
-    // Set up one argument.
-    jit.move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
+    // This will implicitly pass GPRInfo::callFrameRegister as the first argument based on the operation type.
+    jit.setupArguments<decltype(operationCompileOSRExit)>(bufferGPR);
     jit.prepareCallOperation(vm);
 
     MacroAssembler::Call functionCall = jit.call(OperationPtrTag);

Modified: branches/safari-612.1.22.0-branch/Source/_javascript_Core/runtime/VM.h (279586 => 279587)


--- branches/safari-612.1.22.0-branch/Source/_javascript_Core/runtime/VM.h	2021-07-06 17:04:34 UTC (rev 279586)
+++ branches/safari-612.1.22.0-branch/Source/_javascript_Core/runtime/VM.h	2021-07-06 17:05:10 UTC (rev 279587)
@@ -260,6 +260,11 @@
         return result;
     }
 
+    static ScratchBuffer* fromData(void* buffer)
+    {
+        return bitwise_cast<ScratchBuffer*>(static_cast<char*>(buffer) - OBJECT_OFFSETOF(ScratchBuffer, m_buffer));
+    }
+
     static size_t allocationSize(Checked<size_t> bufferSize) { return sizeof(ScratchBuffer) + bufferSize; }
     void setActiveLength(size_t activeLength) { u.m_activeLength = activeLength; }
     size_t activeLength() const { return u.m_activeLength; };
@@ -282,7 +287,7 @@
 
 class ActiveScratchBufferScope {
 public:
-    ActiveScratchBufferScope(VM&, size_t activeScratchBufferSizeInJSValues);
+    ActiveScratchBufferScope(ScratchBuffer*, size_t activeScratchBufferSizeInJSValues);
     ~ActiveScratchBufferScope();
 
 private:

Modified: branches/safari-612.1.22.0-branch/Source/_javascript_Core/runtime/VMInlines.h (279586 => 279587)


--- branches/safari-612.1.22.0-branch/Source/_javascript_Core/runtime/VMInlines.h	2021-07-06 17:04:34 UTC (rev 279586)
+++ branches/safari-612.1.22.0-branch/Source/_javascript_Core/runtime/VMInlines.h	2021-07-06 17:05:10 UTC (rev 279587)
@@ -32,12 +32,12 @@
 
 namespace JSC {
 
-inline ActiveScratchBufferScope::ActiveScratchBufferScope(VM& vm, size_t activeScratchBufferSizeInJSValues)
-    : m_scratchBuffer(vm.scratchBufferForSize(activeScratchBufferSizeInJSValues * sizeof(EncodedJSValue)))
+inline ActiveScratchBufferScope::ActiveScratchBufferScope(ScratchBuffer* buffer, size_t activeScratchBufferSizeInJSValues)
+    : m_scratchBuffer(buffer)
 {
     // Tell GC mark phase how much of the scratch buffer is active during the call operation this scope is used in.
     if (m_scratchBuffer)
-        m_scratchBuffer->u.m_activeLength = activeScratchBufferSizeInJSValues * sizeof(EncodedJSValue);
+        m_scratchBuffer->setActiveLength(activeScratchBufferSizeInJSValues * sizeof(EncodedJSValue));
 }
 
 inline ActiveScratchBufferScope::~ActiveScratchBufferScope()
@@ -44,7 +44,7 @@
 {
     // Tell the GC that we're not using the scratch buffer anymore.
     if (m_scratchBuffer)
-        m_scratchBuffer->u.m_activeLength = 0;
+        m_scratchBuffer->setActiveLength(0);
 }
 
 bool VM::ensureStackCapacityFor(Register* newTopOfStack)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to