<div>Looks good with some comments. Don't mind the code sharing, I'm fine duplicating this kind of code. It so happens that sometimes the interface looks the same, but that's just pure luck.</div><div><br></div>
On Thu, Oct 27, 2011 at 4:20 AM, Will Dietz <span dir="ltr"><<a href="mailto:wdietz2@illinois.edu">wdietz2@illinois.edu</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Inlined below.<br>
<br>
While very similar, didn't see a particularly good way to share this<br>
with the Classpath version of the same, as I recall.<br>
<br>
~Will<br>
<br>
<br>
>From 502715afce04e56d3c8c30156a1c670dfb62b17e Mon Sep 17 00:00:00 2001<br>
From: Will Dietz <<a href="mailto:w@wdtz.org">w@wdtz.org</a>><br>
Date: Wed, 26 Oct 2011 21:11:48 -0500<br>
Subject: [PATCH 2/2] Add support for stack traces in OpenJDK<br>
<br>
---<br>
 lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp |   51 ++++++++++-<br>
 lib/J3/ClassLib/OpenJDK/ClasspathReflect.h   |   18 ++--<br>
 lib/J3/ClassLib/OpenJDK/OpenJDK.inc          |  134 +++++++++++++++++++++++++-<br>
 3 files changed, 190 insertions(+), 13 deletions(-)<br>
<br>
diff --git a/lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp<br>
b/lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp<br>
index 31dd417..a4f9ab2 100644<br>
--- a/lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp<br>
+++ b/lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp<br>
@@ -11,6 +11,10 @@<br>
 #include "JavaClass.h"<br>
 #include "JavaObject.h"<br>
 #include "JavaThread.h"<br>
+#include "Jnjvm.h"<br>
+#include "JavaUpcalls.h"<br>
+<br>
+#include <map><br>
<br>
 namespace j3 {<br>
<br>
@@ -21,11 +25,56 @@ JavaMethod*<br>
JavaObjectConstructor::getInternalMethod(JavaObjectConstructor* self<br>
   return &(cls->asClass()->virtualMethods[self->slot]);<br>
 }<br>
<br>
-<br>
 JavaMethod* JavaObjectMethod::getInternalMethod(JavaObjectMethod* self) {<br>
   llvm_gcroot(self, 0);<br>
   UserCommonClass* cls = JavaObjectClass::getClass(self->declaringClass);<br>
   return &(cls->asClass()->virtualMethods[self->slot]);<br>
 }<br>
<br>
+int JavaObjectThrowable::getStackTraceBase(JavaObjectThrowable * self) {<br>
+  if (!self->backtrace) return 0;<br>
+<br>
+  Jnjvm* vm = JavaThread::get()->getJVM();<br>
+<br>
+  JavaObject * stack = self->backtrace;<br>
+  llvm_gcroot(self, 0);<br>
+  llvm_gcroot(stack, 0);<br></blockquote><div><br></div><div>This is not written anywhere, but I usually follow the rule to declare GC variables and llvm_gcroot as the first things in a method. This makes it easier to review the code and find potential GC bugs.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
+  sint32 index = 2;;<br>
+  while (index != JavaArray::getSize(stack)) {<br>
+    mvm::FrameInfo* FI =<br>
vm->IPToFrameInfo(ArrayPtr::getElement((ArrayPtr*)stack, index));<br>
+    if (FI->Metadata == NULL) ++index;<br>
+    else {<br>
+      JavaMethod* meth = (JavaMethod*)FI->Metadata;<br>
+      assert(meth && "Wrong stack trace");<br>
+      if (meth->classDef->isAssignableFrom(vm->upcalls->newThrowable)) {<br>
+        ++index;<br>
+      } else return index;<br>
+    }<br>
+  }<br>
+<br>
+  assert(0 && "Invalid stack trace!");<br>
+  return 0;<br>
+}<br>
+<br>
+int JavaObjectThrowable::getStackTraceDepth(JavaObjectThrowable * self) {<br>
+  if (!self->backtrace) return 0;<br>
+<br>
+  Jnjvm* vm = JavaThread::get()->getJVM();<br>
+<br>
+  JavaObject * stack = self->backtrace;<br>
+  llvm_gcroot(self, 0);<br>
+  llvm_gcroot(stack, 0);<br></blockquote><div><br></div><div>Same here.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
+  sint32 index = getStackTraceBase(self);<br>
+<br>
+  sint32 size = 0;<br>
+  sint32 cur = index;<br>
+  while (cur < JavaArray::getSize(stack)) {<br>
+    mvm::FrameInfo* FI =<br>
vm->IPToFrameInfo(ArrayPtr::getElement((ArrayPtr*)stack, cur));<br>
+    ++cur;<br>
+    if (FI->Metadata != NULL) ++size;<br>
+  }<br>
+<br>
+  return size;<br>
+}<br>
+<br>
 }<br>
