Bug 1125770 - Always trace |this| slot in Ion frames, r=jandem.
--- 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",