fix all the bugs

So there I was, planning to just fix one little bug: Thread.holdsLock
and Thread.yield were missing for the Android class library.  Easy
enough, right?  So, I added a test, got it passing, and figured I'd go
ahead and run ci.sh with all three class libraries.  Big mistake.

Here's the stuff I found:

 * minor inconsistency in README.md about OpenSSL version

 * untested, broken Class.getEnclosingMethod (reported by Josh)

 * JNI test failed for tails=true Android build

 * Runtime.nativeExit missing for Android build

 * obsolete assertion in CallEvent broke tails=true Android build

 * obsolete superclass field offset padding broke bootimage=true Android build

 * runtime annotation parsing broke bootimage=true Android build
   (because we couldn't modify Addendum.annotationTable for classes in
   the heap image)

 * ci.sh tried building with both android=... and openjdk=..., which
   the makefile rightfully balked at

Sorry this is all in a single commit; I didn't expect so many
unrelated issues, and I'm too lazy to break them apart.

README.md

@@ -432,7 +432,7 @@ Also note that we use the upstream OpenSSL repository and apply the
 Android patches to it.  This is because it is not clear how to build
 the Android fork of OpenSSL directly without checking out and building
 the entire platform.  As of this writing, the patches apply cleanly
-against OpenSSL 1.0.1e, so that's the tag we check out, but this may
+against OpenSSL 1.0.1h, so that's the tag we check out, but this may
 change in the future when the Android fork rebases against a new
 OpenSSL version.
 

classpath/avian/ClassAddendum.java

@@ -22,9 +22,7 @@ public class ClassAddendum extends Addendum {
    */
   public int declaredMethodCount;
 
-  // Either a byte[] or a Pair, apparently...
+  public byte[] enclosingClass;
-  // TODO: make it monomorphic
-  public Object enclosingClass;
 
-  public Object enclosingMethod;
+  public Pair enclosingMethod;
 }

classpath/avian/Pair.java

@@ -0,0 +1,5 @@
+package avian;
+
+abstract class Pair {
+  // VM-visible fields in types.def
+}

classpath/java/lang/Class.java

@@ -392,6 +392,12 @@ public final class Class <T> implements Type, AnnotatedElement {
     }
   }
 
