[vmkit-commits] [PATCH] Various patches

[Nicolas Geoffray]

And some comments on the JNI methods. Code looks good, feel free to apply
once all the comments are addressed (especially the one on the Throw
method).

>From 0b2f106489d221af52509ee794130a0e946d1186 Mon Sep 17 00:00:00 2001
From: Will Dietz <w at wdtz.org>
Date: Fri, 14 Oct 2011 05:47:10 -0500
Subject: [PATCH] Jni.cpp: DefineClass,GetSuperClass,Throw, misc String
 functions.

---
 lib/J3/VMCore/Jni.cpp |  116
+++++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 99 insertions(+), 17 deletions(-)

diff --git a/lib/J3/VMCore/Jni.cpp b/lib/J3/VMCore/Jni.cpp
index 5e76931..10c71ed 100644
--- a/lib/J3/VMCore/Jni.cpp
+++ b/lib/J3/VMCore/Jni.cpp
@@ -20,6 +20,7 @@
 #include "JavaTypes.h"
 #include "JavaUpcalls.h"
 #include "Jnjvm.h"
+#include "Reader.h"

 using namespace j3;

@@ -51,10 +52,30 @@ jint GetVersion(JNIEnv *env) {
 }


-jclass DefineClass(JNIEnv *env, const char *name, jobject loader,
-   const jbyte *buf, jsize bufLen) {
-  NYI();
-  abort();
+jclass DefineClass(JNIEnv *env, const char *name, jobject _loader,
+   const jbyte *buf, jsize len) {
+  BEGIN_JNI_EXCEPTION
+
+  JavaObject * loader = _loader ? *(JavaObject**)_loader : 0;
+  llvm_gcroot(loader, 0);
+
+  jclass res;
+
+  Jnjvm* vm = JavaThread::get()->getJVM();
+  JnjvmClassLoader* JCL = NULL;
+  JCL = JnjvmClassLoader::getJnjvmLoaderFromJavaObject(loader, vm);

Merge the two lines above.

+
+  ClassBytes * bytes = new (JCL->allocator, len) ClassBytes(len);
+  memcpy(bytes->elements,buf,len);
+  const UTF8* utfName = JCL->asciizConstructUTF8(name);
+  UserClass *cl = JCL->constructClass(utfName, bytes);
+
+  if (cl) res = (jclass)cl->getClassDelegateePtr(vm);
+
+  RETURN_FROM_JNI(res);
+
+  END_JNI_EXCEPTION
+
   return 0;
 }

@@ -109,9 +130,25 @@ jmethodID FromReflectedMethod(JNIEnv *env, jobject
method) {


 jclass GetSuperclass(JNIEnv *env, jclass sub) {
-  NYI();
-  abort();
-  return 0;
+  BEGIN_JNI_EXCEPTION
+
+  JavaObject* res = 0;

res should be a jclass. You're returning a JNI ref, not a JavaObject.

+  JavaObject* Cl = *(JavaObject**)sub;
+  llvm_gcroot(Cl, 0);
+  llvm_gcroot(res, 0);
+
+  Jnjvm* vm = JavaThread::get()->getJVM();
+  UserCommonClass* cl = UserCommonClass::resolvedImplClass(vm, Cl, false);
+  if (cl->isInterface()) res = 0;
+  else {
+    if (cl->getSuper()) res =
(JavaObject*)cl->getSuper()->getClassDelegateePtr(vm);
+    else res = 0;

You could save some lines by just doing:
if (!cl->isInterface() && cl->getSuper() != NULL) {
  res = (jcass)cl->getSuper()->getClassDelegateePtr(vm);
}

+  }
+  RETURN_FROM_JNI((jclass)res);
+
+  END_JNI_EXCEPTION
+
+  RETURN_FROM_JNI(0);
 }


@@ -141,8 +178,10 @@ jboolean IsAssignableFrom(JNIEnv *env, jclass _sub,
jclass _sup) {


 jint Throw(JNIEnv *env, jthrowable obj) {
-  NYI();
-  abort();
+  BEGIN_JNI_EXCEPTION
+  JavaThread::get()->throwException(*(JavaObject**)obj);

You must set JavaThread::get()->pendingException instead of throwing
directly. Throwing directly will make you do a longjmp, to a previous CATCH
clause, and this is not what the JNI spec says. Take a look at what ThrowNew
does in the same file.

+
+  END_JNI_EXCEPTION
   return 0;
 }

@@ -2831,10 +2870,17 @@ jstring NewStringUTF(JNIEnv *env, const char *bytes)
{
 }


-jsize GetStringUTFLength (JNIEnv *env, jstring string) {
-  NYI();
-  abort();
-  return 0;
+jsize GetStringUTFLength (JNIEnv *env, jstring string) {
+  BEGIN_JNI_EXCEPTION
+  JavaThread* th = JavaThread::get();
+
+  JavaString * s = *(JavaString**)string;
+  llvm_gcroot(s, 0);
+  // TODO: Review why this is! (Especially coupled with +1 in
StringUTFRegion)

Remove the TODO?

+  RETURN_FROM_JNI(s->count);
+  END_JNI_EXCEPTION
+
+  RETURN_FROM_JNI(0)
 }


@@ -3779,15 +3825,51 @@ jint GetJavaVM(JNIEnv *env, JavaVM **vm) {

 void GetStringRegion(JNIEnv* env, jstring str, jsize start, jsize len,
                      jchar *buf) {
-  NYI();
-  abort();
+  BEGIN_JNI_EXCEPTION
+
+  JavaString * s = *(JavaString**)str;
+  llvm_gcroot(s, 0);
+  UserClass * cl = JavaObject::getClass(s)->asClass();
+  const UTF8 * utf = JavaString::javaToInternal(s,
cl->classLoader->hashUTF8);
+
+  ssize_t end = start+len;
+  if (end > utf->size) {
+    assert(0 && "Throw string out of bounds exception here!");
+  }
+
+  Jnjvm* vm = JavaThread::get()->getJVM();
+  UTF8Map* map = vm->bootstrapLoader->hashUTF8;
+
+  const UTF8 * result = utf->extract(map, start, start + len);
+  assert(result->size == len);
+  for(sint32 i = 0; i < len; ++i)
+    buf[i] = result->elements[i];
+
+  RETURN_VOID_FROM_JNI;
+  END_JNI_EXCEPTION
+  RETURN_VOID_FROM_JNI;
 }


 void GetStringUTFRegion(JNIEnv* env, jstring str, jsize start, jsize len,
                         char *buf) {
-  NYI();
-  abort();
+  BEGIN_JNI_EXCEPTION
+
+  JavaString * s = *(JavaString**)str;
+  llvm_gcroot(s, 0);
+
+  int end = start+len;
+  if (end > s->count) {
+    assert(0 && "Throw string out of bounds exception here!");
+  }
+
+  char * internalStr = JavaString::strToAsciiz(s);
+  assert((int)strlen(internalStr) == len);
+  memcpy(buf, internalStr, len + 1);
+
+  RETURN_VOID_FROM_JNI;
+  END_JNI_EXCEPTION
+  RETURN_VOID_FROM_JNI;
 }


-- 
1.7.5.1


On Mon, Oct 24, 2011 at 12:53 AM, Will Dietz <willdtz at gmail.com> wrote:

> Sorry for the noise, just to try to stay on top of things (maybe
> sending all these patches in 1 thread was a bad idea...I was trying to
> *avoid* unnecessary clutter!)
>
> Patches outstanding:
>
> 4 (updated version attached addressing cast, printf, and NATIVE_JNI
> issues mentioned)
>
> 5, 6 --approved as-is, should I commit these?
>
> 7-- the "+1" is unnecessary as far as I can tell upon looking into it
> further--the function should return the strlen, not the space needed
> for a buffer containing the string.  Sorry for the confusion.  Updated
> version also attached.
>
> 8-- the registerNatives patch, which was sent in previous email.
>
> Thanks again! :)
>
> ~Will
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/vmkit-commits/attachments/20111024/eae318a2/attachment.html>