Title: [231830] branches/safari-605-branch/Source/_javascript_Core
Revision
231830
Author
[email protected]
Date
2018-05-15 18:41:58 -0700 (Tue, 15 May 2018)

Log Message

Cherry-pick r231317. rdar://problem/39988108

    WebContent crash loading page on seas.upenn.edu @ _javascript_Core: vmEntryToJavaScript
    https://bugs.webkit.org/show_bug.cgi?id=185231

    Reviewed by Saam Barati.

    We weren't clearing the scratch register cache when switching back and forth between
    allowing scratch register usage.  We disallow scratch register usage when we are in
    code that will freely allocate and use any register.  Such usage can change the
    contents of scratch registers.  For ARM64, where we cache the contents of scratch
    registers to reuse some or all of the contained values, we need to invalidate these
    caches.  We do this when re-enabling scratch register usage, that is when we transition
    from disallow to allow scratch register usage.

    Added a new Air regression test.

    * assembler/AllowMacroScratchRegisterUsage.h:
    (JSC::AllowMacroScratchRegisterUsage::AllowMacroScratchRegisterUsage):
    * assembler/AllowMacroScratchRegisterUsageIf.h:
    (JSC::AllowMacroScratchRegisterUsageIf::AllowMacroScratchRegisterUsageIf):
    * assembler/DisallowMacroScratchRegisterUsage.h:
    (JSC::DisallowMacroScratchRegisterUsage::~DisallowMacroScratchRegisterUsage):
    * b3/air/testair.cpp:

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

Modified Paths

Diff

Modified: branches/safari-605-branch/Source/_javascript_Core/ChangeLog (231829 => 231830)


--- branches/safari-605-branch/Source/_javascript_Core/ChangeLog	2018-05-16 01:41:56 UTC (rev 231829)
+++ branches/safari-605-branch/Source/_javascript_Core/ChangeLog	2018-05-16 01:41:58 UTC (rev 231830)
@@ -1,5 +1,60 @@
 2018-05-15  Kocsen Chung  <[email protected]>
 
+        Cherry-pick r231317. rdar://problem/39988108
+
+    WebContent crash loading page on seas.upenn.edu @ _javascript_Core: vmEntryToJavaScript
+    https://bugs.webkit.org/show_bug.cgi?id=185231
+    
+    Reviewed by Saam Barati.
+    
+    We weren't clearing the scratch register cache when switching back and forth between 
+    allowing scratch register usage.  We disallow scratch register usage when we are in
+    code that will freely allocate and use any register.  Such usage can change the
+    contents of scratch registers.  For ARM64, where we cache the contents of scratch
+    registers to reuse some or all of the contained values, we need to invalidate these
+    caches.  We do this when re-enabling scratch register usage, that is when we transition
+    from disallow to allow scratch register usage.
+    
+    Added a new Air regression test.
+    
+    * assembler/AllowMacroScratchRegisterUsage.h:
+    (JSC::AllowMacroScratchRegisterUsage::AllowMacroScratchRegisterUsage):
+    * assembler/AllowMacroScratchRegisterUsageIf.h:
+    (JSC::AllowMacroScratchRegisterUsageIf::AllowMacroScratchRegisterUsageIf):
+    * assembler/DisallowMacroScratchRegisterUsage.h:
+    (JSC::DisallowMacroScratchRegisterUsage::~DisallowMacroScratchRegisterUsage):
+    * b3/air/testair.cpp:
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231317 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2018-05-03  Michael Saboff  <[email protected]>
+
+            WebContent crash loading page on seas.upenn.edu @ _javascript_Core: vmEntryToJavaScript
+            https://bugs.webkit.org/show_bug.cgi?id=185231
+
+            Reviewed by Saam Barati.
+
+            We weren't clearing the scratch register cache when switching back and forth between
+            allowing scratch register usage.  We disallow scratch register usage when we are in
+            code that will freely allocate and use any register.  Such usage can change the
+            contents of scratch registers.  For ARM64, where we cache the contents of scratch
+            registers to reuse some or all of the contained values, we need to invalidate these
+            caches.  We do this when re-enabling scratch register usage, that is when we transition
+            from disallow to allow scratch register usage.
+
+            Added a new Air regression test.
+
+            * assembler/AllowMacroScratchRegisterUsage.h:
+            (JSC::AllowMacroScratchRegisterUsage::AllowMacroScratchRegisterUsage):
+            * assembler/AllowMacroScratchRegisterUsageIf.h:
+            (JSC::AllowMacroScratchRegisterUsageIf::AllowMacroScratchRegisterUsageIf):
+            * assembler/DisallowMacroScratchRegisterUsage.h:
+            (JSC::DisallowMacroScratchRegisterUsage::~DisallowMacroScratchRegisterUsage):
+            * b3/air/testair.cpp:
+
+2018-05-15  Kocsen Chung  <[email protected]>
+
         Cherry-pick r230486. rdar://problem/39988121
 
     ExecutableToCodeBlockEdge::visitChildren() should be cool with m_codeBlock being null since we clear it in finalizeUnconditionally()

