[vmkit-commits] [PATCH] Add support for stack traces in OpenJDK
Nicolas Geoffray
nicolas.geoffray at gmail.com
Tue Nov 1 09:37:26 PDT 2011
On Tue, Nov 1, 2011 at 5:41 AM, Will Dietz <wdietz2 at illinois.edu> wrote:
> On Thu, Oct 27, 2011 at 4:19 PM, Nicolas Geoffray
> <nicolas.geoffray at gmail.com> wrote:
> >> +int JavaObjectThrowable::getStackTraceBase(JavaObjectThrowable * self)
> {
> >> + if (!self->backtrace) return 0;
> >> +
> >> + Jnjvm* vm = JavaThread::get()->getJVM();
> >> +
> >> + JavaObject * stack = self->backtrace;
> >> + llvm_gcroot(self, 0);
> >> + llvm_gcroot(stack, 0);
> >
> > 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.
> >
>
> Yep, sure thing. Fixed (in both spots mentioned)
>
> >> + uint32 length = th->getFrameContextLength();
> >> +
> >> + if (sizeof(void*) == 4)
> >
> > I know this probably comes from my code, but please use the macros
> ARCH_32
> > and ARCH_64 for these kinds of checks.
> >
>
> Fixed.
>
> >
> > Ah that's not true anymore :) Please remove the cl->getBytes() check.
>
> Sure thing.
>
> >> +
> >> + sint32 cur = 0;
> >> + for (sint32 i = base; i < JavaArray::getSize(stack); ++i) {
> >> + intptr_t sp = ArrayPtr::getElement((ArrayPtr*)stack, i);
> >
> > sp -> ip
> >
>
> Fixed :).
>
> >>
> >> + mvm::FrameInfo* FI = vm->IPToFrameInfo(sp);
> >> + if (FI->Metadata != NULL) {
> >> + if (cur == index) {
> >> + result = consStackElement(FI, sp);
> >> + break;
> >> + }
> >> + cur++;
> >> + }
> >> + }
> >
> > Maybe check that result is not null?
> >
>
> I'm not sure what you mean, sorry. Would you mind elaborating?
>
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?
>
> Updated version inlined below, FWIW.
>
> ~Will
>
> From e04dad6e4322ba91e854c2ce7f7292f2476c89c3 Mon Sep 17 00:00:00 2001
> From: Will Dietz <w at wdtz.org>
> Date: Fri, 28 Oct 2011 09:21:42 -0500
> Subject: [PATCH 1/8] Add OpenJDK support for stack traces
>
> ---
> lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp | 55 +++++++++++-
> lib/J3/ClassLib/OpenJDK/ClasspathReflect.h | 19 ++--
> lib/J3/ClassLib/OpenJDK/OpenJDK.inc | 133
> +++++++++++++++++++++++++-
> 3 files changed, 194 insertions(+), 13 deletions(-)
>
> diff --git a/lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp
> b/lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp
> index 74117ce..4842bb9 100644
> --- a/lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp
> +++ b/lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp
> @@ -11,6 +11,9 @@
> #include "JavaClass.h"
> #include "JavaObject.h"
> #include "JavaThread.h"
> +#include "Jnjvm.h"
> +#include "JavaUpcalls.h"
> +#include "JavaArray.h"
>
> namespace j3 {
>
> @@ -20,11 +23,61 @@ JavaMethod*
> JavaObjectConstructor::getInternalMethod(JavaObjectConstructor* self
> return &(cls->asClass()->virtualMethods[self->slot]);
> }
>
> -
> JavaMethod* JavaObjectMethod::getInternalMethod(JavaObjectMethod* self) {
> llvm_gcroot(self, 0);
> UserCommonClass* cls = JavaObjectClass::getClass(self->clazz);
> return &(cls->asClass()->virtualMethods[self->slot]);
> }
>
> +
> +int JavaObjectThrowable::getStackTraceBase(JavaObjectThrowable * self) {
> + JavaObject * stack = NULL;
> + llvm_gcroot(self, 0);
> + llvm_gcroot(stack, 0);
> +
> + if (!self->backtrace) return 0;
> +
> + Jnjvm* vm = JavaThread::get()->getJVM();
> +
> + stack = self->backtrace;
> + sint32 index = 2;;
> + while (index != JavaArray::getSize(stack)) {
> + mvm::FrameInfo* FI =
> vm->IPToFrameInfo(ArrayPtr::getElement((ArrayPtr*)stack, index));
> + if (FI->Metadata == NULL) ++index;
> + else {
> + JavaMethod* meth = (JavaMethod*)FI->Metadata;
> + assert(meth && "Wrong stack trace");
> + if (meth->classDef->isAssignableFrom(vm->upcalls->newThrowable)) {
> + ++index;
> + } else return index;
> + }
> + }
> +
> + assert(0 && "Invalid stack trace!");
> + return 0;
> +}
> +
> +int JavaObjectThrowable::getStackTraceDepth(JavaObjectThrowable * self) {
> + JavaObject * stack = NULL;
> + llvm_gcroot(self, 0);
> + llvm_gcroot(stack, 0);
> +
> + if (!self->backtrace) return 0;
> +
> + Jnjvm* vm = JavaThread::get()->getJVM();
> +
> + stack = self->backtrace;
> + sint32 index = getStackTraceBase(self);
> +
> + sint32 size = 0;
> + sint32 cur = index;
> + while (cur < JavaArray::getSize(stack)) {
> + mvm::FrameInfo* FI =
> vm->IPToFrameInfo(ArrayPtr::getElement((ArrayPtr*)stack, cur));
> + ++cur;
> + if (FI->Metadata != NULL) ++size;
> + }
> +
> + return size;
> +}
> +
> }
> diff --git a/lib/J3/ClassLib/OpenJDK/ClasspathReflect.h
> b/lib/J3/ClassLib/OpenJDK/ClasspathReflect.h
> index 7deecbe..0055a73 100644
> --- a/lib/J3/ClassLib/OpenJDK/ClasspathReflect.h
> +++ b/lib/J3/ClassLib/OpenJDK/ClasspathReflect.h
> @@ -17,6 +17,7 @@
> #include "JavaThread.h"
> #include "JavaString.h"
>
> +extern "C" j3::JavaObject* internalFillInStackTrace(j3::JavaObject*);
> namespace j3 {
>
> class JavaObjectClass : public JavaObject {
> @@ -262,18 +263,18 @@ public:
> llvm_gcroot(self, 0);
> llvm_gcroot(stackTrace, 0);
>
> - // TODO: Implement this right
> - assert(0 && "fillInStackTrace not implemented yet!");
> -
> - // stackTrace = internalFillInStackTrace(self);
> - // mvm::Collector::objectReferenceWriteBarrier(
> - // (gc*)self, (gc**)&(self->vmState), (gc*)stackTrace);
> + stackTrace = internalFillInStackTrace(self);
> + mvm::Collector::objectReferenceWriteBarrier(
> + (gc*)self, (gc**)&(self->backtrace), (gc*)stackTrace);
>
> - // mvm::Collector::objectReferenceWriteBarrier(
> - // (gc*)self, (gc**)&(self->cause), (gc*)self);
> + mvm::Collector::objectReferenceWriteBarrier(
> + (gc*)self, (gc**)&(self->cause), (gc*)self);
>
> - // self->stackTrace = NULL;
> + self->stackTrace = NULL;
> }
> +
> + static int getStackTraceDepth(JavaObjectThrowable * self);
> + static int getStackTraceBase(JavaObjectThrowable * self);
> };
>
> class JavaObjectReference : public JavaObject {
> diff --git a/lib/J3/ClassLib/OpenJDK/OpenJDK.inc
> b/lib/J3/ClassLib/OpenJDK/OpenJDK.inc
> index 31bd044..d9c1a78 100644
> --- a/lib/J3/ClassLib/OpenJDK/OpenJDK.inc
> +++ b/lib/J3/ClassLib/OpenJDK/OpenJDK.inc
> @@ -22,6 +22,76 @@
> #define NYI() \
> do{ assert(0 && "Not yet implemented!"); abort(); } while(0)
>
> +JavaObject* internalFillInStackTrace(JavaObject* throwable) {
> +
> + ArrayPtr* result = 0;
> + llvm_gcroot(throwable, 0);
> + llvm_gcroot(result, 0);
> +
> + JavaThread* th = JavaThread::get();
> + Jnjvm* vm = th->getJVM();
> + assert(th);
> + assert(vm);
> +
> + uint32 length = th->getFrameContextLength();
> +
> +#ifdef ARCH_64
> + ClassArray* cl = vm->upcalls->ArrayOfInt;
> + result = (ArrayPtr*) cl->doNew(length, vm);
> +#else
> + ClassArray* cl = vm->upcalls->ArrayOfLong;
> + result = (ArrayPtr*) cl->doNew(length, vm);
> +#endif
> +
> + // Don't call th->getFrameContext because it is not GC-safe.
> + mvm::StackWalker Walker(th);
> + uint32_t i = 0;
> +
> + while (intptr_t ip = *Walker) {
> + ArrayPtr::setElement(result, ip, i);
> + ++i;
> + ++Walker;
> + }
> +
> + return result;
> +}
> +
> +JavaObject* consStackElement(mvm::FrameInfo* FI, intptr_t ip) {
> +
> + JavaString* methodName = 0;
> + JavaString* className = 0;
> + JavaString* sourceName = 0;
> + JavaObject* res = 0;
> + llvm_gcroot(methodName, 0);
> + llvm_gcroot(className, 0);
> + llvm_gcroot(sourceName, 0);
> + llvm_gcroot(res, 0);
> +
> + Jnjvm* vm = JavaThread::get()->getJVM();
> + JavaMethod* meth = (JavaMethod*)FI->Metadata;
> + methodName = vm->internalUTF8ToStr(meth->name);
> + Class* cl = meth->classDef;
> + className = JavaString::internalToJava(cl->name, vm);
> +
> + Attribut* sourceAtt = cl->lookupAttribut(Attribut::sourceFileAttribut);
> +
> + if (sourceAtt) {
> + Reader reader(sourceAtt, cl->bytes);
> + uint16 index = reader.readU2();
> + sourceName =
> vm->internalUTF8ToStr(cl->getConstantPool()->UTF8At(index));
> + }
> +
> + uint16 lineNumber = meth->lookupLineNumber(FI);
> +
> + UserClass* newS = vm->upcalls->newStackTraceElement;
> + res = newS->doNew(vm);
> + vm->upcalls->initStackTraceElement->invokeIntSpecial(vm, newS, res,
> + &className,
> + &methodName,
> + &sourceName,
> + lineNumber);
> + return res;
> +}
>
> JNIEXPORT jint JNICALL
> JVM_GetInterfaceVersion(void) {
> @@ -266,7 +336,17 @@ JVM_IsNaN(jdouble d) {
> */
> JNIEXPORT void JNICALL
> JVM_FillInStackTrace(JNIEnv *env, jobject throwable) {
> - NYI();
> + JavaObjectThrowable * T = 0;
> + llvm_gcroot(T, 0);
> +
> + BEGIN_JNI_EXCEPTION
> +
> + T = *(JavaObjectThrowable**)throwable;
> + JavaObjectThrowable::fillInStackTrace(T);
> +
> + RETURN_VOID_FROM_JNI;
> +
> + END_JNI_EXCEPTION
> }
>
> JNIEXPORT void JNICALL
> @@ -276,12 +356,59 @@ JVM_PrintStackTrace(JNIEnv *env, jobject
> throwable, jobject printable) {
>
> JNIEXPORT jint JNICALL
> JVM_GetStackTraceDepth(JNIEnv *env, jobject throwable) {
> - NYI();
> + JavaObjectThrowable * T = 0;
> + llvm_gcroot(T, 0);
> +
> + jint res = -1;
> +
> + BEGIN_JNI_EXCEPTION
> +
> + T = *(JavaObjectThrowable**)throwable;
> + res = JavaObjectThrowable::getStackTraceDepth(T);
> +
> + RETURN_FROM_JNI(res);
> +
> + END_JNI_EXCEPTION
> +
> + return 0;
> }
>
> JNIEXPORT jobject JNICALL
> JVM_GetStackTraceElement(JNIEnv *env, jobject throwable, jint index) {
> - NYI();
> + JavaObjectThrowable * T = 0;
> + JavaObject * result = 0;
> + JavaObject * stack = 0;
> + llvm_gcroot(T, 0);
> + llvm_gcroot(result, 0);
> + llvm_gcroot(stack, 0);
> +
> + BEGIN_JNI_EXCEPTION
> +
> + T = *(JavaObjectThrowable**)throwable;
> + Jnjvm* vm = JavaThread::get()->getJVM();
> + stack = vm->upcalls->backtrace->getInstanceObjectField(T);
> + verifyNull(stack);
> +
> + sint32 base = JavaObjectThrowable::getStackTraceBase(T);
> +
> + sint32 cur = 0;
> + for (sint32 i = base; i < JavaArray::getSize(stack); ++i) {
> + intptr_t ip = ArrayPtr::getElement((ArrayPtr*)stack, i);
> + mvm::FrameInfo* FI = vm->IPToFrameInfo(ip);
> + if (FI->Metadata != NULL) {
> + if (cur == index) {
> + result = consStackElement(FI, ip);
> + break;
> + }
> + cur++;
> + }
> + }
> +
> + RETURN_FROM_JNI((jobject)th->pushJNIRef(result));
> +
> + END_JNI_EXCEPTION
> +
> + return 0;
> }
>
> /*
> --
> 1.7.5.1
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/vmkit-commits/attachments/20111101/86dc009f/attachment.html>
More information about the vmkit-commits
mailing list