request branch patch approval: libgcj stack traces from reflection data (required for Win32)

request branch patch approval: libgcj stack traces from reflection data (required for Win32)

[Adam Megacz]

Mitchell and java@gcc,

I'm sorry that this is so late, but I think it warrants consideration
for 3.1. Without it, printStackTrace() doesn't work on Win32. This is
probably the last really-important feature that gcj-Win32 is missing.

If this is too risky, please consider accepting the patch with "#ifdef
WIN32" wrapped around it; that way it is unlikely to break anything
that already works.

This prerequires the patch which I mistakenly committed this afternoon
without your approval (sorry about that; I'm behind on reading
gcc@gcc).

  - a


2002-04-23  Adam Megacz <adam@xwt.org>

        * java/lang/Class.h (_Jv_GetAllMethods): Added this.
        * java/lang/ClassLoader.java (getAllMethods,
        getAllMethodAddrs): Added these.
        * java/lang/Throwable.java (longFromStackTraceBytes,
        printStackTrace): Added a fallback -- if CPlusPlusDemangler
        fails, libgcj will use reflection data to generate a stack
        trace without line numbers.
        * java/lang/natClassLoader.cc (getAllMethods,
        getAllMethodAddrs, _Jv_GetAllMethods): Added functions to dump
        a list of all methods of all registered classes.
        * java/lang/natThrowable.cc (longFromStackTraceBytes): Added
        this helper function, which reads a properly-endianized
        pointer out of a jbyte[] and returns it as a jlong.

Index: Class.h
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/lang/Class.h,v
retrieving revision 1.43
diff -u -r1.43 Class.h
--- Class.h	21 Dec 2001 19:47:50 -0000	1.43
+++ Class.h	24 Apr 2002 03:06:14 -0000
@@ -308,6 +308,7 @@
   friend void _Jv_LayoutVTableMethods (jclass klass);
   friend void _Jv_SetVTableEntries (jclass, _Jv_VTable *);
   friend void _Jv_MakeVTable (jclass);
+  friend JArray<java::lang::reflect::Method*>* _Jv_GetAllMethods();
 
   // Return array class corresponding to element type KLASS, creating it if
   // necessary.
Index: ClassLoader.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/lang/ClassLoader.java,v
retrieving revision 1.16
diff -u -r1.16 ClassLoader.java
--- ClassLoader.java	7 Dec 2001 23:34:12 -0000	1.16
+++ ClassLoader.java	24 Apr 2002 03:06:14 -0000
@@ -577,4 +577,8 @@
     // Default to returning null.  Derived classes implement this.
     return null;
   }
+
+  static native java.lang.reflect.Method[] getAllMethods();
+  static native long[] getAllMethodAddrs();
+
 }
Index: Throwable.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/lang/Throwable.java,v
retrieving revision 1.10
diff -u -r1.10 Throwable.java
--- Throwable.java	24 Feb 2001 03:52:49 -0000	1.10
+++ Throwable.java	24 Apr 2002 03:06:14 -0000
@@ -123,21 +123,64 @@
     printStackTrace (writer);
   }
 
