fix incorrect reporting of fixie collection status in heap.cpp

This led to fixed-position objects being considered unreachable when
they were actually still reachable, causing global weak JNI references
to be cleared prematurely, most notably leading to crashes in AWT
buffered image code.

This commit also fixes a field offset calculation mismatch in
bootimage.cpp relative to machine.cpp.

src/bootimage.cpp

@@ -34,7 +34,7 @@ const unsigned TargetFixieSizeInBytes = 8 + (TargetBytesPerWord * 2);
 const unsigned TargetFixieSizeInWords = ceiling
   (TargetFixieSizeInBytes, TargetBytesPerWord);
 const unsigned TargetFixieAge = 0;
-const unsigned TargetFixieHasMask = 1;
+const unsigned TargetFixieFlags = 2;
 const unsigned TargetFixieSize = 4;
 
 const bool DebugNativeTarget = false;
@@ -441,11 +441,7 @@ makeCodeImage(Thread* t, Zone* zone, BootImage* image, uint8_t* code,
 
             memberFields[memberIndex] = *f;
 
-            while (targetMemberOffset % f->targetSize) {
+            targetMemberOffset = f->targetOffset + f->targetSize;
-              ++ targetMemberOffset;
-            }
-
-            targetMemberOffset += f->targetSize;
 
             ++ memberIndex;
           }
@@ -540,8 +536,6 @@ makeCodeImage(Thread* t, Zone* zone, BootImage* image, uint8_t* code,
           }
         }
 
-        // if (strcmp(name, "avian/VMClass.class") == 0) trap();
-
         if (hashMapFind(t, typeMaps, c, objectHash, objectEqual) == 0) {
           object array = makeByteArray
             (t, TypeMap::sizeInBytes
@@ -1175,13 +1169,13 @@ makeHeapImage(Thread* t, BootImage* image, target_uintptr_t* heap,
 
           memset(heap + position, 0, TargetFixieSizeInBytes);
 
-          uint8_t age = FixieTenureThreshold + 1;
+          uint16_t age = targetV2(FixieTenureThreshold + 1);
           memcpy(reinterpret_cast<uint8_t*>(heap + position)
-                 + TargetFixieAge, &age, 1);
+                 + TargetFixieAge, &age, 2);
 
-          uint8_t hasMask = true;
+          uint16_t flags = targetV2(1);
           memcpy(reinterpret_cast<uint8_t*>(heap + position)
-                 + TargetFixieHasMask, &hasMask, 1);
+                 + TargetFixieFlags, &flags, 2);
 
           uint32_t targetSize = targetV4(size);
           memcpy(reinterpret_cast<uint8_t*>(heap + position)

src/heap.cpp

@@ -463,12 +463,15 @@ class Segment {
 
 class Fixie {
  public:
+  static const unsigned HasMask = 1 << 0;
+  static const unsigned Marked = 1 << 1;
+  static const unsigned Dirty = 1 << 2;
+  static const unsigned Dead = 1 << 3;
+
   Fixie(Context* c, unsigned size, bool hasMask, Fixie** handle,
         bool immortal):
     age(immortal ? FixieTenureThreshold + 1 : 0),
-    hasMask(hasMask),
+    flags(hasMask ? HasMask : 0),
-    marked(false),
-    dirty(false),
     size(size),
     next(0),
     handle(0)
@@ -536,16 +539,54 @@ class Fixie {
   }
 
   unsigned totalSize() {
-    return totalSize(size, hasMask);
+    return totalSize(size, hasMask());
+  }
+
+  bool hasMask() {
+    return (flags & HasMask) != 0;
+  }
+
+  bool marked() {
+    return (flags & Marked) != 0;
+  }
+
+  void marked(bool v) {
+    if (v) {
+      flags |= Marked;
+    } else {
+      flags &= ~Marked;
+    }
+  }
+
+  bool dirty() {
+    return (flags & Dirty) != 0;
+  }
+
+  void dirty(bool v) {
+    if (v) {
+      flags |= Dirty;
+    } else {
+      flags &= ~Dirty;
+    }
+  }
+
+  bool dead() {
+    return (flags & Dead) != 0;
+  }
+
+  void dead(bool v) {
+    if (v) {
+      flags |= Dead;
+    } else {
+      flags &= ~Dead;
+    }
   }
 
   // be sure to update e.g. TargetFixieSizeInBytes in bootimage.cpp if
   // you add/remove/change fields in this class:
 
-  uint8_t age;
+  uint16_t age;
-  uint8_t hasMask;
+  uint16_t flags;
-  uint8_t marked;
-  uint8_t dirty;
   uint32_t size;
   Fixie* next;
   Fixie** handle;
@@ -850,11 +891,11 @@ free(Context* c, Fixie** fixies, bool resetImmortal)
           fprintf(stderr, "reset immortal fixie %p\n", f);
         }
         *p = f->next;
-        memset(f->mask(), 0, Fixie::maskSize(f->size, f->hasMask));
+        memset(f->mask(), 0, Fixie::maskSize(f->size, f->hasMask()));
         f->next = 0;
         f->handle = 0;
-        f->marked = false;
+        f->marked(false);
-        f->dirty = false;
+        f->dirty(false);
       } else {      
         p = &(f->next);
       }
@@ -868,6 +909,28 @@ free(Context* c, Fixie** fixies, bool resetImmortal)
   }
 }
 
