frysk vs libunwind symbol lookup

[Mark Wielaard <mark at klomp dot org>]

Hi,

This looks like a small patch, but it actually is a pretty significant
change in how we are using libunwind. Since we weren't trusting the IP
address given by libunwind unw_get_reg() because that did a lookup of
the current address through the current frame even though that could
have missing or bogus dwarf info, we were using a full proc and symbol
name lookup for each frame through libunwind. With this patch, and a
small change to libunwind to always use the cached (and correct) ip
value of the cursor, we don't need to do any symbol lookup through
libunwind anymore (which will get rid of a lot of our workarounds, since
the libunwind symbol lookup was basically just a guess).

This also fixes a couple of issues with the load command that Rick was
seeing, like bug #5267. You can now do all the normal fhpd commands
(although the results are kind of boring of course). One last issue is
bug #5286 (and the duplicate? #5345) with print, but those seem to
happen also with other exe or core targets.

frysk-core/frysk/stack/ChangeLog
2007-11-19  Mark Wielaard  <mwielaard@redhat.com>

    * LibunwindFrame.java (getAddress): Don't do a proc name lookup,
    use new Cursor.getIP().

frysk-imports/libunwind/ChangeLog
2007-11-19  Mark Wielaard  <mwielaard@redhat.com>

    * src/mi/Gget_reg.c (unw_get_reg): Use cached value from cursor
    when looking for UNW_REG_IP.

frysk-sys/lib/unwind/ChangeLog
2007-11-19  Mark Wielaard  <mwielaard@redhat.com>

    * Cursor.java (getIP): New method.
    (unwind): Check current ip.
    * Unwind.java (getIP): New method.
    * cni/UnwindH.hxx (getIP): Likewise.
    (getContext): Check for null elfImage.

All tests PASS before and after this patch on x86 and x86_64 (Fedora 8).

I will clean up the libunwind interfaces up next to purge the stuff we
aren't using anymore and out custom patches to some of the symbol lookup
stuff in libunwind itself.

Cheers,

Mark

diff --git a/frysk-core/frysk/stack/LibunwindFrame.java b/frysk-core/frysk/stack/LibunwindFrame.java
index 080df12..64e9c28 100644
--- a/frysk-core/frysk/stack/LibunwindFrame.java
+++ b/frysk-core/frysk/stack/LibunwindFrame.java
@@ -44,7 +44,6 @@ import frysk.isa.Register;
 import java.util.logging.Logger;
 import lib.unwind.Cursor;
 import lib.unwind.ProcInfo;
-import lib.unwind.ProcName;
 import frysk.isa.ISA;
 import frysk.proc.Task;
 import frysk.symtab.Symbol;
@@ -92,13 +91,7 @@ class LibunwindFrame extends Frame
      * Returns the current program counter of this Frame.
      */
     public long getAddress() {
-	ProcInfo myInfo = cursor.getProcInfo();
-	ProcName myName = cursor.getProcName(0);
-    
-	if (myInfo.getError() != 0 || myName.getError() != 0)
-	    return 0;
-    
-	return myInfo.getStartIP() + myName.getOffset();
+      return cursor.getIP();
     }
   
     /**
diff --git a/frysk-imports/libunwind/src/mi/Gget_reg.c b/frysk-imports/libunwind/src/mi/Gget_reg.c
index 23b72be..5179a88 100644
--- a/frysk-imports/libunwind/src/mi/Gget_reg.c
+++ b/frysk-imports/libunwind/src/mi/Gget_reg.c
@@ -30,5 +30,12 @@ unw_get_reg (unw_cursor_t *cursor, int regnum, unw_word_t *valp)
 {
   struct cursor *c = (struct cursor *) cursor;
 
+  // No need to go look up the IP value since it is cached in the cursor.
+  if (regnum == UNW_REG_IP)
+    {
+      *valp = c->dwarf.ip;
+      return 0;
+    }
+
   return tdep_access_reg (c, regnum, valp, 0);
 }
diff --git a/frysk-sys/lib/unwind/Cursor.java b/frysk-sys/lib/unwind/Cursor.java
index 190e315..6433ace 100644
--- a/frysk-sys/lib/unwind/Cursor.java
+++ b/frysk-sys/lib/unwind/Cursor.java
@@ -81,6 +81,10 @@ public class Cursor
 			     bytes, start);
     }
 
+    public long getIP() {
+	return unwinder.getIP(cursor);
+    }
+  
     public long getSP() {
 	return unwinder.getSP(cursor);
     }
@@ -110,8 +114,8 @@ public class Cursor
     public Cursor unwind() {
 	logger.log(Level.FINE, "{0}, unwind\n", this);
 
-	//XXX: Don't unwind if no more frames.
-	if (step == 0)
+	//XXX: Don't unwind if no more, or unknown frames.
+	if (step == 0 || getIP() == 0)
 	    return null;
     
 	Cursor newCursor = new Cursor(addressSpace,
diff --git a/frysk-sys/lib/unwind/Unwind.java b/frysk-sys/lib/unwind/Unwind.java
index 23181ec..8e9d3d3 100644
--- a/frysk-sys/lib/unwind/Unwind.java
+++ b/frysk-sys/lib/unwind/Unwind.java
@@ -70,6 +70,7 @@ public abstract class Unwind
     abstract void setRegister(RawDataManaged cursor, int regNum,
 			      long offset, int length, byte[] word, int start);
   
+    public abstract long getIP(RawDataManaged cursor);
     public abstract long getSP(RawDataManaged cursor);
   
   abstract RawDataManaged copyCursor(RawDataManaged cursor);  
diff --git a/frysk-sys/lib/unwind/cni/UnwindH.hxx b/frysk-sys/lib/unwind/cni/UnwindH.hxx
index c13576c..f48494e 100644
--- a/frysk-sys/lib/unwind/cni/UnwindH.hxx
+++ b/frysk-sys/lib/unwind/cni/UnwindH.hxx
@@ -402,6 +402,17 @@ lib::unwind::TARGET::getSP(gnu::gcj::RawDataManaged* cursor)
     return sp;
 }
 
+jlong
+lib::unwind::TARGET::getIP(gnu::gcj::RawDataManaged* cursor)
+{
+  unw_word_t ip;
+  int status = unw_get_reg((::unw_cursor_t *) cursor, UNW_REG_IP, &ip);
+  if (status < 0)
+    return 0; // bottom of stack.
+  else
+    return ip;
+}
+
 
 jint
 lib::unwind::TARGET::getContext(gnu::gcj::RawDataManaged* context)
@@ -445,6 +456,9 @@ lib::unwind::TARGET::createProcInfoFromElfImage(lib::unwind::AddressSpace* addre
 						jboolean needUnwindInfo,
 						lib::unwind::ElfImage* elfImage)
 {
+  if (elfImage == NULL)
+    return new lib::unwind::ProcInfo(UNW_ENOINFO);
+
   unw_proc_info_t *procInfo
     = (::unw_proc_info_t *) JvAllocBytes(sizeof (::unw_proc_info_t));