+  public native Class getEnclosingClass();
+
+  public native Method getEnclosingMethod();
+
+  public native Constructor getEnclosingConstructor();
+
   public T[] getEnumConstants() {
     if (Enum.class.isAssignableFrom(this)) {
       try {

classpath/java/lang/reflect/Method.java

@@ -25,6 +25,10 @@ public class Method<T> extends AccessibleObject implements Member {
     this.vmMethod = vmMethod;
   }
 
+  public boolean equals(Object o) {
+    return o instanceof Method && ((Method) o).vmMethod == vmMethod;
+  }
+
   public boolean isAccessible() {
     return accessible;
   }

makefile

@@ -1,7 +1,7 @@
 MAKEFLAGS = -s
 
 name = avian
-version = 1.0.1
+version = 1.0.2
 
 build-arch := $(shell uname -m \
 	| sed 's/^i.86$$/i386/' \
@@ -1355,21 +1355,22 @@ ifneq ($(classpath),avian)
 # them to synthesize a class:
 	classpath-sources := \
 		$(classpath-src)/avian/Addendum.java \
-		$(classpath-src)/avian/Code.java \
 		$(classpath-src)/avian/AnnotationInvocationHandler.java \
 		$(classpath-src)/avian/Assembler.java \
 		$(classpath-src)/avian/Callback.java \
 		$(classpath-src)/avian/Cell.java \
 		$(classpath-src)/avian/ClassAddendum.java \
-		$(classpath-src)/avian/InnerClassReference.java \
 		$(classpath-src)/avian/Classes.java \
+		$(classpath-src)/avian/Code.java \
 		$(classpath-src)/avian/ConstantPool.java \
 		$(classpath-src)/avian/Continuations.java \
 		$(classpath-src)/avian/FieldAddendum.java \
 		$(classpath-src)/avian/Function.java \
 		$(classpath-src)/avian/IncompatibleContinuationException.java \
+		$(classpath-src)/avian/InnerClassReference.java \
 		$(classpath-src)/avian/Machine.java \
 		$(classpath-src)/avian/MethodAddendum.java \
+		$(classpath-src)/avian/Pair.java \
 		$(classpath-src)/avian/Singleton.java \
 		$(classpath-src)/avian/Stream.java \
 		$(classpath-src)/avian/SystemClassLoader.java \
@@ -1919,6 +1920,7 @@ $(bootimage-generator): $(bootimage-generator-objects) $(vm-objects)
 		armv6=$(armv6) \
 		platform=$(bootimage-platform) \
 		target-format=$(target-format) \
+		android=$(android) \
 		openjdk=$(openjdk) \
 		openjdk-src=$(openjdk-src) \
 		bootimage-generator= \

src/avian/classpath-common.h

@@ -760,27 +760,6 @@ object getDeclaredClasses(Thread* t, GcClass* c, bool publicOnly)
   return makeObjectArray(t, type(t, GcJclass::Type), 0);
 }
 
-GcJclass* getDeclaringClass(Thread* t, GcClass* c)
-{
-  GcClassAddendum* addendum = c->addendum();
-  if (addendum) {
-    GcArray* table = cast<GcArray>(t, addendum->innerClassTable());
-    if (table) {
-      for (unsigned i = 0; i < table->length(); ++i) {
-        GcInnerClassReference* reference
-            = cast<GcInnerClassReference>(t, table->body()[i]);
-        if (reference->outer()
-            and strcmp(reference->inner()->body().begin(),
-                       c->name()->body().begin()) == 0) {
-          return getJClass(t, resolveClass(t, c->loader(), reference->outer()));
-        }
-      }
-    }
-  }
-
-  return 0;
-}
-
 unsigned classModifiers(Thread* t, GcClass* c)
 {
   GcClassAddendum* addendum = c->addendum();

src/avian/machine.h

@@ -2332,6 +2332,8 @@ inline void scanMethodSpec(Thread* t,
 
 GcClass* findLoadedClass(Thread* t, GcClassLoader* loader, GcByteArray* spec);
 
+GcJclass* getDeclaringClass(Thread* t, GcClass* c);
+
 inline bool emptyMethod(Thread* t UNUSED, GcMethod* method)
 {
   return ((method->flags() & ACC_NATIVE) == 0)

src/builtin.cpp

@@ -1215,3 +1215,57 @@ extern "C" AVIAN_EXPORT int64_t JNICALL
 {
   return reinterpret_cast<int64_t>(primitiveClass(t, arguments[0]));
 }
+
+extern "C" AVIAN_EXPORT int64_t JNICALL
+    Avian_java_lang_Class_getDeclaringClass(Thread* t,
+                                            object,
+                                            uintptr_t* arguments)
+{
+  return reinterpret_cast<intptr_t>(getDeclaringClass(
+      t, cast<GcJclass>(t, reinterpret_cast<object>(arguments[0]))->vmClass()));
+}
+
+extern "C" AVIAN_EXPORT int64_t JNICALL
+    Avian_java_lang_Class_getEnclosingMethod(Thread* t,
+                                             object,
+                                             uintptr_t* arguments)
+{
+  GcClass* c
+      = cast<GcJclass>(t, reinterpret_cast<object>(arguments[0]))->vmClass();
+  PROTECT(t, c);
+
+  GcClassAddendum* addendum = c->addendum();
+  if (addendum) {
+    PROTECT(t, addendum);
+
+    GcByteArray* enclosingClass
+        = cast<GcByteArray>(t, addendum->enclosingClass());
+
+    if (enclosingClass) {
+      GcClass* enclosing = resolveClass(t, c->loader(), enclosingClass);
+
+      GcPair* enclosingMethod = cast<GcPair>(t, addendum->enclosingMethod());
+
+      if (enclosingMethod) {
+        return reinterpret_cast<uintptr_t>(t->m->classpath->makeJMethod(
+            t,
+            cast<GcMethod>(
+                t,
+                findMethodInClass(
+                    t,
+                    enclosing,
+                    cast<GcByteArray>(t, enclosingMethod->first()),
+                    cast<GcByteArray>(t, enclosingMethod->second())))));
+      }
+    }
+  }
+  return 0;
+}
+
+extern "C" AVIAN_EXPORT int64_t JNICALL
+    Avian_java_lang_Class_getEnclosingConstructor(Thread* t,
+                                                  object method,
+                                                  uintptr_t* arguments)
+{
+  return Avian_java_lang_Class_getEnclosingMethod(t, method, arguments);
+}

src/classpath-android.cpp

@@ -583,13 +583,27 @@ class MyClasspath : public Classpath {
     return fieldAtOffset<int32_t>(b, field->offset());
   }
 
-  virtual bool canTailCall(Thread*,
+  virtual bool canTailCall(Thread* t UNUSED,
                            GcMethod*,
-                           GcByteArray*,
+                           GcByteArray* calleeClassName,
-                           GcByteArray*,
+                           GcByteArray* calleeMethodName,
                            GcByteArray*)
   {
-    return true;
+    // we can't tail call System.load[Library] or
+    // Runtime.load[Library] due to their use of
+    // ClassLoader.getCaller, which gets confused if we elide stack
+    // frames.
+
+    return (
+        (strcmp("loadLibrary",
+                reinterpret_cast<char*>(calleeMethodName->body().begin()))
+         and strcmp("load",
+                    reinterpret_cast<char*>(calleeMethodName->body().begin())))
+        or (strcmp("java/lang/System",
+                   reinterpret_cast<char*>(calleeClassName->body().begin()))
+            and strcmp(
+                    "java/lang/Runtime",
+                    reinterpret_cast<char*>(calleeClassName->body().begin()))));
   }
 
   virtual GcClassLoader* libraryClassLoader(Thread* t, GcMethod* caller)
@@ -1150,63 +1164,6 @@ extern "C" AVIAN_EXPORT int64_t JNICALL
       arguments[1]));
 }
 
-extern "C" AVIAN_EXPORT int64_t JNICALL
-    Avian_java_lang_Class_getDeclaringClass(Thread* t,
-                                            object,
-                                            uintptr_t* arguments)
-{
-  return reinterpret_cast<intptr_t>(getDeclaringClass(
-      t, cast<GcJclass>(t, reinterpret_cast<object>(arguments[0]))->vmClass()));
-}
-
-extern "C" AVIAN_EXPORT int64_t JNICALL
-    Avian_java_lang_Class_getEnclosingMethod(Thread* t,
-                                             object,
-                                             uintptr_t* arguments)
-{
-  GcClass* c
-      = cast<GcJclass>(t, reinterpret_cast<object>(arguments[0]))->vmClass();
-  PROTECT(t, c);
-
-  GcClassAddendum* addendum = c->addendum();
-  if (addendum) {
-    object enclosingClass = addendum->enclosingClass();
-    if (enclosingClass) {
-      PROTECT(t, enclosingClass);
-
-      // enclosingClass = getJClass
-      //   (t, resolveClass(t, classLoader(t, c), enclosingClass));
-
-      object enclosingMethod = addendum->enclosingMethod();
-      if (enclosingMethod) {
-        PROTECT(t, enclosingMethod);
-
-        abort(t);
-        // TODO: the following violates type safety; enclosingClass at this
-        // point is a GcJclass (having come from "getJClass()") - but the method
-        // expects a GcClass.
-        // Figure it out.
-
-        // return reinterpret_cast<uintptr_t>
-        //   (t->m->classpath->makeJMethod
-        //    (t, findMethodInClass
-        //     (t, cast<GcClass>(t, enclosingClass), pairFirst(t,
-        //     enclosingMethod),
-        //      pairSecond(t, enclosingMethod))));
-      }
-    }
-  }
-  return 0;
-}
-
-extern "C" AVIAN_EXPORT int64_t JNICALL
-    Avian_java_lang_Class_getEnclosingConstructor(Thread* t,
-                                                  object method,
-                                                  uintptr_t* arguments)
-{
-  return Avian_java_lang_Class_getEnclosingMethod(t, method, arguments);
-}
-
 extern "C" AVIAN_EXPORT int64_t JNICALL
     Avian_java_lang_Class_newInstanceImpl(Thread* t,
                                           object,
@@ -1417,6 +1374,14 @@ extern "C" AVIAN_EXPORT void JNICALL
   collect(t, Heap::MajorCollection);
 }
 
+extern "C" AVIAN_EXPORT void JNICALL
+    Avian_java_lang_Runtime_nativeExit(Thread* t, object, uintptr_t* arguments)
+{
+  shutDown(t);
+
+  t->m->system->exit(arguments[0]);
+}
+
 extern "C" AVIAN_EXPORT int64_t JNICALL
     Avian_java_lang_Runtime_nativeLoad(Thread* t, object, uintptr_t* arguments)
 {
@@ -1537,6 +1502,34 @@ extern "C" AVIAN_EXPORT void JNICALL
   release(t, t->javaThread->sleepLock());
 }
 
+extern "C" AVIAN_EXPORT int64_t JNICALL
+    Avian_java_lang_VMThread_holdsLock(Thread* t, object, uintptr_t* arguments)
+{
+  object vmThread = reinterpret_cast<object>(arguments[0]);
+  PROTECT(t, vmThread);
+
+  GcField* field = resolveField(
+      t, objectClass(t, vmThread), "thread", "Ljava/lang/Thread;");
+
+  if (cast<GcThread>(t, fieldAtOffset<object>(vmThread, field->offset()))
+      != t->javaThread) {
+    throwNew(t,
+             GcIllegalStateException::Type,
+             "VMThread.holdsLock may only be called on current thread");
+  }
+
+  GcMonitor* m
+      = objectMonitor(t, reinterpret_cast<object>(arguments[1]), false);
+
+  return m and m->owner() == t;
+}
+
+extern "C" AVIAN_EXPORT void JNICALL
+    Avian_java_lang_VMThread_yield(Thread* t, object, uintptr_t*)
+{
+  t->m->system->yield();
+}
+
 extern "C" AVIAN_EXPORT int64_t JNICALL
     Avian_dalvik_system_VMStack_getThreadStackTrace(Thread* t,
                                                     object,

src/classpath-avian.cpp

@@ -730,6 +730,15 @@ extern "C" AVIAN_EXPORT int64_t JNICALL
   return count;
 }
 
+extern "C" AVIAN_EXPORT int64_t JNICALL
+    Avian_java_lang_Thread_holdsLock(Thread* t, object, uintptr_t* arguments)
+{
+  GcMonitor* m
+      = objectMonitor(t, reinterpret_cast<object>(arguments[0]), false);
+
+  return m and m->owner() == t;
+}
+
 extern "C" AVIAN_EXPORT void JNICALL
     Avian_java_lang_Thread_yield(Thread* t, object, uintptr_t*)
 {

src/codegen/compiler/event.cpp

@@ -475,8 +475,6 @@ class CallEvent : public Event {
       int frameOffset;
 
       if (TailCalls and (flags & Compiler::TailJump)) {
-        assertT(c, arguments.count == 0);
-
         int base = frameBase(c);
         returnAddressIndex = base + c->arch->returnAddressOffset();
         if (UseFramePointer) {

src/machine.cpp

@@ -2820,13 +2820,14 @@ void parseAttributeTable(Thread* t,
 
       addendum->setEnclosingClass(
           t,
-          reinterpret_cast<object>(
           cast<GcReference>(t, singletonObject(t, pool, enclosingClass - 1))
-                  ->name()));
+              ->name());
 
       addendum->setEnclosingMethod(
           t,
-          enclosingMethod ? singletonObject(t, pool, enclosingMethod - 1) : 0);
+          enclosingMethod
+              ? cast<GcPair>(t, singletonObject(t, pool, enclosingMethod - 1))
+              : 0);
     } else {
       s.skip(length);
     }
@@ -5949,6 +5950,27 @@ bool threadIsInterrupted(Thread* t, GcThread* thread, bool clear)
   return v;
 }
 
+GcJclass* getDeclaringClass(Thread* t, GcClass* c)
+{
+  GcClassAddendum* addendum = c->addendum();
+  if (addendum) {
+    GcArray* table = cast<GcArray>(t, addendum->innerClassTable());
+    if (table) {
+      for (unsigned i = 0; i < table->length(); ++i) {
+        GcInnerClassReference* reference
+            = cast<GcInnerClassReference>(t, table->body()[i]);
+        if (reference->outer()
+            and strcmp(reference->inner()->body().begin(),
+                       c->name()->body().begin()) == 0) {
+          return getJClass(t, resolveClass(t, c->loader(), reference->outer()));
+        }
+      }
+    }
+  }
+
+  return 0;
+}
+
 void noop()
 {
 }

src/tools/bootimage-generator/main.cpp

@@ -51,7 +51,6 @@ const bool DebugNativeTarget = false;
 
 enum Type {
   Type_none,
-  Type_pad,
   Type_object,
   Type_object_nogc,
   Type_int8_t,
@@ -293,18 +292,18 @@ TypeMap* typeMap(Thread* t, GcHashMap* typeMaps, object p)
 
 unsigned targetFieldOffset(Thread* t, GcHashMap* typeMaps, GcField* field)
 {
-  // if (strcmp(reinterpret_cast<const char*>
+  unsigned offset
-  //            (&byteArrayBody(t, className(t, field->class_()), 0)),
+      = ((field->flags() & ACC_STATIC)
-  //            "java/lang/Throwable") == 0) trap();
+             ? typeMap(t,
-
-  return ((field->flags() & ACC_STATIC)
-              ? typeMap(
-                    t,
                        typeMaps,
                        reinterpret_cast<object>(field->class_()->staticTable()))
              : classTypeMap(
                    t, typeMaps, reinterpret_cast<object>(field->class_())))
             ->targetFixedOffsets()[field->offset()];
+
+  assertT(t, not((field->offset() == 0) xor (offset == 0)));
+
+  return offset;
 }
 
 GcTriple* makeCodeImage(Thread* t,
@@ -1284,10 +1283,11 @@ HeapWalker* makeHeapImage(Thread* t,
         if ((currentObject
              and objectClass(t, currentObject) == type(t, GcClass::Type)
              and (currentOffset * BytesPerWord) == ClassStaticTable)
-            or instanceOf(t, type(t, GcSystemClassLoader::Type), p)) {
+            or instanceOf(t, type(t, GcSystemClassLoader::Type), p)
-          // Static tables and system classloaders must be allocated
+            or instanceOf(t, type(t, GcAddendum::Type), p)) {
-          // as fixed objects in the heap image so that they can be
+          // Static tables, system classloaders, and addendums must be
-          // marked as dirty and visited during GC.  Otherwise,
+          // allocated as fixed objects in the heap image so that they
+          // can be marked as dirty and visited during GC.  Otherwise,
           // attempts to update references in these objects to point
           // to runtime-allocated memory would fail because we don't
           // scan non-fixed objects in the heap image during GC.
@@ -1492,10 +1492,8 @@ void writeBootImage2(Thread* t,
       unsigned fieldCount = 1;
       while (source[typeCount] != Type_none) {
         ++typeCount;
-        if (source[typeCount] != Type_pad) {
         ++fieldCount;
       }
-      }
 
       THREAD_RUNTIME_ARRAY(t, Field, fields, fieldCount);
 
@@ -1515,12 +1513,6 @@ void writeBootImage2(Thread* t,
       unsigned fieldOffset = 1;
       for (unsigned j = 0; j < typeCount; ++j) {
         switch (source[j]) {
-        case Type_pad:
-          type = Type_pad;
-          buildSize = 0;
-          targetSize = 0;
-          break;
-
         case Type_object:
           type = Type_object;
           buildSize = BytesPerWord;
@@ -1580,11 +1572,7 @@ void writeBootImage2(Thread* t,
           sawArray = true;
         }
 
-        if (type == Type_pad) {
+        if (not sawArray) {
-          buildOffset = pad(buildOffset, BytesPerWord);
-
-          targetOffset = pad(targetOffset, TargetBytesPerWord);
-        } else if (not sawArray) {
           buildOffset = pad(buildOffset, buildSize);
 
           targetOffset = pad(targetOffset, targetSize);

src/tools/type-generator/main.cpp

@@ -1534,17 +1534,11 @@ void writeNameInitializations(Output* out, Module& module)
 void writeMap(Output* out, Module& module, Class* cl)
 {
   std::ostringstream ss;
-  uintptr_t ownerId = 0;
   for (std::vector<Field*>::iterator it = cl->fields.begin();
        it != cl->fields.end();
        it++) {
     Field& f = **it;
 
-    if (ownerId && ownerId != f.ownerId) {
-      ss << "Type_pad, ";
-    }
-    ownerId = f.ownerId;
-
     ss << "Type_";
     ss << enumName(module, f);
     if (f.nogc) {
@@ -1556,9 +1550,6 @@ void writeMap(Output* out, Module& module, Class* cl)
 
   if (cl->arrayField) {
     Field& f = *cl->arrayField;
-    if (ownerId && ownerId != f.ownerId) {
-      ss << "Type_pad, ";
-    }
     ss << "Type_array, ";
     ss << "Type_";
     ss << enumName(module, f);

src/types.def

@@ -130,7 +130,7 @@
 (type weakHashMap
   (extends hashMap))
 
-(type pair
+(type pair avian/Pair
   (object first)
   (object second))
 

test/Reflection.java

@@ -255,6 +255,11 @@ public class Reflection {
 
     expect(Baz.class.getField("foo").getAnnotation(Ann.class) == null);
     expect(Baz.class.getField("foo").getAnnotations().length == 0);
+
+    expect(new Runnable() { public void run() { } }.getClass()
+           .getEnclosingMethod().equals
+           (Reflection.class.getMethod
+            ("main", new Class[] { String[].class })));
   }
 
   protected static class Baz {

test/Threads.java

@@ -1,4 +1,10 @@
 public class Threads implements Runnable {  
+  private static boolean success = false;
+
+  private static void expect(boolean v) {
+    if (! v) throw new RuntimeException();
+  }
+
   public static void main(String[] args) throws Exception {
     { Threads test = new Threads();
       Thread thread = new Thread(test);
@@ -40,15 +46,24 @@ public class Threads implements Runnable {
       thread.join();
     }
 
-    System.out.println("finished");
+    System.out.println("finished; success? " + success);
+
+    if (! success) {
+      System.exit(-1);
+    }
   }
 
   public void run() {
-    synchronized (this) {
     int i = 0;
     try {
+      expect(! Thread.holdsLock(this));
+      synchronized (this) {
+        expect(Thread.holdsLock(this));
+
         System.out.println("I'm running in a separate thread!");
 
+        Thread.yield(); // just to prove Thread.yield exists and is callable
+
         final int arrayCount = 16;
         final int arraySize = 4;
         System.out.println("Allocating and discarding " + arrayCount +
@@ -57,14 +72,18 @@ public class Threads implements Runnable {
           byte[] array = new byte[arraySize * 1024 * 1024];
         }
 
-        long nap = 5;
+        long nap = 500;
-        System.out.println("sleeping for " + nap + " seconds");
+        System.out.println("sleeping for " + nap + " milliseconds");
-        Thread.sleep(nap * 1000);
+        Thread.sleep(nap);
+        notifyAll();
+      }
+      success = true;
     } catch (Throwable e) {
       System.err.println("caught something in second thread after " + i +
                          " iterations");
       e.printStackTrace();
     } finally {
+      synchronized (this) {
         notifyAll();
       }
     }

test/ci.sh

@@ -25,8 +25,10 @@ run make ${flags} process=interpret ${test_target}
 if [ -z "${openjdk}" ]; then
   run make ${flags} bootimage=true ${test_target}
   run make ${flags} mode=debug bootimage=true ${test_target}
+  if [ -z "${android}" ]; then
     # might as well do an openjdk test while we're here:
     run make openjdk=$JAVA_HOME ${flags} ${test_target}
+  fi
 fi
 run make ${flags} tails=true continuations=true ${test_target}
 run make ${flags} codegen-targets=all