+void
+kill(Fixie* fixies)
+{
+  for (Fixie* f = fixies; f; f = f->next) {
+    if (! f->immortal()) {
+      f->dead(true);
+    }
+  }
+}
+
+void
+killFixies(Context* c)
+{
+  assert(c, c->markedFixies == 0);
+
+  if (c->mode == Heap::MajorCollection) {
+    kill(c->tenuredFixies);
+    kill(c->dirtyTenuredFixies);
+  }
+  kill(c->fixies);
+}
+
 void
 sweepFixies(Context* c)
 {
@@ -898,14 +961,14 @@ sweepFixies(Context* c)
 
     if (f->age >= FixieTenureThreshold) {
       if (DebugFixies) {
-        fprintf(stderr, "tenure fixie %p (dirty: %d)\n", f, f->dirty);
+        fprintf(stderr, "tenure fixie %p (dirty: %d)\n", f, f->dirty());
       }
 
       if (not f->immortal()) {
         c->tenuredFixieFootprint += f->totalSize();
       }
 
-      if (f->dirty) {
+      if (f->dirty()) {
         f->add(c, &(c->dirtyTenuredFixies));
       } else {
         f->add(c, &(c->tenuredFixies));
@@ -916,7 +979,7 @@ sweepFixies(Context* c)
       f->add(c, &(c->fixies));
     }
 
-    f->marked = false;
+    f->marked(false);
   }
 
   c->tenuredFixieCeiling = max
@@ -1006,14 +1069,14 @@ update3(Context* c, void* o, bool* needsVisit)
 {
   if (c->client->isFixed(o)) {
     Fixie* f = fixie(o);
-    if ((not f->marked)
+    if ((not f->marked())
         and (c->mode == Heap::MajorCollection
              or f->age < FixieTenureThreshold))
     {
       if (DebugFixies) {
         fprintf(stderr, "mark fixie %p\n", f);
       }
-      f->marked = true;
+      f->marked(true);
       f->move(c, &(c->markedFixies));
     }
     *needsVisit = false;
@@ -1044,13 +1107,13 @@ update2(Context* c, void* o, bool* needsVisit)
 void
 markDirty(Context* c, Fixie* f)
 {
-  if (not f->dirty) {
+  if (not f->dirty()) {
 #ifdef USE_ATOMIC_OPERATIONS
     ACQUIRE(c->lock);
 #endif
 
-    if (not f->dirty) {
+    if (not f->dirty()) {
-      f->dirty = true;
+      f->dirty(true);
       f->move(c, &(c->dirtyTenuredFixies));
     }
   }
@@ -1059,8 +1122,8 @@ markDirty(Context* c, Fixie* f)
 void
 markClean(Context* c, Fixie* f)
 {
-  if (f->dirty) {
+  if (f->dirty()) {
-    f->dirty = false;
+    f->dirty(false);
     if (f->immortal()) {
       f->remove(c);
     } else {
@@ -1090,7 +1153,7 @@ updateHeapMap(Context* c, void* p, void* target, unsigned offset, void* result)
   {
     if (target and c->client->isFixed(target)) {
       Fixie* f = fixie(target);
-      assert(c, offset == 0 or f->hasMask);
+      assert(c, offset == 0 or f->hasMask());
 
       if (static_cast<unsigned>(f->age + 1) >= FixieTenureThreshold) {
         if (DebugFixies) {
@@ -1098,7 +1161,7 @@ updateHeapMap(Context* c, void* p, void* target, unsigned offset, void* result)
                   f, offset, f->body() + offset, result);
         }
 
-        f->dirty = true;
+        f->dirty(true);
         markBit(f->mask(), offset);
       }
     } else if (seg->contains(p)) {
@@ -1871,7 +1934,7 @@ class MyHeap: public Heap {
 
       if (c.client->isFixed(p)) {
         Fixie* f = fixie(p);
-        assert(&c, offset == 0 or f->hasMask);
+        assert(&c, offset == 0 or f->hasMask());
 
         bool dirty = false;
         for (unsigned i = 0; i < count; ++i) {
@@ -1946,11 +2009,22 @@ class MyHeap: public Heap {
     }
   }
 
+  virtual void postVisit() {
+    killFixies(&c);
+  }
+
   virtual Status status(void* p) {
     p = mask(p);
 
     if (p == 0) {
       return Null;
+    } else if (c.client->isFixed(p)) {
+      Fixie* f = fixie(p);
+      return f->dead()
+        ? Unreachable
+        : (static_cast<unsigned>(f->age + 1) < FixieTenureThreshold
+           ? Reachable
+           : Tenured);
     } else if (c.nextGen1.contains(p)) {
       return Reachable;
     } else if (c.nextGen2.contains(p)

src/heap.h

@@ -69,6 +69,7 @@ class Heap: public Allocator {
   virtual void mark(void* p, unsigned offset, unsigned count) = 0;
   virtual void pad(void* p) = 0;
   virtual void* follow(void* p) = 0;
+  virtual void postVisit() = 0;
   virtual Status status(void* p) = 0;
   virtual CollectionType collectionType() = 0;
   virtual void disposeFixies() = 0;

src/machine.cpp

@@ -2690,6 +2690,8 @@ class HeapClient: public Heap::Client {
   virtual void visitRoots(Heap::Visitor* v) {
     ::visitRoots(m, v);
 
+    m->heap->postVisit();
+
     postVisit(m->rootThread, v);
   }