8317635: Improve GetClassFields test to verify correctness of field order

Alex Menkov · Alex Menkov · commit 599560a83238 · 2023-10-19T18:12:16.000Z
Reviewed-by: cjplummer, sspitsyn
diff --git a/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetClassFields/getclfld007.java b/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetClassFields/getclfld007.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2023, 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
@@ -24,6 +24,15 @@
 package nsk.jvmti.GetClassFields;
 
 import java.io.PrintStream;
+import java.io.InputStream;
+import java.util.List;
+import java.util.ArrayList;
+
+import jdk.internal.org.objectweb.asm.ClassReader;
+import jdk.internal.org.objectweb.asm.ClassVisitor;
+import jdk.internal.org.objectweb.asm.FieldVisitor;
+import jdk.internal.org.objectweb.asm.Opcodes;
+
 
 public class getclfld007 {
 
@@ -40,7 +49,7 @@ public class getclfld007 {
         }
     }
 
-    native static void check(int i, Class cls);
+    native static void check(Class cls, String[] expectedFields);
     native static int getRes();
 
     public static void main(String args[]) {
@@ -52,22 +61,64 @@ public static void main(String args[]) {
 
     public static int run(String args[], PrintStream out) {
         try {
-            check(0, Class.forName(InnerClass1.class.getName()));
-            check(1, Class.forName(InnerInterface.class.getName()));
-            check(2, Class.forName(InnerClass2.class.getName()));
-            check(3, Class.forName(OuterClass1.class.getName()));
-            check(4, Class.forName(OuterClass2.class.getName()));
-            check(5, Class.forName(OuterClass3.class.getName()));
-            check(6, Class.forName(OuterInterface1.class.getName()));
-            check(7, Class.forName(OuterInterface2.class.getName()));
-            check(8, Class.forName(OuterClass4.class.getName()));
-            check(9, Class.forName(OuterClass5.class.getName()));
-        } catch (ClassNotFoundException e) {
+            check(Class.forName(InnerClass1.class.getName()));
+            check(Class.forName(InnerInterface.class.getName()));
+            check(Class.forName(InnerClass2.class.getName()));
+            check(Class.forName(OuterClass1.class.getName()));
+            check(Class.forName(OuterClass2.class.getName()));
+            check(Class.forName(OuterClass3.class.getName()));
+            check(Class.forName(OuterInterface1.class.getName()));
+            check(Class.forName(OuterInterface2.class.getName()));
+            check(Class.forName(OuterClass4.class.getName()));
+            check(Class.forName(OuterClass5.class.getName()));
+        } catch (Exception e) {
             throw new RuntimeException(e);
         }
         return getRes();
     }
 
+
+    static void check(Class cls) throws Exception {
+        FieldExplorer explorer = new FieldExplorer(cls);
+        List<String> fields = explorer.get();
+        check(cls, fields.toArray(new String[0]));
+    }
+
+    // helper class to get list of the class fields
+    // in the order they appear in the class file
+    static class FieldExplorer extends ClassVisitor {
+        private final Class cls;
+        private List<String> fieldNameAndSig = new ArrayList<>();
+        private FieldExplorer(Class cls) {
+            super(Opcodes.ASM7);
+            this.cls = cls;
+        }
+
+        @Override
+        public FieldVisitor visitField(int access, String name, String descriptor, String signature, Object value) {
+            System.out.println("  field '" + name + "', type = " + descriptor);
+            fieldNameAndSig.add(name);
+            fieldNameAndSig.add(descriptor);
+            return super.visitField(access, name, descriptor, signature, value);
+        }
+
+        private InputStream getClassBytes() throws Exception {
+            String clsName = cls.getName();
+            String clsPath = clsName.replace('.', '/') + ".class";
+            return cls.getClassLoader().getResourceAsStream(clsPath);
+        }
+
+        // each field is represented by 2 Strings in the list: name and type descriptor
+        public List<String> get() throws Exception {
+            System.out.println("Class " + cls.getName());
+            try (InputStream classBytes = getClassBytes()) {
+                ClassReader classReader = new ClassReader(classBytes);
+                classReader.accept(this, 0);
+            }
+            return fieldNameAndSig;
+        }
+    }
+
     static class InnerClass1 {
         String fld_1;
         void meth(String s) {
@@ -119,8 +170,13 @@ public int meth_i2() {
     }
 }
 
+// class with multiple fields to verify correctness of the field order
 class OuterClass5 extends OuterClass4 {
     int fld_i1 = 1;
+    String fld_s1 = "str";
+    int fld_i2 = 2;
+    String fld_s2 = "str2";
+
     public int meth_i1() {
         return 1;
     }
diff --git a/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetClassFields/getclfld007/TestDescription.java b/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetClassFields/getclfld007/TestDescription.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2023, 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
@@ -31,9 +31,9 @@
  * DESCRIPTION
  *     The test exercises JVMTI function
  *         GetClassFields(clazz, fieldCountPtr, fieldsPtr).
- *     The test checks if the function returns the expected list of fields.
- *     That is the field list contains only directly declared (not inherited)
- *     fields.
+ *     The test checks if the function returns the expected list of fields:
+ *         - the list contains only directly declared (not inherited) fields;
+ *         - fields are returned in the order they occur in the class file.
  * COMMENTS
  *     Ported from JVMDI.
  *     Test fixed due to test bug:
@@ -45,6 +45,7 @@
  *
  * @library /vmTestbase
  *          /test/lib
+ * @modules java.base/jdk.internal.org.objectweb.asm
  * @run main/othervm/native -agentlib:getclfld007 nsk.jvmti.GetClassFields.getclfld007
  */
 
diff --git a/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetClassFields/getclfld007/getclfld007.cpp b/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetClassFields/getclfld007/getclfld007.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2023, 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
@@ -33,69 +33,25 @@ extern "C" {
 #define PASSED 0
 #define STATUS_FAILED 2
 
-typedef struct {
-    const char *name;
-    const char *sig;
-} fld_info;
-
-typedef struct {
-    const char *name;
-    jint fcount;
-    fld_info *flds;
-} class_info;
-
 static jvmtiEnv *jvmti = NULL;
 static jint result = PASSED;
-static jboolean printdump = JNI_FALSE;
-
-static fld_info f0[] = {
-    { "fld_1", "Ljava/lang/String;" }
-};
-
-static fld_info f1[] = {
-    { "fld_n1", "I" }
-};
-
-static fld_info f2[] = {
-    { "fld_n2", "I" }
-};
-
-static fld_info f4[] = {
-    { "fld_o2", "I" }
-};
-
-static fld_info f5[] = {
-    { "fld_o3", "I" }
-};
-
-static fld_info f6[] = {
-    { "fld_i1", "I" }
-};
-
-static fld_info f7[] = {
-    { "fld_i2", "I" }
-};
-
-static fld_info f8[] = {
-    { "fld_i2", "I" }
-};
-
-static fld_info f9[] = {
-    { "fld_i1", "I" }
-};
-
-static class_info classes[] = {
-    { "InnerClass1", 1, f0 },
-    { "InnerInterface", 1, f1 },
-    { "InnerClass2", 1, f2 },
-    { "OuterClass1", 0, NULL },
-    { "OuterClass2", 1, f4 },
-    { "OuterClass3", 1, f5 },
-    { "OuterInterface1", 1, f6 },
-    { "OuterInterface2", 1, f7 },
-    { "OuterClass4", 1, f8 },
-    { "OuterClass5", 1, f9 }
-};
+
+
+// compares 'value' with jobject_arr[index]
+static bool equals_str(JNIEnv *env, const char *value, jobjectArray jobject_arr, jint index) {
+    jstring jstr = (jstring)env->GetObjectArrayElement(jobject_arr, index);
+    const char* utf = env->GetStringUTFChars(jstr, NULL);
+    bool res = false;
+    if (utf != NULL) {
+        res = strcmp(value, utf) == 0;
+        env->ReleaseStringUTFChars(jstr, utf);
+    } else {
+        printf("GetStringUTFChars failed\n");
+        result = STATUS_FAILED;
+    }
+    env->DeleteLocalRef(jstr);
+    return res;
+}
 
 #ifdef STATIC_BUILD
 JNIEXPORT jint JNICALL Agent_OnLoad_getclfld007(JavaVM *jvm, char *options, void *reserved) {
@@ -111,10 +67,6 @@ JNIEXPORT jint JNI_OnLoad_getclfld007(JavaVM *jvm, char *options, void *reserved
 jint Agent_Initialize(JavaVM *jvm, char *options, void *reserved) {
     jint res;
 
-    if (options != NULL && strcmp(options, "printdump") == 0) {
-        printdump = JNI_TRUE;
-    }
-
     res = jvm->GetEnv((void **) &jvmti, JVMTI_VERSION_1_1);
     if (res != JNI_OK || jvmti == NULL) {
         printf("Wrong result of a valid call to GetEnv!\n");
@@ -125,61 +77,62 @@ jint Agent_Initialize(JavaVM *jvm, char *options, void *reserved) {
 }
 
 JNIEXPORT void JNICALL
-Java_nsk_jvmti_GetClassFields_getclfld007_check(JNIEnv *env, jclass cls, jint i, jclass clazz) {
+Java_nsk_jvmti_GetClassFields_getclfld007_check(JNIEnv *env, jclass cls, jclass clazz, jobjectArray fieldArr) {
     jvmtiError err;
     jint fcount;
     jfieldID *fields;
-    char *name, *sig, *generic;
+    char *name, *sig;
     int j;
 
     if (jvmti == NULL) {
         printf("JVMTI client was not properly loaded!\n");
+        fflush(0);
         result = STATUS_FAILED;
         return;
     }
 
-    if (printdump == JNI_TRUE) {
-        printf(">>> %s:\n", classes[i].name);
-    }
+    // fieldArr contains 2 elements for each field
+    jint field_count = env->GetArrayLength(fieldArr) / 2;
 
     err = jvmti->GetClassFields(clazz, &fcount, &fields);
     if (err != JVMTI_ERROR_NONE) {
-        printf("(GetClassFields#%d) unexpected error: %s (%d)\n",
-               i, TranslateError(err), err);
+        printf("GetClassFields unexpected error: %s (%d)\n",
+               TranslateError(err), err);
+        fflush(0);
         result = STATUS_FAILED;
         return;
     }
 
-    if (fcount != classes[i].fcount) {
-        printf("(%d) wrong number of fields: %d, expected: %d\n",
-               i, fcount, classes[i].fcount);
+    if (fcount != field_count) {
+        printf("wrong number of fields: %d, expected: %d\n",
+               fcount, field_count);
         result = STATUS_FAILED;
     }
     for (j = 0; j < fcount; j++) {
         if (fields[j] == NULL) {
-            printf("(%d:%d) fieldID = null\n", i, j);
-        } else {
-            err = jvmti->GetFieldName(clazz, fields[j],
-                &name, &sig, &generic);
-            if (err != JVMTI_ERROR_NONE) {
-                printf("(GetFieldName#%d:%d) unexpected error: %s (%d)\n",
-                       i, j, TranslateError(err), err);
-            } else {
-                if (printdump == JNI_TRUE) {
-                    printf(">>>   [%d]: %s, sig = \"%s\"\n", j, name, sig);
-                }
-                if ((j < classes[i].fcount) &&
-                       (name == NULL || sig == NULL ||
-                        strcmp(name, classes[i].flds[j].name) != 0 ||
-                        strcmp(sig, classes[i].flds[j].sig) != 0)) {
-                    printf("(%d:%d) wrong field: \"%s%s\"", i, j, name, sig);
-                    printf(", expected: \"%s%s\"\n",
-                           classes[i].flds[j].name, classes[i].flds[j].sig);
-                    result = STATUS_FAILED;
-                }
-            }
+            printf("(%d) fieldID = null\n", j);
+            result = STATUS_FAILED;
+            continue;
+        }
+        err = jvmti->GetFieldName(clazz, fields[j], &name, &sig, NULL);
+        if (err != JVMTI_ERROR_NONE) {
+            printf("(GetFieldName#%d) unexpected error: %s (%d)\n",
+                   j, TranslateError(err), err);
+            result = STATUS_FAILED;
+            continue;
+        }
+        printf(">>>   [%d]: %s, sig = \"%s\"\n", j, name, sig);
+        if ((j < field_count) &&
+               (name == NULL || sig == NULL ||
+                !equals_str(env, name, fieldArr, j * 2) ||
+                !equals_str(env, sig, fieldArr, j * 2 + 1))) {
+            printf("(%d) wrong field: \"%s%s\"", j, name, sig);
+            result = STATUS_FAILED;
         }
+        jvmti->Deallocate((unsigned char *)name);
+        jvmti->Deallocate((unsigned char *)sig);
     }
+    fflush(0);
 }
 
 JNIEXPORT int JNICALL
