fix Class.getDeclaredMethods

getDeclaredMethods was returning methods which were inherited from
interfaces but not (re)declared in the class itself, due to the VM's
internal use of VMClass.methodTable differing from its role in
reflection.  For reflection, we must only include the declared
methods, not the inherited but un-redeclared ones.

Previously, we saved the original method table in
ClassAddendum.methodTable before creating a new one which contains
both declared and inherited methods.  That wasted space, so this patch
replaces ClassAddendum.methodTable with
ClassAddendum.declaredMethodCount, which specifies how many of the
methods in VMClass.methodTable were declared in that class.

Alternatively, we could ensure that undeclared methods always have
their VMMethod.class_ field set to the declaring class instead of the
inheriting class.  I tried this, but it led to subtle crashes in
interface method lookup.  The rest of the VM relies not only on
VMClass.methodTable containing all inherited interface methods but
also that those methods point to the inheriting class, not the
declaring class.  Changing those assumptions would be a much bigger
(and more dangerous in terms of regression potential) effort than I
care to take on right now.  The solution I chose is a bit ugly, but
it's safe.

classpath/avian/ClassAddendum.java

@@ -13,7 +13,14 @@ package avian;
 public class ClassAddendum extends Addendum {
   public Object[] interfaceTable;
   public InnerClassReference[] innerClassTable;
-  public Object[] methodTable;
+  /**
+   * If this value is negative, all the methods in VMClass.methodTable
+   * were declared in that class.  Otherwise, only the first
+   * declaredMethodCount methods in that table were declared in that
+   * class, while the rest were declared in interfaces implemented or
+   * extended by that class.
+   */
+  public int declaredMethodCount;
   public Object enclosingClass;
   public Object enclosingMethod;
 }

classpath/avian/Classes.java

@@ -221,6 +221,18 @@ public class Classes {
     return result;
   }
 
