Title: [214927] trunk
- Revision
- 214927
- Author
- [email protected]
- Date
- 2017-04-04 20:50:07 -0700 (Tue, 04 Apr 2017)
Log Message
On ARM64, DFG::SpeculativeJIT::compileArithMod() failed to ensure result is of DataFormatInt32.
https://bugs.webkit.org/show_bug.cgi?id=170473
<rdar://problem/29912391>
Reviewed by Saam Barati.
JSTests:
* stress/regress-170473.js: Added.
Source/_javascript_Core:
In Unchecked mode, when DFG::SpeculativeJIT::compileArithMod() detects that the
divisor is 0, we want it to return 0. The result is expected to be of
DataFormatIn32.
The ARM implementation just returns the value in the divisor register. However,
the divisor in this case can be of DataFormatJSInt32. On ARM64, returning the
divisor register yields the wrong result format because the same register also
holds the upper 32-bit of the JSValue encoding. The fix is to return an
immediate 0 instead.
Also turned on the assertion in jitAssertIsInt32 for ARM64. This assertion being
disabled may have contributed to this bug going unnoticed all this time.
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileArithMod):
* jit/AssemblyHelpers.cpp:
(JSC::AssemblyHelpers::jitAssertIsInt32):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (214926 => 214927)
--- trunk/JSTests/ChangeLog 2017-04-05 03:43:44 UTC (rev 214926)
+++ trunk/JSTests/ChangeLog 2017-04-05 03:50:07 UTC (rev 214927)
@@ -1,3 +1,13 @@
+2017-04-04 Mark Lam <[email protected]>
+
+ On ARM64, DFG::SpeculativeJIT::compileArithMod() failed to ensure result is of DataFormatInt32.
+ https://bugs.webkit.org/show_bug.cgi?id=170473
+ <rdar://problem/29912391>
+
+ Reviewed by Saam Barati.
+
+ * stress/regress-170473.js: Added.
+
2017-04-03 Mark Lam <[email protected]>
Fix incorrect capacity delta calculation reported in SparseArrayValueMap::add().
Added: trunk/JSTests/stress/regress-170473.js (0 => 214927)
--- trunk/JSTests/stress/regress-170473.js (rev 0)
+++ trunk/JSTests/stress/regress-170473.js 2017-04-05 03:50:07 UTC (rev 214927)
@@ -0,0 +1,14 @@
+var heap = new SharedArrayBuffer(4096);
+var Uint8ArrayView = new Uint8Array(heap);
+
+function test(k) {
+ var d = new Float32Array();
+ var c = d | 0;
+ var b = 1 % c;
+ var a = b | 0;
+ Uint8ArrayView[a] = 0;
+}
+noInline(test);
+
+for (var k = 0; k < 200; ++k)
+ test(k);
Modified: trunk/Source/_javascript_Core/ChangeLog (214926 => 214927)
--- trunk/Source/_javascript_Core/ChangeLog 2017-04-05 03:43:44 UTC (rev 214926)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-04-05 03:50:07 UTC (rev 214927)
@@ -1,3 +1,29 @@
+2017-04-04 Mark Lam <[email protected]>
+
+ On ARM64, DFG::SpeculativeJIT::compileArithMod() failed to ensure result is of DataFormatInt32.
+ https://bugs.webkit.org/show_bug.cgi?id=170473
+ <rdar://problem/29912391>
+
+ Reviewed by Saam Barati.
+
+ In Unchecked mode, when DFG::SpeculativeJIT::compileArithMod() detects that the
+ divisor is 0, we want it to return 0. The result is expected to be of
+ DataFormatIn32.
+
+ The ARM implementation just returns the value in the divisor register. However,
+ the divisor in this case can be of DataFormatJSInt32. On ARM64, returning the
+ divisor register yields the wrong result format because the same register also
+ holds the upper 32-bit of the JSValue encoding. The fix is to return an
+ immediate 0 instead.
+
+ Also turned on the assertion in jitAssertIsInt32 for ARM64. This assertion being
+ disabled may have contributed to this bug going unnoticed all this time.
+
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileArithMod):
+ * jit/AssemblyHelpers.cpp:
+ (JSC::AssemblyHelpers::jitAssertIsInt32):
+
2017-04-04 Filip Pizlo <[email protected]>
Air::eliminateDeadCode should not repeatedly process the same live instructions
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (214926 => 214927)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2017-04-05 03:43:44 UTC (rev 214926)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2017-04-05 03:50:07 UTC (rev 214927)
@@ -5101,7 +5101,10 @@
speculationCheck(Overflow, JSValueRegs(), 0, m_jit.branchTest32(JITCompiler::Zero, divisorGPR));
else {
JITCompiler::Jump denominatorNotZero = m_jit.branchTest32(JITCompiler::NonZero, divisorGPR);
- m_jit.move(divisorGPR, quotientThenRemainderGPR);
+ // We know that the low 32-bit of divisorGPR is 0, but we don't know if the high bits are.
+ // So, use TrustedImm32(0) on ARM instead because done expects the result to be in DataFormatInt32.
+ // Using an immediate 0 doesn't cost anything extra on ARM.
+ m_jit.move(TrustedImm32(0), quotientThenRemainderGPR);
done.append(m_jit.jump());
denominatorNotZero.link(&m_jit);
}
Modified: trunk/Source/_javascript_Core/jit/AssemblyHelpers.cpp (214926 => 214927)
--- trunk/Source/_javascript_Core/jit/AssemblyHelpers.cpp 2017-04-05 03:43:44 UTC (rev 214926)
+++ trunk/Source/_javascript_Core/jit/AssemblyHelpers.cpp 2017-04-05 03:50:07 UTC (rev 214927)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011, 2013-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2011-2017 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -192,7 +192,7 @@
#if USE(JSVALUE64)
void AssemblyHelpers::jitAssertIsInt32(GPRReg gpr)
{
-#if CPU(X86_64)
+#if CPU(X86_64) || CPU(ARM64)
Jump checkInt32 = branch64(BelowOrEqual, gpr, TrustedImm64(static_cast<uintptr_t>(0xFFFFFFFFu)));
abortWithReason(AHIsNotInt32);
checkInt32.link(this);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
