Merge "Fix cert-dcl16-c clang-tidy warnings."
diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc
index 4c2074d..5045888 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -654,6 +654,22 @@
const CodeVisitor code_visitor_;
};
+template <typename ReferenceVisitor>
+class ImageSpace::ClassTableVisitor final {
+ public:
+ explicit ClassTableVisitor(const ReferenceVisitor& reference_visitor)
+ : reference_visitor_(reference_visitor) {}
+
+ void VisitRoot(mirror::CompressedReference<mirror::Object>* root) const
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ DCHECK(root->AsMirrorPtr() != nullptr);
+ root->Assign(reference_visitor_(root->AsMirrorPtr()));
+ }
+
+ private:
+ ReferenceVisitor reference_visitor_;
+};
+
// Helper class encapsulating loading, so we can access private ImageSpace members (this is a
// nested class), but not declare functions in the header.
class ImageSpace::Loader {
@@ -701,11 +717,22 @@
TimingLogger::ScopedTiming timing("RelocateImage", &logger);
ImageHeader* image_header = reinterpret_cast<ImageHeader*>(space->GetMemMap()->Begin());
- if (!RelocateInPlace(*image_header,
- space->GetMemMap()->Begin(),
- space->GetLiveBitmap(),
- oat_file,
- error_msg)) {
+ const PointerSize pointer_size = image_header->GetPointerSize();
+ bool result;
+ if (pointer_size == PointerSize::k64) {
+ result = RelocateInPlace<PointerSize::k64>(*image_header,
+ space->GetMemMap()->Begin(),
+ space->GetLiveBitmap(),
+ oat_file,
+ error_msg);
+ } else {
+ result = RelocateInPlace<PointerSize::k32>(*image_header,
+ space->GetMemMap()->Begin(),
+ space->GetLiveBitmap(),
+ oat_file,
+ error_msg);
+ }
+ if (!result) {
return nullptr;
}
Runtime* runtime = Runtime::Current();
@@ -1089,11 +1116,8 @@
class FixupObjectVisitor : public FixupVisitor {
public:
template<typename... Args>
- explicit FixupObjectVisitor(gc::accounting::ContinuousSpaceBitmap* visited,
- const PointerSize pointer_size,
- Args... args)
+ explicit FixupObjectVisitor(gc::accounting::ContinuousSpaceBitmap* visited, Args... args)
: FixupVisitor(args...),
- pointer_size_(pointer_size),
visited_(visited) {}
// Fix up separately since we also need to fix up method entrypoints.
@@ -1105,39 +1129,14 @@
ALWAYS_INLINE void operator()(ObjPtr<mirror::Object> obj,
MemberOffset offset,
-
bool is_static ATTRIBUTE_UNUSED) const
NO_THREAD_SAFETY_ANALYSIS {
- // There could be overlap between ranges, we must avoid visiting the same reference twice.
- // Avoid the class field since we already fixed it up in FixupClassVisitor.
- if (offset.Uint32Value() != mirror::Object::ClassOffset().Uint32Value()) {
- // Space is not yet added to the heap, don't do a read barrier.
- mirror::Object* ref = obj->GetFieldObject<mirror::Object, kVerifyNone, kWithoutReadBarrier>(
- offset);
- // Use SetFieldObjectWithoutWriteBarrier to avoid card marking since we are writing to the
- // image.
- obj->SetFieldObjectWithoutWriteBarrier<false, true, kVerifyNone>(offset, ForwardObject(ref));
- }
- }
-
- // Visit a pointer array and forward corresponding native data. Ignores pointer arrays in the
- // boot image. Uses the bitmap to ensure the same array is not visited multiple times.
- template <typename Visitor>
- void UpdatePointerArrayContents(mirror::PointerArray* array, const Visitor& visitor) const
- NO_THREAD_SAFETY_ANALYSIS {
- DCHECK(array != nullptr);
- DCHECK(visitor.IsInAppImage(array));
- // The bit for the array contents is different than the bit for the array. Since we may have
- // already visited the array as a long / int array from walking the bitmap without knowing it
- // was a pointer array.
- static_assert(kObjectAlignment == 8u, "array bit may be in another object");
- mirror::Object* const contents_bit = reinterpret_cast<mirror::Object*>(
- reinterpret_cast<uintptr_t>(array) + kObjectAlignment);
- // If the bit is not set then the contents have not yet been updated.
- if (!visited_->Test(contents_bit)) {
- array->Fixup<kVerifyNone>(array, pointer_size_, visitor);
- visited_->Set(contents_bit);
- }
+ // Space is not yet added to the heap, don't do a read barrier.
+ mirror::Object* ref = obj->GetFieldObject<mirror::Object, kVerifyNone, kWithoutReadBarrier>(
+ offset);
+ // Use SetFieldObjectWithoutWriteBarrier to avoid card marking since we are writing to the
+ // image.
+ obj->SetFieldObjectWithoutWriteBarrier<false, true, kVerifyNone>(offset, ForwardObject(ref));
}
// java.lang.ref.Reference visitor.
@@ -1152,81 +1151,16 @@
void operator()(mirror::Object* obj) const
NO_THREAD_SAFETY_ANALYSIS {
- if (visited_->Test(obj)) {
- // Already visited.
- return;
- }
- visited_->Set(obj);
-
- // Handle class specially first since we need it to be updated to properly visit the rest of
- // the instance fields.
- {
- mirror::Class* klass = obj->GetClass<kVerifyNone, kWithoutReadBarrier>();
- DCHECK(klass != nullptr) << "Null class in image";
- // No AsClass since our fields aren't quite fixed up yet.
- mirror::Class* new_klass = down_cast<mirror::Class*>(ForwardObject(klass));
- if (klass != new_klass) {
- obj->SetClass<kVerifyNone>(new_klass);
- }
- if (new_klass != klass && IsInAppImage(new_klass)) {
- // Make sure the klass contents are fixed up since we depend on it to walk the fields.
- operator()(new_klass);
- }
- }
-
- if (obj->IsClass()) {
- mirror::Class* klass = obj->AsClass<kVerifyNone>();
- // Fixup super class before visiting instance fields which require
- // information from their super class to calculate offsets.
- mirror::Class* super_class =
- klass->GetSuperClass<kVerifyNone, kWithoutReadBarrier>().Ptr();
- if (super_class != nullptr) {
- mirror::Class* new_super_class = down_cast<mirror::Class*>(ForwardObject(super_class));
- if (new_super_class != super_class && IsInAppImage(new_super_class)) {
- // Recursively fix all dependencies.
- operator()(new_super_class);
- }
- }
- }
-
- obj->VisitReferences</*visit native roots*/false, kVerifyNone, kWithoutReadBarrier>(
- *this,
- *this);
- // Note that this code relies on no circular dependencies.
- // We want to use our own class loader and not the one in the image.
- if (obj->IsClass<kVerifyNone>()) {
- mirror::Class* as_klass = obj->AsClass<kVerifyNone>();
- FixupObjectAdapter visitor(boot_image_, app_image_, app_oat_);
- as_klass->FixupNativePointers<kVerifyNone>(as_klass, pointer_size_, visitor);
- // Deal with the pointer arrays. Use the helper function since multiple classes can reference
- // the same arrays.
- mirror::PointerArray* const vtable = as_klass->GetVTable<kVerifyNone, kWithoutReadBarrier>();
- if (vtable != nullptr && IsInAppImage(vtable)) {
- operator()(vtable);
- UpdatePointerArrayContents(vtable, visitor);
- }
- mirror::IfTable* iftable = as_klass->GetIfTable<kVerifyNone, kWithoutReadBarrier>();
- // Ensure iftable arrays are fixed up since we need GetMethodArray to return the valid
- // contents.
- if (IsInAppImage(iftable)) {
- operator()(iftable);
- for (int32_t i = 0, count = iftable->Count(); i < count; ++i) {
- if (iftable->GetMethodArrayCount<kVerifyNone, kWithoutReadBarrier>(i) > 0) {
- mirror::PointerArray* methods =
- iftable->GetMethodArray<kVerifyNone, kWithoutReadBarrier>(i);
- if (visitor.IsInAppImage(methods)) {
- operator()(methods);
- DCHECK(methods != nullptr);
- UpdatePointerArrayContents(methods, visitor);
- }
- }
- }
- }
+ if (!visited_->Set(obj)) {
+ // Not already visited.
+ obj->VisitReferences</*visit native roots*/false, kVerifyNone, kWithoutReadBarrier>(
+ *this,
+ *this);
+ CHECK(!obj->IsClass());
}
}
private:
- const PointerSize pointer_size_;
gc::accounting::ContinuousSpaceBitmap* const visited_;
};
@@ -1306,6 +1240,7 @@
// Relocate an image space mapped at target_base which possibly used to be at a different base
// address. In place means modifying a single ImageSpace in place rather than relocating from
// one ImageSpace to another.
+ template <PointerSize kPointerSize>
static bool RelocateInPlace(ImageHeader& image_header,
uint8_t* target_base,
accounting::ContinuousSpaceBitmap* bitmap,
@@ -1317,7 +1252,6 @@
uint32_t boot_image_end = 0;
uint32_t boot_oat_begin = 0;
uint32_t boot_oat_end = 0;
- const PointerSize pointer_size = image_header.GetPointerSize();
gc::Heap* const heap = Runtime::Current()->GetHeap();
heap->GetBootImagesSize(&boot_image_begin, &boot_image_end, &boot_oat_begin, &boot_oat_end);
if (boot_image_begin == boot_image_end) {
@@ -1359,11 +1293,8 @@
return true;
}
ScopedDebugDisallowReadBarriers sddrb(Thread::Current());
- // Need to update the image to be at the target base.
- const ImageSection& objects_section = image_header.GetObjectsSection();
- uintptr_t objects_begin = reinterpret_cast<uintptr_t>(target_base + objects_section.Offset());
- uintptr_t objects_end = reinterpret_cast<uintptr_t>(target_base + objects_section.End());
FixupObjectAdapter fixup_adapter(boot_image, app_image, app_oat);
+ PatchObjectVisitor<kPointerSize, FixupObjectAdapter> patch_object_visitor(fixup_adapter);
if (fixup_image) {
// Two pass approach, fix up all classes first, then fix up non class-objects.
// The visited bitmap is used to ensure that pointer arrays are not forwarded twice.
@@ -1371,16 +1302,64 @@
gc::accounting::ContinuousSpaceBitmap::Create("Relocate bitmap",
target_base,
image_header.GetImageSize()));
- FixupObjectVisitor fixup_object_visitor(visited_bitmap.get(),
- pointer_size,
- boot_image,
- app_image,
- app_oat);
- TimingLogger::ScopedTiming timing("Fixup classes", &logger);
- // Fixup objects may read fields in the boot image, use the mutator lock here for sanity. Though
- // its probably not required.
+ FixupObjectVisitor fixup_object_visitor(visited_bitmap.get(), boot_image, app_image, app_oat);
+ {
+ TimingLogger::ScopedTiming timing("Fixup classes", &logger);
+ const auto& class_table_section = image_header.GetClassTableSection();
+ if (class_table_section.Size() > 0u) {
+ ScopedObjectAccess soa(Thread::Current());
+ ClassTableVisitor class_table_visitor(fixup_adapter);
+ size_t read_count = 0u;
+ const uint8_t* data = target_base + class_table_section.Offset();
+ // We avoid making a copy of the data since we want modifications to be propagated to the
+ // memory map.
+ ClassTable::ClassSet temp_set(data, /*make_copy_of_data=*/ false, &read_count);
+ for (ClassTable::TableSlot& slot : temp_set) {
+ slot.VisitRoot(class_table_visitor);
+ mirror::Class* klass = slot.Read<kWithoutReadBarrier>();
+ if (!fixup_adapter.IsInAppImage(klass)) {
+ continue;
+ }
+ const bool already_marked = visited_bitmap->Set(klass);
+ CHECK(!already_marked) << "App image class already visited";
+ patch_object_visitor.VisitClass(klass);
+ // Then patch the non-embedded vtable and iftable.
+ mirror::PointerArray* vtable = klass->GetVTable<kVerifyNone, kWithoutReadBarrier>();
+ if (vtable != nullptr &&
+ fixup_object_visitor.IsInAppImage(vtable) &&
+ !visited_bitmap->Set(vtable)) {
+ patch_object_visitor.VisitPointerArray(vtable);
+ }
+ auto* iftable = klass->GetIfTable<kVerifyNone, kWithoutReadBarrier>();
+ if (iftable != nullptr && fixup_object_visitor.IsInAppImage(iftable)) {
+ // Avoid processing the fields of iftable since we will process them later anyways
+ // below.
+ int32_t ifcount = klass->GetIfTableCount<kVerifyNone>();
+ for (int32_t i = 0; i != ifcount; ++i) {
+ mirror::PointerArray* unpatched_ifarray =
+ iftable->GetMethodArrayOrNull<kVerifyNone, kWithoutReadBarrier>(i);
+ if (unpatched_ifarray != nullptr) {
+ // The iftable has not been patched, so we need to explicitly adjust the pointer.
+ mirror::PointerArray* ifarray = fixup_adapter(unpatched_ifarray);
+ if (fixup_object_visitor.IsInAppImage(ifarray) &&
+ !visited_bitmap->Set(ifarray)) {
+ patch_object_visitor.VisitPointerArray(ifarray);
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+
+ // Fixup objects may read fields in the boot image, use the mutator lock here for sanity.
+ // Though its probably not required.
+ TimingLogger::ScopedTiming timing("Fixup cobjects", &logger);
ScopedObjectAccess soa(Thread::Current());
- timing.NewTiming("Fixup objects");
+ // Need to update the image to be at the target base.
+ const ImageSection& objects_section = image_header.GetObjectsSection();
+ uintptr_t objects_begin = reinterpret_cast<uintptr_t>(target_base + objects_section.Offset());
+ uintptr_t objects_end = reinterpret_cast<uintptr_t>(target_base + objects_section.End());
bitmap->VisitMarkedRange(objects_begin, objects_end, fixup_object_visitor);
// Fixup image roots.
CHECK(app_image.InSource(reinterpret_cast<uintptr_t>(
@@ -1392,96 +1371,19 @@
AsObjectArray<mirror::DexCache, kVerifyNone>();
for (int32_t i = 0, count = dex_caches->GetLength(); i < count; ++i) {
mirror::DexCache* dex_cache = dex_caches->Get<kVerifyNone, kWithoutReadBarrier>(i);
- // Fix up dex cache pointers.
- mirror::StringDexCacheType* strings = dex_cache->GetStrings();
- if (strings != nullptr) {
- mirror::StringDexCacheType* new_strings = fixup_adapter.ForwardObject(strings);
- if (strings != new_strings) {
- dex_cache->SetStrings(new_strings);
- }
- dex_cache->FixupStrings<kWithoutReadBarrier>(new_strings, fixup_adapter);
- }
- mirror::TypeDexCacheType* types = dex_cache->GetResolvedTypes();
- if (types != nullptr) {
- mirror::TypeDexCacheType* new_types = fixup_adapter.ForwardObject(types);
- if (types != new_types) {
- dex_cache->SetResolvedTypes(new_types);
- }
- dex_cache->FixupResolvedTypes<kWithoutReadBarrier>(new_types, fixup_adapter);
- }
- mirror::MethodDexCacheType* methods = dex_cache->GetResolvedMethods();
- if (methods != nullptr) {
- mirror::MethodDexCacheType* new_methods = fixup_adapter.ForwardObject(methods);
- if (methods != new_methods) {
- dex_cache->SetResolvedMethods(new_methods);
- }
- for (size_t j = 0, num = dex_cache->NumResolvedMethods(); j != num; ++j) {
- auto pair = mirror::DexCache::GetNativePairPtrSize(new_methods, j, pointer_size);
- ArtMethod* orig = pair.object;
- ArtMethod* copy = fixup_adapter.ForwardObject(orig);
- if (orig != copy) {
- pair.object = copy;
- mirror::DexCache::SetNativePairPtrSize(new_methods, j, pair, pointer_size);
- }
- }
- }
- mirror::FieldDexCacheType* fields = dex_cache->GetResolvedFields();
- if (fields != nullptr) {
- mirror::FieldDexCacheType* new_fields = fixup_adapter.ForwardObject(fields);
- if (fields != new_fields) {
- dex_cache->SetResolvedFields(new_fields);
- }
- for (size_t j = 0, num = dex_cache->NumResolvedFields(); j != num; ++j) {
- mirror::FieldDexCachePair orig =
- mirror::DexCache::GetNativePairPtrSize(new_fields, j, pointer_size);
- mirror::FieldDexCachePair copy(fixup_adapter.ForwardObject(orig.object), orig.index);
- if (orig.object != copy.object) {
- mirror::DexCache::SetNativePairPtrSize(new_fields, j, copy, pointer_size);
- }
- }
- }
-
- mirror::MethodTypeDexCacheType* method_types = dex_cache->GetResolvedMethodTypes();
- if (method_types != nullptr) {
- mirror::MethodTypeDexCacheType* new_method_types =
- fixup_adapter.ForwardObject(method_types);
- if (method_types != new_method_types) {
- dex_cache->SetResolvedMethodTypes(new_method_types);
- }
- dex_cache->FixupResolvedMethodTypes<kWithoutReadBarrier>(new_method_types, fixup_adapter);
- }
- GcRoot<mirror::CallSite>* call_sites = dex_cache->GetResolvedCallSites();
- if (call_sites != nullptr) {
- GcRoot<mirror::CallSite>* new_call_sites = fixup_adapter.ForwardObject(call_sites);
- if (call_sites != new_call_sites) {
- dex_cache->SetResolvedCallSites(new_call_sites);
- }
- dex_cache->FixupResolvedCallSites<kWithoutReadBarrier>(new_call_sites, fixup_adapter);
- }
-
- GcRoot<mirror::String>* preresolved_strings = dex_cache->GetPreResolvedStrings();
- if (preresolved_strings != nullptr) {
- GcRoot<mirror::String>* new_array = fixup_adapter.ForwardObject(preresolved_strings);
- if (preresolved_strings != new_array) {
- dex_cache->SetPreResolvedStrings(new_array);
- }
- const size_t num_preresolved_strings = dex_cache->NumPreResolvedStrings();
- for (size_t j = 0; j < num_preresolved_strings; ++j) {
- new_array[j] = GcRoot<mirror::String>(
- fixup_adapter(new_array[j].Read<kWithoutReadBarrier>()));
- }
- }
+ CHECK(dex_cache != nullptr);
+ patch_object_visitor.VisitDexCacheArrays(dex_cache);
}
}
{
// Only touches objects in the app image, no need for mutator lock.
TimingLogger::ScopedTiming timing("Fixup methods", &logger);
FixupArtMethodVisitor method_visitor(fixup_image,
- pointer_size,
+ kPointerSize,
boot_image,
app_image,
app_oat);
- image_header.VisitPackedArtMethods(&method_visitor, target_base, pointer_size);
+ image_header.VisitPackedArtMethods(&method_visitor, target_base, kPointerSize);
}
if (fixup_image) {
{
@@ -1492,26 +1394,14 @@
}
{
TimingLogger::ScopedTiming timing("Fixup imt", &logger);
- image_header.VisitPackedImTables(fixup_adapter, target_base, pointer_size);
+ image_header.VisitPackedImTables(fixup_adapter, target_base, kPointerSize);
}
{
TimingLogger::ScopedTiming timing("Fixup conflict tables", &logger);
- image_header.VisitPackedImtConflictTables(fixup_adapter, target_base, pointer_size);
+ image_header.VisitPackedImtConflictTables(fixup_adapter, target_base, kPointerSize);
}
// In the app image case, the image methods are actually in the boot image.
image_header.RelocateImageMethods(boot_image.Delta());
- const auto& class_table_section = image_header.GetClassTableSection();
- if (class_table_section.Size() > 0u) {
- // Note that we require that ReadFromMemory does not make an internal copy of the elements.
- // This also relies on visit roots not doing any verification which could fail after we update
- // the roots to be the image addresses.
- ScopedObjectAccess soa(Thread::Current());
- WriterMutexLock mu(Thread::Current(), *Locks::classlinker_classes_lock_);
- ClassTable temp_table;
- temp_table.ReadFromMemory(target_base + class_table_section.Offset());
- FixupRootVisitor root_visitor(boot_image, app_image, app_oat);
- temp_table.VisitRoots(root_visitor);
- }
// Fix up the intern table.
const auto& intern_table_section = image_header.GetInternedStringsSection();
if (intern_table_section.Size() > 0u) {
@@ -1766,22 +1656,6 @@
BitMemoryRegion visited_objects_;
};
- template <typename ReferenceVisitor>
- class ClassTableVisitor final {
- public:
- explicit ClassTableVisitor(const ReferenceVisitor& reference_visitor)
- : reference_visitor_(reference_visitor) {}
-
- void VisitRoot(mirror::CompressedReference<mirror::Object>* root) const
- REQUIRES_SHARED(Locks::mutator_lock_) {
- DCHECK(root->AsMirrorPtr() != nullptr);
- root->Assign(reference_visitor_(root->AsMirrorPtr()));
- }
-
- private:
- ReferenceVisitor reference_visitor_;
- };
-
template <PointerSize kPointerSize>
static void DoRelocateSpaces(const std::vector<std::unique_ptr<ImageSpace>>& spaces,
uint32_t diff) REQUIRES_SHARED(Locks::mutator_lock_) {
diff --git a/runtime/gc/space/image_space.h b/runtime/gc/space/image_space.h
index dbc12d1..9049a53 100644
--- a/runtime/gc/space/image_space.h
+++ b/runtime/gc/space/image_space.h
@@ -189,6 +189,8 @@
private:
class BootImageLoader;
+ template <typename ReferenceVisitor>
+ class ClassTableVisitor;
class Loader;
template <typename PatchObjectVisitor>
class PatchArtFieldVisitor;
