Attachment 368858 Details for Bug 197531 – patch

[patch] patch

b-backup.diff (text/plain), 9.72 KB, created by Saam Barati on 2019-05-02 18:12:47 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2019-05-02 18:12:47 PDT

Size: 9.72 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 244897)
>+++ JSTests/ChangeLog	(working copy)
>@@ -1,3 +1,13 @@
>+2019-05-02  Saam barati  <sbarati@apple.com>
>+
>+        Don't OSR enter into an FTL CodeBlock that has been jettisoned
>+        https://bugs.webkit.org/show_bug.cgi?id=197531
>+        <rdar://problem/50162379>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/dont-osr-enter-into-jettisoned-ftl-code-block.js: Added.
>+
> 2019-05-02  Michael Saboff  <msaboff@apple.com>
> 
>         Unreviewed rollout of r244862.
>Index: JSTests/stress/dont-osr-enter-into-jettisoned-ftl-code-block.js
>===================================================================
>--- JSTests/stress/dont-osr-enter-into-jettisoned-ftl-code-block.js	(nonexistent)
>+++ JSTests/stress/dont-osr-enter-into-jettisoned-ftl-code-block.js	(working copy)
>@@ -0,0 +1,14 @@
>+//@ runDefault("--useRandomizingFuzzerAgent=1", "--validateAbstractInterpreterState=1", "--jitPolicyScale=0", "--useConcurrentJIT=0", "--validateAbstractInterpreterStateProbability=1.0")
>+
>+let x = [];
>+let k = 1;
>+z = 0;
>+
>+for (var i = 0; i < 36; i++) {
>+    k = k * 2;
>+    x[k - 2] = k;
>+}
>+
>+for (var j = 0; j === -1; j++) {
>+    z = z;
>+}
>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 244878)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,34 @@
>+2019-05-02  Saam barati  <sbarati@apple.com>
>+
>+        Don't OSR enter into an FTL CodeBlock that has been jettisoned
>+        https://bugs.webkit.org/show_bug.cgi?id=197531
>+        <rdar://problem/50162379>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Sometimes we make silly mistakes. This is one of those times. It's invalid to OSR
>+        enter into an FTL OSR entry code block that has been jettisoned already.
>+
>+        * bytecode/CodeBlock.cpp:
>+        (JSC::CodeBlock::jettison):
>+        * dfg/DFGJITCode.cpp:
>+        (JSC::DFG::JITCode::setOSREntryBlock):
>+        (JSC::DFG::JITCode::clearOSREntryBlockAndResetThresholds):
>+        * dfg/DFGJITCode.h:
>+        (JSC::DFG::JITCode::clearOSREntryBlock): Deleted.
>+        * dfg/DFGOSREntry.cpp:
>+        (JSC::DFG::prepareOSREntry):
>+        (JSC::DFG::prepareCatchOSREntry):
>+        * dfg/DFGOperations.cpp:
>+        * ftl/FTLForOSREntryJITCode.cpp:
>+        (JSC::FTL::ForOSREntryJITCode::didInvalidate):
>+        * ftl/FTLForOSREntryJITCode.h:
>+        (JSC::FTL::ForOSREntryJITCode::setDFGJITCode):
>+        * ftl/FTLOSREntry.cpp:
>+        (JSC::FTL::prepareOSREntry):
>+        * jit/JITCode.h:
>+        (JSC::JITCode::didInvalidate):
>+
> 2019-05-02  Michael Saboff  <msaboff@apple.com>
> 
>         Unreviewed rollout of r244862.
>Index: Source/JavaScriptCore/bytecode/CodeBlock.cpp
>===================================================================
>--- Source/JavaScriptCore/bytecode/CodeBlock.cpp	(revision 244878)
>+++ Source/JavaScriptCore/bytecode/CodeBlock.cpp	(working copy)
>@@ -2014,6 +2014,8 @@ void CodeBlock::jettison(Profiler::Jetti
>             RELEASE_ASSERT(this != replacement() || (vm.heap.isCurrentThreadBusy() && !vm.heap.isMarked(ownerExecutable())));
>             return;
>         }
>+
>+        jitCode()->didInvalidate();
>     }
>     
>     if (DFG::shouldDumpDisassembly())
>Index: Source/JavaScriptCore/dfg/DFGJITCode.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGJITCode.cpp	(revision 244878)
>+++ Source/JavaScriptCore/dfg/DFGJITCode.cpp	(working copy)
>@@ -210,6 +210,18 @@ void JITCode::setOSREntryBlock(VM& vm, c
>         dataLog("OSR entries will go to ", osrEntryBlock->jitCode()->ftlForOSREntry()->addressForCall(ArityCheckNotRequired), "\n");
>     }
>     m_osrEntryBlock.set(vm, owner, osrEntryBlock);
>+    osrEntryBlock->jitCode()->ftlForOSREntry()->setDFGJITCode(this);
>+}
>+
>+void JITCode::clearOSREntryBlockAndResetThresholds()
>+{ 
>+    if (!m_osrEntryBlock)
>+        return;
>+
>+    unsigned osrEntryBytecode = m_osrEntryBlock->jitCode()->ftlForOSREntry()->bytecodeIndex();
>+    m_osrEntryBlock.clear();
>+    osrEntryRetry = 0;
>+    tierUpEntryTriggers.set(osrEntryBytecode, JITCode::TriggerReason::DontTrigger);
> }
> #endif // ENABLE(FTL_JIT)
> 
>Index: Source/JavaScriptCore/dfg/DFGJITCode.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGJITCode.h	(revision 244878)
>+++ Source/JavaScriptCore/dfg/DFGJITCode.h	(working copy)
>@@ -121,7 +121,7 @@ public:
> #if ENABLE(FTL_JIT)
>     CodeBlock* osrEntryBlock() { return m_osrEntryBlock.get(); }
>     void setOSREntryBlock(VM&, const JSCell* owner, CodeBlock* osrEntryBlock);
>-    void clearOSREntryBlock() { m_osrEntryBlock.clear(); }
>+    void clearOSREntryBlockAndResetThresholds();
> #endif
> 
>     static ptrdiff_t commonDataOffset() { return OBJECT_OFFSETOF(JITCode, common); }
>Index: Source/JavaScriptCore/dfg/DFGOSREntry.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOSREntry.cpp	(revision 244878)
>+++ Source/JavaScriptCore/dfg/DFGOSREntry.cpp	(working copy)
>@@ -98,6 +98,7 @@ void* prepareOSREntry(ExecState* exec, C
>     ASSERT(codeBlock->alternative());
>     ASSERT(codeBlock->alternative()->jitType() == JITType::BaselineJIT);
>     ASSERT(!codeBlock->jitCodeMap());
>+    ASSERT(codeBlock->jitCode()->dfgCommon()->isStillValid);
> 
>     if (!Options::useOSREntryToDFG())
>         return nullptr;
>@@ -342,6 +343,7 @@ void* prepareOSREntry(ExecState* exec, C
> MacroAssemblerCodePtr<ExceptionHandlerPtrTag> prepareCatchOSREntry(ExecState* exec, CodeBlock* codeBlock, unsigned bytecodeIndex)
> { 
>     ASSERT(codeBlock->jitType() == JITType::DFGJIT || codeBlock->jitType() == JITType::FTLJIT);
>+    ASSERT(codeBlock->jitCode()->dfgCommon()->isStillValid);
> 
>     if (!Options::useOSREntryToDFG() && codeBlock->jitCode()->jitType() == JITType::DFGJIT)
>         return nullptr;
>Index: Source/JavaScriptCore/dfg/DFGOperations.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOperations.cpp	(revision 244878)
>+++ Source/JavaScriptCore/dfg/DFGOperations.cpp	(working copy)
>@@ -3315,12 +3315,8 @@ static char* tierUpCommon(ExecState* exe
>         // OSR entry failed. Oh no! This implies that we need to retry. We retry
>         // without exponential backoff and we only do this for the entry code block.
>         CODEBLOCK_LOG_EVENT(codeBlock, "delayFTLCompile", ("OSR entry failed too many times"));
>-        unsigned osrEntryBytecode = entryBlock->jitCode()->ftlForOSREntry()->bytecodeIndex();
>-        jitCode->clearOSREntryBlock();
>-        jitCode->osrEntryRetry = 0;
>-        jitCode->tierUpEntryTriggers.set(osrEntryBytecode, JITCode::TriggerReason::DontTrigger);
>-        jitCode->setOptimizationThresholdBasedOnCompilationResult(
>-            codeBlock, CompilationDeferred);
>+        jitCode->clearOSREntryBlockAndResetThresholds();
>+        jitCode->setOptimizationThresholdBasedOnCompilationResult(codeBlock, CompilationDeferred);
>         return nullptr;
>     }
> 
>Index: Source/JavaScriptCore/ftl/FTLForOSREntryJITCode.cpp
>===================================================================
>--- Source/JavaScriptCore/ftl/FTLForOSREntryJITCode.cpp	(revision 244878)
>+++ Source/JavaScriptCore/ftl/FTLForOSREntryJITCode.cpp	(working copy)
>@@ -50,6 +50,15 @@ void ForOSREntryJITCode::initializeEntry
>     m_entryBuffer = vm.scratchBufferForSize(numCalleeLocals * sizeof(EncodedJSValue));
> }
> 
>+void ForOSREntryJITCode::didInvalidate()
>+{
>+    if (!m_dfgJITCode)
>+        return;
>+
>+    m_dfgJITCode->clearOSREntryBlockAndResetThresholds();
>+    m_dfgJITCode = nullptr;
>+}
>+
> } } // namespace JSC::FTL
> 
> #endif // ENABLE(FTL_JIT)
>Index: Source/JavaScriptCore/ftl/FTLForOSREntryJITCode.h
>===================================================================
>--- Source/JavaScriptCore/ftl/FTLForOSREntryJITCode.h	(revision 244878)
>+++ Source/JavaScriptCore/ftl/FTLForOSREntryJITCode.h	(working copy)
>@@ -27,6 +27,7 @@
> 
> #if ENABLE(FTL_JIT)
> 
>+#include "DFGJITCode.h"
> #include "FTLJITCode.h"
> 
> namespace JSC { namespace FTL {
>@@ -54,9 +55,14 @@ public:
>     void countEntryFailure() { m_entryFailureCount++; }
>     unsigned entryFailureCount() const { return m_entryFailureCount; }
>     
>-    ForOSREntryJITCode* ftlForOSREntry();
>+    ForOSREntryJITCode* ftlForOSREntry() override;
>+
>+    void didInvalidate() override;
>+
>+    void setDFGJITCode(DFG::JITCode* dfgJITCode) { m_dfgJITCode = dfgJITCode; }
>     
> private:
>+    RefPtr<DFG::JITCode> m_dfgJITCode;
>     ScratchBuffer* m_entryBuffer; // Only for OSR entry code blocks.
>     unsigned m_bytecodeIndex;
>     unsigned m_entryFailureCount;
>Index: Source/JavaScriptCore/ftl/FTLOSREntry.cpp
>===================================================================
>--- Source/JavaScriptCore/ftl/FTLOSREntry.cpp	(revision 244878)
>+++ Source/JavaScriptCore/ftl/FTLOSREntry.cpp	(working copy)
>@@ -48,6 +48,8 @@ void* prepareOSREntry(
>     ExecutableBase* executable = dfgCodeBlock->ownerExecutable();
>     DFG::JITCode* dfgCode = dfgCodeBlock->jitCode()->dfg();
>     ForOSREntryJITCode* entryCode = entryCodeBlock->jitCode()->ftlForOSREntry();
>+
>+    ASSERT(entryCode->dfgCommon()->isStillValid);
>     
>     if (Options::verboseOSR()) {
>         dataLog(
>Index: Source/JavaScriptCore/jit/JITCode.h
>===================================================================
>--- Source/JavaScriptCore/jit/JITCode.h	(revision 244878)
>+++ Source/JavaScriptCore/jit/JITCode.h	(working copy)
>@@ -203,6 +203,8 @@ public:
>     
>     virtual bool contains(void*) = 0;
> 
>+    virtual void didInvalidate() { }
>+
> #if ENABLE(JIT)
>     virtual RegisterSet liveRegistersToPreserveAtExceptionHandlingCallSite(CodeBlock*, CallSiteIndex);
>     virtual Optional<CodeOrigin> findPC(CodeBlock*, void* pc) { UNUSED_PARAM(pc); return WTF::nullopt; }

Flags:

ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 197531: 368824 | 368843 | 368858 | 368908 | 369174 | 369176 | 369275