Fix issue with copied methods not being checked.
In several places we were using IsMiranda to check if a method is
copied. This misses cases involving default methods.
(cherrypicked commit fcea56f9cc51957161fe7a6e35e895fd8c4c4a7f)
Bug: 27216437
Change-Id: I8c800e3e622a9c0ca0f8752c3d5202f433af9a1c
diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc
index 73574ba..2e9e287 100644
--- a/compiler/image_writer.cc
+++ b/compiler/image_writer.cc
@@ -907,10 +907,10 @@
mirror::DexCache::GetElementPtrSize(resolved_methods, i, target_ptr_size_);
DCHECK(method != nullptr) << "Expected resolution method instead of null method";
mirror::Class* declaring_class = method->GetDeclaringClass();
- // Miranda methods may be held live by a class which was not an image class but have a
+ // Copied methods may be held live by a class which was not an image class but have a
// declaring class which is an image class. Set it to the resolution method to be safe and
// prevent dangling pointers.
- if (method->IsMiranda() || !KeepClass(declaring_class)) {
+ if (method->MightBeCopied() || !KeepClass(declaring_class)) {
mirror::DexCache::SetElementPtrSize(resolved_methods,
i,
resolution_method,
diff --git a/compiler/oat_test.cc b/compiler/oat_test.cc
index 894d29e..d3b404a 100644
--- a/compiler/oat_test.cc
+++ b/compiler/oat_test.cc
@@ -415,7 +415,9 @@
size_t visited_virtuals = 0;
// TODO We should also check copied methods in this test.
for (auto& m : klass->GetDeclaredVirtualMethods(pointer_size)) {
- EXPECT_FALSE(m.IsMiranda());
+ if (!klass->IsInterface()) {
+ EXPECT_FALSE(m.MightBeCopied());
+ }
CheckMethod(&m, oat_class.GetOatMethod(method_index), dex_file);
++method_index;
++visited_virtuals;
diff --git a/runtime/art_method.h b/runtime/art_method.h
index 078a978..f3e8d6b 100644
--- a/runtime/art_method.h
+++ b/runtime/art_method.h
@@ -132,6 +132,11 @@
return (GetAccessFlags() & kAccFinal) != 0;
}
+ // Returns true if this method might be copied from another class.
+ bool MightBeCopied() {
+ return IsMiranda() || IsDefault() || IsDefaultConflicting();
+ }
+
bool IsMiranda() {
return (GetAccessFlags() & kAccMiranda) != 0;
}
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 5278d1b..936c988 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -759,7 +759,7 @@
SHARED_REQUIRES(Locks::mutator_lock_) {
if (m->IsRuntimeMethod()) {
CHECK(m->GetDeclaringClass() == nullptr) << PrettyMethod(m);
- } else if (m->IsMiranda()) {
+ } else if (m->MightBeCopied()) {
CHECK(m->GetDeclaringClass() != nullptr) << PrettyMethod(m);
} else if (expected_class != nullptr) {
CHECK_EQ(m->GetDeclaringClassUnchecked(), expected_class) << PrettyMethod(m);
@@ -1137,18 +1137,18 @@
virtual void Visit(ArtMethod* method) SHARED_REQUIRES(Locks::mutator_lock_) {
GcRoot<mirror::Class>* resolved_types = method->GetDexCacheResolvedTypes(sizeof(void*));
- const bool is_miranda = method->IsMiranda();
+ const bool maybe_copied = method->MightBeCopied();
if (resolved_types != nullptr) {
bool in_image_space = false;
- if (kIsDebugBuild || is_miranda) {
+ if (kIsDebugBuild || maybe_copied) {
in_image_space = header_.GetImageSection(ImageHeader::kSectionDexCacheArrays).Contains(
reinterpret_cast<const uint8_t*>(resolved_types) - header_.GetImageBegin());
}
// Must be in image space for non-miranda method.
- DCHECK(is_miranda || in_image_space)
+ DCHECK(maybe_copied || in_image_space)
<< resolved_types << " is not in image starting at "
<< reinterpret_cast<void*>(header_.GetImageBegin());
- if (!is_miranda || in_image_space) {
+ if (!maybe_copied || in_image_space) {
// Go through the array so that we don't need to do a slow map lookup.
method->SetDexCacheResolvedTypes(*reinterpret_cast<GcRoot<mirror::Class>**>(resolved_types),
sizeof(void*));
@@ -1157,15 +1157,15 @@
ArtMethod** resolved_methods = method->GetDexCacheResolvedMethods(sizeof(void*));
if (resolved_methods != nullptr) {
bool in_image_space = false;
- if (kIsDebugBuild || is_miranda) {
+ if (kIsDebugBuild || maybe_copied) {
in_image_space = header_.GetImageSection(ImageHeader::kSectionDexCacheArrays).Contains(
reinterpret_cast<const uint8_t*>(resolved_methods) - header_.GetImageBegin());
}
// Must be in image space for non-miranda method.
- DCHECK(is_miranda || in_image_space)
+ DCHECK(maybe_copied || in_image_space)
<< resolved_methods << " is not in image starting at "
<< reinterpret_cast<void*>(header_.GetImageBegin());
- if (!is_miranda || in_image_space) {
+ if (!maybe_copied || in_image_space) {
// Go through the array so that we don't need to do a slow map lookup.
method->SetDexCacheResolvedMethods(*reinterpret_cast<ArtMethod***>(resolved_methods),
sizeof(void*));
diff --git a/runtime/class_linker_test.cc b/runtime/class_linker_test.cc
index 3a0f3e5..4d528ed 100644
--- a/runtime/class_linker_test.cc
+++ b/runtime/class_linker_test.cc
@@ -263,7 +263,7 @@
for (ArtMethod& method : klass->GetCopiedMethods(sizeof(void*))) {
AssertMethod(&method);
EXPECT_FALSE(method.IsDirect());
- EXPECT_TRUE(method.IsMiranda() || method.IsDefault() || method.IsDefaultConflicting());
+ EXPECT_TRUE(method.MightBeCopied());
EXPECT_TRUE(method.GetDeclaringClass()->IsInterface())
<< "declaring class: " << PrettyClass(method.GetDeclaringClass());
EXPECT_TRUE(method.GetDeclaringClass()->IsAssignableFrom(klass.Get()))
