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.

Bug: 27216437
Change-Id: I8c800e3e622a9c0ca0f8752c3d5202f433af9a1c

5 files changed, 19 insertions, 12 deletions

diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc
index 73574ba673..2e9e2879a6 100644
--- a/compiler/image_writer.cc
+++ b/compiler/image_writer.cc
@@ -907,10 +907,10 @@ void ImageWriter::PruneNonImageClasses() {
           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 894d29ee99..d3b404a3b6 100644
--- a/compiler/oat_test.cc
+++ b/compiler/oat_test.cc
@@ -415,7 +415,9 @@ TEST_F(OatTest, WriteRead) {
     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 078a978505..f3e8d6bd0b 100644
--- a/runtime/art_method.h
+++ b/runtime/art_method.h
@@ -132,6 +132,11 @@ class ArtMethod FINAL {
     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 5278d1bb05..936c98875f 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -759,7 +759,7 @@ static void SanityCheckArtMethod(ArtMethod* m,
     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 @@ class FixupArtMethodArrayVisitor : public ArtMethodVisitor {
 
   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 @@ class FixupArtMethodArrayVisitor : public ArtMethodVisitor {
     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 3a0f3e5065..4d528ed74b 100644
--- a/runtime/class_linker_test.cc
+++ b/runtime/class_linker_test.cc
@@ -263,7 +263,7 @@ class ClassLinkerTest : public CommonRuntimeTest {
     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()))