Changeset 146174 in webkit


Ignore:
Timestamp:
Mar 18, 2013 8:22:53 PM (11 years ago)
Author:
msaboff@apple.com
Message:

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

Location:
trunk/Source/JavaScriptCore
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r146164 r146174  
     12013-03-18  Michael Saboff  <msaboff@apple.com>
     2
     3        EFL: Unsafe branch detected in compilePutByValForFloatTypedArray()
     4        https://bugs.webkit.org/show_bug.cgi?id=112609
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Created local valueFPR and scratchFPR and filled them with valueOp.fpr() and scratch.fpr()
     9        respectively so that if valueOp.fpr() causes a spill during allocation, it occurs before the
     10        branch and also to follow convention.  Added register allocation checks to FPRTemporary.
     11        Cleaned up a couple of other places to follow the "AllocatVirtualRegType foo, get machine
     12        reg from foo" pattern.
     13
     14        * dfg/DFGSpeculativeJIT.cpp:
     15        (JSC::DFG::SpeculativeJIT::compilePutByValForFloatTypedArray):
     16        * dfg/DFGSpeculativeJIT.h:
     17        (JSC::DFG::SpeculativeJIT::fprAllocate):
     18        * dfg/DFGSpeculativeJIT32_64.cpp:
     19        (JSC::DFG::SpeculativeJIT::convertToDouble):
     20        (JSC::DFG::SpeculativeJIT::compile):
     21        * dfg/DFGSpeculativeJIT64.cpp:
     22        (JSC::DFG::SpeculativeJIT::compile):
     23
    1242013-03-18  Filip Pizlo  <fpizlo@apple.com>
    225

  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

    r146164 r146174  
    27102710    Edge baseUse = m_jit.graph().varArgChild(node, 0);
    27112711    Edge valueUse = m_jit.graph().varArgChild(node, 2);
    2712    
     2712
    27132713    SpeculateDoubleOperand valueOp(this, valueUse);
    2714    
     2714    FPRTemporary scratch(this);
     2715    FPRReg valueFPR = valueOp.fpr();
     2716    FPRReg scratchFPR = scratch.fpr();
     2717
    27152718    ASSERT_UNUSED(baseUse, node->arrayMode().alreadyChecked(m_jit.graph(), node, m_state.forNode(baseUse)));
    2716    
    2717     GPRTemporary result(this);
    27182719   
    27192720    MacroAssembler::Jump outOfBounds;
    … …  
    27232724    switch (elementSize) {
    27242725    case 4: {
    2725         FPRTemporary scratch(this);
    2726         m_jit.moveDouble(valueOp.fpr(), scratch.fpr());
    2727         m_jit.convertDoubleToFloat(valueOp.fpr(), scratch.fpr());
    2728         m_jit.storeFloat(scratch.fpr(), MacroAssembler::BaseIndex(storageReg, property, MacroAssembler::TimesFour));
     2726        m_jit.moveDouble(valueFPR, scratchFPR);
     2727        m_jit.convertDoubleToFloat(valueFPR, scratchFPR);
     2728        m_jit.storeFloat(scratchFPR, MacroAssembler::BaseIndex(storageReg, property, MacroAssembler::TimesFour));
    27292729        break;
    27302730    }
    27312731    case 8:
    2732         m_jit.storeDouble(valueOp.fpr(), MacroAssembler::BaseIndex(storageReg, property, MacroAssembler::TimesEight));
     2732        m_jit.storeDouble(valueFPR, MacroAssembler::BaseIndex(storageReg, property, MacroAssembler::TimesEight));
    27332733        break;
    27342734    default:

  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h

    r146164 r146174  
    213213    FPRReg fprAllocate()
    214214    {
     215#if ENABLE(DFG_REGISTER_ALLOCATION_VALIDATION)
     216        m_jit.addRegisterAllocationAtOffset(m_jit.debugOffset());
     217#endif
    215218        VirtualRegister spillMe;
    216219        FPRReg fpr = m_fprs.allocate(spillMe);

  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp

    r146100 r146174  
    12591259    FPRTemporary scratch(this);
    12601260
    1261     JITCompiler::Jump isInteger = m_jit.branch32(MacroAssembler::Equal, op.tagGPR(), TrustedImm32(JSValue::Int32Tag));
    1262     JITCompiler::Jump notNumber = m_jit.branch32(MacroAssembler::AboveOrEqual, op.payloadGPR(), TrustedImm32(JSValue::LowestTag));
    1263 
    1264     unboxDouble(op.tagGPR(), op.payloadGPR(), result, scratch.fpr());
     1261    GPRReg opPayloadGPR = op.payloadGPR();
     1262    GPRReg opTagGPR = op.tagGPR();
     1263    FPRReg scratchFPR = scratch.fpr();
     1264
     1265    JITCompiler::Jump isInteger = m_jit.branch32(MacroAssembler::Equal, opTagGPR, TrustedImm32(JSValue::Int32Tag));
     1266    JITCompiler::Jump notNumber = m_jit.branch32(MacroAssembler::AboveOrEqual, opPayloadGPR, TrustedImm32(JSValue::LowestTag));
     1267
     1268    unboxDouble(opTagGPR, opPayloadGPR, result, scratchFPR);
    12651269    JITCompiler::Jump done = m_jit.jump();
    12661270
    12671271    isInteger.link(&m_jit);
    1268     m_jit.convertInt32ToDouble(op.payloadGPR(), result);
     1272    m_jit.convertInt32ToDouble(opPayloadGPR, result);
    12691273
    12701274    done.link(&m_jit);
    … …  
    23152319            SpeculateStrictInt32Operand op2(this, node->child2());
    23162320            GPRTemporary result(this, op1);
    2317            
    2318             MacroAssembler::Jump op1Less = m_jit.branch32(op == ArithMin ? MacroAssembler::LessThan : MacroAssembler::GreaterThan, op1.gpr(), op2.gpr());
    2319             m_jit.move(op2.gpr(), result.gpr());
    2320             if (op1.gpr() != result.gpr()) {
     2321
     2322            GPRReg op1GPR = op1.gpr();
     2323            GPRReg op2GPR = op2.gpr();
     2324            GPRReg resultGPR - result.gpr();
     2325
     2326            MacroAssembler::Jump op1Less = m_jit.branch32(op == ArithMin ? MacroAssembler::LessThan : MacroAssembler::GreaterThan, op1GPR, op2GPR);
     2327            m_jit.move(op2GPR, resultGPR);
     2328            if (op1GPR != resultGPR) {
    23212329                MacroAssembler::Jump done = m_jit.jump();
    23222330                op1Less.link(&m_jit);
    2323                 m_jit.move(op1.gpr(), result.gpr());
     2331                m_jit.move(op1GPR, resultGPR);
    23242332                done.link(&m_jit);
    23252333            } else
    23262334                op1Less.link(&m_jit);
    23272335           
    2328             integerResult(result.gpr(), node);
     2336            integerResult(resultGPR, node);
    23292337            break;
    23302338        }
    … …  
    23342342            SpeculateDoubleOperand op2(this, node->child2());
    23352343            FPRTemporary result(this, op1);
    2336        
     2344
     2345            FPRReg op1FPR = op1.fpr();
     2346            FPRReg op2FPR = op2.fpr();
     2347            FPRReg resultFPR = result.fpr();
     2348
    23372349            MacroAssembler::JumpList done;
    23382350       
    2339             MacroAssembler::Jump op1Less = m_jit.branchDouble(op == ArithMin ? MacroAssembler::DoubleLessThan : MacroAssembler::DoubleGreaterThan, op1.fpr(), op2.fpr());
     2351            MacroAssembler::Jump op1Less = m_jit.branchDouble(op == ArithMin ? MacroAssembler::DoubleLessThan : MacroAssembler::DoubleGreaterThan, op1FPR, op2FPR);
    23402352       
    23412353            // op2 is eather the lesser one or one of then is NaN
    2342             MacroAssembler::Jump op2Less = m_jit.branchDouble(op == ArithMin ? MacroAssembler::DoubleGreaterThanOrEqual : MacroAssembler::DoubleLessThanOrEqual, op1.fpr(), op2.fpr());
     2354            MacroAssembler::Jump op2Less = m_jit.branchDouble(op == ArithMin ? MacroAssembler::DoubleGreaterThanOrEqual : MacroAssembler::DoubleLessThanOrEqual, op1FPR, op2FPR);
    23432355       
    23442356            // Unordered case. We don't know which of op1, op2 is NaN. Manufacture NaN by adding
    23452357            // op1 + op2 and putting it into result.
    2346             m_jit.addDouble(op1.fpr(), op2.fpr(), result.fpr());
     2358            m_jit.addDouble(op1FPR, op2FPR, resultFPR);
    23472359            done.append(m_jit.jump());
    23482360       
    23492361            op2Less.link(&m_jit);
    2350             m_jit.moveDouble(op2.fpr(), result.fpr());
    2351        
    2352             if (op1.fpr() != result.fpr()) {
     2362            m_jit.moveDouble(op2FPR, resultFPR);
     2363       
     2364            if (op1FPR != resultFPR) {
    23532365                done.append(m_jit.jump());
    23542366           
    23552367                op1Less.link(&m_jit);
    2356                 m_jit.moveDouble(op1.fpr(), result.fpr());
     2368                m_jit.moveDouble(op1FPR, resultFPR);
    23572369            } else
    23582370                op1Less.link(&m_jit);
    … …  
    23602372            done.link(&m_jit);
    23612373       
    2362             doubleResult(result.fpr(), node);
     2374            doubleResult(resultFPR, node);
    23632375            break;
    23642376        }

  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp

    r146100 r146174  
    22692269            SpeculateDoubleOperand op2(this, node->child2());
    22702270            FPRTemporary result(this, op1);
     2271           
     2272            FPRReg op1FPR = op1.fpr();
     2273            FPRReg op2FPR = op2.fpr();
     2274            FPRReg resultFPR = result.fpr();
    22712275       
    22722276            MacroAssembler::JumpList done;
    22732277       
    2274             MacroAssembler::Jump op1Less = m_jit.branchDouble(op == ArithMin ? MacroAssembler::DoubleLessThan : MacroAssembler::DoubleGreaterThan, op1.fpr(), op2.fpr());
     2278            MacroAssembler::Jump op1Less = m_jit.branchDouble(op == ArithMin ? MacroAssembler::DoubleLessThan : MacroAssembler::DoubleGreaterThan, op1FPR, op2FPR);
    22752279       
    22762280            // op2 is eather the lesser one or one of then is NaN
    2277             MacroAssembler::Jump op2Less = m_jit.branchDouble(op == ArithMin ? MacroAssembler::DoubleGreaterThanOrEqual : MacroAssembler::DoubleLessThanOrEqual, op1.fpr(), op2.fpr());
     2281            MacroAssembler::Jump op2Less = m_jit.branchDouble(op == ArithMin ? MacroAssembler::DoubleGreaterThanOrEqual : MacroAssembler::DoubleLessThanOrEqual, op1FPR, op2FPR);
    22782282       
    22792283            // Unordered case. We don't know which of op1, op2 is NaN. Manufacture NaN by adding
    22802284            // op1 + op2 and putting it into result.
    2281             m_jit.addDouble(op1.fpr(), op2.fpr(), result.fpr());
     2285            m_jit.addDouble(op1FPR, op2FPR, resultFPR);
    22822286            done.append(m_jit.jump());
    22832287       
    22842288            op2Less.link(&m_jit);
    2285             m_jit.moveDouble(op2.fpr(), result.fpr());
    2286        
    2287             if (op1.fpr() != result.fpr()) {
     2289            m_jit.moveDouble(op2FPR, resultFPR);
     2290       
     2291            if (op1FPR != resultFPR) {
    22882292                done.append(m_jit.jump());
    22892293           
    22902294                op1Less.link(&m_jit);
    2291                 m_jit.moveDouble(op1.fpr(), result.fpr());
     2295                m_jit.moveDouble(op1FPR, resultFPR);
    22922296            } else
    22932297                op1Less.link(&m_jit);
    … …  
    22952299            done.link(&m_jit);
    22962300       
    2297             doubleResult(result.fpr(), node);
     2301            doubleResult(resultFPR, node);
    22982302            break;
    22992303        }
Note: See TracChangeset for help on using the changeset viewer.