From 25d69f38ee1760824315ea0683afb2b288b6c41a Mon Sep 17 00:00:00 2001 From: Joel Dice Date: Thu, 6 Mar 2014 15:01:44 -0700 Subject: [PATCH] match Java's schizophrenic concept of inner class access modifiers An inner class has two sets of modifier flags: one is declared in the usual place in the class file and the other is part of the InnerClasses attribute. Not only is that redundant, but they can contradict, and the VM can't just pick one and roll with it. Instead, Class.getModifiers must return the InnerClasses version, whereas reflection must check the top-level version. So even if Class.getModifiers says the class is protected, it might still be public for the purpose of reflection depending on what the InnerClasses attribute says. Crazy? Yes. --- classpath/java/lang/Class.java | 14 ++++++++++++++ src/avian/classpath-common.h | 22 ++++++++++++++++++++++ src/classpath-android.cpp | 2 +- src/classpath-openjdk.cpp | 15 ++++++++++++--- src/machine.cpp | 7 ------- test/Reflection.java | 13 +++++++++++-- test/avian/TestReflection.java | 9 +++++++++ 7 files changed, 69 insertions(+), 13 deletions(-) create mode 100644 test/avian/TestReflection.java diff --git a/classpath/java/lang/Class.java b/classpath/java/lang/Class.java index 32ff0b583b..6aceca3606 100644 --- a/classpath/java/lang/Class.java +++ b/classpath/java/lang/Class.java @@ -30,6 +30,7 @@ import java.lang.annotation.Annotation; import java.io.InputStream; import java.io.IOException; import java.net.URL; +import java.util.Arrays; import java.util.ArrayList; import java.util.Map; import java.util.HashMap; @@ -439,6 +440,19 @@ public final class Class implements Type, AnnotatedElement { } public int getModifiers() { + ClassAddendum addendum = vmClass.addendum; + if (addendum != null) { + InnerClassReference[] table = addendum.innerClassTable; + if (table != null) { + for (int i = 0; i < table.length; ++i) { + InnerClassReference reference = table[i]; + if (Arrays.equals(vmClass.name, reference.inner)) { + return reference.flags; + } + } + } + } + return vmClass.flags; } diff --git a/src/avian/classpath-common.h b/src/avian/classpath-common.h index c1bb129518..994b1b1897 100644 --- a/src/avian/classpath-common.h +++ b/src/avian/classpath-common.h @@ -746,6 +746,28 @@ getDeclaringClass(Thread* t, object c) return 0; } +unsigned +classModifiers(Thread* t, object c) +{ + object addendum = classAddendum(t, c); + if (addendum) { + object table = classAddendumInnerClassTable(t, addendum); + if (table) { + for (unsigned i = 0; i < arrayLength(t, table); ++i) { + object reference = arrayBody(t, table, i); + if (0 == strcmp + (&byteArrayBody(t, className(t, c), 0), + &byteArrayBody(t, innerClassReferenceInner(t, reference), 0))) + { + return innerClassReferenceFlags(t, reference); + } + } + } + } + + return classFlags(t, c); +} + } // namespace vm #endif//CLASSPATH_COMMON_H diff --git a/src/classpath-android.cpp b/src/classpath-android.cpp index 3cbbc33292..a3c30d38ef 100644 --- a/src/classpath-android.cpp +++ b/src/classpath-android.cpp @@ -1634,7 +1634,7 @@ extern "C" AVIAN_EXPORT int64_t JNICALL Avian_java_lang_Class_getModifiers (Thread* t, object, uintptr_t* arguments) { - return classFlags + return classModifiers (t, jclassVmClass(t, reinterpret_cast(arguments[0]))); } diff --git a/src/classpath-openjdk.cpp b/src/classpath-openjdk.cpp index 91d61a2685..76e797de10 100644 --- a/src/classpath-openjdk.cpp +++ b/src/classpath-openjdk.cpp @@ -4123,12 +4123,19 @@ EXPORT(JVM_GetComponentType)(Thread* t, jclass c) return reinterpret_cast(run(t, jvmGetComponentType, arguments)); } +uint64_t +jvmGetClassModifiers(Thread* t, uintptr_t* arguments) +{ + return classModifiers + (t, jclassVmClass(t, *reinterpret_cast(arguments[0]))); +} + extern "C" AVIAN_EXPORT jint JNICALL EXPORT(JVM_GetClassModifiers)(Thread* t, jclass c) { - ENTER(t, Thread::ActiveState); + uintptr_t arguments[] = { reinterpret_cast(c) }; - return classFlags(t, jclassVmClass(t, *c)); + return run(t, jvmGetClassModifiers, arguments); } uint64_t @@ -4349,7 +4356,9 @@ EXPORT(JVM_GetClassDeclaredConstructors)(Thread* t, jclass c, extern "C" AVIAN_EXPORT jint JNICALL EXPORT(JVM_GetClassAccessFlags)(Thread* t, jclass c) { - return EXPORT(JVM_GetClassModifiers)(t, c); + ENTER(t, Thread::ActiveState); + + return classFlags(t, jclassVmClass(t, *c)); } uint64_t diff --git a/src/machine.cpp b/src/machine.cpp index 4d93e2801e..ca99b330e0 100644 --- a/src/machine.cpp +++ b/src/machine.cpp @@ -2314,13 +2314,6 @@ parseAttributeTable(Thread* t, Stream& s, object class_, object pool) flags); set(t, table, ArrayBody + (i * BytesPerWord), reference); - - if (0 == strcmp - (&byteArrayBody(t, className(t, class_), 0), - &byteArrayBody(t, innerClassReferenceInner(t, reference), 0))) - { - classFlags(t, class_) = flags; - } } object addendum = getClassAddendum(t, class_, pool); diff --git a/test/Reflection.java b/test/Reflection.java index 90d3da0eb0..8607cacc45 100644 --- a/test/Reflection.java +++ b/test/Reflection.java @@ -49,8 +49,9 @@ public class Reflection { private static void innerClasses() throws Exception { Class c = Reflection.class; Class[] inner = c.getDeclaredClasses(); - expect(1 == inner.length); - expect(Hello.class == inner[0]); + expect(2 == inner.length); + expect(Hello.class == inner[0] + || Hello.class == inner[1]); } private int egads; @@ -236,6 +237,14 @@ public class Reflection { expect((Foo.class.getMethod("toString").getModifiers() & Modifier.PUBLIC) != 0); + + expect(avian.TestReflection.get(Baz.class.getField("foo"), new Baz()) + .equals(42)); + expect((Baz.class.getModifiers() & Modifier.PUBLIC) == 0); + } + + protected static class Baz { + public int foo = 42; } } diff --git a/test/avian/TestReflection.java b/test/avian/TestReflection.java new file mode 100644 index 0000000000..7b17dff52b --- /dev/null +++ b/test/avian/TestReflection.java @@ -0,0 +1,9 @@ +package avian; + +import java.lang.reflect.Field; + +public class TestReflection { + public static Object get(Field field, Object target) throws Exception { + return field.get(target); + } +}