Changeset 244058 in webkit

Timestamp:

Apr 8, 2019, 5:00:24 PM (7 years ago)

Author:

ysuzuki@apple.com

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/JavaScriptCore:

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):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/is-rope-check-in-string-slice-should-not-jump-over-register-allocations.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2019-04-08  Yusuke Suzuki  <ysuzuki@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  <ysuzuki@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-04-08  Yusuke Suzuki  <ysuzuki@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  <ysuzuki@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -1548 +1548 @@
 
     GPRTemporary temp(this);
+    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);
-
-    GPRTemporary temp2(this);
-    GPRTemporary startIndex(this);
-
-    GPRReg temp2GPR = temp2.gpr();
-    GPRReg startIndexGPR = startIndex.gpr();
     {
         m_jit.load32(MacroAssembler::Address(tempGPR, StringImpl::lengthMemoryOffset()), temp2GPR);