Title: [225322] trunk/Source/_javascript_Core
- Revision
- 225322
- Author
- [email protected]
- Date
- 2017-11-30 03:27:43 -0800 (Thu, 30 Nov 2017)
Log Message
REGRESSION(r225314): [Linux] More than 2000 jsc tests are failing after r225314
https://bugs.webkit.org/show_bug.cgi?id=180185
Reviewed by Carlos Garcia Campos.
After r225314, we start using AllocatorForMode::MustAlreadyHaveAllocator for JSRopeString's allocatorFor.
But it is different from the original code used before r225314. Since DFGSpeculativeJIT::emitAllocateJSCell
can accept nullptr allocator, the behavior of the original code is AllocatorForMode::AllocatorIfExists.
And JSRopeString's allocator may not exist at this point if any JSRopeString is not allocated. But MakeRope
DFG node can be emitted if we see untaken path includes String + String code.
This patch fixes Linux JSC crashes by changing JSRopeString's AllocatorForMode to AllocatorIfExists.
As a result, only one user of AllocatorForMode::MustAlreadyHaveAllocator is MaterializeNewObject in FTL.
I'm not sure why this condition (MustAlreadyHaveAllocator) is ensured. But this code is the same to the
original code used before r225314.
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileMakeRope):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileMakeRope):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (225321 => 225322)
--- trunk/Source/_javascript_Core/ChangeLog 2017-11-30 08:21:25 UTC (rev 225321)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-11-30 11:27:43 UTC (rev 225322)
@@ -1,3 +1,26 @@
+2017-11-30 Yusuke Suzuki <[email protected]>
+
+ REGRESSION(r225314): [Linux] More than 2000 jsc tests are failing after r225314
+ https://bugs.webkit.org/show_bug.cgi?id=180185
+
+ Reviewed by Carlos Garcia Campos.
+
+ After r225314, we start using AllocatorForMode::MustAlreadyHaveAllocator for JSRopeString's allocatorFor.
+ But it is different from the original code used before r225314. Since DFGSpeculativeJIT::emitAllocateJSCell
+ can accept nullptr allocator, the behavior of the original code is AllocatorForMode::AllocatorIfExists.
+ And JSRopeString's allocator may not exist at this point if any JSRopeString is not allocated. But MakeRope
+ DFG node can be emitted if we see untaken path includes String + String code.
+
+ This patch fixes Linux JSC crashes by changing JSRopeString's AllocatorForMode to AllocatorIfExists.
+ As a result, only one user of AllocatorForMode::MustAlreadyHaveAllocator is MaterializeNewObject in FTL.
+ I'm not sure why this condition (MustAlreadyHaveAllocator) is ensured. But this code is the same to the
+ original code used before r225314.
+
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileMakeRope):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileMakeRope):
+
2017-11-28 Filip Pizlo <[email protected]>
CodeBlockSet::deleteUnmarkedAndUnreferenced can be a little more efficient
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (225321 => 225322)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2017-11-30 08:21:25 UTC (rev 225321)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2017-11-30 11:27:43 UTC (rev 225322)
@@ -4249,7 +4249,7 @@
GPRReg scratchGPR = scratch.gpr();
JITCompiler::JumpList slowPath;
- MarkedAllocator* markedAllocator = subspaceFor<JSString>(*m_jit.vm())->allocatorForNonVirtual(sizeof(JSRopeString), AllocatorForMode::MustAlreadyHaveAllocator);
+ MarkedAllocator* markedAllocator = subspaceFor<JSRopeString>(*m_jit.vm())->allocatorForNonVirtual(sizeof(JSRopeString), AllocatorForMode::AllocatorIfExists);
m_jit.move(TrustedImmPtr(markedAllocator), allocatorGPR);
emitAllocateJSCell(resultGPR, markedAllocator, allocatorGPR, TrustedImmPtr(m_jit.graph().registerStructure(m_jit.vm()->stringStructure.get())), scratchGPR, slowPath);
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (225321 => 225322)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2017-11-30 08:21:25 UTC (rev 225321)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2017-11-30 11:27:43 UTC (rev 225322)
@@ -5783,7 +5783,7 @@
LBasicBlock lastNext = m_out.insertNewBlocksBefore(slowPath);
- MarkedAllocator* allocator = subspaceFor<JSRopeString>(vm())->allocatorForNonVirtual(sizeof(JSRopeString), AllocatorForMode::MustAlreadyHaveAllocator);
+ MarkedAllocator* allocator = subspaceFor<JSRopeString>(vm())->allocatorForNonVirtual(sizeof(JSRopeString), AllocatorForMode::AllocatorIfExists);
LValue result = allocateCell(
m_out.constIntPtr(allocator), vm().stringStructure.get(), slowPath);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