Modified: branches/safari-605-branch/Source/_javascript_Core/assembler/AllowMacroScratchRegisterUsage.h (231829 => 231830)


--- branches/safari-605-branch/Source/_javascript_Core/assembler/AllowMacroScratchRegisterUsage.h	2018-05-16 01:41:56 UTC (rev 231829)
+++ branches/safari-605-branch/Source/_javascript_Core/assembler/AllowMacroScratchRegisterUsage.h	2018-05-16 01:41:58 UTC (rev 231830)
@@ -37,6 +37,10 @@
         : m_masm(masm)
         , m_oldValueOfAllowScratchRegister(masm.m_allowScratchRegister)
     {
+#if CPU(ARM64)
+        if (!m_oldValueOfAllowScratchRegister)
+            m_masm.invalidateAllTempRegisters();
+#endif
         masm.m_allowScratchRegister = true;
     }
 

Modified: branches/safari-605-branch/Source/_javascript_Core/assembler/AllowMacroScratchRegisterUsageIf.h (231829 => 231830)


--- branches/safari-605-branch/Source/_javascript_Core/assembler/AllowMacroScratchRegisterUsageIf.h	2018-05-16 01:41:56 UTC (rev 231829)
+++ branches/safari-605-branch/Source/_javascript_Core/assembler/AllowMacroScratchRegisterUsageIf.h	2018-05-16 01:41:58 UTC (rev 231830)
@@ -38,8 +38,13 @@
         , m_allowIfTrue(allowIfTrue)
         , m_oldValueOfAllowScratchRegister(masm.m_allowScratchRegister)
     {
-        if (m_allowIfTrue)
+        if (m_allowIfTrue) {
+#if CPU(ARM64)
+            if (!m_oldValueOfAllowScratchRegister)
+                m_masm.invalidateAllTempRegisters();
+#endif
             masm.m_allowScratchRegister = true;
+        }
     }
 
     ~AllowMacroScratchRegisterUsageIf()

Modified: branches/safari-605-branch/Source/_javascript_Core/assembler/DisallowMacroScratchRegisterUsage.h (231829 => 231830)


--- branches/safari-605-branch/Source/_javascript_Core/assembler/DisallowMacroScratchRegisterUsage.h	2018-05-16 01:41:56 UTC (rev 231829)
+++ branches/safari-605-branch/Source/_javascript_Core/assembler/DisallowMacroScratchRegisterUsage.h	2018-05-16 01:41:58 UTC (rev 231830)
@@ -42,6 +42,10 @@
 
     ~DisallowMacroScratchRegisterUsage()
     {
+#if CPU(ARM64)
+        if (m_oldValueOfAllowScratchRegister)
+            m_masm.invalidateAllTempRegisters();
+#endif
         m_masm.m_allowScratchRegister = m_oldValueOfAllowScratchRegister;
     }
 

Modified: branches/safari-605-branch/Source/_javascript_Core/b3/air/testair.cpp (231829 => 231830)


--- branches/safari-605-branch/Source/_javascript_Core/b3/air/testair.cpp	2018-05-16 01:41:56 UTC (rev 231829)
+++ branches/safari-605-branch/Source/_javascript_Core/b3/air/testair.cpp	2018-05-16 01:41:58 UTC (rev 231830)
@@ -1833,6 +1833,93 @@
 }
 #endif // #if CPU(X86) || CPU(X86_64)
 
