[MinGW] RFA: Make Stack Traces Work for Interpreted Code on Windows

[MinGW] RFA: Make Stack Traces Work for Interpreted Code on Windows

[Ranjit Mathew]

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello,

  When run under the interpreter, an exception-throwing programme
crashes the interpreter on Windows since JvRunMain() does not
occur in the call stack and we once again roam the wilderness of
arbitrary EBP values.

The fix was to change fallback_backtrace() in
"sysdep/i386/backtrace.h" to stop on _Jv_RunMain() instead since
this does occur in both interpreted and natively-executed call
stacks. (Since this function is overloaded, I needed to use an
interim variable with the appropriate cast to tell the compiler
which of these function variants to pick up.)

fallback_backtrace() also does not correctly handle unwinding
through interpreted code. The fix was as simple as copying the
relevant bits of code from _Jv_StackTrace::Unwind_TraceFn() and
adjusting it a little bit.

This exposed the fact that _Jv_InterpMethod::run() is a private
method. Therefore I had to declare fallback_backtrace() as
a friend function for this class (similar to that for the
_Jv_StackTrace class). This declaration is done only if
HAVE_FALLBACK_BACKTRACE is defined. This definition in turn was
moved out of "sysdep/i386/backtrace.h" to configure.host, where
it is added to libgcj_cxxflags (this jugglery avoids having to deal
with included file ordering issues).

The attached patch contains all these changes. Note that it also
contains the "sysdep/i386/backtrace.h" changes from:

  http://gcc.gnu.org/ml/java-patches/2006-q3/msg00098.html

purely as a matter of convenience for yours truly.

With this patch, we now get proper stack traces for interpreted
code also on Windows. (I note that under the interpreter we do
not get line numbers for the innermost call frame, even on Linux.
I haven't investigated this issue yet.)

Tested via a crossed-native i686-pc-mingw32 compiler built on
i686-pc-linux-gnu.

OK to apply?

Thanks,
Ranjit.

- --
Ranjit Mathew       Email: rmathew AT gmail DOT com

Bangalore, INDIA.     Web: http://rmathew.com/




-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEsMo5Yb1hx2wRS48RAmZmAJsHu0Iyx3r9W4ILKg27BMBDC1sOZwCgix8m
CFfW9c39jTSvqNxoH6lBLxY=
=wLCp
-----END PGP SIGNATURE-----

[-- Attachment #2: p2.txt --]
[-- Type: text/plain, Size: 4113 bytes --]

Index: ChangeLog
from  Ranjit Mathew  <rmathew@gcc.gnu.org>

	* configure.host: Add a definition of HAVE_FALLBACK_BACKTRACE to
	libgcj_cxxflags for Windows.
	* include/java-interp.h (_Jv_InterpMethod): Declare
	fallback_backtrace() as a friend function.
	* sysdep/i386/backtrace.h: Do not define HAVE_FALLBACK_BACKTRACE.
	(fallback_backtrace): If we see _Jv_InterpMethod::run(), make sure
	the trace goes through the interpreted java stack.
	Stop unwinding when we see _Jv_RunMain().

Index: configure.host
===================================================================
--- configure.host	(revision 115260)
+++ configure.host	(working copy)
@@ -283,6 +283,7 @@ esac
 case "${host}" in
   *-cygwin* | *-mingw*)
 	fallback_backtrace_h=sysdep/i386/backtrace.h  
+	libgcj_cxxflags="${libgcj_cxxflags} -DHAVE_FALLBACK_BACKTRACE"
 	# We need a frame pointer on Windows, so override BACKTRACESPEC
   	BACKTRACESPEC=
   ;;
Index: include/java-interp.h
===================================================================
--- include/java-interp.h	(revision 115260)
+++ include/java-interp.h	(working copy)
@@ -34,6 +34,11 @@ details.  */
 
 struct _Jv_ResolvedMethod;
 
+#ifdef HAVE_FALLBACK_BACKTRACE
+struct _Jv_UnwindState;
+extern "C" void fallback_backtrace (_Jv_UnwindState *);
+#endif
+
 void _Jv_InitInterpreter ();
 void _Jv_DefineClass (jclass, jbyteArray, jint, jint,
 		      java::security::ProtectionDomain *,
@@ -209,6 +214,10 @@ class _Jv_InterpMethod : public _Jv_Meth
   friend class _Jv_StackTrace;
   friend class _Jv_InterpreterEngine;
 
+#ifdef HAVE_FALLBACK_BACKTRACE
+  friend void fallback_backtrace (_Jv_UnwindState *);
+#endif
+
 #ifdef JV_MARKOBJ_DECL
   friend JV_MARKOBJ_DECL;
 #endif
Index: sysdep/i386/backtrace.h
===================================================================
--- sysdep/i386/backtrace.h	(revision 115261)
+++ sysdep/i386/backtrace.h	(working copy)
@@ -13,8 +13,6 @@ details.  */
 
 #include <java-stack.h>
 
-#define HAVE_FALLBACK_BACKTRACE
-
 /* Store return addresses of the current program stack in
    STATE and return the exact number of values stored.  */
 void
@@ -65,17 +63,44 @@ fallback_backtrace (_Jv_UnwindState *sta
           if (scan_bytes[0] == 0x55 && scan_bytes[1] == 0x89
               && scan_bytes[2] == 0xE5)
             {
-              state->frames[i].start_ip = (void *)scan_addr;
+              // If we see the interpreter's main function, "pop" an entry
+              // off the interpreter stack and use that instead, so that the
+              // trace goes through the java code and not the interpreter
+              // itself. This assumes a 1:1 correspondance between call frames
+              // in the interpreted stack and occurances of
+              // _Jv_InterpMethod::run() on the native stack.
+            #ifdef INTERPRETER
+              void *interp_run = (void *) &_Jv_InterpMethod::run;
+              if ((void *)scan_addr == interp_run)
+                {
+                  state->frames[i].type = frame_interpreter;
+                  state->frames[i].interp.meth = state->interp_frame->self;
+                  state->frames[i].interp.pc = state->interp_frame->pc;
+                  state->interp_frame = state->interp_frame->next;
+                }
+              else
+            #endif /* INTERPRETER */
+                {
+                  state->frames[i].start_ip = (void *)scan_addr;
+                }
+
+              // If we have a trace callback function, call it now.
+              if (state->trace_function != NULL)
+                (state->trace_function) (state);
+
               break;
             }
         }
 
-      /* No need to unwind beyond JvRunMain().  */
-      if (state->frames[i].start_ip == (void *)JvRunMain)
+      /* No need to unwind beyond _Jv_RunMain().  */
+      void *jv_runmain
+        = (void *)(void (*)(JvVMInitArgs *, jclass, const char *, int,
+                            const char **, bool))_Jv_RunMain;
+      if (state->frames[i].start_ip == jv_runmain)
         break;
 
       i++;
+      state->pos = i;
     }
-  state->pos = i;
 }
 #endif

