<div>Are the patches attached the patches outstanding? I'm a little confused :) Could you send one email per patch, and inline the patch int the email? I think that will make things easier :)</div><div><br></div><div>In any case, here are some comments on the Unsafe implementation:</div>
<div><br></div><div><div>From 77cb613be0277291c9b5592ea9548e52a608c42a 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 06:07:15 -0500</div>
<div>Subject: [PATCH] Implement a number of Java.sun.misc.Unsafe.* methods.</div><div><br></div><div>Overall these implementations are generally weak robustness-wise.</div><div>---</div><div> lib/J3/Classpath/Classpath.inc |  178 ++++++++++++++++++++++++++++++++++++++++</div>
<div> 1 files changed, 178 insertions(+), 0 deletions(-)</div><div><br></div><div>diff --git a/lib/J3/Classpath/Classpath.inc b/lib/J3/Classpath/Classpath.inc</div><div>index 792b606..e7bc2c4 100644</div><div>--- a/lib/J3/Classpath/Classpath.inc</div>
<div>+++ b/lib/J3/Classpath/Classpath.inc</div><div>@@ -15,6 +15,7 @@</div><div> #include "JavaThread.h"</div><div> #include "JavaUpcalls.h"</div><div> #include "Jnjvm.h"</div><div>+#include "Reader.h"</div>
<div> </div><div> </div><div> using namespace j3;</div><div>@@ -359,6 +360,183 @@ JavaObject* unsafe, JavaObject* obj, jlong offset, JavaObject* value) {</div><div>   mvm::Collector::objectReferenceWriteBarrier((gc*)obj, (gc**)ptr, (gc*)value);</div>
<div> }</div><div> </div><div>+JNIEXPORT jlong JNICALL Java_sun_misc_Unsafe_allocateMemory(</div><div>+JavaObject* unsafe, jlong size) {</div><div>+  // TODO: Invalid size/OOM/etc handling!</div><div>+  jlong res = 0;</div>
<div>+  BEGIN_NATIVE_EXCEPTION(0)</div><div>+  res = (jlong)malloc(size);</div><div>+  END_NATIVE_EXCEPTION</div><div>+  return res;</div><div>+}</div><div>+</div><div>+JNIEXPORT void JNICALL Java_sun_misc_Unsafe_freeMemory(</div>
<div>+JavaObject* unsafe, jlong ptr) {</div><div>+  // TODO: Exception handling...</div><div>+  BEGIN_NATIVE_EXCEPTION(0)</div><div>+  free((void*)ptr);</div><div>+  END_NATIVE_EXCEPTION</div><div>+}</div><div>+</div><div>
+JNIEXPORT void JNICALL Java_sun_misc_Unsafe_putLong__JJ(</div><div>+JavaObject* unsafe, jlong ptr, jlong value) {</div><div>+  BEGIN_NATIVE_EXCEPTION(0)</div><div>+  *(jlong*)ptr = value;</div><div>+  END_NATIVE_EXCEPTION</div>
<div>+}</div><div>+</div><div>+JNIEXPORT jbyte JNICALL Java_sun_misc_Unsafe_getByte__J(</div><div>+JavaObject* unsafe, jlong ptr) {</div><div>+  jbyte res = 0;</div><div>+  BEGIN_NATIVE_EXCEPTION(0)</div><div>+  res =  *(jbyte*)ptr;</div>
<div>+  END_NATIVE_EXCEPTION</div><div>+</div><div>+  return res;</div><div>+}</div><div>+</div><div>+JNIEXPORT void JNICALL Java_sun_misc_Unsafe_ensureClassInitialized(</div><div>+JavaObject* unsafe, JavaObject* clazz) {</div>
<div>+  llvm_gcroot(unsafe, 0);</div><div>+  llvm_gcroot(clazz, 0);</div><div>+  BEGIN_NATIVE_EXCEPTION(0)</div><div>+</div><div>+  Jnjvm* vm = JavaThread::get()->getJVM();</div><div>+</div><div>+  CommonClass * cl = JavaObject::getClass(clazz);</div>
<div>+  assert(cl && cl->isClass());</div><div>+  cl->asClass()->initialiseClass(vm);</div><div>+</div><div>+  END_NATIVE_EXCEPTION;</div><div>+}</div><div>+</div><div>+JNIEXPORT jlong JNICALL Java_sun_misc_Unsafe_staticFieldOffset(</div>
<div>+JavaObject* unsafe, JavaObjectField* _field) {</div><div>+  llvm_gcroot(unsafe, 0);</div><div>+  llvm_gcroot(_field, 0);</div><div>+</div><div>+  jlong res = 0;</div><div>+  BEGIN_NATIVE_EXCEPTION(0)</div><div>+</div>
<div>+  JavaField * field = JavaObjectField::getInternalField(_field);</div><div>+  assert(field);</div><div>+</div><div>+  res = field->ptrOffset;</div><div>+</div><div>+  END_NATIVE_EXCEPTION;</div><div>+</div><div>+  return res;</div>
<div>+}</div><div>+</div><div>+JNIEXPORT JavaObject* JNICALL Java_sun_misc_Unsafe_staticFieldBase(</div><div>+JavaObject* unsafe, JavaObjectField* _field) {</div><div>+  llvm_gcroot(unsafe, 0);</div><div>+  llvm_gcroot(_field, 0);</div>
<div>+  JavaObject* res = 0;</div><div>+  BEGIN_NATIVE_EXCEPTION(0)</div><div>+</div><div>+  JavaField * field = JavaObjectField::getInternalField(_field);</div><div>+  assert(field);</div><div>+  field->classDef->initialiseClass(JavaThread::get()->getJVM());</div>
<div>+</div><div>+  res = (JavaObject*)field->classDef->getStaticInstance();</div><div><br></div><div>Unfortunately, this does not work. The static instance of a class is a malloc'ed memory, not allocated by the GC. I looked at the comments in Unsafe.java, saying that it's fine for the method to return something that is not an object, but that is plain.... evil!</div>
<div><br></div><div>It looks like we have two options:</div><div>1) Return a dummy Object, that the 'put' and 'get' methods check for. The dummy Object is specific to the VM, and contains a reference to the static instance.</div>
<div>2) Make the static instance a GC-allocated object.</div><div><br></div><div>Both cases are tricky to implement. The first one has the advantage to be localized: you don't need to change code in the existing code base. The second one does need lots of changes in the existing code base, including in the JIT which assumes that a static instance is a non-heap object.</div>
<div><br></div><div>Therefore, I would go with option 1). Take a look at the VMClassLoader class in JnjvmClassLoader.h to see how that works. We fake up a Java Object, that will be scanned by the GC. In your case, it looks like in order to stay valid, the object must make sure that the class is not garbage collected.</div>
<div><br></div><div>+</div><div>+  END_NATIVE_EXCEPTION;</div><div>+</div><div>+  return res;</div><div>+}</div><div>+</div><div>+JNIEXPORT JavaObject* JNICALL Java_sun_misc_Unsafe_getObjectVolatile(</div><div>+JavaObject* unsafe, JavaObject* base, jlong offset) {</div>
<div>+  JavaObject * res = 0;</div><div>+  llvm_gcroot(unsafe, 0);</div><div>+  llvm_gcroot(base, 0);</div><div>+  llvm_gcroot(res, 0);</div><div>+</div><div>+  BEGIN_NATIVE_EXCEPTION(0)</div><div>+  JavaObject** ptr = (JavaObject**) (((uint8 *) base) + offset);</div>
<div>+  res = *ptr;</div><div>+  END_NATIVE_EXCEPTION;</div><div>+</div><div>+  return res;</div><div>+}</div><div>+</div><div>+JNIEXPORT jlong JNICALL Java_sun_misc_Unsafe_arrayBaseOffset(</div><div>+JavaObject* unsafe, JavaObject* clazz) {</div>
<div>+  // TODO: Implement me</div><div>+  return 0;</div><div>+}</div><div>+</div><div>+JNIEXPORT jlong JNICALL Java_sun_misc_Unsafe_arrayIndexScale(</div><div>+#ifdef NATIVE_JNI</div><div>+JNIEnv *env,</div><div>+#endif</div>
<div>+JavaObject* unsafe, JavaObject* clazz) {</div><div>+  // TODO: Implement me</div><div>+  return 0;</div><div>+}</div><div>+</div><div>+JNIEXPORT JavaObject* JNICALL Java_sun_misc_Unsafe_defineClass__Ljava_lang_String_2_3BIILjava_lang_ClassLoader_2Ljava_security_ProtectionDomain_2(</div>
<div>+JavaObject* unsafe, JavaString *name, ArrayObject * bytesArr, jint off, jint len, JavaObject * loader, JavaObject * pd) {</div><div>+  JavaObject* res = 0;</div><div>+  llvm_gcroot(res, 0);</div><div>+  llvm_gcroot(unsafe, 0);</div>
<div>+  llvm_gcroot(name, 0);</div><div>+  llvm_gcroot(bytesArr, 0);</div><div>+  llvm_gcroot(loader, 0);</div><div>+  llvm_gcroot(pd, 0);</div><div>+  BEGIN_NATIVE_EXCEPTION(0)</div><div>+</div><div>+  Jnjvm* vm = JavaThread::get()->getJVM();</div>
<div>+  JnjvmClassLoader* JCL = NULL;</div><div>+  JCL = JnjvmClassLoader::getJnjvmLoaderFromJavaObject(loader, vm);</div><div>+</div><div>+  jint last = off + len;</div><div>+  if (last < bytesArr->size) {</div><div>
+    assert(0 && "What exception to throw here?");</div><div>+  }</div><div>+  ClassBytes * bytes = new (JCL->allocator, len) ClassBytes(len);</div><div>+  memcpy(bytes->elements, JavaArray::getElements(bytesArr)+off, len);</div>
<div>+  const UTF8* utfName = JavaString::javaToInternal(name, JCL->hashUTF8);</div><div>+  UserClass *cl = JCL->constructClass(utfName, bytes);</div><div>+</div><div>+  if (cl) res = (JavaObject*)cl->getClassDelegatee(vm);</div>
<div>+</div><div>+  END_NATIVE_EXCEPTION;</div><div>+</div><div>+  return res;</div><div>+}</div><div>+</div><div>+JNIEXPORT JavaObject* JNICALL Java_sun_misc_Unsafe_allocateInstance__Ljava_lang_Class_2(</div><div>+JavaObject* unsafe, JavaObjectClass * clazz) {</div>
<div>+  JavaObject* res = 0;</div><div>+  llvm_gcroot(unsafe, 0);</div><div>+  llvm_gcroot(clazz, 0);</div><div>+  llvm_gcroot(res, 0);</div><div>+</div><div>+  BEGIN_NATIVE_EXCEPTION(0)</div><div>+</div><div>+  JavaThread* th = JavaThread::get();</div>
<div>+  Jnjvm* vm = th->getJVM();</div><div>+</div><div>+  UserCommonClass* cl = UserCommonClass::resolvedImplClass(vm, clazz, true);</div><div>+  if (cl->isClass())</div><div>+    res = cl->asClass()->doNew(vm);</div>
<div>+</div><div>+  END_NATIVE_EXCEPTION;</div><div>+</div><div>+  return res;</div><div>+}</div><div>+</div><div>+</div><div>+JNIEXPORT void JNICALL Java_sun_misc_Unsafe_throwException(</div><div>+JavaObject* unsafe, JavaObject * obj) {</div>
<div>+  llvm_gcroot(unsafe, 0);</div><div>+  llvm_gcroot(obj, 0);</div><div>+</div><div>+  JavaThread::get()->pendingException = obj;</div><div>+}</div><div>+</div><div> // TODO: Add the Volatile variants</div><div> #define GET_PUT_OFFSET(Type,jtype,shorttype) \</div>
<div> JNIEXPORT jtype JNICALL Java_sun_misc_Unsafe_get ## Type ## __Ljava_lang_Object_2J( \</div><div>-- </div><div>1.7.5.1</div></div><div><br></div><br><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>