+  private static int declaredMethodCount(VMClass c) {
+    ClassAddendum a = c.addendum;
+    if (a != null) {
+      int count = a.declaredMethodCount;
+      if (count >= 0) {
+        return count;
+      }
+    }
+    VMMethod[] table = c.methodTable;
+    return table == null ? 0 : table.length;
+  }
+
   public static void link(VMClass c, ClassLoader loader) {
     acquireClassLock();
     try {
@@ -238,9 +250,10 @@ public class Classes {
           }
         }
 
-        if (c.methodTable != null) {
-          for (int i = 0; i < c.methodTable.length; ++i) {
-            VMMethod m = c.methodTable[i];
+        VMMethod[] methodTable = c.methodTable;
+        if (methodTable != null) {
+          for (int i = 0; i < methodTable.length; ++i) {
+            VMMethod m = methodTable[i];
 
             for (int j = 1; j < m.spec.length;) {
               j = resolveSpec(loader, m.spec, j);
@@ -394,17 +407,18 @@ public class Classes {
   public static int findMethod(VMClass vmClass, String name,
                                 Class[] parameterTypes)
   {
-    if (vmClass.methodTable != null) {
+    VMMethod[] methodTable = vmClass.methodTable;
+    if (methodTable != null) {
       Classes.link(vmClass);
 
       if (parameterTypes == null) {
         parameterTypes = new Class[0];
       }
 
-      for (int i = 0; i < vmClass.methodTable.length; ++i) {
-        if (toString(vmClass.methodTable[i].name).equals(name)
-            && match(parameterTypes,
-                     getParameterTypes(vmClass.methodTable[i])))
+      for (int i = 0; i < methodTable.length; ++i) {
+        VMMethod m = methodTable[i];
+        if (toString(m.name).equals(name)
+            && match(parameterTypes, getParameterTypes(m)))
         {
           return i;
         }
@@ -415,12 +429,12 @@ public class Classes {
 
   public static int countMethods(VMClass vmClass, boolean publicOnly) {
     int count = 0;
-    if (vmClass.methodTable != null) {
-      for (int i = 0; i < vmClass.methodTable.length; ++i) {
-        if (((! publicOnly)
-             || ((vmClass.methodTable[i].flags & Modifier.PUBLIC))
-             != 0)
-            && (! toString(vmClass.methodTable[i].name).startsWith("<")))
+    VMMethod[] methodTable = vmClass.methodTable;
+    if (methodTable != null) {
+      for (int i = 0, j = declaredMethodCount(vmClass); i < j; ++i) {
+        VMMethod m = methodTable[i];
+        if (((! publicOnly) || ((m.flags & Modifier.PUBLIC)) != 0)
+            && (! toString(m.name).startsWith("<")))
         {
           ++ count;
         }
@@ -431,14 +445,15 @@ public class Classes {
 
   public static Method[] getMethods(VMClass vmClass, boolean publicOnly) {
     Method[] array = new Method[countMethods(vmClass, publicOnly)];
-    if (vmClass.methodTable != null) {
+    VMMethod[] methodTable = vmClass.methodTable;
+    if (methodTable != null) {
       Classes.link(vmClass);
 
       int ai = 0;
-      for (int i = 0; i < vmClass.methodTable.length; ++i) {
-        if ((((vmClass.methodTable[i].flags & Modifier.PUBLIC) != 0)
-             || (! publicOnly))
-            && ! toString(vmClass.methodTable[i].name).startsWith("<"))
+      for (int i = 0, j = declaredMethodCount(vmClass); i < j; ++i) {
+        VMMethod m = methodTable[i];
+        if (((! publicOnly) || ((m.flags & Modifier.PUBLIC) != 0))
+            && ! toString(m.name).startsWith("<"))
         {
           array[ai++] = makeMethod(SystemClassLoader.getClass(vmClass), i);
         }

classpath/avian/VMClass.java

@@ -24,6 +24,15 @@ public class VMClass {
   public Object[] interfaceTable;
   public VMMethod[] virtualTable;
   public VMField[] fieldTable;
+  /**
+   * Methods declared in this class, plus any abstract virtual methods
+   * inherited from implemented or extended interfaces.  If addendum
+   * is non-null and addendum.declaredMethodCount is non-negative,
+   * then the first addendum.declaredMethodCount methods are declared
+   * methods, while the rest are abstract virtual methods.  If
+   * addendum is null or addendum.declaredMethodCount is negative, all
+   * are declared methods.
+   */
   public VMMethod[] methodTable;
   public ClassAddendum addendum;
   public Object staticTable;

src/classpath-openjdk.cpp

@@ -2121,25 +2121,26 @@ interceptFileOperations(Thread* t, bool updateRuntimeData)
 }
 #endif // AVIAN_OPENJDK_SRC
 
-object
-getClassMethodTable(Thread* t, object c)
+unsigned
+classDeclaredMethodCount(Thread* t, object c)
 {
   object addendum = classAddendum(t, c);
   if (addendum) {
-    object table = classAddendumMethodTable(t, addendum);
-    if (table) {
-      return table;
+    int count = classAddendumDeclaredMethodCount(t, addendum);
+    if (count >= 0) {
+      return count;
     }
   }
-  return classMethodTable(t, c);
+  object table = classMethodTable(t, c);
+  return table == 0 ? 0 : arrayLength(t, table);
 }
 
 unsigned
 countMethods(Thread* t, object c, bool publicOnly)
 {
-  object table = getClassMethodTable(t, c);
+  object table = classMethodTable(t, c);
   unsigned count = 0;
-  for (unsigned i = 0; i < arrayLength(t, table); ++i) {
+  for (unsigned i = 0, j = classDeclaredMethodCount(t, c); i < j; ++i) {
     object vmMethod = arrayBody(t, table, i);
     if (((not publicOnly) or (methodFlags(t, vmMethod) & ACC_PUBLIC))
         and byteArrayBody(t, methodName(t, vmMethod), 0) != '<')
@@ -2171,9 +2172,9 @@ countFields(Thread* t, object c, bool publicOnly)
 unsigned
 countConstructors(Thread* t, object c, bool publicOnly)
 {
-  object table = getClassMethodTable(t, c);
+  object table = classMethodTable(t, c);
   unsigned count = 0;
-  for (unsigned i = 0; i < arrayLength(t, table); ++i) {
+  for (unsigned i = 0, j = classDeclaredMethodCount(t, c); i < j; ++i) {
     object vmMethod = arrayBody(t, table, i);
     if (((not publicOnly) or (methodFlags(t, vmMethod) & ACC_PUBLIC))
         and strcmp(reinterpret_cast<char*>
@@ -4214,7 +4215,7 @@ jvmGetClassDeclaredMethods(Thread* t, uintptr_t* arguments)
   jclass c = reinterpret_cast<jclass>(arguments[0]);
   jboolean publicOnly = arguments[1];
 
-  object table = getClassMethodTable(t, jclassVmClass(t, *c));
+  object table = classMethodTable(t, jclassVmClass(t, *c));
   if (table) {
     PROTECT(t, table);
 
@@ -4224,7 +4225,9 @@ jvmGetClassDeclaredMethods(Thread* t, uintptr_t* arguments)
     PROTECT(t, array);
 
     unsigned ai = 0;
-    for (unsigned i = 0; i < arrayLength(t, table); ++i) {
+    for (unsigned i = 0, j = classDeclaredMethodCount(t, jclassVmClass(t, *c));
+         i < j; ++i)
+    {
       object vmMethod = arrayBody(t, table, i);
       PROTECT(t, vmMethod);
 
@@ -4308,7 +4311,7 @@ jvmGetClassDeclaredConstructors(Thread* t, uintptr_t* arguments)
   jclass c = reinterpret_cast<jclass>(arguments[0]);
   jboolean publicOnly = arguments[1];
 
-  object table = getClassMethodTable(t, jclassVmClass(t, *c));
+  object table = classMethodTable(t, jclassVmClass(t, *c));
   if (table) {
     PROTECT(t, table);
 
@@ -4318,7 +4321,9 @@ jvmGetClassDeclaredConstructors(Thread* t, uintptr_t* arguments)
     PROTECT(t, array);
 
     unsigned ai = 0;
-    for (unsigned i = 0; i < arrayLength(t, table); ++i) {
+    for (unsigned i = 0, j = classDeclaredMethodCount(t, jclassVmClass(t, *c));
+         i < j; ++i)
+    {
       object vmMethod = arrayBody(t, table, i);
       PROTECT(t, vmMethod);
 

src/machine.cpp

@@ -1123,7 +1123,7 @@ getClassAddendum(Thread* t, object class_, object pool)
   if (addendum == 0) {
     PROTECT(t, class_);
 
-    addendum = makeClassAddendum(t, pool, 0, 0, 0, 0, 0, 0, 0);
+    addendum = makeClassAddendum(t, pool, 0, 0, 0, 0, -1, 0, 0);
     set(t, class_, ClassAddendum, addendum);
   }
   return addendum;
@@ -2206,13 +2206,15 @@ parseMethodTable(Thread* t, Stream& s, object class_, object pool)
     if (abstractVirtuals) {
       PROTECT(t, vtable);
 
-      object addendum = getClassAddendum(t, class_, pool);
-      set(t, addendum, ClassAddendumMethodTable,
-          classMethodTable(t, class_));
+      object originalMethodTable = classMethodTable(t, class_);
+      PROTECT(t, originalMethodTable);
 
       unsigned oldLength = classMethodTable(t, class_) ?
         arrayLength(t, classMethodTable(t, class_)) : 0;
 
+      object addendum = getClassAddendum(t, class_, pool);
+      classAddendumDeclaredMethodCount(t, addendum) = oldLength;
+
       object newMethodTable = makeArray
         (t, oldLength + listSize(t, abstractVirtuals));
 

test/Reflection.java

@@ -241,6 +241,8 @@ public class Reflection {
     expect(avian.TestReflection.get(Baz.class.getField("foo"), new Baz())
            .equals(42));
     expect((Baz.class.getModifiers() & Modifier.PUBLIC) == 0);
+
+    expect(B.class.getDeclaredMethods().length == 0);
   }
 
   protected static class Baz {
@@ -263,3 +265,9 @@ class Foo {
 }
 
 class MyException extends RuntimeException { }
+
+interface A {
+  void foo();
+}
+
+interface B extends A { }