Add a test and CHECKs around the combination of CHA and default methods.

Test: 823-cha-inlining
Bug: 182538502
Change-Id: Ie3e0014804216802af0addf13751a8f89adbfdfa

8 files changed, 184 insertions, 24 deletions

diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index 96cb605c1c..3e9969de84 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -1286,11 +1286,14 @@ bool HInliner::TryInlineAndReplace(HInvoke* invoke_instruction,
         return false;
       }
 
-      if (method->IsDefault() && method->IsDefaultConflicting()) {
-        // Changing to invoke-virtual cannot be done on default conflict method
-        // since it's not in any vtable.
-        DCHECK(cha_devirtualize);
-        return false;
+      if (kIsDebugBuild && method->IsDefaultConflicting()) {
+        CHECK(!cha_devirtualize) << "CHA cannot have a default conflict method as target";
+        // Devirtualization by exact type/inline-cache always uses a method in the vtable,
+        // so it's OK to change this invoke into a HInvokeVirtual.
+        ObjPtr<mirror::Class> receiver_class = receiver_type.GetTypeHandle().Get();
+        CHECK(!receiver_class->IsInterface());
+        PointerSize pointer_size = Runtime::Current()->GetClassLinker()->GetImagePointerSize();
+        CHECK(method == receiver_class->GetVTableEntry(method->GetMethodIndex(), pointer_size));
       }
 
       uint32_t dex_method_index = FindMethodIndexIn(
@@ -1786,14 +1789,12 @@ void HInliner::SubstituteArguments(HGraph* callee_graph,
 bool HInliner::CanInlineBody(const HGraph* callee_graph,
                              const HBasicBlock* target_block,
                              size_t* out_number_of_instructions) const {
-  const DexFile& callee_dex_file = callee_graph->GetDexFile();
   ArtMethod* const resolved_method = callee_graph->GetArtMethod();
-  const uint32_t method_index = resolved_method->GetMethodIndex();
 
   HBasicBlock* exit_block = callee_graph->GetExitBlock();
   if (exit_block == nullptr) {
     LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedInfiniteLoop)
-        << "Method " << callee_dex_file.PrettyMethod(method_index)
+        << "Method " << resolved_method->PrettyMethod()
         << " could not be inlined because it has an infinite loop";
     return false;
   }
@@ -1804,21 +1805,21 @@ bool HInliner::CanInlineBody(const HGraph* callee_graph,
       if (target_block->IsTryBlock()) {
         // TODO(ngeoffray): Support adding HTryBoundary in Hgraph::InlineInto.
         LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedTryCatch)
-            << "Method " << callee_dex_file.PrettyMethod(method_index)
+            << "Method " << resolved_method->PrettyMethod()
             << " could not be inlined because one branch always throws and"
             << " caller is in a try/catch block";
         return false;
       } else if (graph_->GetExitBlock() == nullptr) {
         // TODO(ngeoffray): Support adding HExit in the caller graph.
         LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedInfiniteLoop)
-            << "Method " << callee_dex_file.PrettyMethod(method_index)
+            << "Method " << resolved_method->PrettyMethod()
             << " could not be inlined because one branch always throws and"
             << " caller does not have an exit block";
         return false;
       } else if (graph_->HasIrreducibleLoops()) {
         // TODO(ngeoffray): Support re-computing loop information to graphs with
         // irreducible loops?
-        VLOG(compiler) << "Method " << callee_dex_file.PrettyMethod(method_index)
+        VLOG(compiler) << "Method " << resolved_method->PrettyMethod()
                        << " could not be inlined because one branch always throws and"
                        << " caller has irreducible loops";
         return false;
@@ -1830,7 +1831,7 @@ bool HInliner::CanInlineBody(const HGraph* callee_graph,
 
   if (!has_one_return) {
     LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedAlwaysThrows)
-        << "Method " << callee_dex_file.PrettyMethod(method_index)
+        << "Method " << resolved_method->PrettyMethod()
         << " could not be inlined because it always throws";
     return false;
   }
@@ -1843,7 +1844,7 @@ bool HInliner::CanInlineBody(const HGraph* callee_graph,
         // Don't inline methods with irreducible loops, they could prevent some
         // optimizations to run.
         LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedIrreducibleLoop)
-            << "Method " << callee_dex_file.PrettyMethod(method_index)
+            << "Method " << resolved_method->PrettyMethod()
             << " could not be inlined because it contains an irreducible loop";
         return false;
       }
@@ -1852,7 +1853,7 @@ bool HInliner::CanInlineBody(const HGraph* callee_graph,
         // loop information to be computed incorrectly when updating after
         // inlining.
         LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedLoopWithoutExit)
-            << "Method " << callee_dex_file.PrettyMethod(method_index)
+            << "Method " << resolved_method->PrettyMethod()
             << " could not be inlined because it contains a loop with no exit";
         return false;
       }
@@ -1863,7 +1864,7 @@ bool HInliner::CanInlineBody(const HGraph* callee_graph,
          instr_it.Advance()) {
       if (++number_of_instructions >= inlining_budget_) {
         LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedInstructionBudget)
-            << "Method " << callee_dex_file.PrettyMethod(method_index)
+            << "Method " << resolved_method->PrettyMethod()
             << " is not inlined because the outer method has reached"
             << " its instruction budget limit.";
         return false;
@@ -1872,7 +1873,7 @@ bool HInliner::CanInlineBody(const HGraph* callee_graph,
       if (current->NeedsEnvironment() &&
           (total_number_of_dex_registers_ >= kMaximumNumberOfCumulatedDexRegisters)) {
         LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedEnvironmentBudget)
-            << "Method " << callee_dex_file.PrettyMethod(method_index)
+            << "Method " << resolved_method->PrettyMethod()
             << " is not inlined because its caller has reached"
             << " its environment budget limit.";
         return false;
@@ -1882,7 +1883,7 @@ bool HInliner::CanInlineBody(const HGraph* callee_graph,
           !CanEncodeInlinedMethodInStackMap(*caller_compilation_unit_.GetDexFile(),
                                             resolved_method)) {
         LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedStackMaps)
-            << "Method " << callee_dex_file.PrettyMethod(method_index)
+            << "Method " << resolved_method->PrettyMethod()
             << " could not be inlined because " << current->DebugName()
             << " needs an environment, is in a different dex file"
             << ", and cannot be encoded in the stack maps.";
@@ -1895,7 +1896,7 @@ bool HInliner::CanInlineBody(const HGraph* callee_graph,
           current->IsUnresolvedInstanceFieldSet()) {
         // Entrypoint for unresolved fields does not handle inlined frames.
         LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedUnresolvedEntrypoint)
-            << "Method " << callee_dex_file.PrettyMethod(method_index)
+            << "Method " << resolved_method->PrettyMethod()
             << " could not be inlined because it is using an unresolved"
             << " entrypoint";
         return false;
diff --git a/runtime/art_method.cc b/runtime/art_method.cc
index a2db0f00ad..89dc93b895 100644
--- a/runtime/art_method.cc
+++ b/runtime/art_method.cc
@@ -94,11 +94,14 @@ ArtMethod* ArtMethod::GetNonObsoleteMethod() {
 }
 
 ArtMethod* ArtMethod::GetSingleImplementation(PointerSize pointer_size) {
-  if (!IsAbstract()) {
-    // A non-abstract's single implementation is itself.
+  if (IsInvokable()) {
+    // An invokable method single implementation is itself.
     return this;
   }
-  return reinterpret_cast<ArtMethod*>(GetDataPtrSize(pointer_size));
+  DCHECK(!IsDefaultConflicting());
+  ArtMethod* m = reinterpret_cast<ArtMethod*>(GetDataPtrSize(pointer_size));
+  CHECK(m == nullptr || !m->IsDefaultConflicting());
+  return m;
 }
 
 ArtMethod* ArtMethod::FromReflectedMethod(const ScopedObjectAccessAlreadyRunnable& soa,
@@ -736,9 +739,9 @@ void ArtMethod::CopyFrom(ArtMethod* src, PointerSize image_pointer_size) {
           image_pointer_size);
     }
   }
-    if (interpreter::IsNterpSupported() &&
-        (GetEntryPointFromQuickCompiledCodePtrSize(image_pointer_size) ==
-            interpreter::GetNterpEntryPoint())) {
+  if (interpreter::IsNterpSupported() &&
+      (GetEntryPointFromQuickCompiledCodePtrSize(image_pointer_size) ==
+          interpreter::GetNterpEntryPoint())) {
     // If the entrypoint is nterp, it's too early to check if the new method
     // will support it. So for simplicity, use the interpreter bridge.
     SetEntryPointFromQuickCompiledCodePtrSize(GetQuickToInterpreterBridge(), image_pointer_size);
diff --git a/test/823-cha-inlining/expected-stderr.txt b/test/823-cha-inlining/expected-stderr.txt
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/test/823-cha-inlining/expected-stderr.txt
diff --git a/test/823-cha-inlining/expected-stdout.txt b/test/823-cha-inlining/expected-stdout.txt
new file mode 100644
index 0000000000..6a5618ebc6
--- /dev/null
+++ b/test/823-cha-inlining/expected-stdout.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/823-cha-inlining/info.txt b/test/823-cha-inlining/info.txt
new file mode 100644
index 0000000000..312a2ef3e4
--- /dev/null
+++ b/test/823-cha-inlining/info.txt
@@ -0,0 +1 @@
+Tests for the combination of CHA and default methods.
diff --git a/test/823-cha-inlining/src/Main.java b/test/823-cha-inlining/src/Main.java
new file mode 100644
index 0000000000..e25047d47e
--- /dev/null
+++ b/test/823-cha-inlining/src/Main.java
@@ -0,0 +1,132 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+interface Itf {
+  // We make the methods below directly throw instead of using the $noinline$
+  // directive to get the inliner actually try to inline but decide not to.
+  // This will then make the compiler try to generate an HInvokeVirtual instead
+  // of an HInvokeInterface.
+
+  public default void m() throws Exception {
+    throw new Exception("Don't inline me");
+  }
+  public default void mConflict() throws Exception {
+    throw new Exception("Don't inline me");
+  }
+}
+
+// This is redefined in src2 with a mConflict method.
+interface Itf2 {
+}
+
+interface Itf3 extends Itf, Itf2 {
+}
+
+class Itf3Impl implements Itf3 {
+}
+
+interface Itf4 extends Itf, Itf2 {
+  public default void m() throws Exception {
+    throw new Exception("Don't inline me");
+  }
+}
+
+class Itf4Impl implements Itf4 {
+}
+
+
+public class Main implements Itf, Itf2 {
+
+  public static void main(String[] args) {
+    System.loadLibrary(args[0]);
+
+    // Execute enough time to populate inline caches.
+    for (int i = 0; i < 100000; ++i) {
+      try {
+        $noinline$doCallDefault();
+      } catch (Exception e) {
+        // Expected.
+      }
+    }
+    ensureJitCompiled(Main.class, "$noinline$doCallDefault");
+    try {
+      $noinline$doCallDefault();
+      throw new Error("Expected exception");
+    } catch (Exception e) {
+      // Expected.
+    }
+
+    ensureJitCompiled(Main.class, "$noinline$doCallDefaultConflict");
+    try {
+      $noinline$doCallDefaultConflict();
+      throw new Error("Expected IncompatibleClassChangeError");
+    } catch (Exception e) {
+      throw new Error("Unexpected exception");
+    } catch (IncompatibleClassChangeError e) {
+      // Expected.
+    }
+
+    // Execute enough time to populate inline caches.
+    for (int i = 0; i < 100000; ++i) {
+      try {
+        $noinline$doCallDefaultConflictItf3();
+      } catch (Throwable t) {
+        // Expected.
+      }
+    }
+    ensureJitCompiled(Main.class, "$noinline$doCallDefaultConflictItf3");
+    try {
+      $noinline$doCallDefaultConflictItf3();
+      throw new Error("Expected IncompatibleClassChangeError");
+    } catch (Exception e) {
+      throw new Error("Unexpected exception");
+    } catch (IncompatibleClassChangeError e) {
+      // Expected.
+    }
+
+    ensureJitCompiled(Main.class, "$noinline$doCallDefaultConflictItf4");
+    try {
+      $noinline$doCallDefaultConflictItf4();
+      throw new Error("Expected IncompatibleClassChangeError");
+    } catch (Exception e) {
+      throw new Error("Unexpected exception");
+    } catch (IncompatibleClassChangeError e) {
+      // Expected.
+    }
+  }
+
+  public static void $noinline$doCallDefault() throws Exception {
+    itf.m();
+  }
+
+  public static void $noinline$doCallDefaultConflict() throws Exception {
+    itf.mConflict();
+  }
+
+  public static void $noinline$doCallDefaultConflictItf3() throws Exception {
+    itf3.mConflict();
+  }
+
+  public static void $noinline$doCallDefaultConflictItf4() throws Exception {
+    itf4.mConflict();
+  }
+
+  static Itf itf = new Main();
+  static Itf3 itf3 = new Itf3Impl();
+  static Itf4 itf4 = new Itf4Impl();
+
+  private static native void ensureJitCompiled(Class<?> cls, String methodName);
+}
diff --git a/test/823-cha-inlining/src2/Itf2.java b/test/823-cha-inlining/src2/Itf2.java
new file mode 100644
index 0000000000..6b019c4dfe
--- /dev/null
+++ b/test/823-cha-inlining/src2/Itf2.java
@@ -0,0 +1,21 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+interface Itf2 {
+  public default void mConflict() throws Exception {
+    throw new Exception("Don't inline me");
+  }
+}
diff --git a/test/knownfailures.json b/test/knownfailures.json
index 7f35091295..d4b46ffc6f 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1008,6 +1008,7 @@
           "804-class-extends-itself",
           "816-illegal-new-array",
           "819-verification-runtime",
+          "823-cha-inlining",
           "900-hello-plugin",
           "901-hello-ti-agent",
           "903-hello-tagging",