Fix valgrind tests

Delete large objects in space destructor. Also some cleanup.

Change-Id: I4c4e90149841a156b7a3236201b37683e14890fb

diff --git a/runtime/gc/collector/mark_sweep.cc b/runtime/gc/collector/mark_sweep.cc
index ed2e295..bb8d876 100644 (file)
--- a/runtime/gc/collector/mark_sweep.cc
+++ b/runtime/gc/collector/mark_sweep.cc
@@ -490,29 +490,21 @@ void MarkSweep::VisitRoots(mirror::CompressedReference<mirror::Object>** roots,
 
 class VerifyRootVisitor : public SingleRootVisitor {
  public:
-  explicit VerifyRootVisitor(MarkSweep* collector) : collector_(collector) { }
-
   void VisitRoot(mirror::Object* root, const RootInfo& info) OVERRIDE
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_, Locks::heap_bitmap_lock_) {
-    collector_->VerifyRoot(root, info);
-  }
-
- private:
-  MarkSweep* const collector_;
-};
-
-void MarkSweep::VerifyRoot(const Object* root, const RootInfo& root_info) {
-  // See if the root is on any space bitmap.
-  if (heap_->GetLiveBitmap()->GetContinuousSpaceBitmap(root) == nullptr) {
-    space::LargeObjectSpace* large_object_space = GetHeap()->GetLargeObjectsSpace();
-    if (large_object_space != nullptr && !large_object_space->Contains(root)) {
-      LOG(ERROR) << "Found invalid root: " << root << " " << root_info;
+    // See if the root is on any space bitmap.
+    auto* heap = Runtime::Current()->GetHeap();
+    if (heap->GetLiveBitmap()->GetContinuousSpaceBitmap(root) == nullptr) {
+      space::LargeObjectSpace* large_object_space = heap->GetLargeObjectsSpace();
+      if (large_object_space != nullptr && !large_object_space->Contains(root)) {
+        LOG(ERROR) << "Found invalid root: " << root << " " << info;
+      }
     }
   }
-}
+};
 
 void MarkSweep::VerifyRoots() {
-  VerifyRootVisitor visitor(this);
+  VerifyRootVisitor visitor;
   Runtime::Current()->GetThreadList()->VisitRoots(&visitor);
 }
 

diff --git a/runtime/gc/collector/mark_sweep.h b/runtime/gc/collector/mark_sweep.h
index 31cea17..fad3403 100644 (file)
--- a/runtime/gc/collector/mark_sweep.h
+++ b/runtime/gc/collector/mark_sweep.h
@@ -248,9 +248,6 @@ class MarkSweep : public GarbageCollector {
   // whether or not we care about pauses.
   size_t GetThreadCount(bool paused) const;
 
-  void VerifyRoot(const mirror::Object* root, const RootInfo& root_info)
-      SHARED_LOCKS_REQUIRED(Locks::mutator_lock_, Locks::heap_bitmap_lock_);
-
   // Push a single reference on a mark stack.
   void PushOnMarkStack(mirror::Object* obj) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 

diff --git a/runtime/gc/space/large_object_space.cc b/runtime/gc/space/large_object_space.cc
index 5c8e4b9..a4a9d80 100644 (file)
--- a/runtime/gc/space/large_object_space.cc
+++ b/runtime/gc/space/large_object_space.cc
@@ -37,6 +37,15 @@ class ValgrindLargeObjectMapSpace FINAL : public LargeObjectMapSpace {
   explicit ValgrindLargeObjectMapSpace(const std::string& name) : LargeObjectMapSpace(name) {
   }
 
+  ~ValgrindLargeObjectMapSpace() OVERRIDE {
+    // Keep valgrind happy if there is any large objects such as dex cache arrays which aren't
+    // freed since they are held live by the class linker.
+    MutexLock mu(Thread::Current(), lock_);
+    for (auto& m : mem_maps_) {
+      delete m.second;
+    }
+  }
+
   virtual mirror::Object* Alloc(Thread* self, size_t num_bytes, size_t* bytes_allocated,
                                 size_t* usable_size, size_t* bytes_tl_bulk_allocated)
       OVERRIDE {

diff --git a/runtime/gc_root.h b/runtime/gc_root.h
index bdc7d5c..b67e9c2 100644 (file)
--- a/runtime/gc_root.h
+++ b/runtime/gc_root.h
@@ -162,6 +162,9 @@ class GcRoot {
   ALWAYS_INLINE GcRoot(MirrorType* ref = nullptr) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
  private:
+  // Root visitors take pointers to root_ and place the min CompressedReference** arrays. We use a
+  // CompressedReference<mirror::Object> here since it violates strict aliasing requirements to
+  // cast CompressedReference<MirrorType>* to CompressedReference<mirror::Object>*.
   mutable mirror::CompressedReference<mirror::Object> root_;
 
   template <size_t kBufferSize> friend class BufferedRootVisitor;

diff --git a/runtime/indirect_reference_table.cc b/runtime/indirect_reference_table.cc
index 5012965..d6f9682 100644 (file)
--- a/runtime/indirect_reference_table.cc
+++ b/runtime/indirect_reference_table.cc
@@ -258,7 +258,10 @@ void IndirectReferenceTable::Trim() {
 void IndirectReferenceTable::VisitRoots(RootVisitor* visitor, const RootInfo& root_info) {
   BufferedRootVisitor<kDefaultBufferedRootCount> root_visitor(visitor, root_info);
   for (auto ref : *this) {
-    root_visitor.VisitRootIfNonNull(*ref);
+    if (!ref->IsNull()) {
+      root_visitor.VisitRoot(*ref);
+      DCHECK(!ref->IsNull());
+    }
   }
 }
 

diff --git a/runtime/thread.cc b/runtime/thread.cc
index ac3f089..5ca51fb 100644 (file)
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -1386,6 +1386,8 @@ void Thread::HandleScopeVisitRoots(RootVisitor* visitor, uint32_t thread_id) {
       visitor, RootInfo(kRootNativeStack, thread_id));
   for (HandleScope* cur = tlsPtr_.top_handle_scope; cur; cur = cur->GetLink()) {
     for (size_t j = 0, count = cur->NumberOfReferences(); j < count; ++j) {
+      // GetReference returns a pointer to the stack reference within the handle scope. If this
+      // needs to be updated, it will be done by the root visitor.
       buffered_visitor.VisitRootIfNonNull(cur->GetHandle(j).GetReference());
     }
   }
@@ -2312,6 +2314,7 @@ void Thread::VisitRoots(RootVisitor* visitor) {
   ReleaseLongJumpContext(context);
   for (instrumentation::InstrumentationStackFrame& frame : *GetInstrumentationStack()) {
     visitor->VisitRootIfNonNull(&frame.this_object_, RootInfo(kRootVMInternal, thread_id));
+    DCHECK(frame.method_ != nullptr);
     visitor->VisitRoot(reinterpret_cast<mirror::Object**>(&frame.method_),
                        RootInfo(kRootVMInternal, thread_id));
   }