Pop call frame before boxing result
After reflective method calls, primitive results need to be boxed
up in an appropriate object. This generally requires allocating an
object of the appropriate type. We were doing this before popping
the call frame, which meant that if the allocation caused a GC we
would be scanning results out of a "dead" stack frame.
We hit a case where a register went from holding a reference to
holding a primitive right before the method returned. The exported
PC was pointing at the last GC point, where the register was expected
to hold a reference, so precise GC was getting very confused.
Bug 3102352.
Change-Id: Ic929b725ab81cbaab4c7aa1d85131f7ca3cec961
diff --git a/vm/Thread.c b/vm/Thread.c
index 9011a21..a932889 100644
--- a/vm/Thread.c
+++ b/vm/Thread.c
@@ -4070,6 +4070,9 @@
/* this is very bad */
LOGE("PGC: invalid ref in reg %d: 0x%08x\n",
method->registersSize-1 - i, rval);
+ LOGE("PGC: %s.%s addr 0x%04x\n",
+ method->clazz->descriptor, method->name,
+ saveArea->xtra.currentPc - method->insns);
} else
#endif
{
diff --git a/vm/interp/Stack.c b/vm/interp/Stack.c
index 0456e50..695aa44 100644
--- a/vm/interp/Stack.c
+++ b/vm/interp/Stack.c
@@ -658,6 +658,7 @@
s4* ins;
int verifyCount, argListLength;
JValue retval;
+ bool needPop = false;
/* verify arg count */
if (argList != NULL)
@@ -675,6 +676,7 @@
clazz = callPrep(self, method, obj, !noAccessCheck);
if (clazz == NULL)
return NULL;
+ needPop = true;
/* "ins" for new frame start at frame pointer plus locals */
ins = ((s4*)self->curFrame) + (method->registersSize - method->insSize);
@@ -710,9 +712,10 @@
(*(types-1))->descriptor);
}
dvmPopFrame(self); // throw wants to pull PC out of stack
+ needPop = false;
dvmThrowException("Ljava/lang/IllegalArgumentException;",
"argument type mismatch");
- goto bail_popped;
+ goto bail;
}
ins += width;
@@ -740,6 +743,13 @@
}
/*
+ * Pop the frame immediately. The "wrap" calls below can cause
+ * allocations, and we don't want the GC to walk the now-dead frame.
+ */
+ dvmPopFrame(self);
+ needPop = false;
+
+ /*
* If an exception is raised, wrap and replace. This is necessary
* because the invoked method could have thrown a checked exception
* that the caller wasn't prepared for.
@@ -764,8 +774,9 @@
}
bail:
- dvmPopFrame(self);
-bail_popped:
+ if (needPop) {
+ dvmPopFrame(self);
+ }
return retObj;
}
