Changeset 185772 in webkit

Timestamp:

Jun 19, 2015, 3:28:18 PM (11 years ago)

Author:

msaboff@apple.com

Message:

Crash in com.apple.WebKit.WebContent at com.apple.JavaScriptCore: JSC::FTL::fixFunctionBasedOnStackMaps + 17225
​https://bugs.webkit.org/show_bug.cgi?id=146133

Reviewed by Geoffrey Garen.

When generating code to put in inline caching areas, if there isn't enough space,
then create and link to an out of line area. We connect the inline code to this
out of line code area by planting a jump from the inline area to the out of line
code and appending a jump at the end of the out of line code bck to the instruction
following the inline area. We fill the unused inline area with nops, primarily to
ensure the disassembler doesn't get confused.

• ftl/FTLCompile.cpp:

(generateInlineIfPossibleOutOfLineIfNot): New function that determines if there is enough space
in the inline code area for the code to link. If so, it links inline, otherwise it links the
code out of line and plants appropriate jumps to/from the out of line code.
(generateICFastPath):
(generateCheckInICFastPath):
(fixFunctionBasedOnStackMaps):
Use generateInlineIfPossibleOutOfLineIfNot() to link code intended for inline cache space.

• ftl/FTLJITFinalizer.cpp:

(JSC::FTL::JITFinalizer::finalizeFunction):

• ftl/FTLJITFinalizer.h:

(JSC::FTL::OutOfLineCodeInfo::OutOfLineCodeInfo):
Added code to finalize any out of line LinkBuffer created by generateInlineIfPossibleOutOfLineIfNot().

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• ftl/FTLCompile.cpp (modified) (5 diffs)
• ftl/FTLJITFinalizer.cpp (modified) (1 diff)
• ftl/FTLJITFinalizer.h (modified) (2 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2015-06-19  Michael Saboff  <msaboff@apple.com>
+
+        Crash in com.apple.WebKit.WebContent at com.apple.JavaScriptCore: JSC::FTL::fixFunctionBasedOnStackMaps + 17225
+        https://bugs.webkit.org/show_bug.cgi?id=146133
+
+        Reviewed by Geoffrey Garen.
+
+        When generating code to put in inline caching areas, if there isn't enough space,
+        then create and link to an out of line area.  We connect the inline code to this
+        out of line code area by planting a jump from the inline area to the out of line
+        code and appending a jump at the end of the out of line code bck to the instruction
+        following the inline area.  We fill the unused inline area with nops, primarily to 
+        ensure the disassembler doesn't get confused.
+
+        * ftl/FTLCompile.cpp:
+        (generateInlineIfPossibleOutOfLineIfNot): New function that determines if there is enough space
+        in the inline code area for the code to link.  If so, it links inline, otherwise it links the
+        code out of line and plants appropriate jumps to/from the out of line code.
+        (generateICFastPath):
+        (generateCheckInICFastPath):
+        (fixFunctionBasedOnStackMaps):
+        Use generateInlineIfPossibleOutOfLineIfNot() to link code intended for inline cache space.
+
+        * ftl/FTLJITFinalizer.cpp:
+        (JSC::FTL::JITFinalizer::finalizeFunction):
+        * ftl/FTLJITFinalizer.h:
+        (JSC::FTL::OutOfLineCodeInfo::OutOfLineCodeInfo):
+        Added code to finalize any out of line LinkBuffer created by generateInlineIfPossibleOutOfLineIfNot().
+
 2015-06-19  Geoffrey Garen  <ggaren@apple.com>
 

trunk/Source/JavaScriptCore/ftl/FTLCompile.cpp

@@ -156 +156 @@
 }
 
