Merge change 23362

* changes:
  Minor debugging tweaks to IndirectRefTable.

3 files changed, 76 insertions, 15 deletions

diff --git a/vm/IndirectRefTable.c b/vm/IndirectRefTable.c
index f7c764749..bea0a0f88 100644
--- a/vm/IndirectRefTable.c
+++ b/vm/IndirectRefTable.c
@@ -35,6 +35,12 @@ bool dvmInitIndirectRefTable(IndirectRefTable* pRef, int initialCount,
 #ifndef NDEBUG
     memset(pRef->table, 0xd1, initialCount * sizeof(Object*));
 #endif
+
+    pRef->slotData =
+        (IndirectRefSlot*) calloc(maxCount, sizeof(IndirectRefSlot));
+    if (pRef->slotData == NULL)
+        return false;
+
     pRef->segmentState.all = IRT_FIRST_SEGMENT;
     pRef->allocEntries = initialCount;
     pRef->maxEntries = maxCount;
@@ -79,8 +85,8 @@ bool dvmPopIndirectRefTableSegmentCheck(IndirectRefTable* pRef, u4 cookie)
         return false;
     }
 
-    LOGV("--- after pop, top=%d holes=%d\n",
-        sst.parts.topIndex, sst.parts.numHoles);
+    LOGV("IRT %p[%d]: pop, top=%d holes=%d\n",
+        pRef, pRef->kind, sst.parts.topIndex, sst.parts.numHoles);
 
     return true;
 }