+#if CPU(ARM64)
+void testInvalidateCachedTempRegisters()
+{
+    B3::Procedure proc;
+    Code& code = proc.code();
+    BasicBlock* root = code.addBlock();
+
+    int32_t things[4];
+    things[0] = 0x12000000;
+    things[1] = 0x340000;
+    things[2] = 0x5600;
+    things[3] = 0x78;
+    Tmp base = code.newTmp(GP);
+    GPRReg tmp = GPRInfo::regT1;
+    proc.pinRegister(tmp);
+
+    root->append(Move, nullptr, Arg::bigImm(bitwise_cast<intptr_t>(&things)), base);
+
+    B3::BasicBlock* patchPoint1Root = proc.addBlock();
+    B3::Air::Special* patchpointSpecial = code.addSpecial(std::make_unique<B3::PatchpointSpecial>());
+
+    // In Patchpoint, Load things[0] -> tmp. This will materialize the address in x17 (dataMemoryRegister).
+    B3::PatchpointValue* patchpoint1 = patchPoint1Root->appendNew<B3::PatchpointValue>(proc, B3::Void, B3::Origin());
+    patchpoint1->clobber(RegisterSet::macroScratchRegisters());
+    patchpoint1->setGenerator(
+        [=] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
+            AllowMacroScratchRegisterUsage allowScratch(jit);
+            jit.load32(&things, tmp);
+        });
+    root->append(Patch, patchpoint1, Arg::special(patchpointSpecial));
+
+    // Load things[1] -> x17, trashing dataMemoryRegister.
+    root->append(Move32, nullptr, Arg::addr(base, 1 * sizeof(int32_t)), Tmp(ARM64Registers::x17));
+    root->append(Add32, nullptr, Tmp(tmp), Tmp(ARM64Registers::x17), Tmp(GPRInfo::returnValueGPR));
+
+    // In Patchpoint, Load things[2] -> tmp. This should not reuse the prior contents of x17.
+    B3::BasicBlock* patchPoint2Root = proc.addBlock();
+    B3::PatchpointValue* patchpoint2 = patchPoint2Root->appendNew<B3::PatchpointValue>(proc, B3::Void, B3::Origin());
+    patchpoint2->clobber(RegisterSet::macroScratchRegisters());
+    patchpoint2->setGenerator(
+        [=] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
+            AllowMacroScratchRegisterUsage allowScratch(jit);
+            jit.load32(&things[2], tmp);
+        });
+    root->append(Patch, patchpoint2, Arg::special(patchpointSpecial));
+
+    root->append(Add32, nullptr, Tmp(tmp), Tmp(GPRInfo::returnValueGPR), Tmp(GPRInfo::returnValueGPR));
+
+    // In patchpoint, Store 0x78 -> things[3].
+    // This will use and cache both x16 (dataMemoryRegister) and x17 (dataTempRegister).
+    B3::BasicBlock* patchPoint3Root = proc.addBlock();
+    B3::PatchpointValue* patchpoint3 = patchPoint3Root->appendNew<B3::PatchpointValue>(proc, B3::Void, B3::Origin());
+    patchpoint3->clobber(RegisterSet::macroScratchRegisters());
+    patchpoint3->setGenerator(
+        [=] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
+            AllowMacroScratchRegisterUsage allowScratch(jit);
+            jit.store32(CCallHelpers::TrustedImm32(0x78), &things[3]);
+        });
+    root->append(Patch, patchpoint3, Arg::special(patchpointSpecial));
+
+    // Set x16 to 0xdead, trashing x16.
+    root->append(Move, nullptr, Arg::bigImm(0xdead), Tmp(ARM64Registers::x16));
+    root->append(Xor32, nullptr, Tmp(ARM64Registers::x16), Tmp(GPRInfo::returnValueGPR));
+
+    // In patchpoint, again Store 0x78 -> things[3].
+    // This should rematerialize both x16 (dataMemoryRegister) and x17 (dataTempRegister).
+    B3::BasicBlock* patchPoint4Root = proc.addBlock();
+    B3::PatchpointValue* patchpoint4 = patchPoint4Root->appendNew<B3::PatchpointValue>(proc, B3::Void, B3::Origin());
+    patchpoint4->clobber(RegisterSet::macroScratchRegisters());
+    patchpoint4->setGenerator(
+        [=] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
+            AllowMacroScratchRegisterUsage allowScratch(jit);
+            jit.store32(CCallHelpers::TrustedImm32(0x78), &things[3]);
+        });
+    root->append(Patch, patchpoint4, Arg::special(patchpointSpecial));
+
+    root->append(Move, nullptr, Arg::bigImm(0xdead), Tmp(tmp));
+    root->append(Xor32, nullptr, Tmp(tmp), Tmp(GPRInfo::returnValueGPR));
+    root->append(Move32, nullptr, Arg::addr(base, 3 * sizeof(int32_t)), Tmp(tmp));
+    root->append(Add32, nullptr, Tmp(tmp), Tmp(GPRInfo::returnValueGPR), Tmp(GPRInfo::returnValueGPR));
+    root->append(Ret32, nullptr, Tmp(GPRInfo::returnValueGPR));
+
+    int32_t r = compileAndRun<int32_t>(proc);
+    CHECK(r == 0x12345678);
+}
+#endif // #if CPU(ARM64)
+
 void testArgumentRegPinned()
 {
     B3::Procedure proc;
@@ -2016,6 +2103,10 @@
     RUN(testX86VMULSDBaseIndexNeedRex());
 #endif
 
+#if CPU(ARM64)
+    RUN(testInvalidateCachedTempRegisters());
+#endif
+
     RUN(testArgumentRegPinned());
     RUN(testArgumentRegPinned2());
     RUN(testArgumentRegPinned3());
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to