From 38c8baa0ece45abc2a2a53fe3d53dda84d28dd4c Mon Sep 17 00:00:00 2001 From: Mathieu Chartier Date: Thu, 8 Aug 2013 11:43:30 -0700 Subject: [PATCH] Fix broken card table asserts. We had an off by one error due to getting the card for the heap source limit. This is not a valid card when the heap is at maximum size, but doesn't need to be since we do less than comparison on it. Fixed another error where we didn't take into account the biased begin in an assert. Did a bit of refactoring by removing useless if statement. Fixes 061-out-of-memory, 080-oom-throw. Bug: https://code.google.com/p/android/issues/detail?id=42868 (cherry picked from commit ee903e174872edd0ecc6f1940c7412892cd49123) Change-Id: If677c93e46fe1ee5729eec1820b1bd9682b4f924 --- vm/alloc/CardTable.cpp | 19 ++++++++----------- vm/alloc/MarkSweep.cpp | 5 +++-- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/vm/alloc/CardTable.cpp b/vm/alloc/CardTable.cpp index 2c81fd149..28fe58e00 100644 --- a/vm/alloc/CardTable.cpp +++ b/vm/alloc/CardTable.cpp @@ -54,9 +54,12 @@ bool dvmCardTableStartup(size_t heapMaximumSize, size_t growthLimit) void *allocBase; u1 *biasedBase; GcHeap *gcHeap = gDvm.gcHeap; + int offset; void *heapBase = dvmHeapSourceGetBase(); assert(gcHeap != NULL); assert(heapBase != NULL); + /* All zeros is the correct initial value; all clean. */ + assert(GC_CARD_CLEAN == 0); /* Set up the card table */ length = heapMaximumSize / GC_CARD_SIZE; @@ -69,17 +72,11 @@ bool dvmCardTableStartup(size_t heapMaximumSize, size_t growthLimit) gcHeap->cardTableBase = (u1*)allocBase; gcHeap->cardTableLength = growthLimit / GC_CARD_SIZE; gcHeap->cardTableMaxLength = length; - gcHeap->cardTableOffset = 0; - /* All zeros is the correct initial value; all clean. */ - assert(GC_CARD_CLEAN == 0); - biasedBase = (u1 *)((uintptr_t)allocBase - - ((uintptr_t)heapBase >> GC_CARD_SHIFT)); - if (((uintptr_t)biasedBase & 0xff) != GC_CARD_DIRTY) { - int offset = GC_CARD_DIRTY - ((uintptr_t)biasedBase & 0xff); - gcHeap->cardTableOffset = offset + (offset < 0 ? 0x100 : 0); - biasedBase += gcHeap->cardTableOffset; - } + ((uintptr_t)heapBase >> GC_CARD_SHIFT)); + offset = GC_CARD_DIRTY - ((uintptr_t)biasedBase & 0xff); + gcHeap->cardTableOffset = offset + (offset < 0 ? 0x100 : 0); + biasedBase += gcHeap->cardTableOffset; assert(((uintptr_t)biasedBase & 0xff) == GC_CARD_DIRTY); gDvm.biasedCardTableBase = biasedBase; @@ -165,7 +162,7 @@ bool dvmIsValidCard(const u1 *cardAddr) } /* - * Returns the address of the relevent byte in the card table, given + * Returns the address of the relevant byte in the card table, given * an address on the heap. */ u1 *dvmCardFromAddr(const void *addr) diff --git a/vm/alloc/MarkSweep.cpp b/vm/alloc/MarkSweep.cpp index eb739e552..2781a7cfa 100644 --- a/vm/alloc/MarkSweep.cpp +++ b/vm/alloc/MarkSweep.cpp @@ -558,8 +558,9 @@ static void scanGrayObjects(GcMarkContext *ctx) const u1 *base, *limit, *ptr, *dirty; base = &h->cardTableBase[0]; - limit = dvmCardFromAddr((u1 *)dvmHeapSourceGetLimit()); - assert(limit <= &h->cardTableBase[h->cardTableLength]); + // The limit is the card one after the last accessible card. + limit = dvmCardFromAddr((u1 *)dvmHeapSourceGetLimit() - GC_CARD_SIZE) + 1; + assert(limit <= &base[h->cardTableOffset + h->cardTableLength]); ptr = base; for (;;) { -- 2.11.0