NewGC: Don't put tagged null pointers in garbage objects when updating pointers

src/mark-compact.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 2352 matching lines @@
 
 
 static void UpdatePointer(HeapObject** p, HeapObject* object) {
   ASSERT(*p == object);
 
   Address old_addr = object->address();
 
   Address new_addr = Memory::Address_at(old_addr);
 
   // The new space sweep will overwrite the map word of dead objects
   // with NULL. In this case we do not need to transfer this entry to

[Lasse Reichstein, 2011/08/12 08:47:13]

Does this mean we store NULL in the heap? We should probably call it something
else (Smi::ValueOf(0)) just for precission.

[Erik Corry, 2011/08/12 08:56:40]

It doesn't really have anything to do with a Smi. The map field of the dead
object has to contain something that is heap object tagged, because otherwise it
looks like a forwarding pointer (can't have Smis in the map field).

[Lasse Reichstein, 2011/08/12 09:21:48]

So the comment is incorrect and we don't store NULL, but "HeapObject tagged
NULL"?

   // the store buffer which we are rebuilding.
   if (new_addr != NULL) {
     *p = HeapObject::FromAddress(new_addr);
   } else {
     // We have to zap this pointer, because the store buffer may overflow later,
     // and then we have to scan the entire heap and we don't want to find
     // spurious newspace pointers in the old space.
-    *p = HeapObject::FromAddress(NULL);  // Fake heap object not in new space.
+    *p = reinterpret_cast<HeapObject*>(Smi::FromInt(0));

[Lasse Reichstein, 2011/08/12 08:47:13]

So we are not assuming that the value is HeapObject tagged anywhere? (Quite
possible, since we can't use it as such anyway).
It seems to end up in NewSpace::Contains which just uses address arithmetic, so
it should be safe (but that probably should take a HeapObject and filter out
smis earler). 
Maybe make StoreBuffer::Uniq and similar functions filter out the smis.

[Erik Corry, 2011/08/12 08:56:40]

This is the value in the slot that used to point to new space.  The slot is now
part of a dead object pointing to a dead object so we have to put something
there that neither the scan-on-scavenge page traversal nor the evacuation slot
buffer will choke on.  A Smi zero should be fine.  The signature of this
function gets a little in the way, but changing that is likely to cause more
pain.

[Lasse Reichstein, 2011/08/12 09:21:48]

Ah, so *p is always in the heap, and not in a store-buffer?

   }
 }
 
 
 static String* UpdateReferenceInExternalStringTableEntry(Heap* heap,
                                                          Object** p) {
   MapWord map_word = HeapObject::cast(*p)->map_word();
 
   if (map_word.IsForwardingAddress()) {
     return String::cast(map_word.ToForwardingAddress());
@@ skipping 1035 matching lines @@
   while (buffer != NULL) {
     SlotsBuffer* next_buffer = buffer->next();
     DeallocateBuffer(buffer);
     buffer = next_buffer;
   }
   *buffer_address = NULL;
 }
 
 
 } }  // namespace v8::internal