Bug 625718: Correct SETPROP label offsets. (r=jbramley)
authorChris Leary <cdleary@mozilla.com>
Fri, 14 Jan 2011 07:49:59 -0800
changeset 60601 fb2192c7b8c272818d7fbd9433538e6395bd876d
parent 60600 e8c8df3f17f2ba93a48cc67a084dca3bf0855430
child 60602 fbcba31f6380f931db2f9dbe4b514161ada03936
push idunknown
push userunknown
push dateunknown
reviewersjbramley
bugs625718
milestone2.0b10pre
Bug 625718: Correct SETPROP label offsets. (r=jbramley)
js/src/jit-test/tests/jaeger/bug625718-1.js
js/src/jit-test/tests/jaeger/bug625718-2.js
js/src/jit-test/tests/jaeger/bug625718-3.js
js/src/methodjit/Compiler.cpp
js/src/methodjit/ICLabels.h
js/src/methodjit/PolyIC.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/jaeger/bug625718-1.js
@@ -0,0 +1,12 @@
+function f3() { return 2; };
+function f4(o) { o.g4 = function() {}; };
+
+var f = function() {};
+f.x = undefined;
+f4(new String("x"));
+f3();
+f4(f);
+
+for(var i=0; i<20; i++) {
+    f4(Math);
+}
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/jaeger/bug625718-2.js
@@ -0,0 +1,14 @@
+var o3 = new String("foobarbaz");
+var o10 = Math;
+var o11 = function() {};
+
+function f3(o) { return o; };
+function f4(o) { o.g4 = function() {}; };
+
+for(var i=0; i<20; i++) {
+    o11[3] = undefined;
+    f4(o3);
+    f3(o3);
+    f4(o11);
+    f4(o10);
+}
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/jaeger/bug625718-3.js
@@ -0,0 +1,52 @@
+var o0 = [];
+var o1 = new String("foobarbaz");
+var o2 = {};
+var o3 = new String("foobarbaz");
+var o4 = {};
+var o5 = Math;
+var o6 = {};
+var o7 = new String("foobarbaz");
+var o8 = new String("foobarbaz");
+var o9 = Math;
+var o10 = Math;
+var o11 = function() {};
+var o12 = {};
+var o13 = new String("foobarbaz");
+var o14 = {};
+
+function f1(o) { return o.length;};
+function f2(o) { o.g2 = function() {};};
+function f3(o) { return o.g10;};
+function f4(o) { o.g4 = function() {};};
+function f5(o) { return o == o14;};
+function f6(o) { o[3] = o;};
+function f7(o) { o[3] = undefined;};
+function f8(o) { o[3] = undefined;};
+function f9(o) { return o.length;};
+function f10(o) { return o.__proto__; };
+
+for(var i=0; i<20; i++) {
+    f9(o11);
+    f6(o0);
+    f2(o1);
+    f2(o6);
+    f7(o6);
+    f8(o11);
+    f2(o5);
+    f7(o9);
+    f7(o12);
+    f6(o4);
+    f5(o1);
+    f4(o1);
+    f8(o8);
+    f6(o5);
+    f2(o0);
+    f10(o7);
+    f3(o3);
+    f4(o1);
+    f9(o3);
+    f4(o11);
+    f4(o0);
+    f2(o4);
+    f4(o10);
+}
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -3466,21 +3466,17 @@ mjit::Compiler::jsop_setprop(JSAtom *ato
     }
 
     RETURN_IF_OOM(false);
 
     SetPropLabels &labels = pic.setPropLabels();
     labels.setInlineShapeData(masm, pic.shapeGuard, inlineShapeData);
     labels.setDslotsLoad(masm, pic.fastPathRejoin, dslotsLoadLabel, vr);
     labels.setInlineValueStore(masm, pic.fastPathRejoin, inlineValueStore, vr);
