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/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());