From: Mathieu Chartier Date: Tue, 14 Apr 2015 16:35:18 +0000 (-0700) Subject: Fix valgrind tests X-Git-Tag: android-x86-7.1-r1~889^2~1525^2 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=9086b65b2ad35dd39a8afc62d535be8217208d08;p=android-x86%2Fart.git 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 ed2e2954f..bb8d876bc 100644 --- a/runtime/gc/collector/mark_sweep.cc +++ b/runtime/gc/collector/mark_sweep.cc @@ -490,29 +490,21 @@ void MarkSweep::VisitRoots(mirror::CompressedReference** 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 31cea17ad..fad340343 100644 --- 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 5c8e4b929..a4a9d80fa 100644 --- 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 bdc7d5c8e..b67e9c29b 100644 --- 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 here since it violates strict aliasing requirements to + // cast CompressedReference* to CompressedReference*. mutable mirror::CompressedReference root_; template friend class BufferedRootVisitor; diff --git a/runtime/indirect_reference_table.cc b/runtime/indirect_reference_table.cc index 5012965ea..d6f968205 100644 --- 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 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 ac3f08968..5ca51fbdd 100644 --- 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(&frame.method_), RootInfo(kRootVMInternal, thread_id)); }