@@ -91,15 +97,45 @@ bool dvmPopIndirectRefTableSegmentCheck(IndirectRefTable* pRef, u4 cookie)
 static bool checkEntry(IndirectRefTable* pRef, IndirectRef iref, int idx)
 {
     Object* obj = pRef->table[idx];
-    IndirectRef checkRef = dvmObjectToIndirectRef(obj, idx, pRef->kind);
+    IndirectRef checkRef = dvmObjectToIndirectRef(pRef, obj, idx, pRef->kind);
     if (checkRef != iref) {
-        LOGW("iref mismatch: %p vs %p\n", iref, checkRef);
+        LOGW("IRT %p[%d]: iref mismatch (req=%p vs cur=%p)\n",
+            pRef, pRef->kind, iref, checkRef);
         return false;
     }
     return true;
 }
 
 /*
+ * Update extended debug info when an entry is added.
+ *
+ * We advance the serial number, invalidating any outstanding references to
+ * this slot.
+ */
+static inline void updateSlotAdd(IndirectRefTable* pRef, Object* obj, int slot)
+{
+    if (pRef->slotData != NULL) {
+        IndirectRefSlot* pSlot = &pRef->slotData[slot];
+        pSlot->serial++;
+        //LOGI("+++ add [%d] slot %d (%p->%p), serial=%d\n",
+        //    pRef->kind, slot, obj, iref, pSlot->serial);
+        pSlot->previous[pSlot->serial % kIRTPrevCount] = obj;
+    }
+}
+
+/*
+ * Update extended debug info when an entry is removed.
+ */
+static inline void updateSlotRemove(IndirectRefTable* pRef, int slot)
+{
+    if (pRef->slotData != NULL) {
+        IndirectRefSlot* pSlot = &pRef->slotData[slot];
+        //LOGI("+++ remove [%d] slot %d, serial now %d\n",
+        //    pRef->kind, slot, pSlot->serial);
+    }
+}
+
+/*
  * Add "obj" to "pRef".
  */
 IndirectRef dvmAddToIndirectRefTable(IndirectRefTable* pRef, u4 cookie,
@@ -161,12 +197,15 @@ IndirectRef dvmAddToIndirectRefTable(IndirectRefTable* pRef, u4 cookie,
         while (*--pScan != NULL) {
             assert(pScan >= pRef->table + bottomIndex);
         }
-        result = dvmObjectToIndirectRef(obj, pScan - pRef->table, pRef->kind);
+        updateSlotAdd(pRef, obj, pScan - pRef->table);
+        result = dvmObjectToIndirectRef(pRef, obj, pScan - pRef->table,
+            pRef->kind);
         *pScan = obj;
         pRef->segmentState.parts.numHoles--;
     } else {
         /* add to the end */
-        result = dvmObjectToIndirectRef(obj, topIndex, pRef->kind);
+        updateSlotAdd(pRef, obj, topIndex);
+        result = dvmObjectToIndirectRef(pRef, obj, topIndex, pRef->kind);
         pRef->table[topIndex++] = obj;
         pRef->segmentState.parts.topIndex = topIndex;
     }
@@ -220,6 +259,9 @@ bool dvmGetFromIndirectRefTableCheck(IndirectRefTable* pRef, IndirectRef iref)
  * specified by the cookie, we don't remove anything.  This is the behavior
  * required by JNI's DeleteLocalRef function.
  *
+ * Note this is NOT called when a local frame is popped.  This is only used
+ * for explict single removals.
+ *
  * Returns "false" if nothing was removed.
  */
 bool dvmRemoveFromIndirectRefTable(IndirectRefTable* pRef, u4 cookie,
@@ -255,6 +297,7 @@ bool dvmRemoveFromIndirectRefTable(IndirectRefTable* pRef, u4 cookie,
          */
         if (!checkEntry(pRef, iref, idx))
             return false;
+        updateSlotRemove(pRef, idx);
 
 #ifndef NDEBUG
         pRef->table[idx] = (IndirectRef) 0xd3d3d3d3;
@@ -290,6 +333,7 @@ bool dvmRemoveFromIndirectRefTable(IndirectRefTable* pRef, u4 cookie,
         }
         if (!checkEntry(pRef, iref, idx))
             return false;
+        updateSlotRemove(pRef, idx);
 
         pRef->table[idx] = NULL;
         pRef->segmentState.parts.numHoles++;
@@ -408,7 +452,7 @@ void dvmDumpIndirectRefTable(const IndirectRefTable* pRef, const char* descr)
     qsort(tableCopy, count, sizeof(Object*), compareObject);
     refs = tableCopy;       // use sorted list
 
-    {
+    if (false) {
         int q;
         for (q = 0; q < count; q++)
             LOGI("%d %p\n", q, refs[q]);
diff --git a/vm/IndirectRefTable.h b/vm/IndirectRefTable.h
index c03353b44..6a4db04fc 100644
--- a/vm/IndirectRefTable.h
+++ b/vm/IndirectRefTable.h
@@ -98,6 +98,16 @@ typedef enum IndirectRefKind {
 } IndirectRefKind;
 
 /*
+ * Extended debugging structure.  We keep a parallel array of these, one
+ * per slot in the table.
+ */
+#define kIRTPrevCount   4
+typedef struct IndirectRefSlot {
+    u4          serial;         /* slot serial */
+    Object*     previous[kIRTPrevCount];
+} IndirectRefSlot;
+
+/*
  * Table definition.
  *
  * For the global reference table, the expected common operations are
@@ -179,6 +189,7 @@ typedef struct IndirectRefTable {
     Object**        table;              /* bottom of the stack */
 
     /* private */
+    IndirectRefSlot* slotData;          /* extended debugging info */
     int             allocEntries;       /* #of entries we have space for */
     int             maxEntries;         /* max #of entries allowed */
     IndirectRefKind kind;               /* bit mask, ORed into all irefs */
@@ -198,12 +209,14 @@ typedef struct IndirectRefTable {
  * The object pointer itself is subject to relocation in some GC
  * implementations, so we shouldn't really be using it here.
  */
-INLINE IndirectRef dvmObjectToIndirectRef(Object* obj, u4 tableIndex,
-    IndirectRefKind kind)
+INLINE IndirectRef dvmObjectToIndirectRef(IndirectRefTable* pRef,
+    Object* obj, u4 tableIndex, IndirectRefKind kind)
 {
     assert(tableIndex < 65536);
-    u4 objChunk = (((u4) obj >> 3) ^ ((u4) obj >> 19)) & 0x3fff;
-    u4 uref = objChunk << 18 | (tableIndex << 2) | kind;
+    //u4 objChunk = (((u4) obj >> 3) ^ ((u4) obj >> 19)) & 0x3fff;
+    //u4 uref = objChunk << 18 | (tableIndex << 2) | kind;
+    u4 serialChunk = pRef->slotData[tableIndex].serial;
+    u4 uref = serialChunk << 20 | (tableIndex << 2) | kind;
     return (IndirectRef) uref;
 }
 
@@ -270,7 +283,7 @@ bool dvmPopIndirectRefTableSegmentCheck(IndirectRefTable* pRef, u4 cookie);
  *
  * IMPORTANT: this is implemented as a single instruction in mterp, rather
  * than a call here.  You can add debugging aids for the C-language
- * interpreters, but the basic implementation may not change.
+ * interpreters, but the basic implementation must not change.
  */
 INLINE void dvmPopIndirectRefTableSegment(IndirectRefTable* pRef, u4 cookie)
 {
@@ -323,7 +336,8 @@ INLINE IndirectRef dvmAppendToIndirectRefTable(IndirectRefTable* pRef,
         /* up against alloc or max limit, call the fancy version */
         return dvmAddToIndirectRefTable(pRef, cookie, obj);
     } else {
-        IndirectRef result = dvmObjectToIndirectRef(obj, topIndex, pRef->kind);
+        IndirectRef result = dvmObjectToIndirectRef(pRef, obj, topIndex,
+            pRef->kind);
         pRef->table[topIndex++] = obj;
         pRef->segmentState.parts.topIndex = topIndex;
         return result;
diff --git a/vm/test/TestIndirectRefTable.c b/vm/test/TestIndirectRefTable.c
index 64d843c27..25f1dd147 100644
--- a/vm/test/TestIndirectRefTable.c
+++ b/vm/test/TestIndirectRefTable.c
@@ -245,12 +245,15 @@ static bool basicTest(void)
     dvmRemoveFromIndirectRefTable(&irt, cookie, iref0);
     iref1 = dvmAddToIndirectRefTable(&irt, cookie, obj0);
     if (iref0 != iref1) {
+        /* try 0, should not work */
         if (dvmRemoveFromIndirectRefTable(&irt, cookie, iref0)) {
             LOGE("temporal del succeeded (%p vs %p)\n", iref0, iref1);
             goto bail;
         }
-    } else {
-        dvmRemoveFromIndirectRefTable(&irt, cookie, iref1);
+    }
+    if (!dvmRemoveFromIndirectRefTable(&irt, cookie, iref1)) {
+        LOGE("temporal cleanup failed\n");
+        goto bail;
     }
     if (dvmIndirectRefTableEntries(&irt) != 0) {
         LOGE("temporal del not empty\n");