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.

(cherry-pick from dalvik-dev)

Change-Id: I21f59f1d70689d9e4901deb3100c756fd85223e7

diff --git a/vm/Thread.c b/vm/Thread.c
index 6a5ab1a..c9b6311 100644 (file)
--- a/vm/Thread.c
+++ b/vm/Thread.c
@@ -4054,6 +4054,9 @@ static void gcScanInterpStackReferences(Thread *thread)
                             /* 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 (file)
--- a/vm/interp/Stack.c
+++ b/vm/interp/Stack.c
@@ -658,6 +658,7 @@ Object* dvmInvokeMethod(Object* obj, const Method* method,
     s4* ins;
     int verifyCount, argListLength;
     JValue retval;
+    bool needPop = false;
 
     /* verify arg count */
     if (argList != NULL)
@@ -675,6 +676,7 @@ Object* dvmInvokeMethod(Object* obj, const Method* method,
     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 @@ Object* dvmInvokeMethod(Object* obj, const Method* method,
                     (*(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 @@ Object* dvmInvokeMethod(Object* obj, const Method* method,
     }
 
     /*
+     * 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 @@ Object* dvmInvokeMethod(Object* obj, const Method* method,
     }
 
 bail:
-    dvmPopFrame(self);
-bail_popped:
+    if (needPop) {
+        dvmPopFrame(self);
+    }
     return retObj;
 }