Bug 904315 - Watch for negative integers when loading maybe-hole elements from arrays, r=jandem.

js/src/jit-test/tests/ion/bug904315.js

@@ -0,0 +1,15 @@
+
+function g(o, idx, exp) {
+    for (var i=0; i<3000; i++) {
+	assertEq(o[idx], exp);
+    }
+}
+function f() {
+    var o = [];
+    for (var i=1; i<100; i++) {
+	o[-i] = 1;
+    }
+    g(o, 50, undefined);
+    g(o, -50, 1);
+}
+f();

js/src/jit/BaselineIC.cpp

@@ -3598,6 +3598,12 @@ TryAttachGetElemStub(JSContext *cx, HandleScript script, ICGetElem_Fallback *stu
     if (!obj->isNative() && !obj->is<TypedArrayObject>())
         stub->noteNonNativeAccess();
 
+    // GetElem operations which could access negative indexes generally can't
+    // be optimized without the potential for bailouts, as we can't statically
+    // determine that an object has no properties on such indexes.
+    if (rhs.isNumber() && rhs.toNumber() < 0)
+        stub->noteNegativeIndex();
+
     return true;
 }
 

js/src/jit/BaselineIC.h

@@ -2787,6 +2787,9 @@ class ICGetElem_Fallback : public ICMonitoredFallbackStub
       : ICMonitoredFallbackStub(ICStub::GetElem_Fallback, stubCode)
     { }
 
+    static const uint16_t EXTRA_NON_NATIVE = 0x1;
+    static const uint16_t EXTRA_NEGATIVE_INDEX = 0x2;
+
   public:
     static const uint32_t MAX_OPTIMIZED_STUBS = 16;
 
@@ -2797,10 +2800,17 @@ class ICGetElem_Fallback : public ICMonitoredFallbackStub
     }
 
     void noteNonNativeAccess() {
-        extra_ = 1;
+        extra_ |= EXTRA_NON_NATIVE;
     }
     bool hasNonNativeAccess() const {
-        return extra_;
+        return extra_ & EXTRA_NON_NATIVE;
+    }
+
+    void noteNegativeIndex() {
+        extra_ |= EXTRA_NEGATIVE_INDEX;
+    }
+    bool hasNegativeIndex() const {
+        return extra_ & EXTRA_NEGATIVE_INDEX;
     }
 
     // Compiler for this stub kind.

js/src/jit/BaselineInspector.cpp

@@ -303,6 +303,20 @@ BaselineInspector::hasSeenNonNativeGetElement(jsbytecode *pc)
     return false;
 }
 
+bool
+BaselineInspector::hasSeenNegativeIndexGetElement(jsbytecode *pc)
+{
+    if (!hasBaselineScript())
+        return false;
+
+    const ICEntry &entry = icEntryFromPC(pc);
+    ICStub *stub = entry.fallbackStub();
+
+    if (stub->isGetElem_Fallback())
+        return stub->toGetElem_Fallback()->hasNegativeIndex();
+    return false;
+}
+
 bool
 BaselineInspector::hasSeenAccessedGetter(jsbytecode *pc)
 {

js/src/jit/BaselineInspector.h

@@ -104,6 +104,7 @@ class BaselineInspector
     MIRType expectedBinaryArithSpecialization(jsbytecode *pc);
 
     bool hasSeenNonNativeGetElement(jsbytecode *pc);
+    bool hasSeenNegativeIndexGetElement(jsbytecode *pc);
     bool hasSeenAccessedGetter(jsbytecode *pc);
     bool hasSeenDoubleResult(jsbytecode *pc);
 };

js/src/jit/CodeGenerator.cpp

@@ -6433,6 +6433,8 @@ CodeGenerator::visitLoadElementHole(LLoadElementHole *lir)
     Register initLength = ToRegister(lir->initLength());
     const ValueOperand out = ToOutValue(lir);
 
+    const MLoadElementHole *mir = lir->mir();
+
     // If the index is out of bounds, load |undefined|. Otherwise, load the
     // value.
     Label undefined, done;
@@ -6452,6 +6454,19 @@ CodeGenerator::visitLoadElementHole(LLoadElementHole *lir)
         masm.jump(&done);
 
     masm.bind(&undefined);
+
+    if (mir->needsNegativeIntCheck()) {
+        if (lir->index()->isConstant()) {
+            if (ToInt32(lir->index()) < 0 && !bailout(lir->snapshot()))
+                return false;
+        } else {
+            Label negative;
+            masm.branch32(Assembler::LessThan, ToRegister(lir->index()), Imm32(0), &negative);
+            if (!bailoutFrom(&negative, lir->snapshot()))
+                return false;
+        }
+    }
+
     masm.moveValue(UndefinedValue(), out);
     masm.bind(&done);
     return true;

js/src/jit/IonBuilder.cpp

@@ -6465,6 +6465,11 @@ IonBuilder::getElemTryDense(bool *emitted, MDefinition *obj, MDefinition *index)
     if (ElementAccessHasExtraIndexedProperty(cx, obj) && failedBoundsCheck_)
         return true;
 
+    // Don't generate a fast path if this pc has seen negative indexes accessed,
+    // which will not appear to be extra indexed properties.
+    if (inspector->hasSeenNegativeIndexGetElement(pc))
+        return true;
+
     // Emit dense getelem variant.
     if (!jsop_getelem_dense(obj, index))
         return false;

js/src/jit/Lowering.cpp

@@ -2068,6 +2068,8 @@ LIRGenerator::visitLoadElementHole(MLoadElementHole *ins)
     LLoadElementHole *lir = new LLoadElementHole(useRegister(ins->elements()),
                                                  useRegisterOrConstant(ins->index()),
                                                  useRegister(ins->initLength()));
+    if (ins->needsNegativeIntCheck() && !assignSnapshot(lir))
+        return false;
     return defineBox(lir, ins);
 }
 

js/src/jit/MIR.h

@@ -4821,10 +4821,12 @@ class MLoadElementHole
   : public MTernaryInstruction,
     public SingleObjectPolicy
 {
+    bool needsNegativeIntCheck_;
     bool needsHoleCheck_;
 
     MLoadElementHole(MDefinition *elements, MDefinition *index, MDefinition *initLength, bool needsHoleCheck)
       : MTernaryInstruction(elements, index, initLength),
+        needsNegativeIntCheck_(true),
         needsHoleCheck_(needsHoleCheck)
     {
         setResultType(MIRType_Value);
@@ -4854,12 +4856,16 @@ class MLoadElementHole
     MDefinition *initLength() const {
         return getOperand(2);
     }
+    bool needsNegativeIntCheck() const {
+        return needsNegativeIntCheck_;
+    }
     bool needsHoleCheck() const {
         return needsHoleCheck_;
     }
     AliasSet getAliasSet() const {
         return AliasSet::Load(AliasSet::Element);
     }
+    void collectRangeInfo();
 };
 
 class MStoreElementCommon

js/src/jit/RangeAnalysis.cpp

@@ -2005,6 +2005,12 @@ MInArray::collectRangeInfo()
     needsNegativeIntCheck_ = !index()->range() || index()->range()->lower() < 0;
 }
 
+void
+MLoadElementHole::collectRangeInfo()
+{
+    needsNegativeIntCheck_ = !index()->range() || index()->range()->lower() < 0;
+}
+
 void
 MMod::collectRangeInfo()
 {