Change OSIpoints to record the call address rather than the return address (bug 738124, r=dvander)

js/src/ion/CodeGenerator.cpp

@@ -274,13 +274,13 @@ CodeGenerator::visitOsiPoint(LOsiPoint *lir)
 
     JS_ASSERT(masm.framePushed() == frameSize());
 
-    uint32 osiReturnPointOffset;
-    if (!markOsiPoint(lir, &osiReturnPointOffset))
+    uint32 osiCallPointOffset;
+    if (!markOsiPoint(lir, &osiCallPointOffset))
         return false;
 
     LSafepoint *safepoint = lir->associatedSafepoint();
-    JS_ASSERT(!safepoint->osiReturnPointOffset());
-    safepoint->setOsiReturnPointOffset(osiReturnPointOffset);
+    JS_ASSERT(!safepoint->osiCallPointOffset());
+    safepoint->setOsiCallPointOffset(osiCallPointOffset);
     return true;
 }
 

js/src/ion/IonFrames.cpp

@@ -549,6 +549,14 @@ ion::GetPcScript(JSContext *cx, JSScript **scriptRes, jsbytecode **pcRes)
 void
 OsiIndex::fixUpOffset(MacroAssembler &masm)
 {
-    returnPointDisplacement_ = masm.actualOffset(returnPointDisplacement_);
+    callPointDisplacement_ = masm.actualOffset(callPointDisplacement_);
 }
 
+uint32
+OsiIndex::returnPointDisplacement() const
+{
+    // In general, pointer arithmetic on code is bad, but in this case,
+    // getting the return address from a call instruction, stepping over pools
+    // would be wrong.
+    return callPointDisplacement_ + Assembler::patchWrite_NearCallSize();
+}

js/src/ion/IonFrames.h

