8163105: SIGSEGV: constantPoolHandle::constantPoolHandle(ConstantPool*)

Simplify CompilerToVM.getConstantPool() and related tests.

Reviewed-by: kvn, zmajo

hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/CompilerToVM.java

@@ -265,9 +265,10 @@ final class CompilerToVM {
     native HotSpotResolvedObjectTypeImpl resolveTypeInPool(HotSpotConstantPool constantPool, int cpi) throws LinkageError;
 
     /**
-     * Looks up and attempts to resolve the {@code JVM_CONSTANT_Field} entry for at index {@code cpi} in
+     * Looks up and attempts to resolve the {@code JVM_CONSTANT_Field} entry for at index
-     * {@code constantPool}. For some opcodes, checks are performed that require the {@code method}
+     * {@code cpi} in {@code constantPool}. For some opcodes, checks are performed that require the
-     * that contains {@code opcode} to be specified. The values returned in {@code info} are:
+     * {@code method} that contains {@code opcode} to be specified. The values returned in
+     * {@code info} are:
      *
      * <pre>
      *     [(int) flags,   // only valid if field is resolved
@@ -556,20 +557,18 @@ final class CompilerToVM {
     native HotSpotResolvedJavaMethodImpl getResolvedJavaMethod(Object base, long displacement);
 
     /**
-     * Read a HotSpot ConstantPool* value from the memory location described by {@code base} plus
+     * Gets the {@code ConstantPool*} associated with {@code object} and returns a
-     * {@code displacement} and return the {@link HotSpotConstantPool} wrapping it. This method does
+     * {@link HotSpotConstantPool} wrapping it.
-     * no checking that the memory location actually contains a valid pointer and may crash the VM
-     * if an invalid location is provided. If the {@code base} is null then {@code displacement} is
-     * used by itself. If {@code base} is a {@link HotSpotResolvedJavaMethodImpl},
-     * {@link HotSpotConstantPool} or {@link HotSpotResolvedObjectTypeImpl} then the metaspace
-     * pointer is fetched from that object and added to {@code displacement}. Any other non-null
-     * object type causes an {@link IllegalArgumentException} to be thrown.
      *
-     * @param base an object to read from or null
+     * @param object a {@link HotSpotResolvedJavaMethodImpl} or
-     * @param displacement
+     *            {@link HotSpotResolvedObjectTypeImpl} object
-     * @return null or the resolved method for this location
+     * @return a {@link HotSpotConstantPool} wrapping the {@code ConstantPool*} associated with
+     *         {@code object}
+     * @throws NullPointerException if {@code object == null}
+     * @throws IllegalArgumentException if {@code object} is neither a
+     *             {@link HotSpotResolvedJavaMethodImpl} nor a {@link HotSpotResolvedObjectTypeImpl}
      */
-    native HotSpotConstantPool getConstantPool(Object base, long displacement);
+    native HotSpotConstantPool getConstantPool(Object object);
 
     /**
      * Read a HotSpot Klass* value from the memory location described by {@code base} plus

hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedJavaMethodImpl.java

@@ -121,7 +121,7 @@ final class HotSpotResolvedJavaMethodImpl extends HotSpotMethod implements HotSp
         if (metaspaceConstantPool == holder.getConstantPool().getMetaspaceConstantPool()) {
             this.constantPool = holder.getConstantPool();
         } else {
-            this.constantPool = compilerToVM().getConstantPool(null, constMethod + config.constMethodConstantsOffset);
+            this.constantPool = compilerToVM().getConstantPool(this);
         }
 
         final int nameIndex = UNSAFE.getChar(constMethod + config.constMethodNameIndexOffset);

hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedObjectTypeImpl.java

@@ -445,7 +445,7 @@ final class HotSpotResolvedObjectTypeImpl extends HotSpotResolvedJavaType implem
              * operating on the latest one and that HotSpotResolvedJavaMethodImpls will be able to
              * use the shared copy instead of creating their own instance.
              */
-            constantPool = compilerToVM().getConstantPool(this, config().instanceKlassConstantsOffset);
+            constantPool = compilerToVM().getConstantPool(this);
         }
         return constantPool;
     }

