Remove special treatment of native method for CHA.

This special treatment is not necessary and causes complexity and issues.

Bug: 35104526
Test: m test-art-host
Change-Id: Icb07d147b433388d0b9430212b9b4c4bf04eafae

diff --git a/runtime/art_method.cc b/runtime/art_method.cc
index 61ff417..c796bd1 100644 (file)
--- a/runtime/art_method.cc
+++ b/runtime/art_method.cc
@@ -67,7 +67,6 @@ ArtMethod* ArtMethod::GetNonObsoleteMethod() {
 }
 
 ArtMethod* ArtMethod::GetSingleImplementation(PointerSize pointer_size) {
-  DCHECK(!IsNative());
   if (!IsAbstract()) {
     // A non-abstract's single implementation is itself.
     return this;

diff --git a/runtime/cha.cc b/runtime/cha.cc
index e726bdb..e6681d5 100644 (file)
--- a/runtime/cha.cc
+++ b/runtime/cha.cc
@@ -199,7 +199,8 @@ void ClassHierarchyAnalysis::VerifyNonSingleImplementation(mirror::Class* verify
     if (verify_method != excluded_method) {
       DCHECK(!verify_method->HasSingleImplementation())
           << "class: " << verify_class->PrettyClass()
-          << " verify_method: " << verify_method->PrettyMethod(true);
+          << " verify_method: " << verify_method->PrettyMethod(true)
+          << " excluded_method: " << excluded_method->PrettyMethod(true);
       if (verify_method->IsAbstract()) {
         DCHECK(verify_method->GetSingleImplementation(image_pointer_size) == nullptr);
       }
@@ -256,9 +257,6 @@ void ClassHierarchyAnalysis::CheckSingleImplementationInfo(
     return;
   }
 
-  // Native methods don't have single-implementation flag set.
-  DCHECK(!method_in_super->IsNative());
-
   uint16_t method_index = method_in_super->GetMethodIndex();
   if (method_in_super->IsAbstract()) {
     if (kIsDebugBuild) {
@@ -373,12 +371,12 @@ void ClassHierarchyAnalysis::InitSingleImplementationFlag(Handle<mirror::Class>
     // used for static methods or methods of final classes.
     return;
   }
-  if (method->IsNative()) {
-    // Native method's invocation overhead is already high and it
-    // cannot be inlined. It's not worthwhile to devirtualize the
-    // call which can add a deoptimization point.
-    DCHECK(!method->HasSingleImplementation());
-  } else if (method->IsAbstract()) {
+  if (method->IsAbstract()) {
+    // single-implementation of abstract method shares the same field
+    // that's used for JNI function of native method. It's fine since a method
+    // cannot be both abstract and native.
+    DCHECK(!method->IsNative()) << "Abstract method cannot be native";
+
     if (method->GetDeclaringClass()->IsInstantiable()) {
       // Rare case, but we do accept it (such as 800-smali/smali/b_26143249.smali).
       // Do not attempt to devirtualize it.

diff --git a/test/616-cha-native/expected.txt b/test/616-cha-native/expected.txt
new file mode 100644 (file)
index 0000000..6a5618e
--- /dev/null
+++ b/test/616-cha-native/expected.txt
@@ -0,0 +1 @@
+JNI_OnLoad called

diff --git a/test/616-cha-native/info.txt b/test/616-cha-native/info.txt
new file mode 100644 (file)
index 0000000..a17bcab
--- /dev/null
+++ b/test/616-cha-native/info.txt
@@ -0,0 +1,2 @@
+Test for Class Hierarchy Analysis (CHA) single-implementation status updating
+behavior on an overridden native method.

diff --git a/test/616-cha-native/src/Main.java b/test/616-cha-native/src/Main.java
new file mode 100644 (file)
index 0000000..53a463c
--- /dev/null
+++ b/test/616-cha-native/src/Main.java
@@ -0,0 +1,33 @@
+/*
+ * Copyright (C) 2017 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.
+ */
+
+abstract class A {
+  public abstract void foo();
+}
+
+class B extends A {
+  public native void foo();
+}
+
+class C extends B {
+  public void foo() {}
+}
+
+public class Main {
+  public static void main(String[] args) {
+    System.loadLibrary(args[0]);
+  }
+}

diff --git a/test/616-cha/src/Main.java b/test/616-cha/src/Main.java
index b617944..beea90a 100644 (file)
--- a/test/616-cha/src/Main.java
+++ b/test/616-cha/src/Main.java
@@ -196,8 +196,6 @@ public class Main {
     // should return true for those cases.
     assertSingleImplementation(java.lang.String.class, "charAt", true);
     assertSingleImplementation(java.lang.Thread.class, "join", true);
-    // We don't set single-implementation modifier bit for native methods.
-    assertSingleImplementation(java.lang.Thread.class, "isInterrupted", false);
 
     if (isInterpreted()) {
       sIsOptimizing = false;