+static void generateInlineIfPossibleOutOfLineIfNot(State& state, VM& vm, CodeBlock* codeBlock, CCallHelpers& code, char* startOfInlineCode, size_t sizeOfInlineCode, const char* codeDescription, const std::function<void(LinkBuffer&, CCallHelpers&, bool wasCompiledInline)>& callback)
+{
+    std::unique_ptr<LinkBuffer> codeLinkBuffer;
+    size_t actualCodeSize = code.m_assembler.buffer().codeSize();
+
+    if (actualCodeSize <= sizeOfInlineCode) {
+        LinkBuffer codeLinkBuffer(vm, code, startOfInlineCode, sizeOfInlineCode);
+
+        // Fill the remainder of the inline space with nops to avoid confusing the disassembler.
+        MacroAssembler::AssemblerType_T::fillNops(bitwise_cast<char*>(startOfInlineCode) + actualCodeSize, sizeOfInlineCode - actualCodeSize);
+
+        callback(codeLinkBuffer, code, true);
+
+        return;
+    }
+
+    // If there isn't enough space in the provided inline code area, allocate out of line
+    // executable memory to link the provided code. Place a jump at the beginning of the
+    // inline area and jump to the out of line code. Similarly return by appending a jump
+    // to the provided code that goes to the instruction after the inline code.
+    // Fill the middle with nop's.
+    MacroAssembler::Jump returnToMainline = code.jump();
+
+    // Allocate out of line executable memory and link the provided code there.
+    codeLinkBuffer = std::make_unique<LinkBuffer>(vm, code, codeBlock, JITCompilationMustSucceed);
+
+    // Plant a jmp in the inline buffer to the out of line code.
+    MacroAssembler callToOutOfLineCode;
+    MacroAssembler::Jump jumpToOutOfLine = callToOutOfLineCode.jump();
+    LinkBuffer inlineBuffer(vm, callToOutOfLineCode, startOfInlineCode, sizeOfInlineCode);
+    inlineBuffer.link(jumpToOutOfLine, codeLinkBuffer->entrypoint());
+
+    // Fill the remainder of the inline space with nops to avoid confusing the disassembler.
+    MacroAssembler::AssemblerType_T::fillNops(bitwise_cast<char*>(startOfInlineCode) + inlineBuffer.size(), sizeOfInlineCode - inlineBuffer.size());
+
+    // Link the end of the out of line code to right after the inline area.
+    codeLinkBuffer->link(returnToMainline, CodeLocationLabel(MacroAssemblerCodePtr::createFromExecutableAddress(startOfInlineCode)).labelAtOffset(sizeOfInlineCode));
+
+    callback(*codeLinkBuffer.get(), code, false);
+
+    state.finalizer->outOfLineCodeInfos.append(OutOfLineCodeInfo(WTF::move(codeLinkBuffer), codeDescription));
+}
+
 template<typename DescriptorType>
 void generateICFastPath(
@@ -182 +225 @@
         char* startOfIC =
             bitwise_cast<char*>(generatedFunction) + record.instructionOffset;
-        
-        LinkBuffer linkBuffer(vm, fastPathJIT, startOfIC, sizeOfIC);
-        // Note: we could handle the !isValid() case. We just don't appear to have a
-        // reason to do so, yet.
-        RELEASE_ASSERT(linkBuffer.isValid());
-        
-        MacroAssembler::AssemblerType_T::fillNops(
-            startOfIC + linkBuffer.size(), sizeOfIC - linkBuffer.size());
-        
-        state.finalizer->sideCodeLinkBuffer->link(
-            ic.m_slowPathDone[i], CodeLocationLabel(startOfIC + sizeOfIC));
-        
-        linkBuffer.link(
-            generator.slowPathJump(),
-            state.finalizer->sideCodeLinkBuffer->locationOf(generator.slowPathBegin()));
-        
-        generator.finalize(linkBuffer, *state.finalizer->sideCodeLinkBuffer);
+
+        generateInlineIfPossibleOutOfLineIfNot(state, vm, codeBlock, fastPathJIT, startOfIC, sizeOfIC, "inline cache fast path", [&] (LinkBuffer& linkBuffer, CCallHelpers&, bool) {
+            state.finalizer->sideCodeLinkBuffer->link(ic.m_slowPathDone[i],
+                CodeLocationLabel(startOfIC + sizeOfIC));
+
+            linkBuffer.link(generator.slowPathJump(),
+                state.finalizer->sideCodeLinkBuffer->locationOf(generator.slowPathBegin()));
+
+            generator.finalize(linkBuffer, *state.finalizer->sideCodeLinkBuffer);
+        });
     }
 }