-#ifdef JS_CPU_X64
-    labels.setInlineShapeJump(masm, inlineShapeData, afterInlineShapeJump);
-#else
     labels.setInlineShapeJump(masm, pic.shapeGuard, afterInlineShapeJump);
-#endif
 
     pics.append(pic);
     return true;
 }
 
 void
 mjit::Compiler::jsop_name(JSAtom *atom)
 {
--- a/js/src/methodjit/ICLabels.h
+++ b/js/src/methodjit/ICLabels.h
@@ -77,16 +77,17 @@ struct GetPropLabels : MacroAssemblerTyp
 #ifdef JS_HAS_IC_LABELS
         inlineValueLoadOffset = offset;
 #endif
         /* 
          * Note: the offset between the type and data loads for x86 is asserted
          * in NunboxAssembler::loadValueWithAddressOffsetPatch.
          */
         JS_ASSERT(offset == inlineValueLoadOffset);
+        (void) offset;
     }
 
     CodeLocationLabel getValueLoad(CodeLocationLabel fastPathRejoin) {
         return fastPathRejoin.labelAtOffset(inlineValueLoadOffset);
     }
 
     void setDslotsLoad(MacroAssembler &masm, Label fastPathRejoin, Label dslotsLoad) {
         int offset = masm.differenceBetween(fastPathRejoin, dslotsLoad);
@@ -246,44 +247,45 @@ struct SetPropLabels : MacroAssemblerTyp
         int offset = masm.differenceBetween(shapeGuard, inlineShapeData);
         setInlineShapeDataOffset(offset);
     }
 
     CodeLocationDataLabel32 getInlineShapeData(CodeLocationLabel fastPathStart, int shapeGuardOffset) {
         return fastPathStart.dataLabel32AtOffset(shapeGuardOffset + getInlineShapeDataOffset());
     }
 
-    CodeLocationJump getInlineShapeJump(CodeLocationLabel fastPathStart, int shapeGuardOffset) {
-        return fastPathStart.jumpAtOffset(shapeGuardOffset + getInlineShapeJumpOffset());
-    }
-
     void setDslotsLoad(MacroAssembler &masm, Label fastPathRejoin, Label beforeLoad,
                        const ValueRemat &rhs) {
         int offset = masm.differenceBetween(fastPathRejoin, beforeLoad);
         setDslotsLoadOffset(offset, rhs.isConstant(), rhs.isTypeKnown());
     }
 
     CodeLocationInstruction getDslotsLoad(CodeLocationLabel fastPathRejoin, const ValueRemat &vr) {
         return fastPathRejoin.instructionAtOffset(getDslotsLoadOffset(vr));
     }
 
