This is the mail archive of the
java-patches@gcc.gnu.org
mailing list for the Java project.
Re: [RFA] JMTI GetLocal____ methods
Tom Tromey wrote:
"Kyle" == Kyle Galloway <kgallowa@redhat.com> writes:
Kyle> This patch implements the JVMTI side functions to Get and Set local
Kyle> variables. getLocalFrame is used by all of these methods to do the
Kyle> common error checking and frame retrieval that is needed in every case.
Kyle> If it does not return an error, it returns a newly allocated copy of
Kyle> the frame object containing pointers to the actual local variable and
Kyle> local variable type info. The Get/SetLocal___ methods then gt/set the
Kyle> variable, free the frame structure and return.
Thanks for the explanation.
Kyle> * interpret-run.cc: Add local variable info to frame in the
Kyle> debug interpreter.
Bad wrapping in the ChangeLog entry. Or maybe your mailer?
Yeah my mailer has a way or warping the Changelogs, something to do with
how long the lines it lets me type are and how long the lines it sends are.
Kyle> DEBUG_LOCALS_INSN (I, 'l'); \
Kyle> + DEBUG_LOCALS_INSN (I+1, 'l'); \
Good catch.
But, when we assign a long (or double) to slot N, I think it is better
to mark slot N+1 as invalid. First, this prevents users from
incorrectly treating (combined) slots N+1 and N+2 as a wide type.
Second, if we later assign a narrow type (eg int) to slot N, slot N+1
will already be correctly marked as invalid.
I've changed this now so it marks N+1 with 'x' instead of the long type
in N. I also check when actually getting the variables to see if the
slot being accessed is the lower half of a long type.
Kyle> +#include "interpret-run.cc"
Spurious addition of a space.
Fixed.
Kyle> +#define OBJECT_CHECK_VALID(Aobject)
Kyle> + if (!java::lang::Object::class$.isAssignableFrom (&(Aobject->class$))) \
Two bugs here.
First, Aobject->class$ most likely should be written Aobject->getClass().
Since class$ is a static field, as written this will pick up the
class$ of the declared type of Aobject, which will always be Object.
Second, and more importantly, I don't think this check is useful. All
object are, by definition, assignable to Object.
Your definitely right here, I've totally removed this check. Since you
pass in a jobject anything not an object will cause a compile-time error
in the caller.
Kyle> + if (slot < 0 || slot >= max_locals)
Kyle> + return JVMTI_ERROR_INVALID_SLOT;
If the caller is dealing with a wide type, this must check
'slot + 1 >= max_locals'. I don't see this check being done anywhere.
I've fixed this by expanding the check to check slot+1 >= max_locals if
there is a long type in use.
Kyle> + *iframe = reinterpret_cast<_Jv_InterpFrame *>
Kyle> + (_Jv_MallocUnchecked (sizeof (_Jv_InterpFrame)));
Kyle> + memcpy (*iframe, tmp_iframe, sizeof (_Jv_InterpFrame));
It is better not to allocate a new frame only to free it in the
caller. Instead let the caller pass in a pointer to an unfilled frame
or something like that.
I now allocate a _Jv_InterpFrame then have getLocalFrame copy into it.
[...]
Kyle> + jobject value)
[...]
Kyle> + java::lang::Object *obj = reinterpret_cast<java::lang::Object *> (value);
No need for a cast here. jobject is just a typedef for
java::lang::Object*.
And lastly, this is gone as well.
New patch is attached.
Thanks,
Kyle
Index: libjava/interpret.cc
===================================================================
--- libjava/interpret.cc (revision 121999)
+++ libjava/interpret.cc (working copy)
@@ -190,11 +190,13 @@
# define STOREL(I) \
do { \
DEBUG_LOCALS_INSN (I, 'l'); \
+ DEBUG_LOCALS_INSN (I+1, 'x'); \
(sp -= 2, locals[I].l = sp->l); \
} while (0)
# define STORED(I) \
do { \
DEBUG_LOCALS_INSN (I, 'd'); \
+ DEBUG_LOCALS_INSN (I+1, 'x'); \
(sp -= 2, locals[I].d = sp->d); \
} while (0)
@@ -202,13 +204,15 @@
# define STOREL(I) \
do { \
DEBUG_LOCALS_INSN (I, 'l'); \
+ DEBUG_LOCALS_INSN (I+1, 'x'); \
jint __idx = (I); \
locals[__idx+1].ia[0] = (--sp)->ia[0]; \
locals[__idx].ia[0] = (--sp)->ia[0]; \
} while (0)
# define STORED(I) \
do { \
- DEBUG_LOCALS_INSN(I, 'd'); \
+ DEBUG_LOCALS_INSN (I, 'd'); \
+ DEBUG_LOCALS_INSN (I+1, 'x'); \
jint __idx = (I); \
locals[__idx+1].ia[0] = (--sp)->ia[0]; \
locals[__idx].ia[0] = (--sp)->ia[0]; \
@@ -928,7 +932,7 @@
{
#undef DEBUG
#undef DEBUG_LOCALS_INSN
-#define DEBUG_LOCALS_INSN(s, t) do {} while(0)
+#define DEBUG_LOCALS_INSN(s, t) do {} while (0)
#include "interpret-run.cc"
}
@@ -938,7 +942,12 @@
{
#define DEBUG
#undef DEBUG_LOCALS_INSN
-#define DEBUG_LOCALS_INSN(s, t) do {} while(0)
+#define DEBUG_LOCALS_INSN(s, t) \
+do \
+ { \
+ frame_desc.locals_type[s] = t; \
+ } \
+while (0)
#include "interpret-run.cc"
}
Index: libjava/jvmti.cc
===================================================================
--- libjava/jvmti.cc (revision 121999)
+++ libjava/jvmti.cc (working copy)
@@ -211,7 +211,274 @@
return JVMTI_ERROR_NONE;
}
+// This method performs the common tasks to get and set variables of all types.
+// It is called by the _Jv_JVMTI_Get/SetLocalInt/Object/.... methods.
+static jvmtiError
+getLocalFrame (jvmtiEnv *env, jthread thread, jint depth, jint slot, char type,
+ _Jv_InterpFrame *iframe)
+{
+ using namespace java::lang;
+
+ REQUIRE_PHASE (env, JVMTI_PHASE_LIVE);
+
+ ILLEGAL_ARGUMENT (depth < 0);
+
+ THREAD_DEFAULT_TO_CURRENT (thread);
+ THREAD_CHECK_VALID (thread);
+ THREAD_CHECK_IS_ALIVE (thread);
+
+ _Jv_Frame *frame = reinterpret_cast<_Jv_Frame *> (thread->frame);
+
+ for (int i = 0; i < depth; i++)
+ {
+ frame = frame->next;
+
+ if (frame == NULL)
+ return JVMTI_ERROR_NO_MORE_FRAMES;
+ }
+
+ if (frame->frame_type == frame_native)
+ return JVMTI_ERROR_OPAQUE_FRAME;
+
+ jint max_locals;
+ jvmtiError jerr = env->GetMaxLocals (reinterpret_cast<jmethodID>
+ (frame->self->get_method ()),
+ &max_locals);
+ if (jerr != JVMTI_ERROR_NONE)
+ return jerr;
+
+ _Jv_InterpFrame *tmp_iframe = reinterpret_cast<_Jv_InterpFrame *> (frame);
+
+ // The second slot taken up by a long type is marked as type 'x' meaning it
+ // is not valid for access since it holds only the 4 low bytes of the value.
+ if (tmp_iframe->locals_type[slot] == 'x')
+ return JVMTI_ERROR_INVALID_SLOT;
+
+ if (tmp_iframe->locals_type[slot] != type)
+ return JVMTI_ERROR_TYPE_MISMATCH;
+
+ // Check for invalid slots, if the type is a long type, we must check that
+ // the next slot is valid as well.
+ if (slot < 0 || slot >= max_locals
+ || ((type == 'l' || type == 'd') && slot + 1 >= max_locals))
+ return JVMTI_ERROR_INVALID_SLOT;
+
+ memcpy (iframe, tmp_iframe, sizeof (_Jv_InterpFrame));
+
+ return JVMTI_ERROR_NONE;
+}
+
static jvmtiError JNICALL
+_Jv_JVMTI_GetLocalObject (jvmtiEnv *env, jthread thread, jint depth, jint slot,
+ jobject *value)
+{
+ NULL_CHECK (value);
+
+ _Jv_InterpFrame *frame = reinterpret_cast<_Jv_InterpFrame *>
+ (_Jv_MallocUnchecked (sizeof (_Jv_InterpFrame)));
+ jvmtiError jerr = getLocalFrame (env, thread, depth, slot, 'o', frame);
+
+ if (jerr != JVMTI_ERROR_NONE)
+ return jerr;
+
+ *value = frame->locals[slot].o;
+
+ _Jv_Free (frame);
+ return JVMTI_ERROR_NONE;
+}
+
+static jvmtiError JNICALL
+_Jv_JVMTI_SetLocalObject (jvmtiEnv *env, jthread thread, jint depth, jint slot,
+ jobject value)
+{
+ _Jv_InterpFrame *frame = reinterpret_cast<_Jv_InterpFrame *>
+ (_Jv_MallocUnchecked (sizeof (_Jv_InterpFrame)));
+ jvmtiError jerr = getLocalFrame (env, thread, depth, slot, 'o', frame);
+
+ if (jerr != JVMTI_ERROR_NONE)
+ return jerr;
+
+ frame->locals[slot].o = value;
+
+ _Jv_Free (frame);
+ return JVMTI_ERROR_NONE;
+}
+
+static jvmtiError JNICALL
+_Jv_JVMTI_GetLocalInt (jvmtiEnv *env, jthread thread, jint depth, jint slot,
+ jint *value)
+{
+ NULL_CHECK (value);
+
+ _Jv_InterpFrame *frame = reinterpret_cast<_Jv_InterpFrame *>
+ (_Jv_MallocUnchecked (sizeof (_Jv_InterpFrame)));
+ jvmtiError jerr = getLocalFrame (env, thread, depth, slot, 'i', frame);
+
+ if (jerr != JVMTI_ERROR_NONE)
+ return jerr;
+
+ *value = frame->locals[slot].i;
+
+ _Jv_Free (frame);
+ return JVMTI_ERROR_NONE;
+}
+
+static jvmtiError JNICALL
+_Jv_JVMTI_SetLocalInt (jvmtiEnv *env, jthread thread, jint depth, jint slot,
+ jint value)
+{
+ _Jv_InterpFrame *frame = reinterpret_cast<_Jv_InterpFrame *>
+ (_Jv_MallocUnchecked (sizeof (_Jv_InterpFrame)));
+ jvmtiError jerr = getLocalFrame (env, thread, depth, slot, 'i', frame);
+
+ if (jerr != JVMTI_ERROR_NONE)
+ return jerr;
+
+ frame->locals[slot].i = value;
+
+ _Jv_Free (frame);
+ return JVMTI_ERROR_NONE;
+}
+
+static jvmtiError JNICALL
+_Jv_JVMTI_GetLocalLong (jvmtiEnv *env, jthread thread, jint depth, jint slot,
+ jlong *value)
+{
+ NULL_CHECK (value);
+
+ _Jv_InterpFrame *frame = reinterpret_cast<_Jv_InterpFrame *>
+ (_Jv_MallocUnchecked (sizeof (_Jv_InterpFrame)));
+ jvmtiError jerr = getLocalFrame (env, thread, depth, slot, 'l', frame);
+
+ if (jerr != JVMTI_ERROR_NONE)
+ return jerr;
+
+#if SIZEOF_VOID_P==8
+ *value = frame->locals[slot].l;
+#else
+ _Jv_word2 val;
+ val.ia[0] = frame->locals[slot].ia[0];
+ val.ia[1] = frame->locals[slot+1].ia[0];
+ *value = val.l;
+#endif
+
+ _Jv_Free (frame);
+ return JVMTI_ERROR_NONE;
+}
+
+static jvmtiError JNICALL
+_Jv_JVMTI_SetLocalLong (jvmtiEnv *env, jthread thread, jint depth, jint slot,
+ jlong value)
+{
+ _Jv_InterpFrame *frame = reinterpret_cast<_Jv_InterpFrame *>
+ (_Jv_MallocUnchecked (sizeof (_Jv_InterpFrame)));
+ jvmtiError jerr = getLocalFrame (env, thread, depth, slot, 'l', frame);
+
+ if (jerr != JVMTI_ERROR_NONE)
+ return jerr;
+
+#if SIZEOF_VOID_P==8
+ frame->locals[slot].l = value;
+#else
+ _Jv_word2 val;
+ val.l = value;
+ frame->locals[slot].ia[0] = val.ia[0];
+ frame->locals[slot+1].ia[0] = val.ia[1];
+#endif
+
+ _Jv_Free (frame);
+ return JVMTI_ERROR_NONE;
+}
+
+
+static jvmtiError JNICALL
+_Jv_JVMTI_GetLocalFloat (jvmtiEnv *env, jthread thread, jint depth, jint slot,
+ jfloat *value)
+{
+ NULL_CHECK (value);
+
+ _Jv_InterpFrame *frame = reinterpret_cast<_Jv_InterpFrame *>
+ (_Jv_MallocUnchecked (sizeof (_Jv_InterpFrame)));
+ jvmtiError jerr = getLocalFrame (env, thread, depth, slot, 'f', frame);
+
+ if (jerr != JVMTI_ERROR_NONE)
+ return jerr;
+
+ *value = frame->locals[slot].f;
+
+ _Jv_Free (frame);
+ return JVMTI_ERROR_NONE;
+}
+
+static jvmtiError JNICALL
+_Jv_JVMTI_SetLocalFloat (jvmtiEnv *env, jthread thread, jint depth, jint slot,
+ jfloat value)
+{
+ _Jv_InterpFrame *frame = reinterpret_cast<_Jv_InterpFrame *>
+ (_Jv_MallocUnchecked (sizeof (_Jv_InterpFrame)));
+ jvmtiError jerr = getLocalFrame (env, thread, depth, slot, 'f', frame);
+
+ if (jerr != JVMTI_ERROR_NONE)
+ return jerr;
+
+ frame->locals[slot].f = value;
+
+ _Jv_Free (frame);
+ return JVMTI_ERROR_NONE;
+}
+
+
+static jvmtiError JNICALL
+_Jv_JVMTI_GetLocalDouble (jvmtiEnv *env, jthread thread, jint depth, jint slot,
+ jdouble *value)
+{
+ NULL_CHECK (value);
+
+ _Jv_InterpFrame *frame = reinterpret_cast<_Jv_InterpFrame *>
+ (_Jv_MallocUnchecked (sizeof (_Jv_InterpFrame)));
+ jvmtiError jerr = getLocalFrame (env, thread, depth, slot, 'd', frame);
+
+ if (jerr != JVMTI_ERROR_NONE)
+ return jerr;
+
+#if SIZEOF_VOID_P==8
+ *value = frame->locals[slot].d;
+#else
+ _Jv_word2 val;
+ val.ia[0] = frame->locals[slot].ia[0];
+ val.ia[1] = frame->locals[slot+1].ia[0];
+ *value = val.d;
+#endif
+
+ _Jv_Free (frame);
+ return JVMTI_ERROR_NONE;
+}
+
+static jvmtiError JNICALL
+_Jv_JVMTI_SetLocalDouble (jvmtiEnv *env, jthread thread, jint depth, jint slot,
+ jdouble value)
+{
+ _Jv_InterpFrame *frame = reinterpret_cast<_Jv_InterpFrame *>
+ (_Jv_MallocUnchecked (sizeof (_Jv_InterpFrame)));
+ jvmtiError jerr = getLocalFrame (env, thread, depth, slot, 'd', frame);
+
+ if (jerr != JVMTI_ERROR_NONE)
+ return jerr;
+
+#if SIZEOF_VOID_P==8
+ frame->locals[slot].d = value;
+#else
+ _Jv_word2 val;
+ val.d = value;
+ frame->locals[slot].ia[0] = val.ia[0];
+ frame->locals[slot+1].ia[0] = val.ia[1];
+#endif
+
+ _Jv_Free (frame);
+ return JVMTI_ERROR_NONE;
+}
+
+static jvmtiError JNICALL
_Jv_JVMTI_GetAllThreads(MAYBE_UNUSED jvmtiEnv *env, jint *thread_cnt,
jthread **threads)
{
@@ -1716,16 +1983,16 @@
RESERVED, // reserved18
UNIMPLEMENTED, // GetFrameLocation
UNIMPLEMENTED, // NotifyPopFrame
- UNIMPLEMENTED, // GetLocalObject
- UNIMPLEMENTED, // GetLocalInt
- UNIMPLEMENTED, // GetLocalLong
- UNIMPLEMENTED, // GetLocalFloat
- UNIMPLEMENTED, // GetLocalDouble
- UNIMPLEMENTED, // SetLocalObject
- UNIMPLEMENTED, // SetLocalInt
- UNIMPLEMENTED, // SetLocalLong
- UNIMPLEMENTED, // SetLocalFloat
- UNIMPLEMENTED, // SetLocalDouble
+ _Jv_JVMTI_GetLocalObject, // GetLocalObject
+ _Jv_JVMTI_GetLocalInt, // GetLocalInt
+ _Jv_JVMTI_GetLocalLong, // GetLocalLong
+ _Jv_JVMTI_GetLocalFloat, // GetLocalFloat
+ _Jv_JVMTI_GetLocalDouble, // GetLocalDouble
+ _Jv_JVMTI_SetLocalObject, // SetLocalObject
+ _Jv_JVMTI_SetLocalInt, // SetLocalInt
+ _Jv_JVMTI_SetLocalLong, // SetLocalLong
+ _Jv_JVMTI_SetLocalFloat, // SetLocalFloat
+ _Jv_JVMTI_SetLocalDouble, // SetLocalDouble
_Jv_JVMTI_CreateRawMonitor, // CreateRawMonitor
_Jv_JVMTI_DestroyRawMonitor, // DestroyRawMonitor
_Jv_JVMTI_RawMonitorEnter, // RawMonitorEnter
Index: libjava/interpret-run.cc
===================================================================
--- libjava/interpret-run.cc (revision 121957)
+++ libjava/interpret-run.cc (working copy)
@@ -26,6 +26,11 @@
_Jv_word *sp = stack;
_Jv_word locals[meth->max_locals];
+#ifdef DEBUG
+ frame_desc.locals = locals;
+ frame_desc.locals_type
+ = reinterpret_cast<char *> (_Jv_AllocBytes (meth->max_locals));
+#endif
#define INSN_LABEL(op) &&insn_##op