diff --git a/lib/J3/ClassLib/OpenJDK/ClasspathReflect.h<br>
b/lib/J3/ClassLib/OpenJDK/ClasspathReflect.h<br>
index 101e726..d371fdc 100644<br>
--- a/lib/J3/ClassLib/OpenJDK/ClasspathReflect.h<br>
+++ b/lib/J3/ClassLib/OpenJDK/ClasspathReflect.h<br>
@@ -157,18 +157,18 @@ public:<br>
     llvm_gcroot(self, 0);<br>
     llvm_gcroot(stackTrace, 0);<br>
<br>
-    // TODO: Implement this right<br>
-    assert(0 && "fillInStackTrace not implemented yet!");<br>
-<br>
-    // stackTrace = internalFillInStackTrace(self);<br>
-    // mvm::Collector::objectReferenceWriteBarrier(<br>
-    //     (gc*)self, (gc**)&(self->vmState), (gc*)stackTrace);<br>
+    stackTrace = internalFillInStackTrace(self);<br>
+    mvm::Collector::objectReferenceWriteBarrier(<br>
+        (gc*)self, (gc**)&(self->backtrace), (gc*)stackTrace);<br>
<br>
-    // mvm::Collector::objectReferenceWriteBarrier(<br>
-    //     (gc*)self, (gc**)&(self->cause), (gc*)self);<br>
+    mvm::Collector::objectReferenceWriteBarrier(<br>
+        (gc*)self, (gc**)&(self->cause), (gc*)self);<br>
<br>
-    // self->stackTrace = NULL;<br>
+    self->stackTrace = NULL;<br>
   }<br>
+<br>
+  static int getStackTraceDepth(JavaObjectThrowable * self);<br>
+  static int getStackTraceBase(JavaObjectThrowable * self);<br>
 };<br>