Re: [MinGW] RFA: Make Stack Traces Work for Interpreted Code on Windows

[Bryce McKinlay]

Hi Ranjit,

Ranjit Mathew wrote:
>   When run under the interpreter, an exception-throwing programme
> crashes the interpreter on Windows since JvRunMain() does not
> occur in the call stack and we once again roam the wilderness of
> arbitrary EBP values.
>
> The fix was to change fallback_backtrace() in
> "sysdep/i386/backtrace.h" to stop on _Jv_RunMain() instead since
> this does occur in both interpreted and natively-executed call
> stacks. (Since this function is overloaded, I needed to use an
> interim variable with the appropriate cast to tell the compiler
> which of these function variants to pick up.)
>   

Only the main thread will have JvRunMain on the stack - you probably 
also need to look for the thread-start routine to handle other threads.

Note that even then there are some situations (invocation API) where 
neither _Jv_RunMain or _Jv_ThreadStart will be on the stack.
> fallback_backtrace() also does not correctly handle unwinding
> through interpreted code. The fix was as simple as copying the
> relevant bits of code from _Jv_StackTrace::Unwind_TraceFn() and
> adjusting it a little bit.
>   

This part of the patch is a bit awkward - duplicating the 
interpreter-specific code is fragile and will make maintenance more 
difficult. Couldn't we instead have fallback_backtrace call the real 
UnwindTraceFn? I believe _Unwind_context is an opaque type, so it should 
be possible to have fallback_backtrace pass its own data there.
> (I note that under the interpreter we do
> not get line numbers for the innermost call frame, even on Linux.
> I haven't investigated this issue yet.)
>   
Hmm. There shouldn't be any problems here (on Linux) - do you have a 
test case?

Bryce

Re: [MinGW] RFA: Make Stack Traces Work for Interpreted Code on Windows

[Ranjit Mathew]

[Apologies if you see this message twice. Gmail's SMTP server is
acting weird.]

Bryce McKinlay wrote:
> Ranjit Mathew wrote:
>>
>> The fix was to change fallback_backtrace() in
>> "sysdep/i386/backtrace.h" to stop on _Jv_RunMain() instead since
>> this does occur in both interpreted and natively-executed call
>> stacks. (Since this function is overloaded, I needed to use an
>> interim variable with the appropriate cast to tell the compiler
>> which of these function variants to pick up.)
>>
>
> Only the main thread will have JvRunMain on the stack - you probably
> also need to look for the thread-start routine to handle other threads.

Good point. So I can stop unwinding when I see either
_Jv_RunMain() or _Jv_ThreadStart().

However...

> Note that even then there are some situations (invocation API) where
> neither _Jv_RunMain or _Jv_ThreadStart will be on the stack.

...would you happen to know what *will* be there on the call stack
in this case?


>> fallback_backtrace() also does not correctly handle unwinding
>> through interpreted code. The fix was as simple as copying the
>> relevant bits of code from _Jv_StackTrace::Unwind_TraceFn() and
>> adjusting it a little bit.
>>
>
> This part of the patch is a bit awkward - duplicating the
> interpreter-specific code is fragile and will make maintenance more
> difficult. Couldn't we instead have fallback_backtrace call the real
> UnwindTraceFn? I believe _Unwind_context is an opaque type, so it should
> be possible to have fallback_backtrace pass its own data there.

I sort of agree. However, if you look at the code there, it
calls _Unwind_GetRegionStart() and _Unwind_GetIPInfo() - I'll
have to #ifdef these out and somehow shoehorn the SJLJ EH
equivalents in there since the default implementations of
these functions provided by GCC for SJLJ EH are of no use to
us.

The bigger picture however is that I expect all this to be
unnecessary in 4.3 - we would hopefully switch to DW2 EH
for MinGW by then.


>> (I note that under the interpreter we do
>> not get line numbers for the innermost call frame, even on Linux.
>> I haven't investigated this issue yet.)
>>
> Hmm. There shouldn't be any problems here (on Linux) - do you have a
> test case?

Yes. See:
- ---------------------------- 8< ----------------------------
 1 public class Ex {
 2   void snafu( ) throws Exception {
 3     throw new Exception( "I do not like you!");
 4   }
 5   void bar( ) throws Exception {
 6     snafu( );
 7   }
 8   void foo( ) throws Exception {
 9     bar( );
10   }
11   public static void main( String[] args) throws Exception {
12     new Ex( ).foo( );
13   }
14 }
- ---------------------------- 8< ----------------------------

- ---------------------------- 8< ----------------------------
~/src/tmp > $MYGCJ --version
gcj (GCC) 4.2.0 20060701 (experimental)
Copyright (C) 2006 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

~/src/tmp > $MYGCJ --main=Ex -g Ex.java
~/src/tmp > ./a.out
Exception in thread "main" java.lang.Exception: I do not like you!
   at Ex.snafu(Ex.java:2)
   at Ex.bar(Ex.java:6)
   at Ex.foo(Ex.java:9)
   at Ex.main(Ex.java:12)
~/src/tmp > $MYGCJ -C Ex.java
~/src/tmp > mygij Ex
Exception in thread "main" java.lang.Exception: I do not like you!
   at Ex.snafu(Ex.java)
   at Ex.bar(Ex.java:6)
   at Ex.foo(Ex.java:9)
   at Ex.main(Ex.java:12)
~/src/tmp > mygij --version
java version "1.4.2"
gij (GNU libgcj) version 4.2.0 20060701 (experimental)

Copyright (C) 2006 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
~/src/tmp >
- ---------------------------- 8< ----------------------------

Note that this is a slightly older build and does not have your
_UnwindGetIPInfo() changes, so the problem has very likely been
fixed in recent builds.

Thanks,
Ranjit.

-- 
Ranjit Mathew      Email: rmathew AT gmail DOT com

Bangalore, INDIA.    Web: http://rmathew.com/

Re: [MinGW] RFA: Make Stack Traces Work for Interpreted Code on Windows

[Bryce McKinlay]

Ranjit Mathew wrote:
>> Note that even then there are some situations (invocation API) where
>> neither _Jv_RunMain or _Jv_ThreadStart will be on the stack.
>
> ...would you happen to know what *will* be there on the call stack
> in this case?

I don't think there is anything guaranteed in the invocation API case. 
For JNI you could look for the various JNI Call* functions, but even 
then it is tricky to figure out whether the frame you are looking at is 
the outermost JNI call.

>> This part of the patch is a bit awkward - duplicating the
>> interpreter-specific code is fragile and will make maintenance more
>> difficult. Couldn't we instead have fallback_backtrace call the real
>> UnwindTraceFn? I believe _Unwind_context is an opaque type, so it should
>> be possible to have fallback_backtrace pass its own data there.
>
> I sort of agree. However, if you look at the code there, it
> calls _Unwind_GetRegionStart() and _Unwind_GetIPInfo() - I'll
> have to #ifdef these out and somehow shoehorn the SJLJ EH
> equivalents in there since the default implementations of
> these functions provided by GCC for SJLJ EH are of no use to
> us.

I think you could just overload them with #defines in backtrace.h? 
Failing that, I think that adding a couple of #ifdef's in UnwindTraceFn 
is not as bad as the code duplication.

>> Hmm. There shouldn't be any problems here (on Linux) - do you have a
>> test case?
>
> Yes. See:
> - ---------------------------- 8< ----------------------------
> 1 public class Ex {
> 2   void snafu( ) throws Exception {
> 3     throw new Exception( "I do not like you!");
> 4   }
> 5   void bar( ) throws Exception {
> 6     snafu( );
> 7   }
> 8   void foo( ) throws Exception {
> 9     bar( );
> 10   }
> 11   public static void main( String[] args) throws Exception {
> 12     new Ex( ).foo( );
> 13   }
> 14 }
> - ---------------------------- 8< ----------------------------

Hmm, ok, I see this too. I've filed a bug:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28352

Bryce

Re: [MinGW] RFA: Make Stack Traces Work for Interpreted Code on Windows

[Ranjit Mathew]

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Bryce McKinlay wrote:
> Ranjit Mathew wrote:
>>> Note that even then there are some situations (invocation API) where
>>> neither _Jv_RunMain or _Jv_ThreadStart will be on the stack.
>> ...would you happen to know what *will* be there on the call stack
>> in this case?
> 
> I don't think there is anything guaranteed in the invocation API case. 
> For JNI you could look for the various JNI Call* functions, but even 
> then it is tricky to figure out whether the frame you are looking at is 
> the outermost JNI call.

For now, I'm giving up when the unwinding reaches "close enough"
to main(). :-/


> I think you could just overload them with #defines in backtrace.h? 
> Failing that, I think that adding a couple of #ifdef's in UnwindTraceFn 
> is not as bad as the code duplication.

That worked pretty nicely, thank you. It reduces unnecessary
duplication of code too. The attached patch implements it and
was tested using a crossed-native compiler for i686-pc-mingw32.
I still get usable stack traces for compiled and interpreted
code.

OK?

Thanks,
Ranjit.

- --
Ranjit Mathew       Email: rmathew AT gmail DOT com

Bangalore, INDIA.     Web: http://rmathew.com/




-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEtTFTYb1hx2wRS48RArjrAJ9mpsgaC7uv24LxehmpwLlxxAsnXQCgkwPf
Gm3Wb/KgCNNOVqZROV14tIU=
=pJ4u
-----END PGP SIGNATURE-----

[-- Attachment #2: p3.txt --]
[-- Type: text/plain, Size: 7421 bytes --]

Index: ChangeLog
from  Ranjit Mathew  <rmathew@gcc.gnu.org>

	* stacktrace.cc (_Jv_StackTrace::GetStackTrace): Pass UnwindTraceFn()
	to fallback_backtrace().
	(_Jv_StackTrace::GetCallerInfo): Enable even for targets using SJLJ
	EH and use fallback_backtrace() in that case.
	(_Jv_StackTrace::GetClassContext): Pass UnwindTraceFn() to
	fallback_backtrace().
	(_Jv_StackTrace::GetFirstNonSystemClassLoader): Likewise.
	* sysdep/generic/backtrace.h (fallback_backtrace): Accept an
	additional unwind trace function argument.
	* sysdep/i386/backtrace.h (HAVE_FALLBACK_BACKTRACE): Do not define.
	(_Unwind_GetIPInfo): Define macro.
	(_Unwind_GetRegionStart): Likewise.
	(fallback_backtrace): Accept additional unwind trace function
	argument.  Call it during unwinding.  Stop when any of _Jv_RunMain(),
	_Jv_ThreadStart() or main() is seen during unwinding.

Index: stacktrace.cc
===================================================================
--- stacktrace.cc	(revision 115260)
+++ stacktrace.cc	(working copy)
@@ -156,7 +156,7 @@ _Jv_StackTrace::GetStackTrace(void)
 #ifdef SJLJ_EXCEPTIONS
   // The Unwind interface doesn't work with the SJLJ exception model.
   // Fall back to a platform-specific unwinder.
-  fallback_backtrace (&state);
+  fallback_backtrace (UnwindTraceFn, &state);
 #else /* SJLJ_EXCEPTIONS */  
   _Unwind_Backtrace (UnwindTraceFn, &state);
 #endif /* SJLJ_EXCEPTIONS */
@@ -421,7 +421,6 @@ void
 _Jv_StackTrace::GetCallerInfo (jclass checkClass, jclass *caller_class,
   _Jv_Method **caller_meth)
 {
-#ifndef SJLJ_EXCEPTIONS
   int trace_size = 20;
   _Jv_StackFrame frames[trace_size];
   _Jv_UnwindState state (trace_size);
@@ -439,15 +438,18 @@ _Jv_StackTrace::GetCallerInfo (jclass ch
   //JvSynchronized (ncodeMap);
   UpdateNCodeMap ();
 
+#ifdef SJLJ_EXCEPTIONS
+  // The Unwind interface doesn't work with the SJLJ exception model.
+  // Fall back to a platform-specific unwinder.
+  fallback_backtrace (UnwindTraceFn, &state);
+#else /* SJLJ_EXCEPTIONS */  
   _Unwind_Backtrace (UnwindTraceFn, &state);
+#endif /* SJLJ_EXCEPTIONS */
   
   if (caller_class)
     *caller_class = trace_data.foundClass;
   if (caller_meth)
     *caller_meth = trace_data.foundMeth;
-#else
-  return;
-#endif
 }
 
 // Return a java array containing the Java classes on the stack above CHECKCLASS.
@@ -467,7 +469,7 @@ _Jv_StackTrace::GetClassContext (jclass 
 #ifdef SJLJ_EXCEPTIONS
   // The Unwind interface doesn't work with the SJLJ exception model.
   // Fall back to a platform-specific unwinder.
-  fallback_backtrace (&state);
+  fallback_backtrace (UnwindTraceFn, &state);
 #else /* SJLJ_EXCEPTIONS */  
   _Unwind_Backtrace (UnwindTraceFn, &state);
 #endif /* SJLJ_EXCEPTIONS */  
@@ -544,7 +546,7 @@ _Jv_StackTrace::GetFirstNonSystemClassLo
 #ifdef SJLJ_EXCEPTIONS
   // The Unwind interface doesn't work with the SJLJ exception model.
   // Fall back to a platform-specific unwinder.
-  fallback_backtrace (&state);
+  fallback_backtrace (UnwindTraceFn, &state);
 #else /* SJLJ_EXCEPTIONS */  
   _Unwind_Backtrace (UnwindTraceFn, &state);
 #endif /* SJLJ_EXCEPTIONS */  
Index: sysdep/generic/backtrace.h
===================================================================
--- sysdep/generic/backtrace.h	(revision 115260)
+++ sysdep/generic/backtrace.h	(working copy)
@@ -16,7 +16,7 @@ details.  */
 /* Store return addresses of the current program stack in
    STATE and return the exact number of values stored.  */
 void
-fallback_backtrace (_Jv_UnwindState *)
+fallback_backtrace (_Unwind_Trace_Fn, _Jv_UnwindState *)
 {
 }
 #endif
Index: sysdep/i386/backtrace.h
===================================================================
--- sysdep/i386/backtrace.h	(revision 115261)
+++ sysdep/i386/backtrace.h	(working copy)
@@ -13,21 +13,39 @@ details.  */
 
 #include <java-stack.h>
 
-#define HAVE_FALLBACK_BACKTRACE
+extern int main (int, char **);
+
+/* The context used to keep track of our position while unwinding through
+   the call stack.  */
+struct _Unwind_Context
+{
+  /* The starting address of the method.  */
+  _Jv_uintptr_t meth_addr;
+
+  /* The return address in the method.  */
+  _Jv_uintptr_t ret_addr;
+};
+
+#undef _Unwind_GetIPInfo
+#define _Unwind_GetIPInfo(ctx,ip_before_insn) \
+  (*(ip_before_insn) = 1, (ctx)->ret_addr)
+
+#undef _Unwind_GetRegionStart
+#define _Unwind_GetRegionStart(ctx) \
+  ((ctx)->meth_addr)
+
 
 /* Store return addresses of the current program stack in
    STATE and return the exact number of values stored.  */
 void
-fallback_backtrace (_Jv_UnwindState *state)
+fallback_backtrace (_Unwind_Trace_Fn trace_fn, _Jv_UnwindState *state)
 {
   register _Jv_uintptr_t *_ebp __asm__ ("ebp");
   register _Jv_uintptr_t _esp __asm__ ("esp");
   _Jv_uintptr_t rfp;
+  _Unwind_Context ctx;
 
-  int i = state->pos;
-  for (rfp = *_ebp;
-       rfp && i < state->length;
-       rfp = *(_Jv_uintptr_t *)rfp)
+  for (rfp = *_ebp; rfp; rfp = *(_Jv_uintptr_t *)rfp)
     {
       /* Sanity checks to eliminate dubious-looking frame pointer chains.
          The frame pointer should be a 32-bit word-aligned stack address.
@@ -42,12 +60,7 @@ fallback_backtrace (_Jv_UnwindState *sta
 
       /* Get the return address in the calling function.  This is stored on
          the stack just before the value of the old frame pointer.  */
-      _Jv_uintptr_t ret_addr
-        = *(_Jv_uintptr_t *)(rfp + sizeof (_Jv_uintptr_t));
-
-      state->frames[i].type = frame_native;
-      state->frames[i].ip = (void *)(ret_addr - 1);
-      state->frames[i].start_ip = NULL;
+      ctx.ret_addr = *(_Jv_uintptr_t *)(rfp + sizeof (_Jv_uintptr_t));
 
       /* Try to locate a "pushl %ebp; movl %esp, %ebp" function prologue
          by scanning backwards at even addresses below the return address.
@@ -56,7 +69,8 @@ fallback_backtrace (_Jv_UnwindState *sta
          FIXME: This is not robust and will probably give us false positives,
          but this is about the best we can do if we do not have DWARF-2 unwind
          information based exception handling.  */
-      _Jv_uintptr_t scan_addr = (ret_addr & 0xFFFFFFFE) - 2;
+      ctx.meth_addr = (_Jv_uintptr_t)NULL;
+      _Jv_uintptr_t scan_addr = (ctx.ret_addr & 0xFFFFFFFE) - 2;
       _Jv_uintptr_t limit_addr
         = (scan_addr > 1024 * 1024) ? (scan_addr - 1024 * 1024) : 2;
       for ( ; scan_addr >= limit_addr; scan_addr -= 2)
@@ -65,17 +79,24 @@ fallback_backtrace (_Jv_UnwindState *sta
           if (scan_bytes[0] == 0x55 && scan_bytes[1] == 0x89
               && scan_bytes[2] == 0xE5)
             {
-              state->frames[i].start_ip = (void *)scan_addr;
+              ctx.meth_addr = scan_addr;
               break;
             }
         }
 
-      /* No need to unwind beyond JvRunMain().  */
-      if (state->frames[i].start_ip == (void *)JvRunMain)
+      /* Now call the unwinder callback function. */
+      if (trace_fn != NULL)
+        (*trace_fn) (&ctx, state);
+
+      /* No need to unwind beyond _Jv_RunMain(), _Jv_ThreadStart or
+         main().  */
+      void *jv_runmain
+        = (void *)(void (*)(JvVMInitArgs *, jclass, const char *, int,
+                            const char **, bool))_Jv_RunMain;
+      if (ctx.meth_addr == (_Jv_uintptr_t)jv_runmain
+          || ctx.meth_addr == (_Jv_uintptr_t)_Jv_ThreadStart
+          || (ctx.meth_addr - (_Jv_uintptr_t)main) < 16)
         break;
-
-      i++;
     }
-  state->pos = i;
 }
 #endif

Re: [MinGW] RFA: Make Stack Traces Work for Interpreted Code on Windows

[Bryce McKinlay]

Ranjit Mathew wrote:
>> I think you could just overload them with #defines in backtrace.h? 
>> Failing that, I think that adding a couple of #ifdef's in UnwindTraceFn 
>> is not as bad as the code duplication.
>>     
>
> That worked pretty nicely, thank you. It reduces unnecessary
> duplication of code too. The attached patch implements it and
> was tested using a crossed-native compiler for i686-pc-mingw32.
> I still get usable stack traces for compiled and interpreted
> code.
>
> OK?
>   

It occurs to me that you could now actually #define fallback_backtrace 
as _Unwind_Backtrace, and get rid of some #ifdefs in stacktrace.cc. Do 
you think that is worth doing? Otherwise, this is OK for trunk.

Bryce

Re: [MinGW] RFA: Make Stack Traces Work for Interpreted Code on Windows

[Ranjit Mathew]

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Bryce McKinlay wrote:
> Ranjit Mathew wrote:
>>> I think you could just overload them with #defines in backtrace.h? 
>>> Failing that, I think that adding a couple of #ifdef's in UnwindTraceFn 
>>> is not as bad as the code duplication.
>>>     
>> That worked pretty nicely, thank you. It reduces unnecessary
>> duplication of code too. The attached patch implements it and
>> was tested using a crossed-native compiler for i686-pc-mingw32.
>> I still get usable stack traces for compiled and interpreted
>> code.
>>
>> OK?
> 
> It occurs to me that you could now actually #define fallback_backtrace 
> as _Unwind_Backtrace, and get rid of some #ifdefs in stacktrace.cc. Do 
> you think that is worth doing? Otherwise, this is OK for trunk.

I've applied the attached patch to trunk. It defines _Unwind_Backtrace()
as fallback_backtrace() if SJLJ EH is in use. It was tested on
i686-pc-mingw32.

Even after this patch everything is not OK with the interpreter
and unwinding. While I get proper stack traces for programmes
that throw uncaught exceptions, I get a crashed interpreter for
programmes where the exception is caught. Some of the frames just
before _Jv_InterpMethod::run() have wild return addresses (even within
GDB) and I do not immediately understand why. I'll investigate this
further.
- -------------------------------- 8< --------------------------------
#13 0x0043bcd0 in _Jv_InterpMethod::run (retp=0x22fd80, args=0x22fda0,
    meth=0x2cd6f60) at /extra/src/gcc/gcc/libjava/interpret.cc:1221
#14 0xf8836606 in ?? ()
#15 0x0022fd80 in ?? ()
#16 0x0022fda0 in ?? ()
#17 0x02cd6f60 in ?? ()
#18 0x02cd6f60 in ?? ()
#19 0x02a1f098 in ?? ()
#20 0x02a139d0 in ?? ()
#21 0x0022fd98 in ?? ()
#22 0x00567e02 in _ZN4java4lang7reflect8Modifier8isPublicEJbi (mod=44138424)
    at /extra/src/gcc/gcc/libjava/java/lang/reflect/Modifier.java:257
#23 0x005e2be8 in gnu::java::lang::MainThread::call_main (this=0x2cc0fa0)
    at /extra/src/gcc/gcc/libjava/gnu/java/lang/natMainThread.cc:50
#24 0x0042fdff in _ZN3gnu4java4lang10MainThread3runEJvv (this=0x2cc0fa0)
    at /extra/src/gcc/gcc/libjava/gnu/java/lang/MainThread.java:108
#25 0x004550f4 in _Jv_ThreadRun (thread=0x2cc0fa0)
    at /extra/src/gcc/gcc/libjava/java/lang/natThread.cc:302
#26 0x00405136 in _Jv_RunMain (vm_args=0x22ff48, klass=0x0,
    name=0x3f2488 "Ex12", argc=1, argv=0x3f2504, is_jar=false)
    at /extra/src/gcc/gcc/libjava/prims.cc:1542
#27 0x004015e8 in main (argc=12, argv=0x7801392e)
    at /extra/src/gcc/gcc/libjava/gij.cc:333
- -------------------------------- 8< --------------------------------

Thanks,
Ranjit.

- --
Ranjit Mathew       Email: rmathew AT gmail DOT com

Bangalore, INDIA.     Web: http://rmathew.com/




-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEt9O4Yb1hx2wRS48RAgyMAJ92tLmo2x3Q3t6jDGrLAEZriwGD5wCfTik8
4tPj7wleovKU8ZLejmbWOpU=
=BJrv
-----END PGP SIGNATURE-----

[-- Attachment #2: p4.txt --]
[-- Type: text/plain, Size: 8171 bytes --]

Index: ChangeLog
from  Ranjit Mathew  <rmathew@gcc.gnu.org>

	* stacktrace.cc (_Jv_StackTrace::GetStackTrace): Unconditionally use
	_Unwind_Backtrace().
	(_Jv_StackTrace::GetCallerInfo): Enable even for targets using SJLJ
	EH.
	(_Jv_StackTrace::GetClassContext): Unconditionally use
	_Unwind_Backtrace().
	(_Jv_StackTrace::GetFirstNonSystemClassLoader): Likewise.
	* sysdep/i386/backtrace.h (HAVE_FALLBACK_BACKTRACE): Do not define.
	(_Unwind_GetIPInfo): Define macro if SJLJ EH is in use.
	(_Unwind_GetRegionStart): Likewise.
	(_Unwind_Backtrace): Likewise.
	(fallback_backtrace): Accept additional unwind trace function
	argument.  Call it during unwinding.  Stop when any of _Jv_RunMain(),
	_Jv_ThreadStart() or main() is seen during unwinding.
	* sysdep/generic/backtrace.h (fallback_backtrace): Accept an
	additional unwind trace function argument.

Index: stacktrace.cc
===================================================================
--- stacktrace.cc	(revision 115260)
+++ stacktrace.cc	(working copy)
@@ -153,13 +153,7 @@ _Jv_StackTrace::GetStackTrace(void)
   _Jv_UnwindState state (trace_size);
   state.frames = (_Jv_StackFrame *) &frames;
 
-#ifdef SJLJ_EXCEPTIONS
-  // The Unwind interface doesn't work with the SJLJ exception model.
-  // Fall back to a platform-specific unwinder.
-  fallback_backtrace (&state);
-#else /* SJLJ_EXCEPTIONS */  
   _Unwind_Backtrace (UnwindTraceFn, &state);
-#endif /* SJLJ_EXCEPTIONS */
   
   // Copy the trace and return it.
   int traceSize = sizeof (_Jv_StackTrace) + 
@@ -421,7 +415,6 @@ void
 _Jv_StackTrace::GetCallerInfo (jclass checkClass, jclass *caller_class,
   _Jv_Method **caller_meth)
 {
-#ifndef SJLJ_EXCEPTIONS
   int trace_size = 20;
   _Jv_StackFrame frames[trace_size];
   _Jv_UnwindState state (trace_size);
@@ -445,9 +438,6 @@ _Jv_StackTrace::GetCallerInfo (jclass ch
     *caller_class = trace_data.foundClass;
   if (caller_meth)
     *caller_meth = trace_data.foundMeth;
-#else
-  return;
-#endif
 }
 
 // Return a java array containing the Java classes on the stack above CHECKCLASS.
@@ -464,13 +454,7 @@ _Jv_StackTrace::GetClassContext (jclass 
   //JvSynchronized (ncodeMap);
   UpdateNCodeMap ();
 
-#ifdef SJLJ_EXCEPTIONS
-  // The Unwind interface doesn't work with the SJLJ exception model.
-  // Fall back to a platform-specific unwinder.
-  fallback_backtrace (&state);
-#else /* SJLJ_EXCEPTIONS */  
   _Unwind_Backtrace (UnwindTraceFn, &state);
-#endif /* SJLJ_EXCEPTIONS */  
 
   // Count the number of Java frames on the stack.
   int jframe_count = 0;
@@ -541,13 +525,7 @@ _Jv_StackTrace::GetFirstNonSystemClassLo
   //JvSynchronized (ncodeMap);
   UpdateNCodeMap ();
   
-#ifdef SJLJ_EXCEPTIONS
-  // The Unwind interface doesn't work with the SJLJ exception model.
-  // Fall back to a platform-specific unwinder.
-  fallback_backtrace (&state);
-#else /* SJLJ_EXCEPTIONS */  
   _Unwind_Backtrace (UnwindTraceFn, &state);
-#endif /* SJLJ_EXCEPTIONS */  
 
   if (state.trace_data)
     return (ClassLoader *) state.trace_data;
Index: sysdep/i386/backtrace.h
===================================================================
--- sysdep/i386/backtrace.h	(revision 115261)
+++ sysdep/i386/backtrace.h	(working copy)
@@ -13,21 +13,46 @@ details.  */
 
 #include <java-stack.h>
 
-#define HAVE_FALLBACK_BACKTRACE
+extern int main (int, char **);
 
-/* Store return addresses of the current program stack in
-   STATE and return the exact number of values stored.  */
-void
-fallback_backtrace (_Jv_UnwindState *state)
+/* The context used to keep track of our position while unwinding through
+   the call stack.  */
+struct _Unwind_Context
+{
+  /* The starting address of the method.  */
+  _Jv_uintptr_t meth_addr;
+
+  /* The return address in the method.  */
+  _Jv_uintptr_t ret_addr;
+};
+
+#ifdef SJLJ_EXCEPTIONS
+
+#undef _Unwind_GetIPInfo
+#define _Unwind_GetIPInfo(ctx,ip_before_insn) \
+  (*(ip_before_insn) = 1, (ctx)->ret_addr)
+
+#undef _Unwind_GetRegionStart
+#define _Unwind_GetRegionStart(ctx) \
+  ((ctx)->meth_addr)
+
+#undef _Unwind_Backtrace
+#define _Unwind_Backtrace(trace_fn,state_ptr) \
+  (fallback_backtrace (trace_fn, state_ptr))
+
+#endif /* SJLJ_EXCEPTIONS */
+
+/* Unwind through the call stack calling TRACE_FN with STATE for each stack
+   frame.  Returns the reason why the unwinding was stopped.  */
+_Unwind_Reason_Code
+fallback_backtrace (_Unwind_Trace_Fn trace_fn, _Jv_UnwindState *state)
 {
   register _Jv_uintptr_t *_ebp __asm__ ("ebp");
   register _Jv_uintptr_t _esp __asm__ ("esp");
   _Jv_uintptr_t rfp;
+  _Unwind_Context ctx;
 
-  int i = state->pos;
-  for (rfp = *_ebp;
-       rfp && i < state->length;
-       rfp = *(_Jv_uintptr_t *)rfp)
+  for (rfp = *_ebp; rfp; rfp = *(_Jv_uintptr_t *)rfp)
     {
       /* Sanity checks to eliminate dubious-looking frame pointer chains.
          The frame pointer should be a 32-bit word-aligned stack address.
@@ -42,12 +67,7 @@ fallback_backtrace (_Jv_UnwindState *sta
 
       /* Get the return address in the calling function.  This is stored on
          the stack just before the value of the old frame pointer.  */
-      _Jv_uintptr_t ret_addr
-        = *(_Jv_uintptr_t *)(rfp + sizeof (_Jv_uintptr_t));
-
-      state->frames[i].type = frame_native;
-      state->frames[i].ip = (void *)(ret_addr - 1);
-      state->frames[i].start_ip = NULL;
+      ctx.ret_addr = *(_Jv_uintptr_t *)(rfp + sizeof (_Jv_uintptr_t));
 
       /* Try to locate a "pushl %ebp; movl %esp, %ebp" function prologue
          by scanning backwards at even addresses below the return address.
@@ -56,7 +76,8 @@ fallback_backtrace (_Jv_UnwindState *sta
          FIXME: This is not robust and will probably give us false positives,
          but this is about the best we can do if we do not have DWARF-2 unwind
          information based exception handling.  */
-      _Jv_uintptr_t scan_addr = (ret_addr & 0xFFFFFFFE) - 2;
+      ctx.meth_addr = (_Jv_uintptr_t)NULL;
+      _Jv_uintptr_t scan_addr = (ctx.ret_addr & 0xFFFFFFFE) - 2;
       _Jv_uintptr_t limit_addr
         = (scan_addr > 1024 * 1024) ? (scan_addr - 1024 * 1024) : 2;
       for ( ; scan_addr >= limit_addr; scan_addr -= 2)
@@ -65,17 +86,26 @@ fallback_backtrace (_Jv_UnwindState *sta
           if (scan_bytes[0] == 0x55 && scan_bytes[1] == 0x89
               && scan_bytes[2] == 0xE5)
             {
-              state->frames[i].start_ip = (void *)scan_addr;
+              ctx.meth_addr = scan_addr;
               break;
             }
         }
 
-      /* No need to unwind beyond JvRunMain().  */
-      if (state->frames[i].start_ip == (void *)JvRunMain)
+      /* Now call the unwinder callback function. */
+      if (trace_fn != NULL)
+        (*trace_fn) (&ctx, state);
+
+      /* No need to unwind beyond _Jv_RunMain(), _Jv_ThreadStart or
+         main().  */
+      void *jv_runmain
+        = (void *)(void (*)(JvVMInitArgs *, jclass, const char *, int,
+                            const char **, bool))_Jv_RunMain;
+      if (ctx.meth_addr == (_Jv_uintptr_t)jv_runmain
+          || ctx.meth_addr == (_Jv_uintptr_t)_Jv_ThreadStart
+          || (ctx.meth_addr - (_Jv_uintptr_t)main) < 16)
         break;
-
-      i++;
     }
-  state->pos = i;
+
+  return _URC_NO_REASON;
 }
 #endif
Index: sysdep/generic/backtrace.h
===================================================================
--- sysdep/generic/backtrace.h	(revision 115260)
+++ sysdep/generic/backtrace.h	(working copy)
@@ -1,6 +1,6 @@
 // backtrace.h - Fallback backtrace implementation. default implementation.
 
-/* Copyright (C) 2005  Free Software Foundation
+/* Copyright (C) 2005, 2006  Free Software Foundation
 
    This file is part of libgcj.
 
@@ -13,10 +13,11 @@ details.  */
 
 #include <java-stack.h>
 
-/* Store return addresses of the current program stack in
-   STATE and return the exact number of values stored.  */
-void
-fallback_backtrace (_Jv_UnwindState *)
+/* Unwind through the call stack calling TRACE_FN with STATE for every stack
+   frame.  Returns the reason why the unwinding was stopped.  */
+_Unwind_Reason_Code
+fallback_backtrace (_Unwind_Trace_Fn, _Jv_UnwindState *)
 {
+  return _URC_NO_REASON;
 }
 #endif

Re: [MinGW] RFA: Make Stack Traces Work for Interpreted Code on Windows

[Bryce McKinlay]

Ranjit Mathew wrote:
>  /* Store return addresses of the current program stack in
>     STATE and return the exact number of values stored.  */
>  void
> -fallback_backtrace (_Jv_UnwindState *state)
> +fallback_backtrace (_Unwind_Trace_Fn trace_fn, _Jv_UnwindState *state)
>  {
>   


The comment here should also be updated.

Bryce