[vmkit-commits] [PATCH] Add support for stack traces in OpenJDK
Nicolas Geoffray
nicolas.geoffray at gmail.com
Thu Oct 27 14:19:45 PDT 2011
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.
On Thu, Oct 27, 2011 at 4:20 AM, Will Dietz <wdietz2 at illinois.edu> wrote:
> Inlined below.
>
> While very similar, didn't see a particularly good way to share this
> with the Classpath version of the same, as I recall.
>
> ~Will
>
>
> >From 502715afce04e56d3c8c30156a1c670dfb62b17e Mon Sep 17 00:00:00 2001
> From: Will Dietz <w at wdtz.org>
> Date: Wed, 26 Oct 2011 21:11:48 -0500
> Subject: [PATCH 2/2] Add support for stack traces in OpenJDK
>
> ---
> lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp | 51 ++++++++++-
> lib/J3/ClassLib/OpenJDK/ClasspathReflect.h | 18 ++--
> lib/J3/ClassLib/OpenJDK/OpenJDK.inc | 134
> +++++++++++++++++++++++++-
> 3 files changed, 190 insertions(+), 13 deletions(-)
>
> diff --git a/lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp
> b/lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp
> index 31dd417..a4f9ab2 100644
> --- a/lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp
> +++ b/lib/J3/ClassLib/OpenJDK/ClasspathReflect.cpp
> @@ -11,6 +11,10 @@
> #include "JavaClass.h"
> #include "JavaObject.h"
> #include "JavaThread.h"
> +#include "Jnjvm.h"
> +#include "JavaUpcalls.h"
> +
> +#include <map>
>
> namespace j3 {
>
> @@ -21,11 +25,56 @@ 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->declaringClass);
> return &(cls->asClass()->virtualMethods[self->slot]);
> }
>
> +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.
> + 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) {
> + if (!self->backtrace) return 0;
> +
> + Jnjvm* vm = JavaThread::get()->getJVM();
> +
> + JavaObject * stack = self->backtrace;
> + llvm_gcroot(self, 0);
> + llvm_gcroot(stack, 0);
>
Same here.
> + 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 101e726..d371fdc 100644
> --- a/lib/J3/ClassLib/OpenJDK/ClasspathReflect.h
> +++ b/lib/J3/ClassLib/OpenJDK/ClasspathReflect.h
> @@ -157,18 +157,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 8ff8035..cbdd3d9 100644
> --- a/lib/J3/ClassLib/OpenJDK/OpenJDK.inc
> +++ b/lib/J3/ClassLib/OpenJDK/OpenJDK.inc
> @@ -17,6 +17,77 @@
> #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();
> +
> + 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.
> {
> + ClassArray* cl = vm->upcalls->ArrayOfInt;
> + result = (ArrayPtr*) cl->doNew(length, vm);
> + } else {
> + ClassArray* cl = vm->upcalls->ArrayOfLong;
> + result = (ArrayPtr*) cl->doNew(length, vm);
> + }
> +
> + // 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);
> +
> + // We don't have the bytes if the class was vmjc'ed.
>
Ah that's not true anymore :) Please remove the cl->getBytes() check.
+ if (sourceAtt && cl->getBytes()) {
> + 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) {
> @@ -261,7 +332,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
> @@ -271,12 +352,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 sp = ArrayPtr::getElement((ArrayPtr*)stack, i);
>
sp -> ip
> + 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?
> +
> + RETURN_FROM_JNI((jobject)th->pushJNIRef(result));
> +
> + END_JNI_EXCEPTION
> +
> + return 0;
> }
>
> /*
> --
> 1.7.5.1
> _______________________________________________
> vmkit-commits mailing list
> vmkit-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/vmkit-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/vmkit-commits/attachments/20111027/3cdde6cf/attachment.html>
More information about the vmkit-commits
mailing list