<br>
 class JavaObjectReference : public JavaObject {<br>
diff --git a/lib/J3/ClassLib/OpenJDK/OpenJDK.inc<br>
b/lib/J3/ClassLib/OpenJDK/OpenJDK.inc<br>
index 8ff8035..cbdd3d9 100644<br>
--- a/lib/J3/ClassLib/OpenJDK/OpenJDK.inc<br>
+++ b/lib/J3/ClassLib/OpenJDK/OpenJDK.inc<br>
@@ -17,6 +17,77 @@<br>
 #define NYI() \<br>
   do{ assert(0 && "Not yet implemented!"); abort(); } while(0)<br>
<br>
+JavaObject* internalFillInStackTrace(JavaObject* throwable) {<br>
+<br>
+  ArrayPtr* result = 0;<br>
+  llvm_gcroot(throwable, 0);<br>
+  llvm_gcroot(result, 0);<br>
+<br>
+  JavaThread* th = JavaThread::get();<br>
+  Jnjvm* vm = th->getJVM();<br>
+  assert(th);<br>
+  assert(vm);<br>
+<br>
+  uint32 length = th->getFrameContextLength();<br>
+<br>
+  if (sizeof(void*) == 4)</blockquote><div><br></div><div>I know this probably comes from my code, but please use the macros ARCH_32 and ARCH_64 for these kinds of checks.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
 {<br>
+    ClassArray* cl = vm->upcalls->ArrayOfInt;<br>
+    result = (ArrayPtr*) cl->doNew(length, vm);<br>
+  } else {<br>
+    ClassArray* cl = vm->upcalls->ArrayOfLong;<br>
+    result = (ArrayPtr*) cl->doNew(length, vm);<br>
+  }<br>
+<br>
+  // Don't call th->getFrameContext because it is not GC-safe.<br>
+  mvm::StackWalker Walker(th);<br>
+  uint32_t i = 0;<br>
+<br>
+  while (intptr_t ip = *Walker) {<br>
+    ArrayPtr::setElement(result, ip, i);<br>
+    ++i;<br>
+    ++Walker;<br>
+  }<br>
+<br>
+  return result;<br>
+}<br>
+<br>
+JavaObject* consStackElement(mvm::FrameInfo* FI, intptr_t ip) {<br>
+<br>
+  JavaString* methodName = 0;<br>
+  JavaString* className = 0;<br>
+  JavaString* sourceName = 0;<br>
+  JavaObject* res = 0;<br>
+  llvm_gcroot(methodName, 0);<br>
+  llvm_gcroot(className, 0);<br>
+  llvm_gcroot(sourceName, 0);<br>
+  llvm_gcroot(res, 0);<br>
+<br>
+  Jnjvm* vm = JavaThread::get()->getJVM();<br>
+  JavaMethod* meth = (JavaMethod*)FI->Metadata;<br>
+  methodName = vm->internalUTF8ToStr(meth->name);<br>
+  Class* cl = meth->classDef;<br>
+  className = JavaString::internalToJava(cl->name, vm);<br>
+<br>
+  Attribut* sourceAtt = cl->lookupAttribut(Attribut::sourceFileAttribut);<br>
+<br>
+  // We don't have the bytes if the class was vmjc'ed.<br></blockquote><div><br></div><div>Ah that's not true anymore :) Please remove the cl->getBytes() check.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

+  if (sourceAtt && cl->getBytes()) {<br>
+    Reader reader(sourceAtt, cl->bytes);<br>
+    uint16 index = reader.readU2();<br>
+    sourceName = vm->internalUTF8ToStr(cl->getConstantPool()->UTF8At(index));<br>
+  }<br>
+<br>
+  uint16 lineNumber = meth->lookupLineNumber(FI);<br>
+<br>
+  UserClass* newS = vm->upcalls->newStackTraceElement;<br>
+  res = newS->doNew(vm);<br>
+  vm->upcalls->initStackTraceElement->invokeIntSpecial(vm, newS, res,<br>
+                                                       &className,<br>
+                                                       &methodName,<br>
+                                                       &sourceName,<br>
+                                                       lineNumber);<br>
+  return res;<br>
+}<br>
<br>
 JNIEXPORT jint JNICALL<br>
 JVM_GetInterfaceVersion(void) {<br>
@@ -261,7 +332,17 @@ JVM_IsNaN(jdouble d) {<br>
  */<br>
 JNIEXPORT void JNICALL<br>
 JVM_FillInStackTrace(JNIEnv *env, jobject throwable) {<br>
-  NYI();<br>
+  JavaObjectThrowable * T = 0;<br>
+  llvm_gcroot(T, 0);<br>
+<br>
+  BEGIN_JNI_EXCEPTION<br>
+<br>
+  T = *(JavaObjectThrowable**)throwable;<br>
+  JavaObjectThrowable::fillInStackTrace(T);<br>
+<br>
+  RETURN_VOID_FROM_JNI;<br>
+<br>
+  END_JNI_EXCEPTION<br>
 }<br>
<br>
 JNIEXPORT void JNICALL<br>
@@ -271,12 +352,59 @@ JVM_PrintStackTrace(JNIEnv *env, jobject<br>
throwable, jobject printable) {<br>
<br>
 JNIEXPORT jint JNICALL<br>
 JVM_GetStackTraceDepth(JNIEnv *env, jobject throwable) {<br>
-  NYI();<br>
+  JavaObjectThrowable * T = 0;<br>
+  llvm_gcroot(T, 0);<br>
+<br>
+  jint res = -1;<br>
+<br>
+  BEGIN_JNI_EXCEPTION<br>
+<br>
+  T = *(JavaObjectThrowable**)throwable;<br>
+  res = JavaObjectThrowable::getStackTraceDepth(T);<br>
+<br>
+  RETURN_FROM_JNI(res);<br>
+<br>
+  END_JNI_EXCEPTION<br>
+<br>
+  return 0;<br>
 }<br>
<br>
 JNIEXPORT jobject JNICALL<br>
 JVM_GetStackTraceElement(JNIEnv *env, jobject throwable, jint index) {<br>
-  NYI();<br>
+  JavaObjectThrowable * T = 0;<br>
+  JavaObject * result = 0;<br>
+  JavaObject * stack = 0;<br>
+  llvm_gcroot(T, 0);<br>
+  llvm_gcroot(result, 0);<br>
+  llvm_gcroot(stack, 0);<br>
+<br>
+  BEGIN_JNI_EXCEPTION<br>
+<br>
+  T = *(JavaObjectThrowable**)throwable;<br>
+  Jnjvm* vm = JavaThread::get()->getJVM();<br>
+  stack = vm->upcalls->backtrace->getInstanceObjectField(T);<br>
+  verifyNull(stack);<br>
+<br>
+  sint32 base = JavaObjectThrowable::getStackTraceBase(T);<br>
+<br>
+  sint32 cur = 0;<br>
+  for (sint32 i = base; i < JavaArray::getSize(stack); ++i) {<br>
+    intptr_t sp = ArrayPtr::getElement((ArrayPtr*)stack, i);<br></blockquote><div><br></div><div>sp -> ip</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

+    mvm::FrameInfo* FI = vm->IPToFrameInfo(sp);<br>
+    if (FI->Metadata != NULL) {<br>
+      if (cur == index) {<br>
+        result = consStackElement(FI, sp);<br>
+        break;<br>
+      }<br>
+      cur++;<br>
+    }<br>
+  }<br></blockquote><div><br></div><div>Maybe check that result is not null?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
+<br>
+  RETURN_FROM_JNI((jobject)th->pushJNIRef(result));<br>
+<br>
+  END_JNI_EXCEPTION<br>
+<br>
+  return 0;<br>
 }<br>
<br>
 /*<br>
<font color="#888888">--<br>
1.7.5.1<br>
_______________________________________________<br>
vmkit-commits mailing list<br>
<a href="mailto:vmkit-commits@cs.uiuc.edu">vmkit-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/vmkit-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/vmkit-commits</a><br>
</font></blockquote></div><br>