Skip to content

Commit

Permalink
fix all the bugs
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dicej committed Jul 12, 2014
1 parent 80f19ab commit 2a43e68
Show file tree
Hide file tree
Showing 19 changed files with 229 additions and 152 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
6 changes: 2 additions & 4 deletions classpath/avian/ClassAddendum.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ public class ClassAddendum extends Addendum {
*/
public int declaredMethodCount;

// Either a byte[] or a Pair, apparently...
// TODO: make it monomorphic
public Object enclosingClass;
public byte[] enclosingClass;

public Object enclosingMethod;
public Pair enclosingMethod;
}
5 changes: 5 additions & 0 deletions classpath/avian/Pair.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package avian;

abstract class Pair {
// VM-visible fields in types.def
}
6 changes: 6 additions & 0 deletions classpath/java/lang/Class.java
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,12 @@ public Class[] getInterfaces() {
}
}

public native Class getEnclosingClass();

public native Method getEnclosingMethod();

public native Constructor getEnclosingConstructor();

public T[] getEnumConstants() {
if (Enum.class.isAssignableFrom(this)) {
try {
Expand Down
4 changes: 4 additions & 0 deletions classpath/java/lang/reflect/Method.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ public Method(VMMethod vmMethod) {
this.vmMethod = vmMethod;
}

public boolean equals(Object o) {
return o instanceof Method && ((Method) o).vmMethod == vmMethod;
}

public boolean isAccessible() {
return accessible;
}
Expand Down
8 changes: 5 additions & 3 deletions makefile
Original file line number Diff line number Diff line change
@@ -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/' \
Expand Down Expand Up @@ -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 \
Expand Down Expand Up @@ -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= \
Expand Down
21 changes: 0 additions & 21 deletions src/avian/classpath-common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions src/avian/machine.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
54 changes: 54 additions & 0 deletions src/builtin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
115 changes: 54 additions & 61 deletions src/classpath-android.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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*,
GcByteArray* calleeClassName,
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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 9 additions & 0 deletions src/classpath-avian.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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*)
{
Expand Down
2 changes: 0 additions & 2 deletions src/codegen/compiler/event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit 2a43e68

Please sign in to comment.