Bug 1125770 - Always trace |this| slot in Ion frames, r=jandem.
authorBrian Hackett <bhackett1024@gmail.com>
Tue, 27 Jan 2015 18:11:44 -0700
changeset 239530 05f7505159ebfadcf03a7890ca14969aa85eadd0
parent 239529 58222952d07345035416a192a1c521f337dc250b
child 239531 1c324316ea7a505bc200d2d207e9b513e6c1ffa4
push id500
push userjoshua.m.grant@gmail.com
push dateThu, 29 Jan 2015 01:48:36 +0000
reviewersjandem
bugs1125770
milestone38.0a1
Bug 1125770 - Always trace |this| slot in Ion frames, r=jandem.
js/src/jit/BacktrackingAllocator.cpp
js/src/jit/JitFrames.cpp
js/src/jit/LinearScan.cpp
js/src/jit/RegisterAllocator.cpp
--- a/js/src/jit/BacktrackingAllocator.cpp
+++ b/js/src/jit/BacktrackingAllocator.cpp
@@ -163,27 +163,44 @@ BacktrackingAllocator::canAddToGroup(Vir
 {
     for (size_t i = 0; i < group->registers.length(); i++) {
         if (LifetimesOverlap(reg, &vregs[group->registers[i]]))
             return false;
     }
     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)
 {
     // See if reg0 and reg1 can be placed in the same group, following the
     // restrictions imposed by VirtualRegisterGroup and any other registers
     // already grouped with reg0 or reg1.
     BacktrackingVirtualRegister *reg0 = &vregs[vreg0], *reg1 = &vregs[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)
         return tryGroupRegisters(vreg1, vreg0);
 
     if (group0) {
         if (group1) {
             if (group0 == group1) {
--- a/js/src/jit/JitFrames.cpp
+++ b/js/src/jit/JitFrames.cpp
@@ -967,33 +967,34 @@ ReadAllocation(const JitFrameIterator &f
         Register reg = a->toGeneralReg()->reg();
         return frame.machineState().read(reg);
     }
     return *frame.jsFrame()->slotRef(SafepointSlotEntry(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();
-
-    // Trace actual arguments. Note + 1 for thisv.
+    size_t nformals = 0;
+    if (CalleeTokenIsFunction(layout->calleeToken()))
+        nformals = CalleeTokenToFunction(layout->calleeToken())->nargs();
+
     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");
 }
 
 #ifdef JS_NUNBOX32
 static inline void
 WriteAllocation(const JitFrameIterator &frame, const LAllocation *a, uintptr_t value)
 {
@@ -1018,17 +1019,17 @@ MarkIonJSFrame(JSTracer *trc, const JitF
         // This frame has been invalidated, meaning that its IonScript is no
         // longer reachable through the callee token (JSFunction/JSScript->ion
         // is now nullptr or recompiled). Manually trace it here.
         IonScript::Trace(trc, ionScript);
     } else {
         ionScript = frame.ionScriptFromCalleeToken();
     }
 
-    MarkExtraActualArguments(trc, frame);
+    MarkThisAndExtraActualArguments(trc, frame);
 
     const SafepointIndex *si = ionScript->getSafepointIndex(frame.returnAddressToFp());
 
     SafepointReader safepoint(ionScript, si);
 
     // Scan through slots which contain pointers (or on punboxing systems,
     // actual values).
     SafepointSlotEntry entry;
@@ -1077,17 +1078,17 @@ static void
 MarkBailoutFrame(JSTracer *trc, const JitFrameIterator &frame)
 {
     JitFrameLayout *layout = (JitFrameLayout *)frame.fp();
 
     layout->replaceCalleeToken(MarkCalleeToken(trc, layout->calleeToken()));
 
     // 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
     // used to reconstruct the Baseline frame.
     //
     // Note that at the time where this function is called, we have not yet
     // started to reconstruct baseline frames.
 
--- a/js/src/jit/LinearScan.cpp
+++ b/js/src/jit/LinearScan.cpp
@@ -479,25 +479,21 @@ LinearScanAllocator::isSpilledAt(LiveInt
     }
 
     return interval->getAllocation() == reg->canonicalSpill();
 }
 
 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;
         }
     }
 
     size_t firstSafepoint = 0;
 
--- a/js/src/jit/RegisterAllocator.cpp
+++ b/js/src/jit/RegisterAllocator.cpp
@@ -273,16 +273,20 @@ AllocationIntegrityState::checkSafepoint
     if (alloc.isRegister()) {
         AnyRegister reg = alloc.toRegister();
         if (populateSafepoints)
             safepoint->addLiveRegister(reg);
 
         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;
 
     switch (type) {
       case LDefinition::OBJECT:
         if (populateSafepoints) {
             JitSpew(JitSpew_RegAlloc, "Safepoint object v%u i%u %s",