More symbol lookup vs libunwind cleanups

[Mark Wielaard <mark at klomp dot org>]

Hi,

Here are some more small symbol lookup vs libunwind cleanups. This helps
make our libunwind interface simpler and is more correct (and hopefully
more efficient). Besides some cleanups of ununsed interfaces there are
two main changes in creating our FrameIdentifier.

First, we no longer use libunwind to get the current function symbol and
starting address, we already know the Symbol of the current Frame.

Second we defer getting the Canonical Frame Address to libunwind instead
of doing it ourselves by unwinding one step and then getting the Stack
Pointer register. libunwind can do this for use more efficiently since
it already holds the CFA value for the current cursor. At least on x86
and x86_64, I have asked upstream to make this generic for all platforms
(ppc32/64 doesn't expose this yet, but Jose agreed this would be a good
thing to have).

frysk-core/frysk/stack/ChangeLog
2007-12-03  Mark Wielaard  <mwielaard@redhat.com>
   
   * LibunwindFrame.java (getFrameIdentifier): Get CFA from cursor.
    
frysk-sys/lib/unwind/ChangeLog
2007-12-03  Mark Wielaard  <mwielaard@redhat.com>
    
   * Cursor.java (getCFA): New method.
   * Unwind.java (getCFA): Likewise.
   * cni/UnwindH.hxx (getCFA): Likewise.

frysk-core/frysk/stack/ChangeLog
2007-11-30  Mark Wielaard  <mwielaard@redhat.com>
    
   * LibunwindAddressSpace.java (getDynInfoListAddr): Removed.
   (resume): Likewise.
    
frysk-sys/lib/unwind/ChangeLog
2007-11-30  Mark Wielaard  <mwielaard@redhat.com>
    
   * AddressSpace.java (getDynInfoListAddr): Removed.
   (resume): Likewise.
   * cni/UnwindH.hxx (get_dyn_info_list_addr): Return -UNW_ENOINFO.
   (resume): return -UNW_EINVAL.


frysk-core/frysk/stack/ChangeLog
2007-11-30  Mark Wielaard  <mwielaard@redhat.com>
    
   * LibunwindFrame (getProcInfo): Removed.
   (getFrameIdentifier): Use Symbol.getAddress(), not ProcInfo.

Although these changes are related and presented as one patch here it is
actually three separate commits, which were all three tested on x86 and
x86_64 (fedora 8).

Cheers,

Mark

diff --git a/frysk-core/frysk/stack/LibunwindAddressSpace.java b/frysk-core/frysk/stack/LibunwindAddressSpace.java
index 0027b0c..5369fd5 100644
--- a/frysk-core/frysk/stack/LibunwindAddressSpace.java
+++ b/frysk-core/frysk/stack/LibunwindAddressSpace.java
@@ -49,12 +49,10 @@ import frysk.dwfl.DwflFactory;
 import frysk.isa.ISA;
 import frysk.proc.MemoryMap;
 import frysk.proc.Task;
-import java.util.Arrays;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import lib.unwind.AddressSpace;
 import lib.unwind.ByteOrder;
-import lib.unwind.Cursor;
 import lib.unwind.ElfImage;
 import lib.unwind.ProcInfo;
 import frysk.isa.RegisterMap;
@@ -138,22 +136,11 @@ class LibunwindAddressSpace extends AddressSpace {
 	return procInfo;
     }
 
-    public int getDynInfoListAddr (byte[] dilap) {
-	//XXX: Todo.
-	Arrays.fill(dilap, (byte) 0);
-	return - lib.unwind.Error.UNW_ENOINFO_;
-    }
-
     public void putUnwindInfo (final ProcInfo procInfo) {
 	// No longer need to hold procInfo.
 	this.procInfo = null;
     }
 
-    public int resume (final Cursor cursor) {
-	//XXX: Todo.
-	return - lib.unwind.Error.UNW_EUNSPEC_;   
-    }
-
     private ElfImage getElfImage (long addr) {
 	logger.log(Level.FINE, "{0} Entering getElfImage, addr: 0x{1}\n", 
 		   new Object [] {this, Long.toHexString(addr)} );
diff --git a/frysk-core/frysk/stack/LibunwindFrame.java b/frysk-core/frysk/stack/LibunwindFrame.java
index 64e9c28..6d8f09e 100644
--- a/frysk-core/frysk/stack/LibunwindFrame.java
+++ b/frysk-core/frysk/stack/LibunwindFrame.java
@@ -43,7 +43,6 @@ import java.util.logging.Level;
 import frysk.isa.Register;
 import java.util.logging.Logger;
 import lib.unwind.Cursor;
-import lib.unwind.ProcInfo;
 import frysk.isa.ISA;
 import frysk.proc.Task;
 import frysk.symtab.Symbol;
@@ -81,13 +80,6 @@ class LibunwindFrame extends Frame
     }
 
     /**
-     * Returns the ProcInfo object for this Frame.
-     */
-    public ProcInfo getProcInfo () {
-	return cursor.getProcInfo();
-    }
-  
-    /**
      * Returns the current program counter of this Frame.
      */
     public long getAddress() {
@@ -140,19 +132,15 @@ class LibunwindFrame extends Frame
   
     /**
      * Return this frame's FrameIdentifier.
+     * The frame identifier is the combination of the current
+     * symbols (function) start address and the call frame address
+     * of the cursor (frame).
      */
     public FrameIdentifier getFrameIdentifier () {
 	if (frameIdentifier == null) {
-	    ProcInfo myInfo = getProcInfo();
-	    long cfa = 0;
-	    Frame outer = getOuter();
-	    if (outer != null)
-		// The previous frame's SP makes for a good CFA for
-		// this frame.  It's a value that needs to be constant
-		// through out the life-time of this frame, and hence
-		// this frame's SP (which changes) is no good.
-		cfa = ((LibunwindFrame)outer).cursor.getSP();
-	    frameIdentifier = new FrameIdentifier(myInfo.getStartIP(), cfa);
+	  long functionAddress = getSymbol().getAddress();
+	  long cfa = cursor.getCFA();
+	  frameIdentifier = new FrameIdentifier(functionAddress, cfa);
 	}
 	return this.frameIdentifier;
     }
diff --git a/frysk-sys/lib/unwind/AddressSpace.java b/frysk-sys/lib/unwind/AddressSpace.java
index fafd777..1fdeff3 100644
--- a/frysk-sys/lib/unwind/AddressSpace.java
+++ b/frysk-sys/lib/unwind/AddressSpace.java
@@ -88,8 +88,6 @@ public abstract class AddressSpace
      */
     public abstract void putUnwindInfo (ProcInfo procInfo);
 
-    public abstract int getDynInfoListAddr (byte[] dilap);
-
     public abstract int accessMem (long addr, byte[] valp, boolean write);
 
     /**
@@ -107,6 +105,4 @@ public abstract class AddressSpace
      * Access LIBUNWIND Register REGNUM.
      */
     public abstract int accessReg(Number regnum, byte[] val, boolean write);
-
-    public abstract int resume (Cursor cursor);
 }
