[webkit-changes] [205742] branches/safari-602-branch

Fri, 09 Sep 2016 08:29:13 -0700

Title: [205742] branches/safari-602-branch

Diff

Modified: branches/safari-602-branch/JSTests/ChangeLog (205741 => 205742)


--- branches/safari-602-branch/JSTests/ChangeLog	2016-09-09 15:17:21 UTC (rev 205741)
+++ branches/safari-602-branch/JSTests/ChangeLog	2016-09-09 15:29:24 UTC (rev 205742)
@@ -1,3 +1,17 @@
+2016-09-09  Babak Shafiei  <[email protected]>
+
+        Merge r204403. rdar://problem/27991568
+
+    2016-08-11  Mark Lam  <[email protected]>
+
+            OverridesHasInstance should not branch across register allocations.
+            https://bugs.webkit.org/show_bug.cgi?id=160792
+            <rdar://problem/27361778>
+
+            Reviewed by Benjamin Poulain.
+
+            * stress/OverrideHasInstance-should-not-branch-across-register-allocations.js: Added.
+
 2016-08-30  Babak Shafiei  <[email protected]>
 
         Merge r204570. rdar://problem/27991567

Added: branches/safari-602-branch/JSTests/stress/OverrideHasInstance-should-not-branch-across-register-allocations.js (0 => 205742)


--- branches/safari-602-branch/JSTests/stress/OverrideHasInstance-should-not-branch-across-register-allocations.js	                        (rev 0)
+++ branches/safari-602-branch/JSTests/stress/OverrideHasInstance-should-not-branch-across-register-allocations.js	2016-09-09 15:29:24 UTC (rev 205742)
@@ -0,0 +1,16 @@
+//@ runDefault
+
+// This test should not crash.
+
+delete this.Function;
+
+var test = function() { 
+    Math.cos("0" instanceof arguments)
+}
+
+for (var k = 0; k < 10000; ++k) {
+    try {
+        test();
+    } catch (e) {
+    }
+}

Modified: branches/safari-602-branch/Source/_javascript_Core/ChangeLog (205741 => 205742)


--- branches/safari-602-branch/Source/_javascript_Core/ChangeLog	2016-09-09 15:17:21 UTC (rev 205741)
+++ branches/safari-602-branch/Source/_javascript_Core/ChangeLog	2016-09-09 15:29:24 UTC (rev 205742)
@@ -1,5 +1,37 @@
 2016-09-09  Babak Shafiei  <[email protected]>
 
+        Merge r204403. rdar://problem/27991568
+
+    2016-08-11  Mark Lam  <[email protected]>
+
+            OverridesHasInstance should not branch across register allocations.
+            https://bugs.webkit.org/show_bug.cgi?id=160792
+            <rdar://problem/27361778>
+
+            Reviewed by Benjamin Poulain.
+
+            The OverrideHasInstance node has a branch test that is emitted conditionally.
+            It also has a bug where it allocated a register after this branch, which is not
+            allowed and would fail an assertion introduced in https://trac.webkit.org/r145931.
+            From the ChangeLog for r145931:
+
+            "This [assertion that register allocations are not branched around] protects
+            against the case where an allocation could have spilled register contents to free
+            up a register and that spill only occurs on one path of many through the code.
+            A subsequent fill of the spilled register may load garbage."
+
+            Because the branch isn't always emitted, this bug has gone unnoticed until now.
+            This patch fixes this issue by pre-allocating the registers before emitting the
+            branch in OverrideHasInstance.
+
+            Note: this issue is only present in DFGSpeculativeJIT64.cpp.  The 32-bit version
+            is doing it right.
+
+            * dfg/DFGSpeculativeJIT64.cpp:
+            (JSC::DFG::SpeculativeJIT::compile):
+
+2016-09-09  Babak Shafiei  <[email protected]>
+
         Merge r204485. rdar://problem/27991572
 
     2016-08-15  Mark Lam  <[email protected]>

Modified: branches/safari-602-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (205741 => 205742)


--- branches/safari-602-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2016-09-09 15:17:21 UTC (rev 205741)
+++ branches/safari-602-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2016-09-09 15:29:24 UTC (rev 205742)
@@ -4521,16 +4521,19 @@
         GPRTemporary result(this);
 
         GPRReg resultGPR = result.gpr();
+        GPRReg baseGPR = base.gpr();
 
         // If we have proven that the constructor's Symbol.hasInstance will always be the one on Function.prototype[Symbol.hasInstance]
         // then we don't need a runtime check here. We don't worry about the case where the constructor's Symbol.hasInstance is a constant
         // but is not the default one as fixup should have converted this check to true.
         ASSERT(!hasInstanceValueNode->isCellConstant() || defaultHasInstanceFunction == hasInstanceValueNode->asCell());
-        if (!hasInstanceValueNode->isCellConstant())
-            notDefault = m_jit.branchPtr(MacroAssembler::NotEqual, hasInstanceValue.gpr(), TrustedImmPtr(defaultHasInstanceFunction));
+        if (!hasInstanceValueNode->isCellConstant()) {
+            GPRReg hasInstanceValueGPR = hasInstanceValue.gpr();
+            notDefault = m_jit.branchPtr(MacroAssembler::NotEqual, hasInstanceValueGPR, TrustedImmPtr(defaultHasInstanceFunction));
+        }
 
         // Check that base 'ImplementsDefaultHasInstance'.
-        m_jit.test8(MacroAssembler::Zero, MacroAssembler::Address(base.gpr(), JSCell::typeInfoFlagsOffset()), MacroAssembler::TrustedImm32(ImplementsDefaultHasInstance), resultGPR);
+        m_jit.test8(MacroAssembler::Zero, MacroAssembler::Address(baseGPR, JSCell::typeInfoFlagsOffset()), MacroAssembler::TrustedImm32(ImplementsDefaultHasInstance), resultGPR);
         m_jit.or32(TrustedImm32(ValueFalse), resultGPR);
         MacroAssembler::Jump done = m_jit.jump();
 

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to