@@ -165,17 +165,18 @@ class MacroAssembler;
 // buffer is the |returnPointDisplacement|.
 class OsiIndex
 {
-    uint32 returnPointDisplacement_;
+    uint32 callPointDisplacement_;
     uint32 snapshotOffset_;
 
   public:
-    OsiIndex(uint32 returnPointDisplacement, uint32 snapshotOffset)
-      : returnPointDisplacement_(returnPointDisplacement),
+    OsiIndex(uint32 callPointDisplacement, uint32 snapshotOffset)
+      : callPointDisplacement_(callPointDisplacement),
         snapshotOffset_(snapshotOffset)
     { }
 
-    uint32 returnPointDisplacement() const {
-        return returnPointDisplacement_;
+    uint32 returnPointDisplacement() const;
+    uint32 callPointDisplacement() const {
+        return callPointDisplacement_;
     }
     uint32 snapshotOffset() const {
         return snapshotOffset_;

js/src/ion/LIR.h

@@ -943,8 +943,8 @@ class LSafepoint : public TempObject
     // INVALID_SAFEPOINT_OFFSET.
     uint32 safepointOffset_;
 
-    // Assembler buffer displacement to OSI point's return location.
-    uint32 osiReturnPointOffset_;
+    // Assembler buffer displacement to OSI point's call location.
+    uint32 osiCallPointOffset_;
 
     // List of stack slots which have gc pointers.
     SlotList gcSlots_;
@@ -957,7 +957,7 @@ class LSafepoint : public TempObject
   public:
     LSafepoint()
       : safepointOffset_(INVALID_SAFEPOINT_OFFSET),
-        osiReturnPointOffset_(0)
+        osiCallPointOffset_(0)
     { }
     void addLiveRegister(AnyRegister reg) {
         liveRegs_.add(reg);
@@ -1017,14 +1017,20 @@ class LSafepoint : public TempObject
         safepointOffset_ = offset;
     }
     uint32 osiReturnPointOffset() const {
-        return osiReturnPointOffset_;
+        // In general, pointer arithmetic on code is bad, but in this case,
+        // getting the return address from a call instruction, stepping over pools
+        // would be wrong.
+        return osiCallPointOffset_ + Assembler::patchWrite_NearCallSize();
     }
-    void setOsiReturnPointOffset(uint32 osiReturnPointOffset) {
-        JS_ASSERT(!osiReturnPointOffset_);
-        osiReturnPointOffset_ = osiReturnPointOffset;
+    uint32 osiCallPointOffset() const {
+        return osiCallPointOffset_;
+    }
+    void setOsiCallPointOffset(uint32 osiCallPointOffset) {
+        JS_ASSERT(!osiCallPointOffset_);
+        osiCallPointOffset_ = osiCallPointOffset;
     }
     void fixupOffset(MacroAssembler *masm) {
-        osiReturnPointOffset_ = masm->actualOffset(osiReturnPointOffset_);
+        osiCallPointOffset_ = masm->actualOffset(osiCallPointOffset_);
         safepointOffset_ = masm->actualOffset(safepointOffset_);
     }
 };

js/src/ion/Safepoints.cpp

@@ -63,9 +63,9 @@ SafepointWriter::startEntry()
 }
 
 void
-SafepointWriter::writeOsiReturnPointOffset(uint32 osiReturnPointOffset)
+SafepointWriter::writeOsiCallPointOffset(uint32 osiCallPointOffset)
 {
-    stream_.writeUnsigned(osiReturnPointOffset);
+    stream_.writeUnsigned(osiCallPointOffset);
 }
 
 static void
@@ -184,20 +184,24 @@ SafepointReader::InvalidationPatchPoint(IonScript *script, const SafepointIndex
 {
     SafepointReader reader(script, si);
 
-    // The size of a call is subtracted off, because the return address of the call to the
-    // invalidateEpilogue must match the value that was recorded previously in markOsiPoint.
-    // The returned value should only be within the code generated for the LOsiPoint,
-    // not the preceeding call.
-    uint32 osiPointOffset = reader.getOsiReturnPointOffset() - Assembler::patchWrite_NearCallSize();
+    uint32 osiPointOffset = reader.getOsiCallPointOffset();
     return CodeLocationLabel(script->method(), osiPointOffset);
 }
 
+uint32
+SafepointReader::getOsiCallPointOffset()
+{
+    return stream_.readUnsigned();
+}
+
 uint32
 SafepointReader::getOsiReturnPointOffset()
 {
-    return stream_.readUnsigned();
+    // In general, pointer arithmetic on code is bad, but in this case,
+    // getting the return address from a call instruction, stepping over pools
+    // would be wrong.
+    return stream_.readUnsigned() + Assembler::patchWrite_NearCallSize();
 }
-
 void
 SafepointReader::getGcRegs(GeneralRegisterSet *actual, GeneralRegisterSet *spilled)
 {

js/src/ion/Safepoints.h

@@ -65,7 +65,7 @@ class SafepointWriter
 
     // A safepoint entry is written in the order these functions appear.
     uint32 startEntry();
-    void writeOsiReturnPointOffset(uint32 osiPointOffset);
+    void writeOsiCallPointOffset(uint32 osiPointOffset);
     void writeGcRegs(GeneralRegisterSet actual, GeneralRegisterSet spilled);
     void writeGcSlots(uint32 nslots, uint32 *slots);
     void writeValueSlots(uint32 nslots, uint32 *slots);
@@ -99,6 +99,7 @@ class SafepointReader
     static CodeLocationLabel InvalidationPatchPoint(IonScript *script, const SafepointIndex *si);
 
     uint32 getOsiReturnPointOffset();
+    uint32 getOsiCallPointOffset();
 
     // A safepoint entry must be read in the order these functions appear.
     void getGcRegs(GeneralRegisterSet *actual, GeneralRegisterSet *spilled);

js/src/ion/shared/CodeGenerator-shared.cpp

@@ -285,9 +285,9 @@ CodeGeneratorShared::encodeSafepoint(LSafepoint *safepoint)
 
     uint32 safepointOffset = safepoints_.startEntry();
 
-    JS_ASSERT(safepoint->osiReturnPointOffset());
+    JS_ASSERT(safepoint->osiCallPointOffset());
 
-    safepoints_.writeOsiReturnPointOffset(safepoint->osiReturnPointOffset());
+    safepoints_.writeOsiCallPointOffset(safepoint->osiCallPointOffset());
     safepoints_.writeGcRegs(safepoint->gcRegs(), safepoint->liveRegs().gprs());
     safepoints_.writeGcSlots(safepoint->gcSlots().length(), safepoint->gcSlots().begin());
 #ifdef JS_NUNBOX32
@@ -309,7 +309,7 @@ CodeGeneratorShared::encodeSafepoints()
         LSafepoint *safepoint = it->safepoint();
 
         // All safepoints must have a valid OSI displacement.
-        JS_ASSERT(safepoint->osiReturnPointOffset());
+        JS_ASSERT(safepoint->osiCallPointOffset());
         encodeSafepoint(safepoint);
         it->resolve();
     }
@@ -330,7 +330,7 @@ CodeGeneratorShared::markSafepointAt(uint32 offset, LInstruction *ins)
 }
 
 bool
-CodeGeneratorShared::markOsiPoint(LOsiPoint *ins, uint32 *returnPointOffset)
+CodeGeneratorShared::markOsiPoint(LOsiPoint *ins, uint32 *callPointOffset)
 {
     if (!encode(ins->snapshot()))
         return false;
@@ -347,10 +347,9 @@ CodeGeneratorShared::markOsiPoint(LOsiPoint *ins, uint32 *returnPointOffset)
     JS_ASSERT(masm.currentOffset() - lastOsiPointOffset_ >= Assembler::patchWrite_NearCallSize());
     lastOsiPointOffset_ = masm.currentOffset();
 
-    *returnPointOffset = masm.currentOffset() + Assembler::patchWrite_NearCallSize();
-
+    *callPointOffset = masm.currentOffset();
     SnapshotOffset so = ins->snapshot()->snapshotOffset();
-    return osiIndices_.append(OsiIndex(*returnPointOffset, so));
+    return osiIndices_.append(OsiIndex(*callPointOffset, so));
 }
 
 // Before doing any call to Cpp, you should ensure that volatile