-    /*
-     * Note: on x64, the base is the inlineShapeLabel DataLabel32, whereas on other
-     * platforms the base is the shapeGuard.
-     */
-    template <typename T>
-    void setInlineShapeJump(MacroAssembler &masm, T base, Label afterJump) {
-        setInlineShapeJumpOffset(masm.differenceBetween(base, afterJump));
+    void setInlineShapeJump(MacroAssembler &masm, Label shapeGuard, Label afterJump) {
+        setInlineShapeJumpOffset(masm.differenceBetween(shapeGuard, afterJump));
+    }
+
+    CodeLocationJump getInlineShapeJump(CodeLocationLabel shapeGuard) {
+        return shapeGuard.jumpAtOffset(getInlineShapeJumpOffset());
     }
 
     void setStubShapeJump(MacroAssembler &masm, Label stubStart, Label afterShapeJump) {
         int offset = masm.differenceBetween(stubStart, afterShapeJump);
         setStubShapeJumpOffset(offset);
     }
 
+    CodeLocationJump getStubShapeJump(CodeLocationLabel stubStart) {
+        return stubStart.jumpAtOffset(getStubShapeJumpOffset());
+    }
+
+  private:
+
     /* Offset-based interface. */
 
     void setDslotsLoadOffset(int offset, bool isConstant, bool isTypeKnown) {
 #if defined JS_HAS_IC_LABELS
         dslotsLoadOffset = offset;
         JS_ASSERT(offset == dslotsLoadOffset);
 #elif defined JS_CPU_X86
         JS_ASSERT_IF(isConstant, offset == INLINE_DSLOTS_BEFORE_CONSTANT);
@@ -335,17 +337,17 @@ struct SetPropLabels : MacroAssemblerTyp
     void setInlineShapeJumpOffset(int offset) {
 #ifdef JS_HAS_IC_LABELS
         inlineShapeJumpOffset = offset;
 #endif
         JS_ASSERT(offset == inlineShapeJumpOffset);
     }
 
     int getInlineShapeJumpOffset() {
-        return POST_INST_OFFSET(inlineShapeDataOffset + INLINE_SHAPE_JUMP);
+        return POST_INST_OFFSET(inlineShapeJumpOffset);
     }
 
     int getInlineShapeDataOffset() {
         return inlineShapeDataOffset;
     }
 
     int getStubShapeJumpOffset() {
         return POST_INST_OFFSET(stubShapeJumpOffset);
@@ -359,17 +361,16 @@ struct SetPropLabels : MacroAssemblerTyp
             return INLINE_VALUE_STORE_CONSTANT;
         else if (isTypeKnown)
             return INLINE_VALUE_STORE_KTYPE;
         else
             return INLINE_VALUE_STORE_DYNAMIC;
 #endif
     }
 
-  private:
     /* Offset from storeBack to beginning of 'mov dslots, addr'. */
 #if defined JS_CPU_X86
     static const int INLINE_DSLOTS_BEFORE_CONSTANT = -23;
     static const int INLINE_DSLOTS_BEFORE_KTYPE = -19;
     static const int INLINE_DSLOTS_BEFORE_DYNAMIC = -15;
 #else
     int32 dslotsLoadOffset : 8;
 #endif
@@ -389,26 +390,16 @@ struct SetPropLabels : MacroAssemblerTyp
     static const int INLINE_VALUE_STORE_KTYPE = -16;
     static const int INLINE_VALUE_STORE_DYNAMIC = -12;
 #else
     int32 inlineValueStoreOffset : 8;
 #endif
 
     /* Offset from shapeGuard to the end of the shape jump. */
     int32 inlineShapeJumpOffset : 8;
-
-#if defined JS_CPU_X86
-    static const int INLINE_SHAPE_JUMP = 0;
-#elif defined JS_CPU_X64
-    static const int INLINE_SHAPE_JUMP = 6;
-#elif defined JS_CPU_ARM
-    static const int INLINE_SHAPE_JUMP = 12;
-#else
-# error
-#endif
 };
 
 /* BindNameCompiler */
 struct BindNameLabels : MacroAssemblerTypedefs {
     friend class ::ICOffsetInitializer;
 
     void setInlineJumpOffset(int offset) {
 #ifdef JS_HAS_IC_LABELS
--- a/js/src/methodjit/PolyIC.cpp
+++ b/js/src/methodjit/PolyIC.cpp
@@ -199,17 +199,17 @@ class SetPropCompiler : public PICStubCo
     { }
 
     static void reset(Repatcher &repatcher, ic::PICInfo &pic)
     {
         SetPropLabels &labels = pic.setPropLabels();
         repatcher.repatchLEAToLoadPtr(labels.getDslotsLoad(pic.fastPathRejoin, pic.u.vr));
         repatcher.repatch(labels.getInlineShapeData(pic.fastPathStart, pic.shapeGuard),
                           int32(JSObjectMap::INVALID_SHAPE));
-        repatcher.relink(labels.getInlineShapeJump(pic.fastPathStart, pic.shapeGuard),
+        repatcher.relink(labels.getInlineShapeJump(pic.fastPathStart.labelAtOffset(pic.shapeGuard)),
                          pic.slowPathStart);
 
         FunctionPtr target(JS_FUNC_TO_DATA_PTR(void *, ic::SetProp));
         repatcher.relink(pic.slowPathCall, target);
     }
 
     LookupStatus patchInline(const Shape *shape, bool inlineSlot)
     {
@@ -257,22 +257,22 @@ class SetPropCompiler : public PICStubCo
     void patchPreviousToHere(CodeLocationLabel cs)
     {
         Repatcher repatcher(pic.lastCodeBlock(f.jit()));
         CodeLocationLabel label = pic.lastPathStart();
 
         // Patch either the inline fast path or a generated stub. The stub
         // omits the prefix of the inline fast path that loads the shape, so
         // the offsets are different.
-        int shapeGuardJumpOffset;
-        if (pic.stubsGenerated)
-            shapeGuardJumpOffset = pic.setPropLabels().getStubShapeJumpOffset();
-        else
-            shapeGuardJumpOffset = pic.shapeGuard + pic.setPropLabels().getInlineShapeJumpOffset();
-        repatcher.relink(label.jumpAtOffset(shapeGuardJumpOffset), cs);
+        if (pic.stubsGenerated) {
+            repatcher.relink(pic.setPropLabels().getStubShapeJump(label), cs);
+        } else {
+            CodeLocationLabel shapeGuard = label.labelAtOffset(pic.shapeGuard);
+            repatcher.relink(pic.setPropLabels().getInlineShapeJump(shapeGuard), cs);
+        }
         if (int secondGuardOffset = getLastStubSecondShapeGuard())
             repatcher.relink(label.jumpAtOffset(secondGuardOffset), cs);
     }
 
     LookupStatus generateStub(uint32 initialShape, const Shape *shape, bool adding, bool inlineSlot)
     {
         /* Exits to the slow path. */
         Vector<Jump, 8> slowExits(cx);
@@ -287,16 +287,18 @@ class SetPropCompiler : public PICStubCo
         }
 
         Label start = masm.label();
         Jump shapeGuard = masm.branch32_force32(Assembler::NotEqual, pic.shapeReg,
                                                 Imm32(initialShape));
 
         Label stubShapeJumpLabel = masm.label();
 
+        pic.setPropLabels().setStubShapeJump(masm, start, stubShapeJumpLabel);
+
         JS_ASSERT_IF(!shape->hasDefaultSetter(), obj->getClass() == &js_CallClass);
 
         MaybeJump skipOver;
 
         if (adding) {
             JS_ASSERT(shape->hasSlot());
             pic.shapeRegHasBaseShape = false;
 
@@ -415,16 +417,17 @@ class SetPropCompiler : public PICStubCo
                 masm.loadPtr(Address(pic.objReg, offsetof(JSObject, slots)), pic.objReg);
 
                 Address dslot(pic.objReg, (slot + JSObject::CALL_RESERVED_SLOTS) * sizeof(Value));
                 masm.storeValue(pic.u.vr, dslot);
             }
 
             pic.shapeRegHasBaseShape = false;
         }
+
         Jump done = masm.jump();
 
         // Common all secondary guards into one big exit.
         MaybeJump slowExit;
         if (otherGuards.length()) {
             for (Jump *pj = otherGuards.begin(); pj != otherGuards.end(); ++pj)
                 pj->linkTo(masm.label(), &masm);
             slowExit = masm.jump();
@@ -460,18 +463,16 @@ class SetPropCompiler : public PICStubCo
         // This function can patch either the inline fast path for a generated
         // stub. The stub omits the prefix of the inline fast path that loads
         // the shape, so the offsets are different.
         patchPreviousToHere(cs);
 
         pic.stubsGenerated++;
         pic.updateLastPath(buffer, start);
 
-        pic.setPropLabels().setStubShapeJump(masm, start, stubShapeJumpLabel);
-
         if (pic.stubsGenerated == MAX_PIC_STUBS)
             disable("max stubs reached");
 
         return Lookup_Cacheable;
     }
 
     LookupStatus update()
     {