8216324: GetClassMethods is confused by the presence of default methods in super interfaces

Reviewed-by: sspitsyn, amenkov

src/hotspot/share/prims/jvmtiEnv.cpp

@@ -2516,54 +2516,54 @@ JvmtiEnv::GetClassMethods(oop k_mirror, jint* method_count_ptr, jmethodID** meth
   jmethodID* result_list = (jmethodID*)jvmtiMalloc(result_length * sizeof(jmethodID));
   int index;
   bool jmethodids_found = true;
+  int skipped = 0;  // skip overpass methods
 
-  if (JvmtiExport::can_maintain_original_method_order()) {
+  for (index = 0; index < result_length; index++) {
-    // Use the original method ordering indices stored in the class, so we can emit
+    Method* m = ik->methods()->at(index);
-    // jmethodIDs in the order they appeared in the class file
+    // Depending on can_maintain_original_method_order capability use the original
-    for (index = 0; index < result_length; index++) {
+    // method ordering indices stored in the class, so we can emit jmethodIDs in
-      Method* m = ik->methods()->at(index);
+    // the order they appeared in the class file or just copy in current order.
-      int original_index = ik->method_ordering()->at(index);
+    int result_index = JvmtiExport::can_maintain_original_method_order() ? ik->method_ordering()->at(index) : index;
-      assert(original_index >= 0 && original_index < result_length, "invalid original method index");
+    assert(result_index >= 0 && result_index < result_length, "invalid original method index");
-      jmethodID id;
+    if (m->is_overpass()) {
-      if (jmethodids_found) {
+      result_list[result_index] = NULL;
-        id = m->find_jmethod_id_or_null();
+      skipped++;
-        if (id == NULL) {
+      continue;
-          // If we find an uninitialized value, make sure there is
+    }
-          // enough space for all the uninitialized values we might
+    jmethodID id;
-          // find.
+    if (jmethodids_found) {
-          ik->ensure_space_for_methodids(index);
+      id = m->find_jmethod_id_or_null();
-          jmethodids_found = false;
+      if (id == NULL) {
-          id = m->jmethod_id();
+        // If we find an uninitialized value, make sure there is
-        }
+        // enough space for all the uninitialized values we might
-      } else {
+        // find.
+        ik->ensure_space_for_methodids(index);
+        jmethodids_found = false;
         id = m->jmethod_id();
       }
-      result_list[original_index] = id;
+    } else {
-    }
+      id = m->jmethod_id();
-  } else {
-    // otherwise just copy in any order
-    for (index = 0; index < result_length; index++) {
-      Method* m = ik->methods()->at(index);
-      jmethodID id;
-      if (jmethodids_found) {
-        id = m->find_jmethod_id_or_null();
-        if (id == NULL) {
-          // If we find an uninitialized value, make sure there is
-          // enough space for all the uninitialized values we might
-          // find.
-          ik->ensure_space_for_methodids(index);
-          jmethodids_found = false;
-          id = m->jmethod_id();
-        }
-      } else {
-        id = m->jmethod_id();
-      }
-      result_list[index] = id;
     }
+    result_list[result_index] = id;
   }
+
   // Fill in return value.
-  *method_count_ptr = result_length;
+  if (skipped > 0) {
-  *methods_ptr = result_list;
+    // copy results skipping NULL methodIDs
+    *methods_ptr = (jmethodID*)jvmtiMalloc((result_length - skipped) * sizeof(jmethodID));
+    *method_count_ptr = result_length - skipped;
+    for (index = 0, skipped = 0; index < result_length; index++) {
+      if (result_list[index] == NULL) {
+        skipped++;
+      } else {
+        (*methods_ptr)[index - skipped] = result_list[index];
+      }
+    }
+    deallocate((unsigned char *)result_list);
+  } else {
+    *method_count_ptr = result_length;
+    *methods_ptr = result_list;
+  }
 
   return JVMTI_ERROR_NONE;
 } /* end GetClassMethods */

test/hotspot/jtreg/serviceability/jvmti/GetClassMethods/OverpassMethods.java

@@ -0,0 +1,94 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * @test
+ * @bug 8216324
+ * @summary GetClassMethods is confused by the presence of default methods in super interfaces
+ * @library /test/lib
+ * @compile OverpassMethods.java
+ * @run main/othervm/native -agentlib:OverpassMethods OverpassMethods
+ * @run main/othervm/native -agentlib:OverpassMethods=maintain_original_method_order OverpassMethods
+  */
+
+import java.lang.reflect.Method;
+import java.util.Arrays;
+
+public class OverpassMethods {
+
+    static {
+        try {
+            System.loadLibrary("OverpassMethods");
+        } catch (UnsatisfiedLinkError ex) {
+            System.err.println("Could not load OverpassMethods library");
+            System.err.println("java.library.path:" + System.getProperty("java.library.path"));
+            throw ex;
+        }
+    }
+
+    static private void log(Object msg) {
+        System.out.println(String.valueOf(msg));
+    }
+
+    static private native Method[] getJVMTIDeclaredMethods(Class<?> klass);
+
+    public interface Parent {
+        default String def() { return "Parent.def"; }
+        String method0();
+        String method1();
+    }
+
+    public interface Child extends Parent {
+        String method2();
+    }
+
+    public static class Impl implements Child {
+        public String method0() { return "Impl.method0"; }
+        public String method1() { return "Impl.method1"; }
+        public String method2() { return "Impl.method2"; }
+    }
+
+    public static void main(String[] args) {
+        new Impl(); // To get classes initialized
+
+        Method[] reflectMethods = Child.class.getDeclaredMethods();
+        Method[] jvmtiMethods = getJVMTIDeclaredMethods(Child.class);
+
+        if (jvmtiMethods == null) {
+            throw new RuntimeException("getJVMTIDeclaredMethods failed");
+        }
+
+        log("Reflection getDeclaredMethods returned: " + Arrays.toString(reflectMethods));
+        log("JVMTI GetClassMethods returned: " + Arrays.toString(jvmtiMethods));
+
+        if (reflectMethods.length != jvmtiMethods.length) {
+            throw new RuntimeException("OverpassMethods failed: Unexpected method count from JVMTI GetClassMethods!");
+        }
+        if (!reflectMethods[0].equals(jvmtiMethods[0])) {
+            throw new RuntimeException("OverpassMethods failed: Unexpected method from JVMTI GetClassMethods!");
+        }
+        log("Test passed: Got expected output from JVMTI GetClassMethods!");
+    }
+}

test/hotspot/jtreg/serviceability/jvmti/GetClassMethods/libOverpassMethods.cpp

@@ -0,0 +1,103 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include "jvmti.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#define ACC_STATIC 0x0008
+
+static jvmtiEnv *jvmti = NULL;
+
+JNIEXPORT
+jint JNICALL JNI_OnLoad(JavaVM *jvm, void *reserved) {
+  return JNI_VERSION_9;
+}
+
+JNIEXPORT jint JNICALL Agent_OnLoad(JavaVM *vm, char *options, void *reserved) {
+  vm->GetEnv((void **)&jvmti, JVMTI_VERSION_11);
+
+  if (options != NULL && strcmp(options, "maintain_original_method_order") == 0) {
+    printf("Enabled capability: maintain_original_method_order\n");
+    jvmtiCapabilities caps = {};
+    caps.can_maintain_original_method_order = 1;
+
+    jvmtiError err = jvmti->AddCapabilities(&caps);
+    if (err != JVMTI_ERROR_NONE) {
+      printf("Agent_OnLoad: AddCapabilities failed with error: %d\n", err);
+      return JNI_ERR;
+    }
+  }
+  return JNI_OK;
+}
+
+JNIEXPORT jobjectArray JNICALL Java_OverpassMethods_getJVMTIDeclaredMethods(JNIEnv *env, jclass static_klass, jclass klass) {
+  jint method_count = 0;
+  jmethodID* methods = NULL;
+  jvmtiError err = jvmti->GetClassMethods(klass, &method_count, &methods);
+  if (err != JVMTI_ERROR_NONE) {
+    printf("GetClassMethods failed with error: %d\n", err);
+    return NULL;
+  }
+
+  jclass method_cls = env->FindClass("java/lang/reflect/Method");
+  if (method_cls == NULL) {
+    printf("FindClass (Method) failed\n");
+    return NULL;
+  }
+  jobjectArray array = env->NewObjectArray(method_count, method_cls, NULL);
+  if (array == NULL) {
+    printf("NewObjectArray failed\n");
+    return NULL;
+  }
+
+  for (int i = 0; i < method_count; i++) {
+    jint modifiers = 0;
+    err = jvmti->GetMethodModifiers(methods[i], &modifiers);
+    if (err != JVMTI_ERROR_NONE) {
+      printf("GetMethodModifiers failed with error: %d\n", err);
+      return NULL;
+    }
+
+    jobject m = env->ToReflectedMethod(klass, methods[i], (modifiers & ACC_STATIC) == ACC_STATIC);
+    if (array == NULL) {
+      printf("ToReflectedMethod failed\n");
+      return NULL;
+    }
+    env->SetObjectArrayElement(array, i, m);
+
+    env->DeleteLocalRef(m);
+  }
+  jvmti->Deallocate((unsigned char *)methods);
+
+  return array;
+}
+#ifdef __cplusplus
+}
+#endif

test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetClassMethods/getclmthd007.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -81,6 +81,7 @@ public class getclmthd007 {
     }
 
     static interface InnerInterface {
+        default void meth_def1() {}
         void meth_n1();
     }
 
@@ -109,7 +110,11 @@ class OuterClass3 {
     }
 }
 
-interface OuterInterface1 {
+interface DefaultInterface {
+    default void default_method() { }
+}
+
+interface OuterInterface1 extends DefaultInterface {
     int meth_i1();
 }
 

test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetClassMethods/getclmthd007/getclmthd007.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -54,7 +54,8 @@ static meth_info m0[] = {
 };
 
 static meth_info m1[] = {
-    { "meth_n1", "()V" }
+    { "meth_n1", "()V" },
+    { "meth_def1", "()V" }
 };
 
 static meth_info m2[] = {
@@ -98,7 +99,7 @@ static meth_info m9[] = {
 
 static class_info classes[] = {
     { "InnerClass1", 2, m0 },
-    { "InnerInterface", 1, m1 },
+    { "InnerInterface", 2, m1 },
     { "InnerClass2", 4, m2 },
     { "OuterClass1", 1, m3 },
     { "OuterClass2", 2, m4 },
@@ -145,6 +146,7 @@ Java_nsk_jvmti_GetClassMethods_getclmthd007_check(JNIEnv *env,
     char *name, *sig, *generic;
     int j, k;
 
+    int failed = JNI_FALSE; // enable debugging on failure
     if (jvmti == NULL) {
         printf("JVMTI client was not properly loaded!\n");
         result = STATUS_FAILED;
@@ -167,12 +169,14 @@ Java_nsk_jvmti_GetClassMethods_getclmthd007_check(JNIEnv *env,
         printf("(%d) wrong number of methods: %d, expected: %d\n",
                i, mcount, classes[i].mcount);
         result = STATUS_FAILED;
+        failed = JNI_TRUE; // show the methods found
+        printf(">>> %s:\n", classes[i].name);
     }
     for (k = 0; k < mcount; k++) {
         if (methods[k] == NULL) {
             printf("(%d:%d) methodID = null\n", i, k);
             result = STATUS_FAILED;
-        } else if (printdump == JNI_TRUE) {
+        } else if (printdump == JNI_TRUE || failed == JNI_TRUE) {
             err = jvmti->GetMethodName(methods[k],
                 &name, &sig, &generic);
             if (err == JVMTI_ERROR_NONE) {