EFL: Unsafe branch detected in compilePutByValForFloatTypedArray()
authormsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Mar 2013 03:22:53 +0000 (03:22 +0000)
committermsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Mar 2013 03:22:53 +0000 (03:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=112609

Reviewed by Geoffrey Garen.

Created local valueFPR and scratchFPR and filled them with valueOp.fpr() and scratch.fpr()
respectively so that if valueOp.fpr() causes a spill during allocation, it occurs before the
branch and also to follow convention.  Added register allocation checks to FPRTemporary.
Cleaned up a couple of other places to follow the "AllocatVirtualRegType foo, get machine
reg from foo" pattern.

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compilePutByValForFloatTypedArray):
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::SpeculativeJIT::fprAllocate):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::convertToDouble):
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@146174 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp

index 738106c..877379b 100644 (file)
@@ -1,3 +1,26 @@
+2013-03-18  Michael Saboff  <msaboff@apple.com>
+
+        EFL: Unsafe branch detected in compilePutByValForFloatTypedArray()
+        https://bugs.webkit.org/show_bug.cgi?id=112609
+
+        Reviewed by Geoffrey Garen.
+
+        Created local valueFPR and scratchFPR and filled them with valueOp.fpr() and scratch.fpr()
+        respectively so that if valueOp.fpr() causes a spill during allocation, it occurs before the
+        branch and also to follow convention.  Added register allocation checks to FPRTemporary.
+        Cleaned up a couple of other places to follow the "AllocatVirtualRegType foo, get machine
+        reg from foo" pattern.
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compilePutByValForFloatTypedArray):
+        * dfg/DFGSpeculativeJIT.h:
+        (JSC::DFG::SpeculativeJIT::fprAllocate):
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::convertToDouble):
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+
 2013-03-18  Filip Pizlo  <fpizlo@apple.com>
 
         DFG should inline binary string concatenations (i.e. ValueAdd with string children)
