Fix confusion over on-disk object id sizes during instance serialization.

runtime/vm/heap_profiler.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/heap_profiler.h"
 
 #include "vm/dart_api_state.h"
 #include "vm/object.h"
 #include "vm/raw_object.h"
 #include "vm/stack_frame.h"
@@ skipping 50 matching lines @@
   uint16_t x = 0xFF;
   if (*reinterpret_cast<uint8_t*>(&x) == 0xFF) {
     uint64_t hi = static_cast<uint64_t>(htonl(value & 0xFFFFFFFF)) << 32;
     uint64_t lo = htonl(value >> 32);
     value = hi | lo;
   }
   body_.Write(reinterpret_cast<uint8_t*>(&value), sizeof(value));
 }
 
 
-void HeapProfiler::Record::WritePointer(const void* value) {
+void HeapProfiler::Record::WriteObjectId(const void* value) {
   Write64(reinterpret_cast<uint64_t>(value));
 }
 
 
 HeapProfiler::SubRecord::SubRecord(uint8_t sub_tag, HeapProfiler* profiler)
     : record_(profiler->heap_dump_record_) {
   record_->Write8(sub_tag);
 }
 
 
@@ skipping 19 matching lines @@
 void HeapProfiler::SubRecord::Write32(uint32_t value) {
   record_->Write32(value);
 }
 
 
 void HeapProfiler::SubRecord::Write64(uint64_t value) {
   record_->Write64(value);
 }
 
 
-void HeapProfiler::SubRecord::WritePointer(const void* value) {
-  record_->WritePointer(value);
+void HeapProfiler::SubRecord::WriteObjectId(const void* value) {
+  record_->WriteObjectId(value);
 }
 
 
 HeapProfiler::HeapProfiler(Dart_FileWriteCallback callback, void* stream)
     : write_callback_(callback),
       output_stream_(stream),
       heap_dump_record_(NULL) {
   WriteHeader();
   WriteStackTrace();
   heap_dump_record_ = new Record(kHeapDump, this);
@@ skipping 66 matching lines @@
   const RawType* super_type = raw_class->ptr()->super_type_;
   if (super_type == Type::null()) {
     return Class::null();
   }
   return reinterpret_cast<const RawClass*>(super_type->ptr()->type_class_);
 }
 
 
 void HeapProfiler::WriteRoot(const RawObject* raw_obj) {
   SubRecord sub(kRootUnknown, this);
-  sub.WritePointer(ObjectId(raw_obj));
+  sub.WriteObjectId(ObjectId(raw_obj));
 }
 
 
 void HeapProfiler::WriteObject(const RawObject* raw_obj) {
   ASSERT(raw_obj->IsHeapObject());
   intptr_t class_id = raw_obj->GetClassId();
   switch (class_id) {
     case kFreeListElement: {
       // Free space has an object-like encoding.  Heap profiles only
       // care about live objects so we skip over these records.
@@ skipping 151 matching lines @@
     for (intptr_t i = 0; i < Smi::Value(twostr->ptr()->length_); ++i) {
       length += Utf8::Length(twostr->ptr()->data_[i]);
     }
     characters = new char[length];
     for (intptr_t i = 0, j = 0; i < Smi::Value(twostr->ptr()->length_); ++i) {
       int32_t ch = twostr->ptr()->data_[i];
       j += Utf8::Encode(ch, &characters[j]);
     }
   }
   Record record(kStringInUtf8, this);
-  record.WritePointer(ObjectId(raw_string));
+  record.WriteObjectId(ObjectId(raw_string));
   for (intptr_t i = 0; i < length; ++i) {
     record.Write8(characters[i]);
   }
   delete[] characters;
 }
 
 
 void HeapProfiler::WriteStringInUtf8(const char* c_string) {
   Record record(kStringInUtf8, this);
-  record.WritePointer(c_string);
+  record.WriteObjectId(c_string);
   for (; *c_string != '\0'; ++c_string) {
     record.Write8(*c_string);
   }
 }
 
 
 // LOAD CLASS - 0x02
 //
 // Format:
 //   u4 - class serial number (always > 0)
 //   ID - class object ID
 //   u4 - stack trace serial number
 //   ID - class name string ID
 void HeapProfiler::WriteLoadClass(const RawClass* raw_class) {
   Record record(kLoadClass, this);
   // class serial number (always > 0)
   record.Write32(1);
   // class object ID
-  record.WritePointer(raw_class);
+  record.WriteObjectId(raw_class);
   // stack trace serial number
   record.Write32(0);
   // class name string ID
   if (raw_class->ptr()->name_ != String::null()) {
-    record.WritePointer(StringId(raw_class->ptr()->name_));
+    record.WriteObjectId(StringId(raw_class->ptr()->name_));
   } else {
     const char* format = "<an unnamed class with id %d>";
     intptr_t len = OS::SNPrint(NULL, 0, format, raw_class->ptr()->id_);
     char* str = new char[len + 1];
     OS::SNPrint(str, len + 1, format, raw_class->ptr()->id_);
-    record.WritePointer(StringId(str));
+    record.WriteObjectId(StringId(str));
     delete[] str;
   }
 }
 
 
 // STACK TRACE - 0x05
 //
 //  u4 - stack trace serial number
 //  u4 - thread serial number
 //  u4 - number of frames
@@ skipping 56 matching lines @@
 //  u2 - Number of static fields:
 //  ID - static field name string ID
 //  u1 - type of field: (See Basic Type)
 //  value - value of entry (u1, u2, u4, or u8 based on type of field)
 //  u2 - Number of instance fields (not including super class's)
 //  ID - field name string ID
 //  u1 - type of field: (See Basic Type)
 void HeapProfiler::WriteClassDump(const RawClass* raw_class) {
   SubRecord sub(kClassDump, this);
   // class object ID
-  sub.WritePointer(ClassId(raw_class));
+  sub.WriteObjectId(ClassId(raw_class));
   // stack trace serial number
   sub.Write32(0);
   // super class object ID
   const RawClass* super_class = GetSuperClass(raw_class);
   if (super_class == Class::null()) {
-    sub.WritePointer(NULL);
+    sub.WriteObjectId(NULL);
   } else {
-    sub.WritePointer(ClassId(super_class));
+    sub.WriteObjectId(ClassId(super_class));
   }
   // class loader object ID
-  sub.WritePointer(NULL);
+  sub.WriteObjectId(NULL);
   // signers object ID
-  sub.WritePointer(NULL);
+  sub.WriteObjectId(NULL);
   // protection domain object ID
-  sub.WritePointer(NULL);
+  sub.WriteObjectId(NULL);
   // reserved
-  sub.WritePointer(NULL);
+  sub.WriteObjectId(NULL);
   // reserved
-  sub.WritePointer(NULL);
+  sub.WriteObjectId(NULL);
 
   intptr_t num_static_fields = 0;
   intptr_t num_instance_fields = 0;
 
   RawArray* raw_array = raw_class->ptr()->fields_;
   if (raw_array != Array::null()) {
     for (intptr_t i = 0; i < Smi::Value(raw_array->ptr()->length_); ++i) {
       RawField* raw_field =
           reinterpret_cast<RawField*>(raw_array->ptr()->data()[i]);
       if (Field::StaticBit::decode(raw_field->ptr()->kind_bits_)) {
@@ skipping 11 matching lines @@
   // Number of static fields
   sub.Write16(num_static_fields);
   // Static fields:
   if (raw_array != Array::null()) {
     for (intptr_t i = 0; i < Smi::Value(raw_array->ptr()->length_); ++i) {
       RawField* raw_field =
           reinterpret_cast<RawField*>(raw_array->ptr()->data()[i]);
       if (Field::StaticBit::decode(raw_field->ptr()->kind_bits_)) {
         ASSERT(raw_field->ptr()->name_ != String::null());
         // static field name string ID
-        sub.WritePointer(StringId(raw_field->ptr()->name_));
+        sub.WriteObjectId(StringId(raw_field->ptr()->name_));
         // type of static field
         sub.Write8(kObject);
         // value of entry
-        sub.WritePointer(ObjectId(raw_field->ptr()->value_));
+        sub.WriteObjectId(ObjectId(raw_field->ptr()->value_));
       }
     }
   }
   // Number of instance fields (not include super class's)
   sub.Write16(num_instance_fields);
   // Instance fields:
   if (raw_array != Array::null()) {
     for (intptr_t i = 0; i < Smi::Value(raw_array->ptr()->length_); ++i) {
       RawField* raw_field =
           reinterpret_cast<RawField*>(raw_array->ptr()->data()[i]);
       if (!Field::StaticBit::decode(raw_field->ptr()->kind_bits_)) {
         ASSERT(raw_field->ptr()->name_ != String::null());
         // field name string ID
-        sub.WritePointer(StringId(raw_field->ptr()->name_));
+        sub.WriteObjectId(StringId(raw_field->ptr()->name_));
         // type of field
         sub.Write8(kObject);
       }
     }
   }
 }
 
 
 // INSTANCE DUMP - 0x21
 //
 // Format:
 //  ID - object ID
 //  u4 - stack trace serial number
 //  ID - class object ID
 //  u4 - number of bytes that follow
 //  [value]* - instance field values (this class, followed by super class, etc)
 void HeapProfiler::WriteInstanceDump(const RawObject* raw_obj) {
   SubRecord sub(kInstanceDump, this);
   // object ID
-  sub.WritePointer(raw_obj);
+  sub.WriteObjectId(raw_obj);
   // stack trace serial number
   sub.Write32(0);
   // class object ID
-  sub.WritePointer(ClassId(GetClass(raw_obj)));
+  sub.WriteObjectId(ClassId(GetClass(raw_obj)));
   // number of bytes that follow
   intptr_t num_instance_fields = 0;
   for (const RawClass* cls = GetClass(raw_obj);
        cls != Class::null();
        cls = GetSuperClass(cls)) {
     RawArray* raw_array = cls->ptr()->fields_;
     if (raw_array != Array::null()) {
       intptr_t length = Smi::Value(raw_array->ptr()->length_);
       for (intptr_t i = 0; i < length; ++i) {
         RawField* raw_field =
             reinterpret_cast<RawField*>(raw_array->ptr()->data()[i]);
         if (!Field::StaticBit::decode(raw_field->ptr()->kind_bits_)) {
           ++num_instance_fields;
         }
       }
     }
   }
-  sub.Write32(num_instance_fields * kWordSize);
+  sub.Write32(num_instance_fields * kObjectIdSize);

[siva, 2013/01/08 02:18:01]

I am assuming that Write32 will truncate the value beyond 32 bits do we should
probably assert here that
ASSERT((num_instance_fields * kObjectSize) < kMaxInt32);

[cshapiro, 2013/01/08 02:53:04]

That will not quite work :-)

Write32 takes a uint32_t argument so kMaxInt32 needs to be a kMaxUint32 and the
check needs to be <= not < as above.  It is, of course, legal for the value to
range up to and including the limit.

Also, it is always true that an uint32_t is going to be less than 0xFFFFFFFF so
to make this check work the multiply needs to be done in higher precision.

Anyway, I fixed the code to do that.  Presumably all we care about is having the
product be in range so I did not bother with the possibility of overflow above
where the fields are counted.

   // instance field values (this class, followed by super class, etc)
   for (const RawClass* cls = GetClass(raw_obj);
        cls != Class::null();
        cls = GetSuperClass(cls)) {
     RawArray* raw_array = cls->ptr()->fields_;
     if (raw_array != Array::null()) {
       intptr_t length = Smi::Value(raw_array->ptr()->length_);
       uint8_t* base = reinterpret_cast<uint8_t*>(raw_obj->ptr());
       for (intptr_t i = 0; i < length; ++i) {
         RawField* raw_field =
             reinterpret_cast<RawField*>(raw_array->ptr()->data()[i]);
         if (!Field::StaticBit::decode(raw_field->ptr()->kind_bits_)) {
           intptr_t offset_in_words =
               Smi::Value(reinterpret_cast<RawSmi*>(raw_field->ptr()->value_));
           intptr_t offset = offset_in_words * kWordSize;
           RawObject* ptr = *reinterpret_cast<RawObject**>(base + offset);
-          sub.WritePointer(ObjectId(ptr));
+          sub.WriteObjectId(ObjectId(ptr));
         }
       }
     }
   }
 }
 
 
 // OBJECT ARRAY DUMP - 0x22
 //
 // Format:
 //  ID - array object ID
 //  u4 - stack trace serial number
 //  u4 - number of elements
 //  ID - array class object ID
 //  [ID]* - elements
 void HeapProfiler::WriteObjectArrayDump(const RawArray* raw_array) {
   SubRecord sub(kObjectArrayDump, this);
   // array object ID
-  sub.WritePointer(raw_array);
+  sub.WriteObjectId(raw_array);
   // stack trace serial number
   sub.Write32(0);
   // number of elements
   intptr_t length = Smi::Value(raw_array->ptr()->length_);
   sub.Write32(length);
   // array class object ID
-  sub.WritePointer(NULL);
+  sub.WriteObjectId(NULL);
   // elements
   for (intptr_t i = 0; i < length; ++i) {
-    sub.WritePointer(ObjectId(raw_array->ptr()->data()[i]));
+    sub.WriteObjectId(ObjectId(raw_array->ptr()->data()[i]));
   }
 }
 
 
 // PRIMITIVE ARRAY DUMP - 0x23
 //
 // Format:
 //  ID - array object ID
 //  u4 - stack trace serial number
 //  u4 - number of elements
 //  u1 - element type
 //  [u1]* - elements
 void HeapProfiler::WritePrimitiveArrayDump(const RawByteArray* raw_byte_array,
                                            uint8_t tag,
                                            const void* data) {
   SubRecord sub(kPrimitiveArrayDump, this);
   // array object ID
-  sub.WritePointer(raw_byte_array);
+  sub.WriteObjectId(raw_byte_array);
   // stack trace serial number
   sub.Write32(0);
   // number of elements
   intptr_t length = Smi::Value(raw_byte_array->ptr()->length_);
   sub.Write32(length);
   // element type
   sub.Write8(tag);
   // elements (packed)
   for (intptr_t i = 0; i < length; ++i) {
     if (tag == kByte) {
@@ skipping 39 matching lines @@
   RawObject* raw_obj = handle->raw();
   visitor_->VisitPointer(&raw_obj);
 }
 
 
 void HeapProfilerObjectVisitor::VisitObject(RawObject* raw_obj) {
   profiler_->WriteObject(raw_obj);
 }
 
 }  // namespace dart