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).<div><br></div><div><div>From 0b2f106489d221af52509ee794130a0e946d1186 Mon Sep 17 00:00:00 2001</div>
<div>From: Will Dietz <<a href="mailto:w@wdtz.org">w@wdtz.org</a>></div><div>Date: Fri, 14 Oct 2011 05:47:10 -0500</div><div>Subject: [PATCH] Jni.cpp: DefineClass,GetSuperClass,Throw, misc String</div><div> functions.</div>
<div><br></div><div>---</div><div> lib/J3/VMCore/Jni.cpp |  116 +++++++++++++++++++++++++++++++++++++++++-------</div><div> 1 files changed, 99 insertions(+), 17 deletions(-)</div><div><br></div><div>diff --git a/lib/J3/VMCore/Jni.cpp b/lib/J3/VMCore/Jni.cpp</div>
<div>index 5e76931..10c71ed 100644</div><div>--- a/lib/J3/VMCore/Jni.cpp</div><div>+++ b/lib/J3/VMCore/Jni.cpp</div><div>@@ -20,6 +20,7 @@</div><div> #include "JavaTypes.h"</div><div> #include "JavaUpcalls.h"</div>
<div> #include "Jnjvm.h"</div><div>+#include "Reader.h"</div><div> </div><div> using namespace j3;</div><div> </div><div>@@ -51,10 +52,30 @@ jint GetVersion(JNIEnv *env) {</div><div> }</div><div> </div>
<div> </div><div>-jclass DefineClass(JNIEnv *env, const char *name, jobject loader,</div><div>-<span class="Apple-tab-span" style="white-space:pre">                            </span>   const jbyte *buf, jsize bufLen) {</div><div>-  NYI();</div>
<div>-  abort();</div><div>+jclass DefineClass(JNIEnv *env, const char *name, jobject _loader,</div><div>+<span class="Apple-tab-span" style="white-space:pre">                         </span>   const jbyte *buf, jsize len) {</div><div>+  BEGIN_JNI_EXCEPTION</div>
<div>+</div><div>+  JavaObject * loader = _loader ? *(JavaObject**)_loader : 0;</div><div>+  llvm_gcroot(loader, 0);</div><div>+</div><div>+  jclass res;</div><div>+</div><div>+  Jnjvm* vm = JavaThread::get()->getJVM();</div>
<div>+  JnjvmClassLoader* JCL = NULL;</div><div>+  JCL = JnjvmClassLoader::getJnjvmLoaderFromJavaObject(loader, vm);</div><div><br></div><div>Merge the two lines above.</div><div><br></div><div>+</div><div>+  ClassBytes * bytes = new (JCL->allocator, len) ClassBytes(len);</div>
<div>+  memcpy(bytes->elements,buf,len);</div><div>+  const UTF8* utfName = JCL->asciizConstructUTF8(name);</div><div>+  UserClass *cl = JCL->constructClass(utfName, bytes);</div><div>+</div><div>+  if (cl) res = (jclass)cl->getClassDelegateePtr(vm);</div>
<div>+</div><div>+  RETURN_FROM_JNI(res);</div><div>+</div><div>+  END_JNI_EXCEPTION</div><div>+</div><div>   return 0;</div><div> }</div><div> </div><div>@@ -109,9 +130,25 @@ jmethodID FromReflectedMethod(JNIEnv *env, jobject method) {</div>
<div> </div><div> </div><div> jclass GetSuperclass(JNIEnv *env, jclass sub) {</div><div>-  NYI();</div><div>-  abort();</div><div>-  return 0;</div><div>+  BEGIN_JNI_EXCEPTION</div><div>+</div><div>+  JavaObject* res = 0;</div>
<div><br></div><div>res should be a jclass. You're returning a JNI ref, not a JavaObject.</div><div><br></div><div>+  JavaObject* Cl = *(JavaObject**)sub;</div><div>+  llvm_gcroot(Cl, 0);</div><div>+  llvm_gcroot(res, 0);</div>
<div>+</div><div>+  Jnjvm* vm = JavaThread::get()->getJVM();</div><div>+  UserCommonClass* cl = UserCommonClass::resolvedImplClass(vm, Cl, false);</div><div>+  if (cl->isInterface()) res = 0;</div><div>+  else {</div>
<div>+    if (cl->getSuper()) res = (JavaObject*)cl->getSuper()->getClassDelegateePtr(vm);</div><div>+    else res = 0;</div><div><br></div><div>You could save some lines by just doing:</div><div>if (!cl->isInterface() && cl->getSuper() != NULL) {</div>
<div>  res = (jcass)cl->getSuper()->getClassDelegateePtr(vm);</div><div>}</div><div><br></div><div>+  }</div><div>+  RETURN_FROM_JNI((jclass)res);</div><div>+</div><div>+  END_JNI_EXCEPTION</div><div>+</div><div>+  RETURN_FROM_JNI(0);</div>
<div> }</div><div>   </div><div>  </div><div>@@ -141,8 +178,10 @@ jboolean IsAssignableFrom(JNIEnv *env, jclass _sub, jclass _sup) {</div><div> </div><div> </div><div> jint Throw(JNIEnv *env, jthrowable obj) {</div><div>-  NYI();</div>
<div>-  abort();</div><div>+  BEGIN_JNI_EXCEPTION</div><div>+  JavaThread::get()->throwException(*(JavaObject**)obj);</div><div><br></div><div>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.</div>
<div><br></div><div>+</div><div>+  END_JNI_EXCEPTION</div><div>   return 0;</div><div> }</div><div> </div><div>@@ -2831,10 +2870,17 @@ jstring NewStringUTF(JNIEnv *env, const char *bytes) {</div><div> }</div><div> </div><div>
 </div><div>-jsize GetStringUTFLength (JNIEnv *env, jstring string) {   </div><div>-  NYI();</div><div>-  abort();</div><div>-  return 0;</div><div>+jsize GetStringUTFLength (JNIEnv *env, jstring string) {</div><div>+  BEGIN_JNI_EXCEPTION</div>
<div>+  JavaThread* th = JavaThread::get();</div><div>+</div><div>+  JavaString * s = *(JavaString**)string;</div><div>+  llvm_gcroot(s, 0);</div><div>+  // TODO: Review why this is! (Especially coupled with +1 in StringUTFRegion)</div>
<div><br></div><div>Remove the TODO?</div><div><br></div><div>+  RETURN_FROM_JNI(s->count);</div><div>+  END_JNI_EXCEPTION</div><div>+</div><div>+  RETURN_FROM_JNI(0)</div><div> }</div><div> </div><div> </div><div>@@ -3779,15 +3825,51 @@ jint GetJavaVM(JNIEnv *env, JavaVM **vm) {</div>
<div> </div><div> void GetStringRegion(JNIEnv* env, jstring str, jsize start, jsize len,</div><div>                      jchar *buf) {</div><div>-  NYI();</div><div>-  abort();</div><div>+  BEGIN_JNI_EXCEPTION</div><div>+</div>
<div>+  JavaString * s = *(JavaString**)str;</div><div>+  llvm_gcroot(s, 0);</div><div>+  UserClass * cl = JavaObject::getClass(s)->asClass();</div><div>+  const UTF8 * utf = JavaString::javaToInternal(s, cl->classLoader->hashUTF8);</div>
<div>+</div><div>+  ssize_t end = start+len;</div><div>+  if (end > utf->size) {</div><div>+    assert(0 && "Throw string out of bounds exception here!");</div><div>+  }</div><div>+</div><div>+  Jnjvm* vm = JavaThread::get()->getJVM();</div>
<div>+  UTF8Map* map = vm->bootstrapLoader->hashUTF8;</div><div>+</div><div>+  const UTF8 * result = utf->extract(map, start, start + len);</div><div>+  assert(result->size == len);</div><div>+  for(sint32 i = 0; i < len; ++i)</div>
<div>+    buf[i] = result->elements[i];</div><div>+</div><div>+  RETURN_VOID_FROM_JNI;</div><div>+  END_JNI_EXCEPTION</div><div>+  RETURN_VOID_FROM_JNI;</div><div> }</div><div> </div><div> </div><div> void GetStringUTFRegion(JNIEnv* env, jstring str, jsize start, jsize len,</div>
<div>                         char *buf) {</div><div>-  NYI();</div><div>-  abort();</div><div>+  BEGIN_JNI_EXCEPTION</div><div>+</div><div>+  JavaString * s = *(JavaString**)str;</div><div>+  llvm_gcroot(s, 0);</div><div>
+</div><div>+  int end = start+len;</div><div>+  if (end > s->count) {</div><div>+    assert(0 && "Throw string out of bounds exception here!");</div><div>+  }</div><div>+</div><div>+  char * internalStr = JavaString::strToAsciiz(s);</div>
<div>+  assert((int)strlen(internalStr) == len);</div><div>+  memcpy(buf, internalStr, len + 1);</div><div>+</div><div>+  RETURN_VOID_FROM_JNI;</div><div>+  END_JNI_EXCEPTION</div><div>+  RETURN_VOID_FROM_JNI;</div><div> }</div>
<div> </div><div> </div><div>-- </div><div>1.7.5.1</div><div><br></div><br><div class="gmail_quote">On Mon, Oct 24, 2011 at 12:53 AM, Will Dietz <span dir="ltr"><<a href="mailto:willdtz@gmail.com">willdtz@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">Sorry for the noise, just to try to stay on top of things (maybe<br>
sending all these patches in 1 thread was a bad idea...I was trying to<br>
*avoid* unnecessary clutter!)<br>
<br>
Patches outstanding:<br>
<br>
4 (updated version attached addressing cast, printf, and NATIVE_JNI<br>
issues mentioned)<br>
<br>
5, 6 --approved as-is, should I commit these?<br>
<br>
7-- the "+1" is unnecessary as far as I can tell upon looking into it<br>
further--the function should return the strlen, not the space needed<br>
for a buffer containing the string.  Sorry for the confusion.  Updated<br>
version also attached.<br>
<br>
8-- the registerNatives patch, which was sent in previous email.<br>
<br>
Thanks again! :)<br>
<font color="#888888"><br>
~Will<br>
</font></blockquote></div><br></div>