fix getModifiers() for inner classes

Author: dougxc

src/hotspot/share/jvmci/jvmciEnv.cpp

@@ -1365,13 +1365,15 @@ JVMCIObject JVMCIEnv::get_jvmci_type(const JVMCIKlassHandle& klass, JVMCI_TRAPS)
   guarantee(klass->is_loader_alive(), "klass must be alive");
 
   jlong pointer = (jlong) klass();
+  int modifiers = klass()->modifier_flags();
   JavaThread* THREAD = JVMCI::compilation_tick(JavaThread::current()); // For exception macros.
   jboolean exception = false;
   if (is_hotspot()) {
     CompilerThreadCanCallJava ccj(THREAD, true);
     JavaValue result(T_OBJECT);
     JavaCallArguments args;
     args.push_long(pointer);
+    args.push_int(modifiers);
     JavaCalls::call_static(&result,
                            HotSpotJVMCI::HotSpotResolvedObjectTypeImpl::klass(),
                            vmSymbols::fromMetaspace_name(),
@@ -1388,7 +1390,7 @@ JVMCIObject JVMCIEnv::get_jvmci_type(const JVMCIKlassHandle& klass, JVMCI_TRAPS)
     HandleMark hm(THREAD);
     type = JNIJVMCI::wrap(jni()->CallStaticObjectMethod(JNIJVMCI::HotSpotResolvedObjectTypeImpl::clazz(),
                                                         JNIJVMCI::HotSpotResolvedObjectTypeImpl_fromMetaspace_method(),
-                                                        pointer));
+                                                        pointer, modifiers));
     exception = jni()->ExceptionCheck();
   }
   if (exception) {

src/hotspot/share/jvmci/vmSymbols_jvmci.hpp

@@ -85,7 +85,7 @@
   template(fromMetaspace_name,                                    "fromMetaspace")                                                        \
   template(method_fromMetaspace_signature,                        "(JLjdk/vm/ci/hotspot/HotSpotResolvedObjectTypeImpl;)Ljdk/vm/ci/hotspot/HotSpotResolvedJavaMethod;") \
   template(constantPool_fromMetaspace_signature,                  "(J)Ljdk/vm/ci/hotspot/HotSpotConstantPool;")                           \
-  template(klass_fromMetaspace_signature,                         "(J)Ljdk/vm/ci/hotspot/HotSpotResolvedObjectTypeImpl;")                 \
+  template(klass_fromMetaspace_signature,                         "(JI)Ljdk/vm/ci/hotspot/HotSpotResolvedObjectTypeImpl;")                \
   template(primitive_fromMetaspace_signature,                     "(Ljdk/vm/ci/hotspot/HotSpotObjectConstantImpl;C)Ljdk/vm/ci/hotspot/HotSpotResolvedPrimitiveType;") \
   template(getRuntime_name,                                       "getRuntime")                                                           \
   template(getRuntime_signature,                                  "()Ljdk/vm/ci/runtime/JVMCIRuntime;")                                   \

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotJVMCIRuntime.java

@@ -687,7 +687,7 @@ HotSpotResolvedJavaType fromClass(Class<?> javaClass) {
         return fromClass0(javaClass);
     }
 
-    synchronized HotSpotResolvedObjectTypeImpl fromMetaspace(Long klassPointer) {
+    synchronized HotSpotResolvedObjectTypeImpl fromMetaspace(Long klassPointer, int modifiers) {
         if (resolvedJavaTypes == null) {
             resolvedJavaTypes = new HashMap<>();
             resolvedJavaTypesQueue = new ReferenceQueue<>();
@@ -700,7 +700,11 @@ synchronized HotSpotResolvedObjectTypeImpl fromMetaspace(Long klassPointer) {
         }
         if (javaType == null) {
             String name = compilerToVm.getSignatureName(klassPointer);
-            javaType = new HotSpotResolvedObjectTypeImpl(klassPointer, name);
+            char charModifiers = (char) modifiers;
+            if (charModifiers != modifiers) {
+                throw new IllegalArgumentException(String.format("%x != %x", modifiers, charModifiers));
+            }
+            javaType = new HotSpotResolvedObjectTypeImpl(klassPointer, name, charModifiers);
             resolvedJavaTypes.put(klassPointer, new KlassWeakReference(klassPointer, javaType, resolvedJavaTypesQueue));
         }
         expungeStaleKlassEntries();

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

@@ -92,6 +92,13 @@ final class HotSpotResolvedObjectTypeImpl extends HotSpotResolvedJavaType implem
      */
     private FieldInfo[] fieldInfo;
 
+    /**
+     * The JVM defined modifiers. This is different from {@code Klass::_access_flags}
+     * in the case of inner classes where the modifiers are provided by an
+     * InnerClasses attribute.
+     */
+    private final char modifiers;
+
     /**
      * Managed exclusively by {@link HotSpotJDKReflection#getField}.
      */
@@ -107,12 +114,13 @@ static HotSpotResolvedObjectTypeImpl getJavaLangObject() {
      * Called from the VM.
      *
      * @param klassPointer a native pointer to the Klass*
+     * @param modifiers the {@linkplain Class#getModifiers() class modifiers}
      * @return the {@link ResolvedJavaType} corresponding to {@code javaClass}
      */
     @SuppressWarnings("unused")
     @VMEntryPoint
-    private static HotSpotResolvedObjectTypeImpl fromMetaspace(long klassPointer) {
-        return runtime().fromMetaspace(klassPointer);
+    private static HotSpotResolvedObjectTypeImpl fromMetaspace(long klassPointer, int modifiers) {
+        return runtime().fromMetaspace(klassPointer, modifiers);
     }
 
     /**
@@ -123,12 +131,14 @@ private static HotSpotResolvedObjectTypeImpl fromMetaspace(long klassPointer) {
      * </p>
      *
      * @param klass the {@code Klass*} for the type
+     * @param modifiers the {@linkplain Class#getModifiers() class modifiers}
      */
     @SuppressWarnings("try")
-    HotSpotResolvedObjectTypeImpl(long klass, String name) {
+    HotSpotResolvedObjectTypeImpl(long klass, String name, char modifiers) {
         super(name);
         assert klass != 0;
         this.klassPointer = klass;
+        this.modifiers = modifiers;
 
         // The mirror object must be in the global scope since
         // this object will be cached in HotSpotJVMCIRuntime.resolvedJavaTypes
@@ -157,16 +167,7 @@ public long getMetaspacePointer() {
 
     @Override
     public int getModifiers() {
-        if (isArray()) {
-            return (getElementalType().getModifiers() & (Modifier.PUBLIC | Modifier.PRIVATE | Modifier.PROTECTED)) | Modifier.FINAL | Modifier.ABSTRACT;
-        } else {
-            return getAccessFlags() & jvmClassModifiers();
-        }
-    }
-
-    public int getAccessFlags() {
-        HotSpotVMConfig config = config();
-        return UNSAFE.getInt(getKlassPointer() + config.klassAccessFlagsOffset);
+        return modifiers;
     }
 
     public int getMiscFlags() {
@@ -461,7 +462,7 @@ public boolean isInstanceClass() {
 
     @Override
     public boolean isInterface() {
-        return (getAccessFlags() & config().jvmAccInterface) != 0;
+        return (modifiers & config().jvmAccInterface) != 0;
     }
 
     @Override

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ModifiersProvider.java

@@ -22,6 +22,7 @@
  */
 package jdk.vm.ci.meta;
 
+import java.lang.reflect.Member;
 import java.lang.reflect.Modifier;
 
 import static java.lang.reflect.Modifier.PRIVATE;
@@ -35,7 +36,10 @@
 public interface ModifiersProvider {
 
     /**
-     * Returns the modifiers for this element.
+     * Returns the Java language modifiers for this element.
+     * 
+     * @see Class#getModifiers
+     * @see Member#getModifiers
      */
     int getModifiers();
 

test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java

@@ -225,19 +225,8 @@ public void lambdaInternalNameTest() {
     public void getModifiersTest() {
         for (Class<?> c : classes) {
             ResolvedJavaType type = metaAccess.lookupJavaType(c);
-            int mask = Modifier.classModifiers() & ~Modifier.STATIC;
-            int expected = c.getModifiers() & mask;
-            int actual = type.getModifiers() & mask;
-            Class<?> elementalType = c;
-            while (elementalType.isArray()) {
-                elementalType = elementalType.getComponentType();
-            }
-            if (elementalType.isMemberClass()) {
-                // member class get their modifiers from the inner-class attribute in the JVM and
-                // from the classfile header in jvmci
-                expected &= ~(Modifier.PUBLIC | Modifier.PRIVATE | Modifier.PROTECTED);
-                actual &= ~(Modifier.PUBLIC | Modifier.PRIVATE | Modifier.PROTECTED);
-            }
+            int expected = c.getModifiers();
+            int actual = type.getModifiers();
             assertEquals(String.format("%s: 0x%x != 0x%x", type, expected, actual), expected, actual);
         }
     }

test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TypeUniverse.java

@@ -39,6 +39,7 @@
 import java.util.HashSet;
 import java.util.IdentityHashMap;
 import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -69,10 +70,10 @@ public class TypeUniverse {
 
     public static final MetaAccessProvider metaAccess = JVMCI.getRuntime().getHostJVMCIBackend().getMetaAccess();
     public static final ConstantReflectionProvider constantReflection = JVMCI.getRuntime().getHostJVMCIBackend().getConstantReflection();
-    public static final Collection<Class<?>> classes = new HashSet<>();
+    public static final Collection<Class<?>> classes = new LinkedHashSet<>();
     public static final Set<ResolvedJavaType> javaTypes;
     public static final ResolvedJavaType predicateType;
-    public static final Map<Class<?>, Class<?>> arrayClasses = new HashMap<>();
+    public static final Map<Class<?>, Class<?>> arrayClasses = new LinkedHashMap<>();
 
     private static List<ConstantValue> constants;
 
@@ -92,10 +93,25 @@ private class PrivateInnerClass {
 
     }
 
+    private static class PrivateStaticInnerClass {
+
+    }
     protected class ProtectedInnerClass {
 
     }
 
+    protected static class ProtectedStaticInnerClass {
+
+    }
+
+    protected interface ProtectedInnerInterface {
+
+    }
+
+    protected static interface ProtectedStaticInnerInterface {
+
+    }
+
     static {
         Unsafe theUnsafe = null;
         try {
@@ -116,7 +132,7 @@ protected class ProtectedInnerClass {
                         byte[][].class, short[][].class, char[][].class, int[][].class, float[][].class, long[][].class, double[][].class, Object[][].class, Class[][].class, List[][].class,
                         ClassLoader.class, String.class, Serializable.class, Cloneable.class, Test.class, TestMetaAccessProvider.class, List.class, Collection.class, Map.class, Queue.class,
                         HashMap.class, LinkedHashMap.class, IdentityHashMap.class, AbstractCollection.class, AbstractList.class, ArrayList.class, InnerClass.class, InnerStaticClass.class,
-                        InnerStaticFinalClass.class, PrivateInnerClass.class, ProtectedInnerClass.class, ScopedMemoryAccess.class};
+                        ScopedMemoryAccess.class, TypeUniverse.class};
         for (Class<?> c : initialClasses) {
             addClass(c);
         }