From 565bc96291623bf617aef40739247b224188fd16 Mon Sep 17 00:00:00 2001
From: Alex Light <allight@google.com>
Date: Thu, 31 Mar 2016 10:03:07 -0700
Subject: [PATCH] Fix issued with non-public interface methods.

Some APK's ship with dex files containing non-public interface
methods. These would cause crashes on newer versions of ART due to
stricter verification of dex files. Make ART emit a warning but allow
this behavior.

Bug: 27928832

(cherry picked from commit d7c10c237bf2631630fac7982c3f374b1a27ed01)

Change-Id: Ia3689ee091aa154fb5954b1f6dd50c489128acce
---
 runtime/class_linker.cc           |  9 +++++++--
 runtime/dex_file_verifier.cc      | 19 ++++++++++++++++++-
 runtime/dex_file_verifier_test.cc |  6 +++---
 3 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 9d26d1305..5ea5f8e18 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -5874,9 +5874,14 @@ ClassLinker::DefaultMethodSearchResult ClassLinker::FindDefaultMethodImplementat
           !target_name_comparator.HasSameNameAndSignature(
               current_method->GetInterfaceMethodIfProxy(image_pointer_size_))) {
         continue;
+      } else if (!current_method->IsPublic()) {
+        // The verifier should have caught the non-public method for dex version 37. Just warn and
+        // skip it since this is from before default-methods so we don't really need to care that it
+        // has code.
+        LOG(WARNING) << "Interface method " << PrettyMethod(current_method) << " is not public! "
+                     << "This will be a fatal error in subsequent versions of android. "
+                     << "Continuing anyway.";
       }
-      // The verifier should have caught the non-public method.
-      DCHECK(current_method->IsPublic()) << "Interface method is not public!";
       if (UNLIKELY(chosen_iface.Get() != nullptr)) {
         // We have multiple default impls of the same method. This is a potential default conflict.
         // We need to check if this possibly conflicting method is either a superclass of the chosen
diff --git a/runtime/dex_file_verifier.cc b/runtime/dex_file_verifier.cc
index 811e76300..681c5f977 100644
--- a/runtime/dex_file_verifier.cc
+++ b/runtime/dex_file_verifier.cc
@@ -2626,6 +2626,23 @@ bool DexFileVerifier::CheckMethodAccessFlags(uint32_t method_index,
   // From here on out it is easier to mask out the bits we're supposed to ignore.
   method_access_flags &= kMethodAccessFlags;
 
+  // Interfaces are special.
+  if ((class_access_flags & kAccInterface) != 0) {
+    // Non-static interface methods must be public.
+    if ((method_access_flags & (kAccPublic | kAccStatic)) == 0) {
+      *error_msg = StringPrintf("Interface virtual method %" PRIu32 "(%s) is not public",
+          method_index,
+          GetMethodDescriptionOrError(begin_, header_, method_index).c_str());
+      if (header_->GetVersion() >= 37) {
+        return false;
+      } else {
+        // Allow in older versions, but warn.
+        LOG(WARNING) << "This dex file is invalid and will be rejected in the future. Error is: "
+                      << *error_msg;
+      }
+    }
+  }
+
   // If there aren't any instructions, make sure that's expected.
   if (!has_code) {
     // Only native or abstract methods may not have code.
@@ -2664,7 +2681,7 @@ bool DexFileVerifier::CheckMethodAccessFlags(uint32_t method_index,
     }
     // Interfaces are special.
     if ((class_access_flags & kAccInterface) != 0) {
-      // Interface methods must be public and abstract.
+      // Interface methods without code must be abstract.
       if ((method_access_flags & (kAccPublic | kAccAbstract)) != (kAccPublic | kAccAbstract)) {
         *error_msg = StringPrintf("Interface method %" PRIu32 "(%s) is not public and abstract",
             method_index,
diff --git a/runtime/dex_file_verifier_test.cc b/runtime/dex_file_verifier_test.cc
index 4a5ed5d71..344d186ad 100644
--- a/runtime/dex_file_verifier_test.cc
+++ b/runtime/dex_file_verifier_test.cc
@@ -767,7 +767,7 @@ TEST_F(DexFileVerifierTest, MethodAccessFlagsInterfaces) {
 
         ApplyMaskToMethodFlags(dex_file, "foo", ~kAccPublic);
       },
-      "Interface method 1(LInterfaceMethodFlags;.foo) is not public and abstract");
+      "Interface virtual method 1(LInterfaceMethodFlags;.foo) is not public");
 
   VerifyModification(
       kMethodFlagsInterface,
@@ -817,7 +817,7 @@ TEST_F(DexFileVerifierTest, MethodAccessFlagsInterfaces) {
 
         ApplyMaskToMethodFlags(dex_file, "foo", ~kAccPublic);
       },
-      "Interface method 1(LInterfaceMethodFlags;.foo) is not public and abstract");
+      "Interface virtual method 1(LInterfaceMethodFlags;.foo) is not public");
 
   VerifyModification(
       kMethodFlagsInterface,
@@ -839,7 +839,7 @@ TEST_F(DexFileVerifierTest, MethodAccessFlagsInterfaces) {
         ApplyMaskToMethodFlags(dex_file, "foo", ~kAccPublic);
         OrMaskToMethodFlags(dex_file, "foo", kAccProtected);
       },
-      "Interface method 1(LInterfaceMethodFlags;.foo) is not public and abstract");
+      "Interface virtual method 1(LInterfaceMethodFlags;.foo) is not public");
 
   constexpr uint32_t kAllMethodFlags =
       kAccPublic |
-- 
2.11.0