@@ -233 +269 @@
         char* startOfIC =
             bitwise_cast<char*>(generatedFunction) + record.instructionOffset;
-        
-        LinkBuffer fastPath(vm, fastPathJIT, startOfIC, sizeOfIC);
-        LinkBuffer& slowPath = *state.finalizer->sideCodeLinkBuffer;
-        // Note: we could handle the !isValid() case. We just don't appear to have a
-        // reason to do so, yet.
-        RELEASE_ASSERT(fastPath.isValid());
-
-        MacroAssembler::AssemblerType_T::fillNops(
-            startOfIC + fastPath.size(), sizeOfIC - fastPath.size());
-        
-        state.finalizer->sideCodeLinkBuffer->link(
-            ic.m_slowPathDone[i], CodeLocationLabel(startOfIC + sizeOfIC));
-        
-        CodeLocationLabel slowPathBeginLoc = slowPath.locationOf(slowPathBegin);
-        fastPath.link(jump, slowPathBeginLoc);
-
-        CodeLocationCall callReturnLocation = slowPath.locationOf(call);
-
-        stubInfo.patch.deltaCallToDone = MacroAssembler::differenceBetweenCodePtr(
-            callReturnLocation, fastPath.locationOf(done));
-
-        stubInfo.patch.deltaCallToJump = MacroAssembler::differenceBetweenCodePtr(
-            callReturnLocation, fastPath.locationOf(jump));
-        stubInfo.callReturnLocation = callReturnLocation;
-        stubInfo.patch.deltaCallToSlowCase = MacroAssembler::differenceBetweenCodePtr(
-            callReturnLocation, slowPathBeginLoc);
-        
+
+        auto postLink = [&] (LinkBuffer& fastPath, CCallHelpers&, bool) {
+            LinkBuffer& slowPath = *state.finalizer->sideCodeLinkBuffer;
+
+            state.finalizer->sideCodeLinkBuffer->link(
+                ic.m_slowPathDone[i], CodeLocationLabel(startOfIC + sizeOfIC));
+
+            CodeLocationLabel slowPathBeginLoc = slowPath.locationOf(slowPathBegin);
+            fastPath.link(jump, slowPathBeginLoc);
+
+            CodeLocationCall callReturnLocation = slowPath.locationOf(call);
+
+            stubInfo.patch.deltaCallToDone = MacroAssembler::differenceBetweenCodePtr(
+                callReturnLocation, fastPath.locationOf(done));
+
+            stubInfo.patch.deltaCallToJump = MacroAssembler::differenceBetweenCodePtr(
+                callReturnLocation, fastPath.locationOf(jump));
+            stubInfo.callReturnLocation = callReturnLocation;
+            stubInfo.patch.deltaCallToSlowCase = MacroAssembler::differenceBetweenCodePtr(
+                callReturnLocation, slowPathBeginLoc);
+        };
+
+        generateInlineIfPossibleOutOfLineIfNot(state, vm, codeBlock, fastPathJIT, startOfIC, sizeOfIC, "CheckIn inline cache", postLink);
     }
 }
@@ -560 +592 @@
         CCallHelpers fastPathJIT(&vm, codeBlock);
         call.emit(fastPathJIT);
-        
+
         char* startOfIC = bitwise_cast<char*>(generatedFunction) + call.m_instructionOffset;