diff --git a/frysk-sys/lib/unwind/Cursor.java b/frysk-sys/lib/unwind/Cursor.java
index f12156d..2192322 100644
--- a/frysk-sys/lib/unwind/Cursor.java
+++ b/frysk-sys/lib/unwind/Cursor.java
@@ -87,6 +87,10 @@ public class Cursor
 	return unwinder.getSP(cursor);
     }
   
+    public long getCFA() {
+	return unwinder.getCFA(cursor);
+    }
+  
     public int step() {
 	return unwinder.step(cursor);
     }
diff --git a/frysk-sys/lib/unwind/Unwind.java b/frysk-sys/lib/unwind/Unwind.java
index 27bfaa3..8c8cdfd 100644
--- a/frysk-sys/lib/unwind/Unwind.java
+++ b/frysk-sys/lib/unwind/Unwind.java
@@ -70,6 +70,7 @@ public abstract class Unwind
   
     public abstract long getIP(RawDataManaged cursor);
     public abstract long getSP(RawDataManaged cursor);
+    public abstract long getCFA(RawDataManaged cursor);
   
   abstract RawDataManaged copyCursor(RawDataManaged cursor);  
  abstract int getContext(RawDataManaged context);
diff --git a/frysk-sys/lib/unwind/cni/UnwindH.hxx b/frysk-sys/lib/unwind/cni/UnwindH.hxx
index 2580365..4b59aa5 100644
--- a/frysk-sys/lib/unwind/cni/UnwindH.hxx
+++ b/frysk-sys/lib/unwind/cni/UnwindH.hxx
@@ -119,16 +119,13 @@ put_unwind_info (::unw_addr_space_t as, ::unw_proc_info_t *proc_info,
 
 /*
  * Get the head of the dynamic unwind registration list.
+ * There is never any dynamic info in our case.
  */
 static int
 get_dyn_info_list_addr (::unw_addr_space_t as, ::unw_word_t *dilap,
 			void *arg)
 {
-  jbyteArray tmp = JvNewByteArray(sizeof (unw_word_t));
-  memcpy (elements(tmp), dilap, sizeof (unw_word_t));
-  int ret = addressSpace(arg)->getDynInfoListAddr (tmp);
-  memcpy(dilap, elements(tmp), sizeof (unw_word_t));
-  return ret;
+  return -UNW_ENOINFO;
 }
 
 /*
@@ -181,12 +178,13 @@ access_fpreg(::unw_addr_space_t as, ::unw_regnum_t regnum,
 }
 
 /*
- * Resumes the process at the provided stack level
+ * Resumes the process at the provided stack level.
+ * We never resume a process through libunwind.
  */
 static int
 resume(::unw_addr_space_t as, ::unw_cursor_t *cp, void *arg)
 {
-  return (int) addressSpace(arg)->resume ((lib::unwind::Cursor *) cp);
+  return -UNW_EINVAL;
 }
 
 /*
@@ -368,6 +366,32 @@ lib::unwind::TARGET::getIP(gnu::gcj::RawDataManaged* cursor)
     return ip;
 }
 
+jlong
+lib::unwind::TARGET::getCFA(gnu::gcj::RawDataManaged* cursor)
+{
+#ifdef UNW_TARGET_X86
+#define FRYSK_UNW_REG_CFA UNW_X86_CFA
+#else
+#ifdef UNW_TARGET_X86_64
+#define FRYSK_UNW_REG_CFA UNW_X86_64_CFA
+#else
+// This is wasteful, but there is no generic UNW_REG_CFA.
+// So just unwind and return the stack pointer.
+#define FRYSK_UNW_REG_CFA UNW_REG_SP
+cursor = copyCursor (cursor);
+if (unw_step((unw_cursor_t *) cursor) < 0)
+  return 0;
+#endif
+#endif
+
+  unw_word_t cfa;
+  int status = unw_get_reg((::unw_cursor_t *) cursor, FRYSK_UNW_REG_CFA, &cfa);
+  if (status < 0)
+    return 0; // bottom of stack.
+  else
+    return cfa;
+}
+
 
 jint
 lib::unwind::TARGET::getContext(gnu::gcj::RawDataManaged* context)