On Tue, Nov 1, 2011 at 5:41 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;">
<div class="im">On Thu, Oct 27, 2011 at 4:19 PM, Nicolas Geoffray<br>
<<a href="mailto:nicolas.geoffray@gmail.com">nicolas.geoffray@gmail.com</a>> wrote:<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>
><br>
</div><div class="im">> This is not written anywhere, but I usually follow the rule to declare GC<br>
> variables and llvm_gcroot as the first things in a method. This makes it<br>
> easier to review the code and find potential GC bugs.<br>
><br>
<br>
</div>Yep, sure thing. Fixed (in both spots mentioned)<br>
<div class="im"><br>
>> + uint32 length = th->getFrameContextLength();<br>
>> +<br>
>> + if (sizeof(void*) == 4)<br>
><br>
</div><div class="im">> I know this probably comes from my code, but please use the macros ARCH_32<br>
> and ARCH_64 for these kinds of checks.<br>
><br>
<br>
</div>Fixed.<br>
<div class="im"><br>
><br>
> Ah that's not true anymore :) Please remove the cl->getBytes() check.<br>
<br>
</div>Sure thing.<br>
<div class="im"><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>
><br>
</div>> sp -> ip<br>
><br>
<br>
Fixed :).<br>
<div class="im"><br>
>><br>
>> + 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>
><br>
</div><div class="im">> Maybe check that result is not null?<br>
><br>
<br>
</div>I'm not sure what you mean, sorry. Would you mind elaborating?<br></blockquote><div><br></div><div>It looks like the method *must* return a stack element, so we must make sure result is not null. So I meant, maybe you should add an assert 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>
Updated version inlined below, FWIW.<br>
<br>
~Will<br>
<br>
>From e04dad6e4322ba91e854c2ce7f7292f2476c89c3 Mon Sep 17 00:00:00 2001<br>
From: Will Dietz <<a href="mailto:w@wdtz.org">w@wdtz.org</a>><br>
Date: Fri, 28 Oct 2011 09:21:42 -0500<br>
Subject: [PATCH 1/8] Add OpenJDK support for stack traces<br>
<br>
---<br>
lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp | 55 +++++++++++-<br>
lib/J3/ClassLib/OpenJDK/ClasspathReflect.h | 19 ++--<br>
lib/J3/ClassLib/OpenJDK/OpenJDK.inc | 133 +++++++++++++++++++++++++-<br>
3 files changed, 194 insertions(+), 13 deletions(-)<br>
<br>
diff --git a/lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp<br>
b/lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp<br>
index 74117ce..4842bb9 100644<br>
--- a/lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp<br>
+++ b/lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp<br>
@@ -11,6 +11,9 @@<br>
<div class="im"> #include "JavaClass.h"<br>
#include "JavaObject.h"<br>
#include "JavaThread.h"<br>
+#include "Jnjvm.h"<br>
+#include "JavaUpcalls.h"<br>
</div>+#include "JavaArray.h"<br>
<br>
namespace j3 {<br>
<br>
@@ -20,11 +23,61 @@ JavaMethod*<br>
<div class="im">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>
</div> UserCommonClass* cls = JavaObjectClass::getClass(self->clazz);<br>
<div class="im"> return &(cls->asClass()->virtualMethods[self->slot]);<br>
}<br>
<br>
</div><div class="im">+<br>
+int JavaObjectThrowable::getStackTraceBase(JavaObjectThrowable * self) {<br>
</div>+ JavaObject * stack = NULL;<br>
<div class="im">+ llvm_gcroot(self, 0);<br>
+ llvm_gcroot(stack, 0);<br>
+<br>
</div><div class="im">+ if (!self->backtrace) return 0;<br>
+<br>
+ Jnjvm* vm = JavaThread::get()->getJVM();<br>
+<br>
</div><div class="im">+ stack = self->backtrace;<br>
</div><div class="im">+ 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>
</div>+ JavaObject * stack = NULL;<br>
<div class="im">+ llvm_gcroot(self, 0);<br>
+ llvm_gcroot(stack, 0);<br>
+<br>
</div><div class="im">+ if (!self->backtrace) return 0;<br>
+<br>
+ Jnjvm* vm = JavaThread::get()->getJVM();<br>
+<br>
</div><div class="im">+ stack = self->backtrace;<br>
</div><div class="im">+ 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>
</div>index 7deecbe..0055a73 100644<br>
--- a/lib/J3/ClassLib/OpenJDK/ClasspathReflect.h<br>
+++ b/lib/J3/ClassLib/OpenJDK/ClasspathReflect.h<br>
@@ -17,6 +17,7 @@<br>
#include "JavaThread.h"<br>
#include "JavaString.h"<br>
<br>
+extern "C" j3::JavaObject* internalFillInStackTrace(j3::JavaObject*);<br>
namespace j3 {<br>
<br>
class JavaObjectClass : public JavaObject {<br>
@@ -262,18 +263,18 @@ public:<br>
<div class="im"> 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>
</div>index 31bd044..d9c1a78 100644<br>
--- a/lib/J3/ClassLib/OpenJDK/OpenJDK.inc<br>
+++ b/lib/J3/ClassLib/OpenJDK/OpenJDK.inc<br>
@@ -22,6 +22,76 @@<br>
<div class="im"> #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>
</div>+#ifdef ARCH_64<br>
<div class="im">+ ClassArray* cl = vm->upcalls->ArrayOfInt;<br>
+ result = (ArrayPtr*) cl->doNew(length, vm);<br>
</div>+#else<br>
<div class="im">+ ClassArray* cl = vm->upcalls->ArrayOfLong;<br>
+ result = (ArrayPtr*) cl->doNew(length, vm);<br>
</div>+#endif<br>
<div><div></div><div class="h5">+<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>
</div></div>+ if (sourceAtt) {<br>
<div class="im">+ 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>
</div>@@ -266,7 +336,17 @@ JVM_IsNaN(jdouble d) {<br>
<div class="im"> */<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>
</div>@@ -276,12 +356,59 @@ JVM_PrintStackTrace(JNIEnv *env, jobject<br>
<div><div></div><div class="h5">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 ip = ArrayPtr::getElement((ArrayPtr*)stack, i);<br>
</div></div>+ mvm::FrameInfo* FI = vm->IPToFrameInfo(ip);<br>
<div><div></div><div class="h5">+ if (FI->Metadata != NULL) {<br>
+ if (cur == index) {<br>
+ result = consStackElement(FI, ip);<br>
+ break;<br>
+ }<br>
+ cur++;<br>
+ }<br>
+ }<br>
+<br>
+ RETURN_FROM_JNI((jobject)th->pushJNIRef(result));<br>
+<br>
+ END_JNI_EXCEPTION<br>
+<br>
+ return 0;<br>
}<br>
<br>
/*<br>
--<br>
1.7.5.1<br>
</div></div></blockquote></div><br>