index b7dfb8e..6bc6754 100644 (file)
@@ -2709,27 +2709,27 @@ void SpeculativeJIT::compilePutByValForFloatTypedArray(const TypedArrayDescripto
     
     Edge baseUse = m_jit.graph().varArgChild(node, 0);
     Edge valueUse = m_jit.graph().varArgChild(node, 2);
-    
+
     SpeculateDoubleOperand valueOp(this, valueUse);
-    
+    FPRTemporary scratch(this);
+    FPRReg valueFPR = valueOp.fpr();
+    FPRReg scratchFPR = scratch.fpr();
+
     ASSERT_UNUSED(baseUse, node->arrayMode().alreadyChecked(m_jit.graph(), node, m_state.forNode(baseUse)));
     
-    GPRTemporary result(this);
-    
     MacroAssembler::Jump outOfBounds;
     if (node->op() == PutByVal)
         outOfBounds = m_jit.branch32(MacroAssembler::AboveOrEqual, property, MacroAssembler::Address(base, descriptor.m_lengthOffset));
     
     switch (elementSize) {
     case 4: {
-        FPRTemporary scratch(this);
-        m_jit.moveDouble(valueOp.fpr(), scratch.fpr());
-        m_jit.convertDoubleToFloat(valueOp.fpr(), scratch.fpr());
-        m_jit.storeFloat(scratch.fpr(), MacroAssembler::BaseIndex(storageReg, property, MacroAssembler::TimesFour));
+        m_jit.moveDouble(valueFPR, scratchFPR);
+        m_jit.convertDoubleToFloat(valueFPR, scratchFPR);
+        m_jit.storeFloat(scratchFPR, MacroAssembler::BaseIndex(storageReg, property, MacroAssembler::TimesFour));
         break;
     }
     case 8:
-        m_jit.storeDouble(valueOp.fpr(), MacroAssembler::BaseIndex(storageReg, property, MacroAssembler::TimesEight));
+        m_jit.storeDouble(valueFPR, MacroAssembler::BaseIndex(storageReg, property, MacroAssembler::TimesEight));
         break;
     default:
         RELEASE_ASSERT_NOT_REACHED();
index 36e5af0..1acd71c 100644 (file)
@@ -212,6 +212,9 @@ public:
     }
     FPRReg fprAllocate()
     {
+#if ENABLE(DFG_REGISTER_ALLOCATION_VALIDATION)
+        m_jit.addRegisterAllocationAtOffset(m_jit.debugOffset());
+#endif
         VirtualRegister spillMe;
         FPRReg fpr = m_fprs.allocate(spillMe);
         if (spillMe != InvalidVirtualRegister)
index e6c5ca2..e83afdf 100644 (file)
@@ -1258,14 +1258,18 @@ JITCompiler::Jump SpeculativeJIT::convertToDouble(JSValueOperand& op, FPRReg res
 {
     FPRTemporary scratch(this);
 
-    JITCompiler::Jump isInteger = m_jit.branch32(MacroAssembler::Equal, op.tagGPR(), TrustedImm32(JSValue::Int32Tag));
-    JITCompiler::Jump notNumber = m_jit.branch32(MacroAssembler::AboveOrEqual, op.payloadGPR(), TrustedImm32(JSValue::LowestTag));
+    GPRReg opPayloadGPR = op.payloadGPR();
+    GPRReg opTagGPR = op.tagGPR();
+    FPRReg scratchFPR = scratch.fpr();
 
-    unboxDouble(op.tagGPR(), op.payloadGPR(), result, scratch.fpr());
+    JITCompiler::Jump isInteger = m_jit.branch32(MacroAssembler::Equal, opTagGPR, TrustedImm32(JSValue::Int32Tag));
+    JITCompiler::Jump notNumber = m_jit.branch32(MacroAssembler::AboveOrEqual, opPayloadGPR, TrustedImm32(JSValue::LowestTag));
+
+    unboxDouble(opTagGPR, opPayloadGPR, result, scratchFPR);
     JITCompiler::Jump done = m_jit.jump();
 
     isInteger.link(&m_jit);
-    m_jit.convertInt32ToDouble(op.payloadGPR(), result);
+    m_jit.convertInt32ToDouble(opPayloadGPR, result);
 
     done.link(&m_jit);
 
@@ -2314,18 +2318,22 @@ void SpeculativeJIT::compile(Node* node)
             SpeculateStrictInt32Operand op1(this, node->child1());
             SpeculateStrictInt32Operand op2(this, node->child2());
             GPRTemporary result(this, op1);
-            
-            MacroAssembler::Jump op1Less = m_jit.branch32(op == ArithMin ? MacroAssembler::LessThan : MacroAssembler::GreaterThan, op1.gpr(), op2.gpr());
-            m_jit.move(op2.gpr(), result.gpr());
-            if (op1.gpr() != result.gpr()) {
+
+            GPRReg op1GPR = op1.gpr();
+            GPRReg op2GPR = op2.gpr();
+            GPRReg resultGPR - result.gpr();
+
+            MacroAssembler::Jump op1Less = m_jit.branch32(op == ArithMin ? MacroAssembler::LessThan : MacroAssembler::GreaterThan, op1GPR, op2GPR);
+            m_jit.move(op2GPR, resultGPR);
+            if (op1GPR != resultGPR) {
                 MacroAssembler::Jump done = m_jit.jump();
                 op1Less.link(&m_jit);
-                m_jit.move(op1.gpr(), result.gpr());
+                m_jit.move(op1GPR, resultGPR);
                 done.link(&m_jit);
             } else
                 op1Less.link(&m_jit);
             
-            integerResult(result.gpr(), node);
+            integerResult(resultGPR, node);
             break;
         }
         
@@ -2333,33 +2341,37 @@ void SpeculativeJIT::compile(Node* node)
             SpeculateDoubleOperand op1(this, node->child1());
             SpeculateDoubleOperand op2(this, node->child2());
             FPRTemporary result(this, op1);
-        
+
+            FPRReg op1FPR = op1.fpr();
+            FPRReg op2FPR = op2.fpr();
+            FPRReg resultFPR = result.fpr();
+
             MacroAssembler::JumpList done;
         
-            MacroAssembler::Jump op1Less = m_jit.branchDouble(op == ArithMin ? MacroAssembler::DoubleLessThan : MacroAssembler::DoubleGreaterThan, op1.fpr(), op2.fpr());
+            MacroAssembler::Jump op1Less = m_jit.branchDouble(op == ArithMin ? MacroAssembler::DoubleLessThan : MacroAssembler::DoubleGreaterThan, op1FPR, op2FPR);
         
             // op2 is eather the lesser one or one of then is NaN
-            MacroAssembler::Jump op2Less = m_jit.branchDouble(op == ArithMin ? MacroAssembler::DoubleGreaterThanOrEqual : MacroAssembler::DoubleLessThanOrEqual, op1.fpr(), op2.fpr());
+            MacroAssembler::Jump op2Less = m_jit.branchDouble(op == ArithMin ? MacroAssembler::DoubleGreaterThanOrEqual : MacroAssembler::DoubleLessThanOrEqual, op1FPR, op2FPR);
         
             // Unordered case. We don't know which of op1, op2 is NaN. Manufacture NaN by adding 
             // op1 + op2 and putting it into result.
-            m_jit.addDouble(op1.fpr(), op2.fpr(), result.fpr());
+            m_jit.addDouble(op1FPR, op2FPR, resultFPR);
             done.append(m_jit.jump());
         
             op2Less.link(&m_jit);
-            m_jit.moveDouble(op2.fpr(), result.fpr());
+            m_jit.moveDouble(op2FPR, resultFPR);
         
-            if (op1.fpr() != result.fpr()) {
+            if (op1FPR != resultFPR) {
                 done.append(m_jit.jump());
             
                 op1Less.link(&m_jit);
-                m_jit.moveDouble(op1.fpr(), result.fpr());
+                m_jit.moveDouble(op1FPR, resultFPR);
             } else
                 op1Less.link(&m_jit);
         
             done.link(&m_jit);
         
-            doubleResult(result.fpr(), node);
+            doubleResult(resultFPR, node);
             break;
         }
             
index c88f2ec..39a3472 100644 (file)
@@ -2268,33 +2268,37 @@ void SpeculativeJIT::compile(Node* node)
             SpeculateDoubleOperand op1(this, node->child1());
             SpeculateDoubleOperand op2(this, node->child2());
             FPRTemporary result(this, op1);
+            
+            FPRReg op1FPR = op1.fpr();
+            FPRReg op2FPR = op2.fpr();
+            FPRReg resultFPR = result.fpr();
         
             MacroAssembler::JumpList done;
         
-            MacroAssembler::Jump op1Less = m_jit.branchDouble(op == ArithMin ? MacroAssembler::DoubleLessThan : MacroAssembler::DoubleGreaterThan, op1.fpr(), op2.fpr());
+            MacroAssembler::Jump op1Less = m_jit.branchDouble(op == ArithMin ? MacroAssembler::DoubleLessThan : MacroAssembler::DoubleGreaterThan, op1FPR, op2FPR);
         
             // op2 is eather the lesser one or one of then is NaN
-            MacroAssembler::Jump op2Less = m_jit.branchDouble(op == ArithMin ? MacroAssembler::DoubleGreaterThanOrEqual : MacroAssembler::DoubleLessThanOrEqual, op1.fpr(), op2.fpr());
+            MacroAssembler::Jump op2Less = m_jit.branchDouble(op == ArithMin ? MacroAssembler::DoubleGreaterThanOrEqual : MacroAssembler::DoubleLessThanOrEqual, op1FPR, op2FPR);
         
             // Unordered case. We don't know which of op1, op2 is NaN. Manufacture NaN by adding 
             // op1 + op2 and putting it into result.
-            m_jit.addDouble(op1.fpr(), op2.fpr(), result.fpr());
+            m_jit.addDouble(op1FPR, op2FPR, resultFPR);
             done.append(m_jit.jump());
         
             op2Less.link(&m_jit);
-            m_jit.moveDouble(op2.fpr(), result.fpr());
+            m_jit.moveDouble(op2FPR, resultFPR);
         
-            if (op1.fpr() != result.fpr()) {
+            if (op1FPR != resultFPR) {
                 done.append(m_jit.jump());
             
                 op1Less.link(&m_jit);
-                m_jit.moveDouble(op1.fpr(), result.fpr());
+                m_jit.moveDouble(op1FPR, resultFPR);
             } else
                 op1Less.link(&m_jit);
         
             done.link(&m_jit);
         
-            doubleResult(result.fpr(), node);
+            doubleResult(resultFPR, node);
             break;
         }