+  private native static long longFromStackTraceBytes(byte[] stackTrace, int i);
+
   public void printStackTrace (PrintWriter wr)
   {
-    try
-      {
-	CPlusPlusDemangler cPlusPlusFilter = new CPlusPlusDemangler (wr);
-	PrintWriter writer = new PrintWriter (cPlusPlusFilter);
-	printRawStackTrace (writer);	
-	writer.close ();
-	if (cPlusPlusFilter.written == 0) // The demangler has failed...
-	  printRawStackTrace (wr);
+      try
+          {
+              CPlusPlusDemangler cPlusPlusFilter = new CPlusPlusDemangler (wr);
+              PrintWriter writer = new PrintWriter (cPlusPlusFilter);
+              printRawStackTrace (writer);	
+              writer.close ();
+              if (cPlusPlusFilter.written > 0) return;
+          }
+      catch (Exception e1)
+          {
+          }
+
+      wr.println(toString());
+      if (stackTrace == null) {
+          wr.flush();
+          return;
       }
-    catch (Exception e1)
-      {
-	printRawStackTrace (wr);
+      
+      long[] allAddrs = ClassLoader.getAllMethodAddrs();
+      java.lang.reflect.Method[] meths = ClassLoader.getAllMethods();
+      
+      // FIXME: assumes little endian
+      for(int i=0; i<stackTrace.length; i++) {
+          long addr = longFromStackTraceBytes(stackTrace, i);
+          if (addr == 0) break;
+          
+          int whichMethod = -1;
+          for(int j=0; j<allAddrs.length; j++) {
+              if (allAddrs[j] <= addr &&
+                  (whichMethod == -1 || allAddrs[whichMethod] < allAddrs[j])) {
+                  whichMethod = j;
+              }
+          }
+          
+          if (whichMethod == -1) {
+              wr.println("[" + Long.toString(addr, 16) + "]  " + "??");
+              continue;
+          }
+          
+          if (meths[whichMethod].getDeclaringClass().getName().equals("gnu.gcj.runtime.FirstThread") &&
+              meths[whichMethod].getName().equals("call_main"))
+              break;
+          
+          wr.println("    [" + Long.toString(addr, 16) + "] " +
+                     meths[whichMethod].getDeclaringClass().getName() + "." +
+                     meths[whichMethod].getName() + "() " +
+                     "+" + (addr - allAddrs[whichMethod])
+                     );
+          
+          if (java.lang.Thread.class.isAssignableFrom(meths[whichMethod].getDeclaringClass()) &&
+              meths[whichMethod].getName().equals("run"))
+              break;
       }
+      wr.flush();
   }
 
   public Throwable ()
Index: natClassLoader.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/lang/natClassLoader.cc,v
retrieving revision 1.47.8.1
diff -u -r1.47.8.1 natClassLoader.cc
--- natClassLoader.cc	2 Apr 2002 22:19:55 -0000	1.47.8.1
+++ natClassLoader.cc	24 Apr 2002 03:06:14 -0000
@@ -40,6 +40,10 @@
 #include <java/lang/StringBuffer.h>
 #include <java/io/Serializable.h>
 #include <java/lang/Cloneable.h>
+#include <java/lang/reflect/Method.h>
+
+#include<java/lang/reflect/Constructor.h>
+#include<gcj/method.h>
 
 // FIXME: remove these.
 #define CloneableClass java::lang::Cloneable::class$
@@ -347,6 +351,50 @@
 static jclass loaded_classes[HASH_LEN];
 
 // This is the root of a linked list of classes
+
+JArray<java::lang::reflect::Method*>*
+java::lang::ClassLoader::getAllMethods()
+{
+    return _Jv_GetAllMethods();
+}
+
+JArray<jlong>*
+java::lang::ClassLoader::getAllMethodAddrs()
+{
+    JArray<java::lang::reflect::Method*>* arr = _Jv_GetAllMethods();
+    java::lang::reflect::Method** el = (java::lang::reflect::Method**)elements(arr);
+    JArray<jlong>* ret = JvNewLongArray(arr->length);
+    jlong* retel = (jlong*)elements(ret);
+    for(int i=0; i<arr->length; i++)
+        retel[i] = (jlong)((unsigned int)(_Jv_FromReflectedMethod (el[i])->ncode));
+    return ret;
+}
+
+JArray<java::lang::reflect::Method*>*
+_Jv_GetAllMethods()
+{
+  int numMethods = 0;
+
+  for(int i=0; i<HASH_LEN; i++)
+      for(jclass c = loaded_classes[i]; c; c = c->next)
+          numMethods += c->getDeclaredMethods()->length;
+
+  JArray<java::lang::reflect::Method*>* ret =
+    (JArray<java::lang::reflect::Method*>*)
+    JvNewObjectArray(numMethods, &java::lang::reflect::Method::class$, NULL);
+
+  java::lang::reflect::Method** el = (java::lang::reflect::Method**)elements(ret);
+
+  for(int i=0; i<HASH_LEN; i++)
+      for(jclass c = loaded_classes[i]; c; c = c->next) {
+          JArray<java::lang::reflect::Method*>* methods = c->getDeclaredMethods();
+          jint len = methods->length;
+          java::lang::reflect::Method** meths = (java::lang::reflect::Method**)elements(methods);
+          for(int j=0; j<len; j++) el[--numMethods] = meths[j];
+      }
+
+  return ret;
+}
 
 \f
 
Index: natThrowable.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/lang/natThrowable.cc,v
retrieving revision 1.11
diff -u -r1.11 natThrowable.cc
--- natThrowable.cc	7 Feb 2002 19:26:06 -0000	1.11
+++ natThrowable.cc	24 Apr 2002 03:06:14 -0000
@@ -32,6 +32,7 @@
 #include <stdio.h>
 
 #include <unistd.h>
+#include <platform.h>
 
 #ifdef HAVE_EXECINFO_H
 #include <execinfo.h>
@@ -102,3 +103,18 @@
 #endif /* HAVE_BACKTRACE */
   wr->flush ();
 }
+
+// Returns the i^th call address in the stackTrace member, or 0 if i
+// is beyond the end of the trace. This has to be done in C++ because
+// the addresses in stackTrace are the same width as the platform's
+// pointers (which is unknown to Java code), and stackTrace is a
+// byte[] using the platform's endianness (which is unknown to Java
+// code).
+jlong
+java::lang::Throwable::longFromStackTraceBytes(jbyteArray stackArr, jint i)
+{
+    if (i * sizeof(void*) > stackArr->length) return 0;
+    unsigned int* stack = (unsigned int*)elements(stackArr);
+    return (jlong)stack[i];
+}
+

Re: request branch patch approval: libgcj stack traces from reflection data (required for Win32)

[Bryce McKinlay]

Adam Megacz wrote:

>I'm sorry that this is so late, but I think it warrants consideration
>for 3.1. Without it, printStackTrace() doesn't work on Win32. This is
>probably the last really-important feature that gcj-Win32 is missing.
>

Stack tracing on win32 is a new feature rather than a bug fix, so it is 
probibly not appropriate to commit this patch to the branch at this 
time. I suggest waiting until the 3.1 release is made before committing 
it to the branch. It is also quite a significant patch and I'd like to 
have a better look at it soon (unfortunatly I'm both busy and 
PowerBook-less right now so don't have a lot of time).

regards

Bryce.

Re: request branch patch approval: libgcj stack traces from reflection data (required for Win32)

[Mark Mitchell]

--On Wednesday, April 24, 2002 05:40:56 PM +1200 Bryce McKinlay 
<bryce@waitaki.otago.ac.nz> wrote:

> Adam Megacz wrote:
>
>> I'm sorry that this is so late, but I think it warrants consideration
>> for 3.1. Without it, printStackTrace() doesn't work on Win32. This is
>> probably the last really-important feature that gcj-Win32 is missing.
>>
>
> Stack tracing on win32 is a new feature rather than a bug fix, so it is
> probibly not appropriate to commit this patch to the branch at this time.

I agree.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: request branch patch approval: libgcj stack traces from reflection data (required for Win32)

[Tom Tromey]

>>>>> "Bryce" == Bryce McKinlay <bryce@waitaki.otago.ac.nz> writes:

Bryce> Stack tracing on win32 is a new feature rather than a bug fix,
Bryce> so it is probibly not appropriate to commit this patch to the
Bryce> branch at this time. I suggest waiting until the 3.1 release is
Bryce> made before committing it to the branch. It is also quite a
Bryce> significant patch

I concur.  I think this patch is too invasive for this point in the
release process.

Adam, Bryce suggested that this time we try to put more changes into
the successive 3.1 releases.  I think that's a good idea too (and
hopefully the SC and/or Mark agrees).  So this patch is something we
should put in 3.1.1.

One unresolved issue about Bryce's plan is how much importance we give
to binary compatibility.  I think it is still early for us to worry
about that, but I could understand why others might disagree.  I build
a compiler twice a week, at least, so rebuilding stuff isn't a pain
for me like it might be for other people.

Tom

Re: request branch patch approval: libgcj stack traces from reflection data (required for Win32)

[Mark Mitchell]

> Adam, Bryce suggested that this time we try to put more changes into
> the successive 3.1 releases.  I think that's a good idea too (and
> hopefully the SC and/or Mark agrees).  So this patch is something we
> should put in 3.1.1.

Let's bring it up after the release.  The dot releases are to fix bugs
in the release -- not add features -- but it's possible we might make
an exception if the whole Java team feels its *vital* and not just
"useful and safe", which is the more common argument.

> One unresolved issue about Bryce's plan is how much importance we give
> to binary compatibility.  I think it is still early for us to worry
> about that, but I could understand why others might disagree.

I think that's something for the Java team to decide.  I don't think the
rest of us know enough to say whether its vital that things stay
compatible or not.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com