Bug 1125770 - Always trace |this| slot in Ion frames, r=jandem.

js/src/jit/BacktrackingAllocator.cpp

@@ -168,6 +168,14 @@ BacktrackingAllocator::canAddToGroup(VirtualRegisterGroup *group, BacktrackingVi
     return true;
 }
 
+static bool
+IsThisSlotDefinition(LDefinition *def)
+{
+    return def->policy() == LDefinition::FIXED &&
+           def->output()->isArgument() &&
+           def->output()->toArgument()->index() < THIS_FRAME_ARGSLOT + sizeof(Value);
+}
+
 bool
 BacktrackingAllocator::tryGroupRegisters(uint32_t vreg0, uint32_t vreg1)
 {
@@ -179,6 +187,15 @@ BacktrackingAllocator::tryGroupRegisters(uint32_t vreg0, uint32_t vreg1)
     if (!reg0->isCompatibleVReg(*reg1))
         return true;
 
+    // Registers which might spill to the frame's |this| slot can only be
+    // grouped with other such registers. The frame's |this| slot must always
+    // hold the |this| value, as required by JitFrame tracing and by the Ion
+    // constructor calling convention.
+    if (IsThisSlotDefinition(reg0->def()) || IsThisSlotDefinition(reg1->def())) {
+        if (*reg0->def()->output() != *reg1->def()->output())
+            return true;
+    }
+
     VirtualRegisterGroup *group0 = reg0->group(), *group1 = reg1->group();
 
     if (!group0 && group1)

js/src/jit/JitFrames.cpp

@@ -972,23 +972,24 @@ ReadAllocation(const JitFrameIterator &frame, const LAllocation *a)
 #endif
 
 static void
-MarkExtraActualArguments(JSTracer *trc, const JitFrameIterator &frame)
+MarkThisAndExtraActualArguments(JSTracer *trc, const JitFrameIterator &frame)
 {
-    // Mark any extra actual arguments for an Ion frame. Marking of |this| and
-    // the formal arguments is taken care of by the frame's safepoint/snapshot.
+    // Mark |this| and any extra actual arguments for an Ion frame. Marking of
+    // formal arguments is taken care of by the frame's safepoint/snapshot.
 
     JitFrameLayout *layout = frame.jsFrame();
 
-    if (!CalleeTokenIsFunction(layout->calleeToken())) {
-        MOZ_ASSERT(frame.numActualArgs() == 0);
-        return;
-    }
-
     size_t nargs = frame.numActualArgs();
-    size_t nformals = CalleeTokenToFunction(layout->calleeToken())->nargs();
+    size_t nformals = 0;
+    if (CalleeTokenIsFunction(layout->calleeToken()))
+        nformals = CalleeTokenToFunction(layout->calleeToken())->nargs();
 
-    // Trace actual arguments. Note + 1 for thisv.
     Value *argv = layout->argv();
+
+    // Trace |this|.
+    gc::MarkValueRoot(trc, argv, "ion-thisv");
+
+    // Trace actual arguments beyond the formals. Note + 1 for thisv.
     for (size_t i = nformals + 1; i < nargs + 1; i++)
         gc::MarkValueRoot(trc, &argv[i], "ion-argv");
 }
@@ -1023,7 +1024,7 @@ MarkIonJSFrame(JSTracer *trc, const JitFrameIterator &frame)
         ionScript = frame.ionScriptFromCalleeToken();
     }
 
-    MarkExtraActualArguments(trc, frame);
+    MarkThisAndExtraActualArguments(trc, frame);
 
     const SafepointIndex *si = ionScript->getSafepointIndex(frame.returnAddressToFp());
 
@@ -1082,7 +1083,7 @@ MarkBailoutFrame(JSTracer *trc, const JitFrameIterator &frame)
 
     // We have to mark the list of actual arguments, as only formal arguments
     // are represented in the Snapshot.
-    MarkExtraActualArguments(trc, frame);
+    MarkThisAndExtraActualArguments(trc, frame);
 
     // Under a bailout, do not have a Safepoint to only iterate over GC-things.
     // Thus we use a SnapshotIterator to trace all the locations which would be

js/src/jit/LinearScan.cpp

@@ -484,15 +484,11 @@ LinearScanAllocator::isSpilledAt(LiveInterval *interval, CodePosition pos)
 bool
 LinearScanAllocator::populateSafepoints()
 {
-    // Populate all safepoints with this/argument slots. These are never changed
+    // Populate all safepoints with argument slots. These are never changed
     // by the allocator and are not necessarily populated by the code below.
     size_t nargs = graph.getBlock(0)->mir()->info().nargs();
     for (size_t i = 0; i < graph.numSafepoints(); i++) {
         LSafepoint *safepoint = graph.getSafepoint(i)->safepoint();
-
-        if (!safepoint->addValueSlot(/* stack = */ false, THIS_FRAME_ARGSLOT * sizeof(Value)))
-            return false;
-
         for (size_t j = 0; j < nargs; j++) {
             if (!safepoint->addValueSlot(/* stack = */ false, (j + 1) * sizeof(Value)))
                 return false;

js/src/jit/RegisterAllocator.cpp

@@ -278,6 +278,10 @@ AllocationIntegrityState::checkSafepointAllocation(LInstruction *ins,
         MOZ_ASSERT(safepoint->liveRegs().has(reg));
     }
 
+    // The |this| argument slot is implicitly included in all safepoints.
+    if (alloc.isArgument() && alloc.toArgument()->index() < THIS_FRAME_ARGSLOT + sizeof(Value))
+        return true;
+
     LDefinition::Type type = virtualRegisters[vreg]
                              ? virtualRegisters[vreg]->type()
                              : LDefinition::GENERAL;