Move most mirror:: args to ObjPtr
Fixed possible moving GC bugs in ClinitImageUpdate class.
Bug: 31113334
Test: test-art-host
Change-Id: I0bf6578553d58b944aaa17665f1350bdf5ed15ec
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index 8d64c65..afaec52 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -1134,6 +1134,7 @@
VLOG(compiler) << "Adding " << descriptor << " to image classes";
for (size_t i = 0; i < klass->NumDirectInterfaces(); ++i) {
StackHandleScope<1> hs2(self);
+ // May cause thread suspension.
MaybeAddToImageClasses(hs2.NewHandle(mirror::Class::GetDirectInterface(self, klass, i)),
image_classes);
}
@@ -1153,15 +1154,14 @@
// Note: we can use object pointers because we suspend all threads.
class ClinitImageUpdate {
public:
- static ClinitImageUpdate* Create(std::unordered_set<std::string>* image_class_descriptors,
- Thread* self, ClassLinker* linker, std::string* error_msg) {
- std::unique_ptr<ClinitImageUpdate> res(new ClinitImageUpdate(image_class_descriptors, self,
+ static ClinitImageUpdate* Create(VariableSizedHandleScope& hs,
+ std::unordered_set<std::string>* image_class_descriptors,
+ Thread* self,
+ ClassLinker* linker) {
+ std::unique_ptr<ClinitImageUpdate> res(new ClinitImageUpdate(hs,
+ image_class_descriptors,
+ self,
linker));
- if (res->dex_cache_class_ == nullptr) {
- *error_msg = "Could not find DexCache class.";
- return nullptr;
- }
-
return res.release();
}
@@ -1171,7 +1171,9 @@
}
// Visitor for VisitReferences.
- void operator()(mirror::Object* object, MemberOffset field_offset, bool /* is_static */) const
+ void operator()(ObjPtr<mirror::Object> object,
+ MemberOffset field_offset,
+ bool /* is_static */) const
REQUIRES_SHARED(Locks::mutator_lock_) {
mirror::Object* ref = object->GetFieldObject<mirror::Object>(field_offset);
if (ref != nullptr) {
@@ -1180,8 +1182,8 @@
}
// java.lang.Reference visitor for VisitReferences.
- void operator()(mirror::Class* klass ATTRIBUTE_UNUSED, mirror::Reference* ref ATTRIBUTE_UNUSED)
- const {}
+ void operator()(ObjPtr<mirror::Class> klass ATTRIBUTE_UNUSED,
+ ObjPtr<mirror::Reference> ref ATTRIBUTE_UNUSED) const {}
// Ignore class native roots.
void VisitRootIfNonNull(mirror::CompressedReference<mirror::Object>* root ATTRIBUTE_UNUSED)
@@ -1193,6 +1195,9 @@
for (mirror::Class* klass_root : image_classes_) {
VisitClinitClassesObject(klass_root);
}
+ for (Handle<mirror::Class> h_klass : to_insert_) {
+ MaybeAddToImageClasses(h_klass, image_class_descriptors_);
+ }
}
private:
@@ -1219,20 +1224,19 @@
ClinitImageUpdate* const data_;
};
- ClinitImageUpdate(std::unordered_set<std::string>* image_class_descriptors, Thread* self,
- ClassLinker* linker)
- REQUIRES_SHARED(Locks::mutator_lock_) :
- image_class_descriptors_(image_class_descriptors), self_(self) {
+ ClinitImageUpdate(VariableSizedHandleScope& hs,
+ std::unordered_set<std::string>* image_class_descriptors,
+ Thread* self,
+ ClassLinker* linker) REQUIRES_SHARED(Locks::mutator_lock_)
+ : hs_(hs),
+ image_class_descriptors_(image_class_descriptors),
+ self_(self) {
CHECK(linker != nullptr);
CHECK(image_class_descriptors != nullptr);
// Make sure nobody interferes with us.
old_cause_ = self->StartAssertNoThreadSuspension("Boot image closure");
- // Find the interesting classes.
- dex_cache_class_ = linker->LookupClass(self, "Ljava/lang/DexCache;",
- ComputeModifiedUtf8Hash("Ljava/lang/DexCache;"), nullptr);
-
// Find all the already-marked classes.
WriterMutexLock mu(self, *Locks::heap_bitmap_lock_);
FindImageClassesVisitor visitor(this);
@@ -1251,25 +1255,25 @@
marked_objects_.insert(object);
if (object->IsClass()) {
- // If it is a class, add it.
- StackHandleScope<1> hs(self_);
- MaybeAddToImageClasses(hs.NewHandle(object->AsClass()), image_class_descriptors_);
+ // Add to the TODO list since MaybeAddToImageClasses may cause thread suspension. Thread
+ // suspensionb is not safe to do in VisitObjects or VisitReferences.
+ to_insert_.push_back(hs_.NewHandle(object->AsClass()));
} else {
// Else visit the object's class.
VisitClinitClassesObject(object->GetClass());
}
// If it is not a DexCache, visit all references.
- mirror::Class* klass = object->GetClass();
- if (klass != dex_cache_class_) {
+ if (!object->IsDexCache()) {
object->VisitReferences(*this, *this);
}
}
+ VariableSizedHandleScope& hs_;
+ mutable std::vector<Handle<mirror::Class>> to_insert_;
mutable std::unordered_set<mirror::Object*> marked_objects_;
std::unordered_set<std::string>* const image_class_descriptors_;
std::vector<mirror::Class*> image_classes_;
- const mirror::Class* dex_cache_class_;
Thread* const self_;
const char* old_cause_;
@@ -1285,12 +1289,12 @@
// Suspend all threads.
ScopedSuspendAll ssa(__FUNCTION__);
+ VariableSizedHandleScope hs(Thread::Current());
std::string error_msg;
- std::unique_ptr<ClinitImageUpdate> update(ClinitImageUpdate::Create(image_classes_.get(),
+ std::unique_ptr<ClinitImageUpdate> update(ClinitImageUpdate::Create(hs,
+ image_classes_.get(),
Thread::Current(),
- runtime->GetClassLinker(),
- &error_msg));
- CHECK(update.get() != nullptr) << error_msg; // TODO: Soft failure?
+ runtime->GetClassLinker()));
// Do the marking.
update->Walk();
diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc
index 13c73dc..412225c 100644
--- a/compiler/image_writer.cc
+++ b/compiler/image_writer.cc
@@ -1323,7 +1323,7 @@
root->Assign(VisitReference(root->AsMirrorPtr()));
}
- ALWAYS_INLINE void operator() (mirror::Object* obj,
+ ALWAYS_INLINE void operator() (ObjPtr<mirror::Object> obj,
MemberOffset offset,
bool is_static ATTRIBUTE_UNUSED) const
REQUIRES_SHARED(Locks::mutator_lock_) {
@@ -1332,8 +1332,8 @@
obj->SetFieldObject</*kTransactionActive*/false>(offset, VisitReference(ref));
}
- ALWAYS_INLINE void operator() (mirror::Class* klass ATTRIBUTE_UNUSED,
- mirror::Reference* ref) const
+ ALWAYS_INLINE void operator() (ObjPtr<mirror::Class> klass ATTRIBUTE_UNUSED,
+ ObjPtr<mirror::Reference> ref) const
REQUIRES_SHARED(Locks::mutator_lock_) {
ref->SetReferent</*kTransactionActive*/false>(
VisitReference(ref->GetReferent<kWithoutReadBarrier>()));
@@ -1941,18 +1941,19 @@
void VisitRoot(mirror::CompressedReference<mirror::Object>* root ATTRIBUTE_UNUSED) const {}
- void operator()(Object* obj, MemberOffset offset, bool is_static ATTRIBUTE_UNUSED) const
+ void operator()(ObjPtr<Object> obj, MemberOffset offset, bool is_static ATTRIBUTE_UNUSED) const
REQUIRES(Locks::mutator_lock_, Locks::heap_bitmap_lock_) {
- Object* ref = obj->GetFieldObject<Object, kVerifyNone>(offset);
+ ObjPtr<Object> ref = obj->GetFieldObject<Object, kVerifyNone>(offset);
// Use SetFieldObjectWithoutWriteBarrier to avoid card marking since we are writing to the
// image.
copy_->SetFieldObjectWithoutWriteBarrier<false, true, kVerifyNone>(
offset,
- image_writer_->GetImageAddress(ref));
+ image_writer_->GetImageAddress(ref.Ptr()));
}
// java.lang.ref.Reference visitor.
- void operator()(mirror::Class* klass ATTRIBUTE_UNUSED, mirror::Reference* ref) const
+ void operator()(ObjPtr<mirror::Class> klass ATTRIBUTE_UNUSED,
+ ObjPtr<mirror::Reference> ref) const
REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(Locks::heap_bitmap_lock_) {
copy_->SetFieldObjectWithoutWriteBarrier<false, true, kVerifyNone>(
mirror::Reference::ReferentOffset(),
@@ -1969,14 +1970,14 @@
FixupClassVisitor(ImageWriter* image_writer, Object* copy) : FixupVisitor(image_writer, copy) {
}
- void operator()(Object* obj, MemberOffset offset, bool is_static ATTRIBUTE_UNUSED) const
+ void operator()(ObjPtr<Object> obj, MemberOffset offset, bool is_static ATTRIBUTE_UNUSED) const
REQUIRES(Locks::mutator_lock_, Locks::heap_bitmap_lock_) {
DCHECK(obj->IsClass());
FixupVisitor::operator()(obj, offset, /*is_static*/false);
}
- void operator()(mirror::Class* klass ATTRIBUTE_UNUSED,
- mirror::Reference* ref ATTRIBUTE_UNUSED) const
+ void operator()(ObjPtr<mirror::Class> klass ATTRIBUTE_UNUSED,
+ ObjPtr<mirror::Reference> ref ATTRIBUTE_UNUSED) const
REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(Locks::heap_bitmap_lock_) {
LOG(FATAL) << "Reference not expected here.";
}
@@ -2045,7 +2046,7 @@
void ImageWriter::FixupClass(mirror::Class* orig, mirror::Class* copy) {
orig->FixupNativePointers(copy, target_ptr_size_, NativeLocationVisitor(this));
FixupClassVisitor visitor(this, copy);
- static_cast<mirror::Object*>(orig)->VisitReferences(visitor, visitor);
+ ObjPtr<mirror::Object>(orig)->VisitReferences(visitor, visitor);
// Remove the clinitThreadId. This is required for image determinism.
copy->SetClinitThreadId(static_cast<pid_t>(0));