Merge "Dump heap reference adjacent memory for logging corruption" into oc-dr1-dev

3 files changed, 46 insertions, 24 deletions

diff --git a/runtime/gc/heap_verification_test.cc b/runtime/gc/heap_verification_test.cc
index a307c519cb..30714ba09e 100644
--- a/runtime/gc/heap_verification_test.cc
+++ b/runtime/gc/heap_verification_test.cc
@@ -54,6 +54,11 @@ TEST_F(VerificationTest, IsValidHeapObjectAddress) {
   Handle<mirror::String> string(
       hs.NewHandle(mirror::String::AllocFromModifiedUtf8(soa.Self(), "test")));
   EXPECT_TRUE(v->IsValidHeapObjectAddress(string.Get()));
+  // Address in the heap that isn't aligned.
+  const void* unaligned_address =
+      reinterpret_cast<const void*>(reinterpret_cast<uintptr_t>(string.Get()) + 1);
+  EXPECT_TRUE(v->IsAddressInHeapSpace(unaligned_address));
+  EXPECT_FALSE(v->IsValidHeapObjectAddress(unaligned_address));
   EXPECT_TRUE(v->IsValidHeapObjectAddress(string->GetClass()));
   const uintptr_t uint_klass = reinterpret_cast<uintptr_t>(string->GetClass());
   // Not actually a valid object but the verification can't know that. Guaranteed to be inside a
diff --git a/runtime/gc/verification.cc b/runtime/gc/verification.cc
index c14f250528..b007d1decd 100644
--- a/runtime/gc/verification.cc
+++ b/runtime/gc/verification.cc
@@ -25,6 +25,28 @@
 namespace art {
 namespace gc {
 
+std::string Verification::DumpRAMAroundAddress(uintptr_t addr, uintptr_t bytes) const {
+  const uintptr_t dump_start = addr - bytes;
+  const uintptr_t dump_end = addr + bytes;
+  std::ostringstream oss;
+  if (dump_start < dump_end &&
+      IsAddressInHeapSpace(reinterpret_cast<const void*>(dump_start)) &&
+      IsAddressInHeapSpace(reinterpret_cast<const void*>(dump_end - 1))) {
+    oss << " adjacent_ram=";
+    for (uintptr_t p = dump_start; p < dump_end; ++p) {
+      if (p == addr) {
+        // Marker of where the address is.
+        oss << "|";
+      }
+      uint8_t* ptr = reinterpret_cast<uint8_t*>(p);
+      oss << std::hex << std::setfill('0') << std::setw(2) << static_cast<uintptr_t>(*ptr);
+    }
+  } else {
+    oss << " <invalid address>";
+  }
+  return oss.str();
+}
+
 std::string Verification::DumpObjectInfo(const void* addr, const char* tag) const {
   std::ostringstream oss;
   oss << tag << "=" << addr;
@@ -50,23 +72,7 @@ std::string Verification::DumpObjectInfo(const void* addr, const char* tag) cons
           card_table->GetCard(reinterpret_cast<const mirror::Object*>(addr)));
     }
     // Dump adjacent RAM.
-    const uintptr_t uint_addr = reinterpret_cast<uintptr_t>(addr);
-    static constexpr size_t kBytesBeforeAfter = 2 * kObjectAlignment;
-    const uintptr_t dump_start = uint_addr - kBytesBeforeAfter;
-    const uintptr_t dump_end = uint_addr + kBytesBeforeAfter;
-    if (dump_start < dump_end &&
-        IsValidHeapObjectAddress(reinterpret_cast<const void*>(dump_start)) &&
-        IsValidHeapObjectAddress(reinterpret_cast<const void*>(dump_end - kObjectAlignment))) {
-      oss << " adjacent_ram=";
-      for (uintptr_t p = dump_start; p < dump_end; ++p) {
-        if (p == uint_addr) {
-          // Marker of where the object is.
-          oss << "|";
-        }
-        uint8_t* ptr = reinterpret_cast<uint8_t*>(p);
-        oss << std::hex << std::setfill('0') << std::setw(2) << static_cast<uintptr_t>(*ptr);
-      }
-    }
+    oss << DumpRAMAroundAddress(reinterpret_cast<uintptr_t>(addr), 4 * kObjectAlignment);
   } else {
     oss << " <invalid address>";
   }
@@ -90,12 +96,15 @@ void Verification::LogHeapCorruption(ObjPtr<mirror::Object> holder,
   if (holder != nullptr) {
     mirror::Class* holder_klass = holder->GetClass<kVerifyNone, kWithoutReadBarrier>();
     if (IsValidClass(holder_klass)) {
-      oss << "field_offset=" << offset.Uint32Value();
+      oss << " field_offset=" << offset.Uint32Value();
       ArtField* field = holder->FindFieldByOffset(offset);
       if (field != nullptr) {
         oss << " name=" << field->GetName();
       }
     }
+    mirror::HeapReference<mirror::Object>* addr = holder->GetFieldObjectReferenceAddr(offset);
+    oss << " reference addr"
+        << DumpRAMAroundAddress(reinterpret_cast<uintptr_t>(addr), 4 * kObjectAlignment);
   }
 
   if (fatal) {
@@ -105,10 +114,7 @@ void Verification::LogHeapCorruption(ObjPtr<mirror::Object> holder,
   }
 }
 
-bool Verification::IsValidHeapObjectAddress(const void* addr, space::Space** out_space) const {
-  if (!IsAligned<kObjectAlignment>(addr)) {
-    return false;
-  }
+bool Verification::IsAddressInHeapSpace(const void* addr, space::Space** out_space) const {
   space::Space* const space = heap_->FindSpaceFromAddress(addr);
   if (space != nullptr) {
     if (out_space != nullptr) {
@@ -119,6 +125,10 @@ bool Verification::IsValidHeapObjectAddress(const void* addr, space::Space** out
   return false;
 }
 
+bool Verification::IsValidHeapObjectAddress(const void* addr, space::Space** out_space) const {
+  return IsAligned<kObjectAlignment>(addr) && IsAddressInHeapSpace(addr, out_space);
+}
+
 bool Verification::IsValidClass(const void* addr) const {
   if (!IsValidHeapObjectAddress(addr)) {
     return false;
diff --git a/runtime/gc/verification.h b/runtime/gc/verification.h
index 3d95d93015..3b5eaf5f46 100644
--- a/runtime/gc/verification.h
+++ b/runtime/gc/verification.h
@@ -49,14 +49,21 @@ class Verification {
                          mirror::Object* ref,
                          bool fatal) const REQUIRES_SHARED(Locks::mutator_lock_);
 
-
   // Return true if the klass is likely to be a valid mirror::Class.
   bool IsValidClass(const void* klass) const REQUIRES_SHARED(Locks::mutator_lock_);
 
-  // Does not allow null.
+  // Does not allow null, checks alignment.
   bool IsValidHeapObjectAddress(const void* addr, space::Space** out_space = nullptr) const
       REQUIRES_SHARED(Locks::mutator_lock_);
 
+  // Does not check alignment, used by DumpRAMAroundAddress.
+  bool IsAddressInHeapSpace(const void* addr, space::Space** out_space = nullptr) const
+      REQUIRES_SHARED(Locks::mutator_lock_);
+
+  // Dump bytes of RAM before and after an address.
+  std::string DumpRAMAroundAddress(uintptr_t addr, uintptr_t bytes) const
+      REQUIRES_SHARED(Locks::mutator_lock_);
+
  private:
   gc::Heap* const heap_;
 };