-        
-        LinkBuffer linkBuffer(vm, fastPathJIT, startOfIC, sizeOfCall());
-        if (!linkBuffer.isValid()) {
-            dataLog("Failed to insert inline cache for call because we thought the size would be ", sizeOfCall(), " but it ended up being ", fastPathJIT.m_assembler.codeSize(), " prior to compaction.\n");
-            RELEASE_ASSERT_NOT_REACHED();
-        }
-        
-        MacroAssembler::AssemblerType_T::fillNops(
-            startOfIC + linkBuffer.size(), sizeOfCall() - linkBuffer.size());
-        
-        call.link(vm, linkBuffer);
+
+        generateInlineIfPossibleOutOfLineIfNot(state, vm, codeBlock, fastPathJIT, startOfIC, sizeOfCall(), "JSCall inline cache", [&] (LinkBuffer& linkBuffer, CCallHelpers&, bool) {
+            call.link(vm, linkBuffer);
+        });
     }
     
@@ -582 +607 @@
         CCallHelpers fastPathJIT(&vm, codeBlock);
         call.emit(fastPathJIT, varargsSpillSlotsOffset);
-        
+
         char* startOfIC = bitwise_cast<char*>(generatedFunction) + call.m_instructionOffset;
         size_t sizeOfIC = sizeOfICFor(call.node());
 
-        LinkBuffer linkBuffer(vm, fastPathJIT, startOfIC, sizeOfIC);
-        if (!linkBuffer.isValid()) {
-            dataLog("Failed to insert inline cache for varargs call (specifically, ", Graph::opName(call.node()->op()), ") because we thought the size would be ", sizeOfIC, " but it ended up being ", fastPathJIT.m_assembler.codeSize(), " prior to compaction.\n");
-            RELEASE_ASSERT_NOT_REACHED();
-        }
-        
-        MacroAssembler::AssemblerType_T::fillNops(
-            startOfIC + linkBuffer.size(), sizeOfIC - linkBuffer.size());
-        
-        call.link(vm, linkBuffer, state.finalizer->handleExceptionsLinkBuffer->entrypoint());
+        generateInlineIfPossibleOutOfLineIfNot(state, vm, codeBlock, fastPathJIT, startOfIC, sizeOfIC, "varargs call inline cache", [&] (LinkBuffer& linkBuffer, CCallHelpers&, bool) {
+            call.link(vm, linkBuffer, state.finalizer->handleExceptionsLinkBuffer->entrypoint());
+        });
     }
     

trunk/Source/JavaScriptCore/ftl/FTLJITFinalizer.cpp

@@ -128 +128 @@
             .executableMemory());
     }
-    
+
+    for (unsigned i = 0; i < outOfLineCodeInfos.size(); ++i) {
+        jitCode->addHandle(FINALIZE_DFG_CODE(
+            *outOfLineCodeInfos[i].m_linkBuffer,
+            ("FTL out of line code for %s", outOfLineCodeInfos[i].m_codeDescription)).executableMemory());
+    }
+
     jitCode->initializeArityCheckEntrypoint(
         FINALIZE_DFG_CODE(

trunk/Source/JavaScriptCore/ftl/FTLJITFinalizer.h

@@ -40 +40 @@
 namespace JSC { namespace FTL {
 
+class OutOfLineCodeInfo {
+public:
+    OutOfLineCodeInfo(std::unique_ptr<LinkBuffer> linkBuffer, const char* codeDescription)
+        : m_linkBuffer(WTF::move(linkBuffer))
+        , m_codeDescription(codeDescription)
+    {
+    }
+
+    std::unique_ptr<LinkBuffer> m_linkBuffer;
+    const char* m_codeDescription;
+};
+
 class JITFinalizer : public DFG::Finalizer {
 public:
     JITFinalizer(DFG::Plan&);
     virtual ~JITFinalizer();
-    
+
     size_t codeSize() override;
     bool finalize() override;
@@ -53 +65 @@
     std::unique_ptr<LinkBuffer> sideCodeLinkBuffer;
     std::unique_ptr<LinkBuffer> handleExceptionsLinkBuffer;
+    Vector<OutOfLineCodeInfo> outOfLineCodeInfos;
     Vector<SlowPathCall> slowPathCalls; // Calls inside the side code.
     Vector<OSRExitCompilationInfo> osrExit;