Title: [244058] trunk
- Revision
- 244058
- Author
- ysuz...@apple.com
- Date
- 2019-04-08 17:00:24 -0700 (Mon, 08 Apr 2019)
Log Message
[JSC] isRope jump in StringSlice should not jump over register allocations
https://bugs.webkit.org/show_bug.cgi?id=196716
Reviewed by Saam Barati.
JSTests:
* stress/is-rope-check-in-string-slice-should-not-jump-over-register-allocations.js: Added.
(foo.bar):
(foo):
Source/_javascript_Core:
Jumping over the register allocation code in DFG (like the following) is wrong.
auto jump = m_jit.branchXXX();
{
GPRTemporary reg(this);
GPRReg regGPR = reg.gpr();
...
}
jump.link(&m_jit);
When GPRTemporary::gpr allocates a new register, it can flush the previous register value into the stack and make the register usable.
Jumping over this register allocation code skips the flushing code, and makes the DFG's stack and register content tracking inconsistent:
DFG thinks that the content is flushed and stored in particular stack slot even while this flushing code is skipped.
In this patch, we perform register allocations before jumping to the slow path based on `isRope` condition in StringSlice.
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileStringSlice):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (244057 => 244058)
--- trunk/JSTests/ChangeLog 2019-04-08 23:33:05 UTC (rev 244057)
+++ trunk/JSTests/ChangeLog 2019-04-09 00:00:24 UTC (rev 244058)
@@ -1,5 +1,16 @@
2019-04-08 Yusuke Suzuki <ysuz...@apple.com>
+ [JSC] isRope jump in StringSlice should not jump over register allocations
+ https://bugs.webkit.org/show_bug.cgi?id=196716
+
+ Reviewed by Saam Barati.
+
+ * stress/is-rope-check-in-string-slice-should-not-jump-over-register-allocations.js: Added.
+ (foo.bar):
+ (foo):
+
+2019-04-08 Yusuke Suzuki <ysuz...@apple.com>
+
[JSC] to_index_string should not assume incoming value is Uint32
https://bugs.webkit.org/show_bug.cgi?id=196713
Added: trunk/JSTests/stress/is-rope-check-in-string-slice-should-not-jump-over-register-allocations.js (0 => 244058)
--- trunk/JSTests/stress/is-rope-check-in-string-slice-should-not-jump-over-register-allocations.js (rev 0)
+++ trunk/JSTests/stress/is-rope-check-in-string-slice-should-not-jump-over-register-allocations.js 2019-04-09 00:00:24 UTC (rev 244058)
@@ -0,0 +1,17 @@
+//@ runDefault("--jitPolicyScale=0", "--useFTLJIT=0", "--useConcurrentJIT=0", "--collectContinuously=1")
+
+function foo() {
+ for (let i = 0; i < 400; i++) {
+ const s = 'a'+'a';
+ function bar() {
+ s.slice(0);
+ }
+ for (const _ in {}) {
+ }
+ const o = {
+ };
+ bar([], [], [], [], {});
+ o ** '';
+ }
+}
+foo();
Modified: trunk/Source/_javascript_Core/ChangeLog (244057 => 244058)
--- trunk/Source/_javascript_Core/ChangeLog 2019-04-08 23:33:05 UTC (rev 244057)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-04-09 00:00:24 UTC (rev 244058)
@@ -1,5 +1,30 @@
2019-04-08 Yusuke Suzuki <ysuz...@apple.com>
+ [JSC] isRope jump in StringSlice should not jump over register allocations
+ https://bugs.webkit.org/show_bug.cgi?id=196716
+
+ Reviewed by Saam Barati.
+
+ Jumping over the register allocation code in DFG (like the following) is wrong.
+
+ auto jump = m_jit.branchXXX();
+ {
+ GPRTemporary reg(this);
+ GPRReg regGPR = reg.gpr();
+ ...
+ }
+ jump.link(&m_jit);
+
+ When GPRTemporary::gpr allocates a new register, it can flush the previous register value into the stack and make the register usable.
+ Jumping over this register allocation code skips the flushing code, and makes the DFG's stack and register content tracking inconsistent:
+ DFG thinks that the content is flushed and stored in particular stack slot even while this flushing code is skipped.
+ In this patch, we perform register allocations before jumping to the slow path based on `isRope` condition in StringSlice.
+
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileStringSlice):
+
+2019-04-08 Yusuke Suzuki <ysuz...@apple.com>
+
[JSC] to_index_string should not assume incoming value is Uint32
https://bugs.webkit.org/show_bug.cgi?id=196713
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (244057 => 244058)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2019-04-08 23:33:05 UTC (rev 244057)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2019-04-09 00:00:24 UTC (rev 244058)
@@ -1547,16 +1547,15 @@
}
GPRTemporary temp(this);
- GPRReg tempGPR = temp.gpr();
-
- m_jit.loadPtr(CCallHelpers::Address(stringGPR, JSString::offsetOfValue()), tempGPR);
- auto isRope = m_jit.branchIfRopeStringImpl(tempGPR);
-
GPRTemporary temp2(this);
GPRTemporary startIndex(this);
+ GPRReg tempGPR = temp.gpr();
GPRReg temp2GPR = temp2.gpr();
GPRReg startIndexGPR = startIndex.gpr();
+
+ m_jit.loadPtr(CCallHelpers::Address(stringGPR, JSString::offsetOfValue()), tempGPR);
+ auto isRope = m_jit.branchIfRopeStringImpl(tempGPR);
{
m_jit.load32(MacroAssembler::Address(tempGPR, StringImpl::lengthMemoryOffset()), temp2GPR);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