hotspot/src/share/vm/jvmci/jvmciCompilerToVM.cpp

@@ -461,31 +461,26 @@ C2V_VMENTRY(jobject, getResolvedJavaMethod, (JNIEnv *, jobject, jobject base, jl
   return JNIHandles::make_local(THREAD, result);
 }
 
-C2V_VMENTRY(jobject, getConstantPool, (JNIEnv *, jobject, jobject base, jlong offset))
+C2V_VMENTRY(jobject, getConstantPool, (JNIEnv *, jobject, jobject object_handle))
   constantPoolHandle cp;
-  oop base_object = JNIHandles::resolve(base);
+  oop object = JNIHandles::resolve(object_handle);
-  jlong base_address = 0;
+  if (object == NULL) {
-  if (base_object != NULL) {
+    THROW_0(vmSymbols::java_lang_NullPointerException());
-    if (base_object->is_a(SystemDictionary::HotSpotResolvedJavaMethodImpl_klass())) {
-      base_address = HotSpotResolvedJavaMethodImpl::metaspaceMethod(base_object);
-    } else if (base_object->is_a(SystemDictionary::HotSpotConstantPool_klass())) {
-      base_address = HotSpotConstantPool::metaspaceConstantPool(base_object);
-    } else if (base_object->is_a(SystemDictionary::HotSpotResolvedObjectTypeImpl_klass())) {
-      base_address = (jlong) CompilerToVM::asKlass(base_object);
-    } else {
-      THROW_MSG_0(vmSymbols::java_lang_IllegalArgumentException(),
-                  err_msg("Unexpected type: %s", base_object->klass()->external_name()));
-    }
   }
-  cp = *((ConstantPool**) (intptr_t) (base_address + offset));
+  if (object->is_a(SystemDictionary::HotSpotResolvedJavaMethodImpl_klass())) {
-  if (!cp.is_null()) {
+    cp = CompilerToVM::asMethod(object)->constMethod()->constants();
-    JavaValue method_result(T_OBJECT);
+  } else if (object->is_a(SystemDictionary::HotSpotResolvedObjectTypeImpl_klass())) {
-    JavaCallArguments args;
+    cp = InstanceKlass::cast(CompilerToVM::asKlass(object))->constants();
-    args.push_long((jlong) (address) cp());
+  } else {
-    JavaCalls::call_static(&method_result, SystemDictionary::HotSpotConstantPool_klass(), vmSymbols::fromMetaspace_name(), vmSymbols::constantPool_fromMetaspace_signature(), &args, CHECK_NULL);
+    THROW_MSG_0(vmSymbols::java_lang_IllegalArgumentException(),
-    return JNIHandles::make_local(THREAD, (oop)method_result.get_jobject());
+                err_msg("Unexpected type: %s", object->klass()->external_name()));
   }
-  return NULL;
+  assert(!cp.is_null(), "npe");
+  JavaValue method_result(T_OBJECT);
+  JavaCallArguments args;
+  args.push_long((jlong) (address) cp());
+  JavaCalls::call_static(&method_result, SystemDictionary::HotSpotConstantPool_klass(), vmSymbols::fromMetaspace_name(), vmSymbols::constantPool_fromMetaspace_signature(), &args, CHECK_NULL);
+  return JNIHandles::make_local(THREAD, (oop)method_result.get_jobject());
 }
 
 C2V_VMENTRY(jobject, getResolvedJavaType, (JNIEnv *, jobject, jobject base, jlong offset, jboolean compressed))
@@ -1522,7 +1517,7 @@ JNINativeMethod CompilerToVM::methods[] = {
   {CC "getMaxCallTargetOffset",                       CC "(J)J",                                                                            FN_PTR(getMaxCallTargetOffset)},
   {CC "getResolvedJavaMethodAtSlot",                  CC "(" CLASS "I)" HS_RESOLVED_METHOD,                                                 FN_PTR(getResolvedJavaMethodAtSlot)},
   {CC "getResolvedJavaMethod",                        CC "(Ljava/lang/Object;J)" HS_RESOLVED_METHOD,                                        FN_PTR(getResolvedJavaMethod)},
-  {CC "getConstantPool",                              CC "(Ljava/lang/Object;J)" HS_CONSTANT_POOL,                                          FN_PTR(getConstantPool)},
+  {CC "getConstantPool",                              CC "(Ljava/lang/Object;)" HS_CONSTANT_POOL,                                           FN_PTR(getConstantPool)},
   {CC "getResolvedJavaType",                          CC "(Ljava/lang/Object;JZ)" HS_RESOLVED_KLASS,                                        FN_PTR(getResolvedJavaType)},
   {CC "readConfiguration",                            CC "()[" OBJECT,                                                                      FN_PTR(readConfiguration)},
   {CC "installCode",                                  CC "(" TARGET_DESCRIPTION HS_COMPILED_CODE INSTALLED_CODE HS_SPECULATION_LOG ")I",    FN_PTR(installCode)},

hotspot/test/compiler/jvmci/common/patches/jdk.vm.ci/jdk/vm/ci/hotspot/CompilerToVMHelper.java

@@ -273,8 +273,8 @@ public class CompilerToVMHelper {
         return CTVM.getResolvedJavaMethod(base, displacement);
     }
 
-    public static HotSpotConstantPool getConstantPool(Object base, long displacement) {
+    public static HotSpotConstantPool getConstantPool(Object object) {
-        return CTVM.getConstantPool(base, displacement);
+        return CTVM.getConstantPool(object);
     }
 
     public static HotSpotResolvedObjectType getResolvedJavaType(Object base,

hotspot/test/compiler/jvmci/compilerToVM/GetConstantPoolTest.java

@@ -29,187 +29,76 @@
  * @library /testlibrary /test/lib /
  * @library ../common/patches
  * @modules java.base/jdk.internal.misc
- * @modules jdk.vm.ci/jdk.vm.ci.hotspot
+ * @modules java.base/jdk.internal.org.objectweb.asm
+            java.base/jdk.internal.org.objectweb.asm.tree
+            jdk.vm.ci/jdk.vm.ci.hotspot
  *          jdk.vm.ci/jdk.vm.ci.meta
+ *          jdk.vm.ci/jdk.vm.ci.code
  *
  * @build jdk.vm.ci/jdk.vm.ci.hotspot.CompilerToVMHelper
- * @build jdk.vm.ci/jdk.vm.ci.hotspot.PublicMetaspaceWrapperObject
  * @build compiler.jvmci.compilerToVM.GetConstantPoolTest
- * @build sun.hotspot.WhiteBox
- * @run driver ClassFileInstaller sun.hotspot.WhiteBox
- *                                sun.hotspot.WhiteBox$WhiteBoxPermission
  * @run main/othervm -Xbootclasspath/a:.
- *                   -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
+ *                   -XX:+UnlockDiagnosticVMOptions
  *                   -XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI
  *                   compiler.jvmci.compilerToVM.GetConstantPoolTest
  */
 package compiler.jvmci.compilerToVM;
 
-import jdk.internal.misc.Unsafe;
 import jdk.test.lib.Utils;
+import compiler.jvmci.common.CTVMUtilities;
+import compiler.jvmci.common.testcases.TestCase;
 import jdk.vm.ci.hotspot.CompilerToVMHelper;
 import jdk.vm.ci.hotspot.HotSpotResolvedJavaMethod;
-import jdk.vm.ci.hotspot.PublicMetaspaceWrapperObject;
+import jdk.vm.ci.hotspot.HotSpotResolvedObjectType;
 import jdk.vm.ci.meta.ConstantPool;
-import sun.hotspot.WhiteBox;
 
 import java.lang.reflect.Field;
+import java.lang.reflect.Executable;
 
 /**
  * Tests for jdk.vm.ci.hotspot.CompilerToVM::getConstantPool method
  */
 public class GetConstantPoolTest {
-    private static enum TestCase {
-        NULL_BASE {
-            @Override
-            ConstantPool getConstantPool() {
-                return CompilerToVMHelper.getConstantPool(null,
-                        getPtrToCpAddress());
-            }
-        },
-        JAVA_METHOD_BASE {
-            @Override
-            ConstantPool getConstantPool() {
-                HotSpotResolvedJavaMethod methodInstance
-                        = CompilerToVMHelper.getResolvedJavaMethodAtSlot(
-                                TEST_CLASS, 0);
-                Field field;
-                try {
-                    // jdk.vm.ci.hotspot.HotSpotResolvedJavaMethodImpl.metaspaceMethod
-                    field = methodInstance.getClass()
-                            .getDeclaredField("metaspaceMethod");
-                    field.setAccessible(true);
-                    field.set(methodInstance, getPtrToCpAddress());
-                } catch (ReflectiveOperationException e) {
-                    throw new Error("TESTBUG : " + e, e);
-                }
 
-                return CompilerToVMHelper.getConstantPool(methodInstance, 0L);
+    public static void testMethod(Executable executable) {
-            }
+        test(CTVMUtilities.getResolvedMethod(executable));
-        },
-        CONSTANT_POOL_BASE {
-            @Override
-            ConstantPool getConstantPool() {
-                ConstantPool cpInst;
-                try {
-                    cpInst = CompilerToVMHelper.getConstantPool(null,
-                            getPtrToCpAddress());
-                    Field field = CompilerToVMHelper.HotSpotConstantPoolClass()
-                            .getDeclaredField("metaspaceConstantPool");
-                    field.setAccessible(true);
-                    field.set(cpInst, getPtrToCpAddress());
-                } catch (ReflectiveOperationException e) {
-                    throw new Error("TESTBUG : " + e.getMessage(), e);
-                }
-                return CompilerToVMHelper.getConstantPool(cpInst, 0L);
-            }
-        },
-        CONSTANT_POOL_BASE_IN_TWO {
-            @Override
-            ConstantPool getConstantPool() {
-                long ptr = getPtrToCpAddress();
-                ConstantPool cpInst;
-                try {
-                    cpInst = CompilerToVMHelper.getConstantPool(null, ptr);
-                    Field field = CompilerToVMHelper.HotSpotConstantPoolClass()
-                            .getDeclaredField("metaspaceConstantPool");
-                    field.setAccessible(true);
-                    field.set(cpInst, ptr / 2L);
-                } catch (ReflectiveOperationException e) {
-                    throw new Error("TESTBUG : " + e.getMessage(), e);
-                }
-                return CompilerToVMHelper.getConstantPool(cpInst,
-                        ptr - ptr / 2L);
-            }
-        },
-        CONSTANT_POOL_BASE_ZERO {
-            @Override
-            ConstantPool getConstantPool() {
-                long ptr = getPtrToCpAddress();
-                ConstantPool cpInst;
-                try {
-                    cpInst = CompilerToVMHelper.getConstantPool(null, ptr);
-                    Field field = CompilerToVMHelper.HotSpotConstantPoolClass()
-                            .getDeclaredField("metaspaceConstantPool");
-                    field.setAccessible(true);
-                    field.set(cpInst, 0L);
-                } catch (ReflectiveOperationException e) {
-                    throw new Error("TESTBUG : " + e.getMessage(), e);
-                }
-                return CompilerToVMHelper.getConstantPool(cpInst, ptr);
-            }
-        },
-        ;
-        abstract ConstantPool getConstantPool();
     }
 
-    private static final WhiteBox WB = WhiteBox.getWhiteBox();
+    public static void testClass(Class cls) {
-    private static final Unsafe UNSAFE = Utils.getUnsafe();
+        HotSpotResolvedObjectType type = CompilerToVMHelper
+                .lookupType(Utils.toJVMTypeSignature(cls),
+                        GetConstantPoolTest.class, /* resolve = */ true);
+        test(type);
+    }
 
-    private static final Class TEST_CLASS = GetConstantPoolTest.class;
+    private static void test(Object object) {
-    private static final long CP_ADDRESS
+        ConstantPool cp = CompilerToVMHelper.getConstantPool(object);
-            = WB.getConstantPool(GetConstantPoolTest.class);
+        System.out.println(object + " -> " + cp);
-
-    public void test(TestCase testCase) {
-        System.out.println(testCase.name());
-        ConstantPool cp = testCase.getConstantPool();
-        String cpStringRep = cp.toString();
-        String cpClassSimpleName
-                = CompilerToVMHelper.HotSpotConstantPoolClass().getSimpleName();
-        if (!cpStringRep.contains(cpClassSimpleName)
-                || !cpStringRep.contains(TEST_CLASS.getName())) {
-            String msg = String.format("%s : "
-                    + " Constant pool is not valid."
-                    + " String representation should contain \"%s\" and \"%s\"",
-                    testCase.name(), cpClassSimpleName,
-                    TEST_CLASS.getName());
-            throw new AssertionError(msg);
-        }
     }
 
     public static void main(String[] args) {
-        GetConstantPoolTest test = new GetConstantPoolTest();
+        TestCase.getAllClasses().forEach(GetConstantPoolTest::testClass);
-        for (TestCase testCase : TestCase.values()) {
+        TestCase.getAllExecutables().forEach(GetConstantPoolTest::testMethod);
-            test.test(testCase);
+        testNull();
-        }
+        testObject();
-        testObjectBase();
-        testMetaspaceWrapperBase();
     }
 
-    private static void testObjectBase() {
+    private static void testNull() {
         try {
-            Object cp = CompilerToVMHelper.getConstantPool(new Object(), 0L);
+            Object cp = CompilerToVMHelper.getConstantPool(null);
-            throw new AssertionError("Test OBJECT_BASE."
+            throw new AssertionError("Test OBJECT."
+                + " Expected IllegalArgumentException has not been caught");
+        } catch (NullPointerException npe) {
+            // expected
+        }
+    }
+    private static void testObject() {
+        try {
+            Object cp = CompilerToVMHelper.getConstantPool(new Object());
+            throw new AssertionError("Test OBJECT."
                 + " Expected IllegalArgumentException has not been caught");
         } catch (IllegalArgumentException iae) {
             // expected
         }
     }
-    private static void testMetaspaceWrapperBase() {
-        try {
-            Object cp = CompilerToVMHelper.getConstantPool(
-                    new PublicMetaspaceWrapperObject() {
-                        @Override
-                        public long getMetaspacePointer() {
-                            return getPtrToCpAddress();
-                        }
-                    }, 0L);
-            throw new AssertionError("Test METASPACE_WRAPPER_BASE."
-                + " Expected IllegalArgumentException has not been caught");
-        } catch (IllegalArgumentException iae) {
-            // expected
-        }
-    }
-
-    private static long getPtrToCpAddress() {
-        Field field;
-        try {
-            field = TEST_CLASS.getDeclaredField("CP_ADDRESS");
-        } catch (NoSuchFieldException nsfe) {
-            throw new Error("TESTBUG : cannot find field \"CP_ADDRESS\" : "
-                    + nsfe.getMessage(), nsfe);
-        }
-        Object base = UNSAFE.staticFieldBase(field);
-        return WB.getObjectAddress(base) + UNSAFE.staticFieldOffset(field);
-    }
 }