[PATCH 1/?] Fix PR38892: "--enable-libgcj-debug" breaks bootstrap.

[PATCH 1/?] Fix PR38892: "--enable-libgcj-debug" breaks bootstrap.

[Dave Korn]

[-- Attachment #1: Type: text/plain, Size: 1921 bytes --]

[ Not subbed, please CC me on replies. ]

    Hello Java team,

  The attached patch fixes the first build problem I run into after turning on
"--enable-libgcj-debug":

[ ... ] -DPIC -o gnu/classpath/jdwp/.libs/natVMVirtualMachine.o
/gnu/gcc/gcc/libjava/gnu/classpath/jdwp/natVMVirtualMachine.cc: In function
'void handle_single_step(jvmtiEnv*, step_info*, java::lang::Thread*,
_Jv_Method*, jlocation)':
/gnu/gcc/gcc/libjava/gnu/classpath/jdwp/natVMVirtualMachine.cc:903: error:
request for member 'frame_type' in 'thread->java::lang::Thread::frame', which
is of non-class type 'gnu::gcj::RawData*'
/gnu/gcc/gcc/libjava/gnu/classpath/jdwp/natVMVirtualMachine.cc: In function
'void jdwpBreakpointCB(jvmtiEnv*, JNIEnv*, java::lang::Thread*, _Jv_Method*,
jlocation)':
/gnu/gcc/gcc/libjava/gnu/classpath/jdwp/natVMVirtualMachine.cc:960: error:
request for member 'frame_type' in 'thread->java::lang::Thread::frame', which
is of non-class type 'gnu::gcj::RawData*'
make[3]: *** [gnu/classpath/jdwp/natVMVirtualMachine.lo] Error 1

  It looks like a couple of asserts didn't get updated at some point when the
class definition was reorganised.  The member they are trying to test is
easily available from the reinterpreted pointer, so I used that instead.

libjava/ChangeLog:

	* gnu/classpath/jdwp/natVMVirtualMachine.cc (handle_single_step):  Use
	casted pointer in debugging assert.
	(jdwpBreakpointCB):  Likewise.

  Tested on i686-pc-cygwin by resuming a failed build and seeing it get
further.  I haven't been able to test the assert in practice yet because I
only get a bit further before running into problem #2, and it occurs to me
that maybe it's entirely superfluous now as the reinterpret_cast will throw a
typeinfo exception if we pass the wrong kind of frame object to it, won't it?
 So perhaps just deleting the asserts would be better.

  I'll describe problem #2 in a follow-up post.

    cheers,
      DaveK


[-- Attachment #2: jdwp-debug-fix.diff --]
[-- Type: text/x-c, Size: 1405 bytes --]

Index: libjava/gnu/classpath/jdwp/natVMVirtualMachine.cc
===================================================================
--- libjava/gnu/classpath/jdwp/natVMVirtualMachine.cc	(revision 146543)
+++ libjava/gnu/classpath/jdwp/natVMVirtualMachine.cc	(working copy)
@@ -900,9 +900,9 @@ handle_single_step (jvmtiEnv *env, struct step_inf
 
   VMMethod *vmmethod = new VMMethod (klass, reinterpret_cast<jlong> (method));
   Location *loc = new Location (vmmethod, location);
-  JvAssert (thread->frame.frame_type == frame_interpreter);
   _Jv_InterpFrame *iframe
     = reinterpret_cast<_Jv_InterpFrame *> (thread->interp_frame);  
+  JvAssert (iframe->frame_type == frame_interpreter);
   jobject instance = iframe->get_this_ptr ();
   event::SingleStepEvent *event
     = new event::SingleStepEvent (thread, loc, instance);
@@ -957,9 +957,9 @@ jdwpBreakpointCB (jvmtiEnv *env, MAYBE_UNUSED JNIE
   jlong methodId = reinterpret_cast<jlong> (method);
   VMMethod *meth = VMVirtualMachine::getClassMethod (klass, methodId);
   Location *loc = new Location (meth, location);
-  JvAssert (thread->frame.frame_type == frame_interpreter);
   _Jv_InterpFrame *iframe
     = reinterpret_cast<_Jv_InterpFrame *> (thread->interp_frame);
+  JvAssert (iframe->frame_type == frame_interpreter);
   jobject instance = iframe->get_this_ptr ();
   BreakpointEvent *event = new BreakpointEvent (thread, loc, instance);
   

Re: [PATCH 1/?] Fix PR38892: "--enable-libgcj-debug" breaks bootstrap.

[Tom Tromey]

>>>>> "Dave" == Dave Korn <dave.korn.cygwin@googlemail.com> writes:

Dave>   The attached patch fixes the first build problem I run into
Dave> after turning on "--enable-libgcj-debug":

I guess we don't do this often :)

Dave> libjava/ChangeLog:
Dave> 	* gnu/classpath/jdwp/natVMVirtualMachine.cc (handle_single_step):  Use
Dave> 	casted pointer in debugging assert.
Dave> 	(jdwpBreakpointCB):  Likewise.

This is ok, thanks.

Dave> that maybe it's entirely superfluous now as the reinterpret_cast
Dave> will throw a typeinfo exception if we pass the wrong kind of
Dave> frame object to it, won't it?

I don't think reinterpret_cast can throw, only dynamic_cast.  If it
can throw then it will probably just make the thread exit.

Tom

Re: [PATCH 1/?] Fix PR38892: "--enable-libgcj-debug" breaks bootstrap.

[Dave Korn]

Tom Tromey wrote:
>>>>>> "Dave" == Dave Korn <dave.korn.cygwin@googlemail.com> writes:

> 
> Dave> that maybe it's entirely superfluous now as the reinterpret_cast
> Dave> will throw a typeinfo exception if we pass the wrong kind of
> Dave> frame object to it, won't it?
> 
> I don't think reinterpret_cast can throw, only dynamic_cast.  

  Just as well I asked :) I always get the new-style C++ casts mixed up :-/

    cheers,
      DaveK