Re: [RFA] _Jv_InterpFrame "this" pointer

[Kyle Galloway <kgallowa at redhat dot com>]

Tom Tromey wrote:
> > > > > > "Kyle" == Kyle Galloway <kgallowa@redhat.com> writes:
> > > > > >             
>
> Kyle> This patch add a method to _Jv_InterpFrame to get the "this" pointer
> Kyle> for an interpreted method.  When the method is run, the first argument
> Kyle> is copied into a new field in _Jv_InterpFrame.  I have added a method
> Kyle> get_this_ptr that hecks if the method is static (and returns null if
> Kyle> it is) and returns the "this" pointer for this frame.
>
> Just a couple notes on this.
>
>
>
> Kyle> Index: libjava/include/java-interp.h
> Kyle> ===================================================================
> Kyle> --- libjava/include/java-interp.h	(revision 121649)
> Kyle> +++ libjava/include/java-interp.h	(working copy)
> Kyle> @@ -372,6 +372,9 @@
> Kyle>    _Jv_word *locals;
> Kyle>    char *locals_type;
>  
> Kyle> +  // Object pointer for this frame ("this")
> Kyle> +  _Jv_word obj_ptr;
>
> This should just be 'jobject'.  We know 'this' will always be an
> object.
>   
Changed.
> Kyle>    _Jv_InterpFrame (void *meth, java::lang::Thread *thr, jclass proxyCls = NULL)
> Kyle>    : _Jv_Frame (reinterpret_cast<_Jv_MethodBase *> (meth), thr,
> Kyle>  	             frame_interpreter)
>
> I think we should initialize obj_ptr to NULL in the constructor.
> I like to avoid uninitialized values at the source when possible.
>   
Done.
> Kyle> +    if ((method->accflags & java::lang::reflect::Modifier::STATIC) == 0)
> Kyle> +      obj = iframe->obj_ptr.o;
> Kyle> +    else
> Kyle> +      obj = NULL;
>
> ...then this can always return obj_ptr.
>
> Kyle> +  // Get the object pointer for this method, if it is non-static,
>
> The test doesn't actually test for non-static-ness.  I think it ought
> to.
>
> Kyle> +  if (meth->args_raw_size > 0)
> Kyle> +    memcpy ((void *) &(frame_desc.obj_ptr), (void *) (&locals[0]), sizeof (_Jv_word));
>
> This is pretty roundabout.  Once there's a check for non-static-ness
> it can just read:
>
>     frame_desc.obj_ptr = locals[0].o;
>   
I now do the static test when I set obj_ptr in interpret-run.cc, which 
reduces the method in _Jv_Frame to just an accessor.  I think this will 
be more what you are looking for.  Patch is attached.

Thanks,
Kyle

Index: libjava/include/java-interp.h
===================================================================
--- libjava/include/java-interp.h	(revision 121700)
+++ libjava/include/java-interp.h	(working copy)
@@ -377,6 +377,9 @@
   _Jv_word *locals;
   char *locals_type;
 
+  // Object pointer for this frame ("this")
+  jobject obj_ptr;
+
   _Jv_InterpFrame (void *meth, java::lang::Thread *thr, jclass proxyCls = NULL)
   : _Jv_Frame (reinterpret_cast<_Jv_MethodBase *> (meth), thr,
 	             frame_interpreter)
@@ -384,12 +387,18 @@
     next_interp = (_Jv_InterpFrame *) thr->interp_frame;
     proxyClass = proxyCls;
     thr->interp_frame = (gnu::gcj::RawData *) this;
+    obj_ptr = NULL;
   }
 
   ~_Jv_InterpFrame ()
   {
     thread->interp_frame = (gnu::gcj::RawData *) next_interp;
   }
+
+  jobject get_this_ptr ()
+  {
+    return obj_ptr;
+  } 
 };
 
 // A native frame in the call stack really just a placeholder
Index: libjava/interpret-run.cc
===================================================================
--- libjava/interpret-run.cc	(revision 121649)
+++ libjava/interpret-run.cc	(working copy)
@@ -349,6 +349,15 @@
   */
   memcpy ((void*) locals, (void*) args, meth->args_raw_size);
 
+#ifdef DEBUG
+  // Get the object pointer for this method, after checking that it is
+  // non-static.
+  _Jv_Method *method = meth->get_method ();
+   
+  if ((method->accflags & java::lang::reflect::Modifier::STATIC) == 0)
+    frame_desc.obj_ptr = locals[0].o;
+#endif
+
   _Jv_word *pool_data = meth->defining_class->constants.data;
 
   /* These three are temporaries for common code used by several