Patch: Use _Unwind_GetIPInfo in stacktrace.cc

[Bryce McKinlay <mckinlay at redhat dot com>]

The new _Unwind_GetIPInfo function allows us to determine if a frame's 
IP points to the instruction following a call instruction, or to a 
faulting instruction itself. In the case where it points to a following 
instruction, we need to "roll back" the IP used in line-number lookup so 
that the line number is that of the call itself, not the next line.

Previously we would do this roll-back unconditionally, but this meant 
that exceptions generated by faults (eg NullPointerException) would 
sometimes show the wrong line number on their inner-most frame. Using 
_Unwind_GetIpInfo fixes this.

I'm also checking in a test case which verifies that stack trace line 
numbers are correct (now that it finally passes!). I'm checking this in 
to trunk.

Bryce



2006-07-06  Bryce McKinlay <mckinlay@redhat.com>

        * stacktrace.cc (ClassForFrame): Remove commented-out code.
        (UnwindTraceFn): Use _Unwind_GetIPInfo and adjust IP
        only when needed.
        (getLineNumberForFrame): Don't adjust IP here.
        * testsuite/libjava.lang/StackTrace.java: New test case.
        * testsuite/libjava.lang/StackTrace.out: Ditto.

Index: stacktrace.cc
===================================================================
--- stacktrace.cc	(revision 115232)
+++ stacktrace.cc	(working copy)
@@ -79,8 +79,6 @@
 {
   JvAssert (frame->type == frame_native);
   jclass klass = NULL;
-  // use _Unwind_FindEnclosingFunction to find start of method
-  //void *entryPoint = _Unwind_FindEnclosingFunction (ip);
 
   // look it up in ncodeMap
   if (frame->start_ip)
@@ -124,13 +122,20 @@
   else
 #endif
     {
+      _Unwind_Ptr ip;
+      int ip_before_insn = 0;
+      ip = _Unwind_GetIPInfo (context, &ip_before_insn);
+
+      // If the unwinder gave us the return address, roll it back a little
+      // to get the correct line number for the call.
+      if (! ip_before_insn)
+	--ip;
+
       state->frames[pos].type = frame_native;
-      state->frames[pos].ip = (void *) _Unwind_GetIP (context);
+      state->frames[pos].ip = (void *) ip;
       state->frames[pos].start_ip = func_addr;
     }
 
-  //printf ("unwind ip: %p\n", _Unwind_GetIP (context));
-
   _Unwind_Reason_Code result = _URC_NO_REASON;
   if (state->trace_function != NULL)
     result = (state->trace_function) (state);
@@ -207,10 +212,6 @@
       else
         offset = (_Unwind_Ptr) ip - (_Unwind_Ptr) info.base;
 
-      // The unwinder gives us the return address. In order to get the right
-      // line number for the stack trace, roll it back a little.
-      offset -= 1;
-
       finder->lookup (binaryName, (jlong) offset);
       *sourceFileName = finder->getSourceFile();
       *lineNum = finder->getLineNum();
Index: testsuite/libjava.lang/StackTrace.java
===================================================================
--- testsuite/libjava.lang/StackTrace.java	(revision 0)
+++ testsuite/libjava.lang/StackTrace.java	(revision 0)
@@ -0,0 +1,76 @@
+// Check that stack trace's work, and stack trace line numbers, if available,
+// are correct.
+
+public class StackTrace
+{
+  public static void main(String[] args) 
+  { 
+    try
+    {
+      a();
+    }
+    catch (Exception x)
+    {
+      StackTraceElement[] trace = x.getStackTrace();
+      checkTrace(trace);
+    }
+  }
+
+  static void a() 
+  { 
+    new Inner();
+  }
+  
+  static class Inner
+  {
+    public Inner()
+    {
+      doCrash(null);
+    }  
+
+    public void doCrash(Object o)
+    {
+      o.toString();
+    }
+  }  
+  
+  static void checkTrace(StackTraceElement[] trace)
+  {
+    System.out.println("Trace length = " + trace.length);
+    checkLine(trace[0], "StackTrace$Inner", "doCrash", 33);
+    checkLine(trace[1], "StackTrace$Inner", "<init>", 28);
+    checkLine(trace[2], "StackTrace", "a", 21);
+    checkLine(trace[3], "StackTrace", "main", 10);
+  }
+  
+  static void checkLine(StackTraceElement frame, String expected_cl, 
+                	String expected_method, int expected_line)
+  {
+    if (frame.getClassName().equals(expected_cl))
+      System.out.print(expected_cl);
+    else
+      System.out.print("FAIL - expected " + expected_cl + ", got: " + 
+		       frame.getClassName());
+    
+    System.out.print(".");
+
+    if (frame.getMethodName().equals(expected_method))
+      System.out.print(expected_method);
+    else
+      System.out.print("FAIL - expected " + expected_method + ", got: " +
+		       frame.getMethodName());
+
+    System.out.print(":");
+    
+    // Permit either the correct line number or no line number. This is so
+    // we don't fail on platforms that don't yet support reading debug info 
+    // for stack traces, or when no debug info is available.
+    if (frame.getLineNumber() < 0
+        || (frame.getLineNumber() == expected_line
+            && frame.getFileName().equals("StackTrace.java")))
+      System.out.println("OK");
+    else
+      System.out.println("FAIL - expected " + expected_line + ", got: " +
+			 frame.getLineNumber());
+  }
+}
Index: testsuite/libjava.lang/StackTrace.out
===================================================================
--- testsuite/libjava.lang/StackTrace.out	(revision 0)
+++ testsuite/libjava.lang/StackTrace.out	(revision 0)
@@ -0,0 +1,5 @@
+Trace length = 4
+StackTrace$Inner.doCrash:OK
+StackTrace$Inner.<init>:OK
+StackTrace.a:OK
+StackTrace.main:OK