[patch] Fix unwind handling for hppa

[patch] Fix unwind handling for hppa

[Randolph Chung]

Frame unwinding on hppa was broken when the frame unwind started at a 
location before the current frame is created (as noted by the comment
below). This patch fixes it and brings the failure count on 
hppa2.0w-hp-hpux11 down from 1000 to 668, and on hppa-linux from 2700+ 
to 1214. still some ways to go :) 

Many thanks to Dan for patiently answering my questions.

ok to commit?

hppa2.0w-hp-hpux11 results:

                === gdb Summary ===

# of expected passes            9348
# of unexpected failures        668
# of unexpected successes       6
# of expected failures          62
# of known failures             21
# of unresolved testcases       12
# of untested testcases         5
# of unsupported tests          15



hppa-unknown-linux results:

                === gdb Summary ===

# of expected passes            8663
# of unexpected failures        1214
# of expected failures          46
# of known failures             35
# of unresolved testcases       80
# of untested testcases         9
# of unsupported tests          3

thanks,
randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

2004-04-16  Randolph Chung  <tausq@debian.org>

	* hppa-tdep.c (hppa_frame_cache): Handle the case when frame unwind
	starts at a pc before the frame is created.

--- gdb/gdb/hppa-tdep.c.orig	2004-04-14 21:49:24.000000000 -0700
+++ gdb/gdb/hppa-tdep.c	2004-04-16 20:19:32.161667784 -0700
@@ -2207,17 +2207,31 @@
        the current function (and is thus equivalent to the "saved"
        stack pointer.  */
     CORE_ADDR this_sp = frame_unwind_register_unsigned (next_frame, HPPA_SP_REGNUM);
-    /* FIXME: cagney/2004-02-22: This assumes that the frame has been
-       created.  If it hasn't everything will be out-of-wack.  */
-    if (u->Save_SP && trad_frame_addr_p (cache->saved_regs, HPPA_SP_REGNUM))
-      /* Both we're expecting the SP to be saved and the SP has been
-	 saved.  The entry SP value is saved at this frame's SP
-	 address.  */
-      cache->base = read_memory_integer (this_sp, TARGET_PTR_BIT / 8);
+
+    if (frame_relative_level (next_frame) >= 0 || 
+        frame_pc_unwind (next_frame) >= 
+	hppa_skip_prologue (frame_func_unwind (next_frame)))
+    {
+      if (u->Save_SP && trad_frame_addr_p (cache->saved_regs, HPPA_SP_REGNUM))
+        {
+          /* Both we're expecting the SP to be saved and the SP has been
+	     saved.  The entry SP value is saved at this frame's SP
+	     address.  */
+          cache->base = read_memory_integer (this_sp, TARGET_PTR_BIT / 8);
+        }
+      else
+        {
+          /* The prologue has been slowly allocating stack space.  Adjust
+	     the SP back.  */
+          cache->base = this_sp - frame_size;
+        }
+      }
     else
-      /* The prologue has been slowly allocating stack space.  Adjust
-	 the SP back.  */
-      cache->base = this_sp - frame_size;
+      {
+	/* This frame has not yet been created. */
+        cache->base = this_sp;
+      }
+
     trad_frame_set_value (cache->saved_regs, HPPA_SP_REGNUM, cache->base);
   }
 

Re: [patch] Fix unwind handling for hppa

[Daniel Jacobowitz]

On Sat, Apr 17, 2004 at 01:05:36AM -0700, Randolph Chung wrote:
> Frame unwinding on hppa was broken when the frame unwind started at a 
> location before the current frame is created (as noted by the comment
> below). This patch fixes it and brings the failure count on 
> hppa2.0w-hp-hpux11 down from 1000 to 668, and on hppa-linux from 2700+ 
> to 1214. still some ways to go :) 
> 
> Many thanks to Dan for patiently answering my questions.
> 
> ok to commit?

Nope, a couple of things:

> +    if (frame_relative_level (next_frame) >= 0 || 
> +        frame_pc_unwind (next_frame) >= 
> +	hppa_skip_prologue (frame_func_unwind (next_frame)))

Andrew and I just had this same discussion with MIPS.  Problems with
this line include:
  - your formatting is wrong; operators always come at the beginning
    of the line.
  - Checking the frame level is wrong.  It's wrong both in practice and
    in principle: in practice, the next frame could be a dummy frame
    or a signal frame.  There's a test case in the testsuite which
    covers this.

    In principle, the point is to unwind without magic knowledge of the
    next frame.  At least, that's what Andrew insists.  I don't think
    that I agree; I think the knowledge of whether we got from this
    frame to the next frame via a call is a legitimate piece of
    information to use during unwinding and can save a lot of
    inefficient code analysis.  But that's unsettled.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [patch] Fix unwind handling for hppa

[Randolph Chung]

> > +    if (frame_relative_level (next_frame) >= 0 || 
> > +        frame_pc_unwind (next_frame) >= 
> > +	hppa_skip_prologue (frame_func_unwind (next_frame)))
> 
>   - your formatting is wrong; operators always come at the beginning
>     of the line.

ok, i have to go and read the coding style doc some more since it is
very different from what i'm used to :)

>   - Checking the frame level is wrong.  It's wrong both in practice and
>     in principle: in practice, the next frame could be a dummy frame
>     or a signal frame.  There's a test case in the testsuite which
>     covers this.

mmmm, ok. so if i just do:

    if (frame_pc_unwind (next_frame) 
        >= hppa_skip_prologue (frame_func_unwind (next_frame)))

then it's ok? or is there a more efficient/correct mechanism to get to 
this info?

thanks,
randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch] Fix unwind handling for hppa

[Daniel Jacobowitz]

On Sat, Apr 17, 2004 at 10:20:27AM -0700, Randolph Chung wrote:
> > > +    if (frame_relative_level (next_frame) >= 0 || 
> > > +        frame_pc_unwind (next_frame) >= 
> > > +	hppa_skip_prologue (frame_func_unwind (next_frame)))
> > 
> >   - your formatting is wrong; operators always come at the beginning
> >     of the line.
> 
> ok, i have to go and read the coding style doc some more since it is
> very different from what i'm used to :)
> 
> >   - Checking the frame level is wrong.  It's wrong both in practice and
> >     in principle: in practice, the next frame could be a dummy frame
> >     or a signal frame.  There's a test case in the testsuite which
> >     covers this.
> 
> mmmm, ok. so if i just do:
> 
>     if (frame_pc_unwind (next_frame) 
>         >= hppa_skip_prologue (frame_func_unwind (next_frame)))
> 
> then it's ok? or is there a more efficient/correct mechanism to get to 
> this info?

Beats me.  I guess that might work.  At that point you're more or less
running the prologue analyzer despite having unwind data; I'm not sure
how I feel about that.  But it does seem pragmatically useful.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [patch] Fix unwind handling for hppa

[Randolph Chung]

> Beats me.  I guess that might work.  At that point you're more or less
> running the prologue analyzer despite having unwind data; I'm not sure
> how I feel about that.  But it does seem pragmatically useful.

yeah, actually it can be optimized a bit. hppa_frame_cache already is
doing some prologue parsing, so there's no point in doing it again....

something like this seems to work just as well.

randolph


2004-04-17  Randolph Chung  <tausq@debian.org>

	* hppa-tdep.c (hppa_frame_cache): Handle the case when frame unwind
	starts at a pc before the frame is created.

Index: hppa-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
retrieving revision 1.147
diff -u -p -r1.147 hppa-tdep.c
--- hppa-tdep.c	17 Apr 2004 17:41:10 -0000	1.147
+++ hppa-tdep.c	17 Apr 2004 18:08:34 -0000
@@ -2039,5 +2039,6 @@ hppa_frame_cache (struct frame_info *nex
   CORE_ADDR this_sp;
   long frame_size;
   struct unwind_table_entry *u;
+  CORE_ADDR end_pc;
   int i;
 
@@ -2085,7 +2086,6 @@ hppa_frame_cache (struct frame_info *nex
   {
     int final_iteration = 0;
     CORE_ADDR pc;
-    CORE_ADDR end_pc;
     int looking_for_sp = u->Save_SP;
     int looking_for_rp = u->Save_RP;
     int fp_loc = -1;
@@ -2207,6 +2207,7 @@ hppa_frame_cache (struct frame_info *nex
 	if (is_branch (inst))
 	  final_iteration = 1;
       }
+    end_pc = pc;
   }
 
   {
@@ -2214,17 +2215,28 @@ hppa_frame_cache (struct frame_info *nex
        the current function (and is thus equivalent to the "saved"
        stack pointer.  */
     CORE_ADDR this_sp = frame_unwind_register_unsigned (next_frame, HPPA_SP_REGNUM);
-    /* FIXME: cagney/2004-02-22: This assumes that the frame has been
-       created.  If it hasn't everything will be out-of-wack.  */
-    if (u->Save_SP && trad_frame_addr_p (cache->saved_regs, HPPA_SP_REGNUM))
-      /* Both we're expecting the SP to be saved and the SP has been
-	 saved.  The entry SP value is saved at this frame's SP
-	 address.  */
-      cache->base = read_memory_integer (this_sp, TARGET_PTR_BIT / 8);
+    if (frame_pc_unwind (next_frame) >= end_pc)
+      {
+        if (u->Save_SP && trad_frame_addr_p (cache->saved_regs, HPPA_SP_REGNUM))
+          {
+          /* Both we're expecting the SP to be saved and the SP has been
+	     saved.  The entry SP value is saved at this frame's SP
+	     address.  */
+          cache->base = read_memory_integer (this_sp, TARGET_PTR_BIT / 8);
+          }
+        else
+          {
+            /* The prologue has been slowly allocating stack space.  Adjust
+	     the SP back.  */
+            cache->base = this_sp - frame_size;
+          }
+      }
     else
-      /* The prologue has been slowly allocating stack space.  Adjust
-	 the SP back.  */
-      cache->base = this_sp - frame_size;
+      {
+	/* This frame has not yet been created. */
+        cache->base = this_sp;
+      }
+
     trad_frame_set_value (cache->saved_regs, HPPA_SP_REGNUM, cache->base);
   }
 


-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch] Fix unwind handling for hppa

[Andrew Cagney]

>     ]snip] the point is to unwind without magic knowledge of the
>     next frame. [snip]

The unwind sniffer uses the debug info and other function specific 
knowledge to identify a `stackless leaf function'.  For an example, look 
at the s390's stub unwinder.

Andrew

Re: [patch] Fix unwind handling for hppa

[Randolph Chung]

> The unwind sniffer uses the debug info and other function specific 
> knowledge to identify a `stackless leaf function'.  For an example, look 
> at the s390's stub unwinder.

hrm, afaict s390 used the same idea as in my original patch -- because
it can be unwinding in the epilogue (or in the prologue in hppa's case)
only in the innermost frame, it only checks this case when 
frame_relative_level (frame) < 0 ....

what am i missing?

anyway i've rewritten it so it doesn't do this... instead it stops
prologue analysis after it hits the pc like several other targets, and
uses this to adjust the sp/pc values.

is this one better? i'm not very sure the "read_register()" call below
is correct. (i.e. should i be going through the regcache or something?)

thanks,
randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/


2004-04-21  Randolph Chung  <tausq@debian.org>

	* hppa-tdep.c (hppa_frame_cache): Stop prologue analysis when we
    hit the pc.

Index: hppa-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
retrieving revision 1.147
diff -u -p -r1.147 hppa-tdep.c
--- hppa-tdep.c	17 Apr 2004 17:41:10 -0000	1.147
+++ hppa-tdep.c	21 Apr 2004 15:10:09 -0000
@@ -2039,5 +2085,6 @@ hppa_frame_cache (struct frame_info *nex
   CORE_ADDR this_sp;
   long frame_size;
   struct unwind_table_entry *u;
+  CORE_ADDR end_pc;
   int i;
 
@@ -2084,15 +2146,18 @@ hppa_frame_cache (struct frame_info *nex
      GCC code.  */
   {
     int final_iteration = 0;
-    CORE_ADDR pc;
-    CORE_ADDR end_pc;
+    CORE_ADDR pc, prologue_end;
     int looking_for_sp = u->Save_SP;
     int looking_for_rp = u->Save_RP;
     int fp_loc = -1;
-    end_pc = skip_prologue_using_sal (frame_func_unwind (next_frame));
-    if (end_pc == 0)
-      end_pc = frame_pc_unwind (next_frame);
+
+    prologue_end = hppa_skip_prologue (frame_func_unwind (next_frame));
+    end_pc = frame_pc_unwind (next_frame) - 4;
+    if (prologue_end != 0 && end_pc > prologue_end)
+      end_pc = prologue_end;
+
     frame_size = 0;
+
     for (pc = frame_func_unwind (next_frame);
 	 ((saved_gr_mask || saved_fr_mask
 	   || looking_for_sp || looking_for_rp
@@ -2104,7 +2169,7 @@ hppa_frame_cache (struct frame_info *nex
 	char buf4[4];
 	long status = target_read_memory (pc, buf4, sizeof buf4);
 	long inst = extract_unsigned_integer (buf4, sizeof buf4);
-	
+
 	/* Note the interesting effects of this instruction.  */
 	frame_size += prologue_inst_adjust_sp (inst);
 	
@@ -2207,6 +2272,7 @@ hppa_frame_cache (struct frame_info *nex
 	if (is_branch (inst))
 	  final_iteration = 1;
       }
+    end_pc = pc;
   }
 
   {
@@ -2214,26 +2280,50 @@ hppa_frame_cache (struct frame_info *nex
        the current function (and is thus equivalent to the "saved"
        stack pointer.  */
     CORE_ADDR this_sp = frame_unwind_register_unsigned (next_frame, HPPA_SP_REGNUM);
-    /* FIXME: cagney/2004-02-22: This assumes that the frame has been
-       created.  If it hasn't everything will be out-of-wack.  */
-    if (u->Save_SP && trad_frame_addr_p (cache->saved_regs, HPPA_SP_REGNUM))
-      /* Both we're expecting the SP to be saved and the SP has been
-	 saved.  The entry SP value is saved at this frame's SP
-	 address.  */
-      cache->base = read_memory_integer (this_sp, TARGET_PTR_BIT / 8);
+
+    if (frame_pc_unwind (next_frame) >= end_pc)
+      {
+        if (u->Save_SP && trad_frame_addr_p (cache->saved_regs, HPPA_SP_REGNUM))
+          {
+            /* Both we're expecting the SP to be saved and the SP has been
+	       saved.  The entry SP value is saved at this frame's SP
+	       address.  */
+            cache->base = read_memory_integer (this_sp, TARGET_PTR_BIT / 8);
+          }
+        else
+          {
+            /* The prologue has been slowly allocating stack space.  Adjust
+	     the SP back.  */
+            cache->base = this_sp - frame_size;
+          }
+      }
     else
-      /* The prologue has been slowly allocating stack space.  Adjust
-	 the SP back.  */
-      cache->base = this_sp - frame_size;
+      {
+	/* This frame has not yet been created. */
+        cache->base = this_sp;
+      }
+
     trad_frame_set_value (cache->saved_regs, HPPA_SP_REGNUM, cache->base);
   }
 
   /* The PC is found in the "return register", "Millicode" uses "r31"
      as the return register while normal code uses "rp".  */
   if (u->Millicode)
-    cache->saved_regs[PCOQ_HEAD_REGNUM] = cache->saved_regs[31];
+    {
+      if (trad_frame_addr_p (cache->saved_regs, RP_REGNUM))
+        cache->saved_regs[PCOQ_HEAD_REGNUM] = cache->saved_regs[31];
+      else
+	trad_frame_set_value (cache->saved_regs, PCOQ_HEAD_REGNUM,
+			      read_register (31));
+    }
   else
-    cache->saved_regs[PCOQ_HEAD_REGNUM] = cache->saved_regs[RP_REGNUM];
+    {
+      if (trad_frame_addr_p (cache->saved_regs, RP_REGNUM))
+        cache->saved_regs[PCOQ_HEAD_REGNUM] = cache->saved_regs[RP_REGNUM];
+      else
+	trad_frame_set_value (cache->saved_regs, PCOQ_HEAD_REGNUM,
+			      read_register (RP_REGNUM));
+    }
 
   {
     /* Convert all the offsets into addresses.  */

Re: [patch] Fix unwind handling for hppa

[Andrew Cagney]

>>> For an example, look
>>> at the s390's stub unwinder.
> 
> 
> hrm, afaict s390 used the same idea as in my original patch -- because
> it can be unwinding in the epilogue (or in the prologue in hppa's case)
> only in the innermost frame, it only checks this case when 
> frame_relative_level (frame) < 0 ....

I ment s390_stub_*.  (I didn't realise that a call to 
frame_relative_level had snuck back into -tdep code, sigh).   Which 
tries to identify frameless (not frame pointer) stackless (no allocated 
stack) leaf functions and thunk code.

> what am i missing?
> 
> anyway i've rewritten it so it doesn't do this... instead it stops
> prologue analysis after it hits the pc like several other targets, and
> uses this to adjust the sp/pc values.

I actually thought I had the HP code already stopping at PC, oops.

> is this one better? i'm not very sure the "read_register()" call below
> is correct. (i.e. should i be going through the regcache or something?)

See comments below.

> Index: hppa-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
> retrieving revision 1.147
> diff -u -p -r1.147 hppa-tdep.c
> --- hppa-tdep.c	17 Apr 2004 17:41:10 -0000	1.147
> +++ hppa-tdep.c	21 Apr 2004 15:10:09 -0000
> @@ -2039,5 +2085,6 @@ hppa_frame_cache (struct frame_info *nex
>    CORE_ADDR this_sp;
>    long frame_size;
>    struct unwind_table_entry *u;
> +  CORE_ADDR end_pc;
>    int i;
>  
> @@ -2084,15 +2146,18 @@ hppa_frame_cache (struct frame_info *nex
>       GCC code.  */
>    {
>      int final_iteration = 0;
> -    CORE_ADDR pc;
> -    CORE_ADDR end_pc;
> +    CORE_ADDR pc, prologue_end;
>      int looking_for_sp = u->Save_SP;
>      int looking_for_rp = u->Save_RP;
>      int fp_loc = -1;
> -    end_pc = skip_prologue_using_sal (frame_func_unwind (next_frame));
> -    if (end_pc == 0)
> -      end_pc = frame_pc_unwind (next_frame);
> +
> +    prologue_end = hppa_skip_prologue (frame_func_unwind (next_frame));

Is hppa_skip_prologue needed?  skip_prologue_using_sal doesn't touch the 
inferior so is more light weight.

> +    end_pc = frame_pc_unwind (next_frame) - 4;

No, the edge case of the test is wrong:

   for (...; ... && pc <= end_pc; ...

should be:

	... pc < end_pc

> +    if (prologue_end != 0 && end_pc > prologue_end)
> +      end_pc = prologue_end;
> +
>      frame_size = 0;
> +
>      for (pc = frame_func_unwind (next_frame);
>  	 ((saved_gr_mask || saved_fr_mask
>  	   || looking_for_sp || looking_for_rp
> @@ -2104,7 +2169,7 @@ hppa_frame_cache (struct frame_info *nex
>  	char buf4[4];
>  	long status = target_read_memory (pc, buf4, sizeof buf4);
>  	long inst = extract_unsigned_integer (buf4, sizeof buf4);
> -	
> +
>  	/* Note the interesting effects of this instruction.  */
>  	frame_size += prologue_inst_adjust_sp (inst);
>  	
> @@ -2207,6 +2272,7 @@ hppa_frame_cache (struct frame_info *nex
>  	if (is_branch (inst))
>  	  final_iteration = 1;
>        }
> +    end_pc = pc;
>    }
>  
>    {
> @@ -2214,26 +2280,50 @@ hppa_frame_cache (struct frame_info *nex
>         the current function (and is thus equivalent to the "saved"
>         stack pointer.  */
>      CORE_ADDR this_sp = frame_unwind_register_unsigned (next_frame, HPPA_SP_REGNUM);
> -    /* FIXME: cagney/2004-02-22: This assumes that the frame has been
> -       created.  If it hasn't everything will be out-of-wack.  */
> -    if (u->Save_SP && trad_frame_addr_p (cache->saved_regs, HPPA_SP_REGNUM))
> -      /* Both we're expecting the SP to be saved and the SP has been
> -	 saved.  The entry SP value is saved at this frame's SP
> -	 address.  */
> -      cache->base = read_memory_integer (this_sp, TARGET_PTR_BIT / 8);
> +
> +    if (frame_pc_unwind (next_frame) >= end_pc)

as the result of the above, this will need tweaking.

> +      {
> +        if (u->Save_SP && trad_frame_addr_p (cache->saved_regs, HPPA_SP_REGNUM))
> +          {
> +            /* Both we're expecting the SP to be saved and the SP has been
> +	       saved.  The entry SP value is saved at this frame's SP
> +	       address.  */
> +            cache->base = read_memory_integer (this_sp, TARGET_PTR_BIT / 8);
> +          }
> +        else
> +          {
> +            /* The prologue has been slowly allocating stack space.  Adjust
> +	     the SP back.  */
> +            cache->base = this_sp - frame_size;
> +          }
> +      }
>      else
> -      /* The prologue has been slowly allocating stack space.  Adjust
> -	 the SP back.  */
> -      cache->base = this_sp - frame_size;
> +      {
> +	/* This frame has not yet been created. */
> +        cache->base = this_sp;
> +      }
> +
>      trad_frame_set_value (cache->saved_regs, HPPA_SP_REGNUM, cache->base);
>    }
>  
>    /* The PC is found in the "return register", "Millicode" uses "r31"
>       as the return register while normal code uses "rp".  */
>    if (u->Millicode)
> -    cache->saved_regs[PCOQ_HEAD_REGNUM] = cache->saved_regs[31];
> +    {
> +      if (trad_frame_addr_p (cache->saved_regs, RP_REGNUM))
> +        cache->saved_regs[PCOQ_HEAD_REGNUM] = cache->saved_regs[31];
> +      else
> +	trad_frame_set_value (cache->saved_regs, PCOQ_HEAD_REGNUM,
> +			      read_register (31));
> +    }
>    else
> -    cache->saved_regs[PCOQ_HEAD_REGNUM] = cache->saved_regs[RP_REGNUM];
> +    {
> +      if (trad_frame_addr_p (cache->saved_regs, RP_REGNUM))
> +        cache->saved_regs[PCOQ_HEAD_REGNUM] = cache->saved_regs[RP_REGNUM];
> +      else
> +	trad_frame_set_value (cache->saved_regs, PCOQ_HEAD_REGNUM,
> +			      read_register (RP_REGNUM));

use frame_register_unwind et.al.

Andrew

Re: [patch] Fix unwind handling for hppa

[Randolph Chung]

> >+    prologue_end = hppa_skip_prologue (frame_func_unwind (next_frame));
> 
> Is hppa_skip_prologue needed?  skip_prologue_using_sal doesn't touch the 
> inferior so is more light weight.

yeah, i've seen a few instances where skip_prologue_using_sal() was
returning 0. i haven't looked into why.

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch] Fix unwind handling for hppa

[Andrew Cagney]

>>>+    prologue_end = hppa_skip_prologue (frame_func_unwind (next_frame));
>>
>>> 
>>> Is hppa_skip_prologue needed?  skip_prologue_using_sal doesn't touch the 
>>> inferior so is more light weight.
> 
> 
> yeah, i've seen a few instances where skip_prologue_using_sal() was
> returning 0. i haven't looked into why.

In that only call the HP function when the generic one fails.  However, 
first can you investigate the details?  This sort of thing needs to be 
commented.

Andrew

Re: [patch] Fix unwind handling for hppa

[Randolph Chung]

> In that only call the HP function when the generic one fails.  However, 
> first can you investigate the details?  This sort of thing needs to be 
> commented.

actually it already does something similar. let me look at this some
more and figure out why it's not working.

thx
randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch] Fix unwind handling for hppa

[Randolph Chung]

> In that only call the HP function when the generic one fails.  However, 
> first can you investigate the details?  This sort of thing needs to be 
> commented.

oh, silly me, this is easy.

it fails if you are stepping through a function that is not compiled
with debug info... e.g. if you are debugging a program (compiled with
-g) that is linked against glibc (non-debug version), and then you 
want to step through a glibc function.

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch] Fix unwind handling for hppa

[Randolph Chung]

> In that only call the HP function when the generic one fails.  However, 
> first can you investigate the details?  This sort of thing needs to be 
> commented.

one more note about this:

skip_prologue_using_sal() sometimes returns a pc that is past the end of
a function. after_prologue() in hppa-tdep.c checks for this explicitly
and returns 0 in this case so that we can fall back to code reading.
this can be observed e.g. with gdb.base/break.exp with the
"breakpoint small function, optimized file" test. Is this expected? or
is it a bug in skip_prologue_using_sal()?

thanks,
randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch] Fix unwind handling for hppa

[Andrew Cagney]

>>In that only call the HP function when the generic one fails.  However, 
>>> first can you investigate the details?  This sort of thing needs to be 
>>> commented.
> 
> 
> one more note about this:
> 
> skip_prologue_using_sal() sometimes returns a pc that is past the end of
> a function. after_prologue() in hppa-tdep.c checks for this explicitly
> and returns 0 in this case so that we can fall back to code reading.
> this can be observed e.g. with gdb.base/break.exp with the
> "breakpoint small function, optimized file" test. Is this expected? or
> is it a bug in skip_prologue_using_sal()?

Ok, can you post the revised change (with comments explaining all this)?

Andrew

Re: [patch] Fix unwind handling for hppa

[Randolph Chung]

> Ok, can you post the revised change (with comments explaining all this)?

Here goes. This fixes the end_pc bug with the earlier patch too. We
really want to compare against prologue_end and not end_pc....

2004-04-22  Randolph Chung  <tausq@debian.org>

	* hppa-tdep.c (hppa_frame_cache): Handle the case when frame unwind
	starts at a pc before the frame is created.

Index: hppa-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
retrieving revision 1.149
diff -u -p -r1.149 hppa-tdep.c
--- hppa-tdep.c	23 Apr 2004 02:54:21 -0000	1.149
+++ hppa-tdep.c	23 Apr 2004 03:06:32 -0000
@@ -1532,6 +1553,7 @@ hppa_frame_cache (struct frame_info *nex
   CORE_ADDR this_sp;
   long frame_size;
   struct unwind_table_entry *u;
+  CORE_ADDR prologue_end;
   int i;
 
   if (hppa_debug)
@@ -1590,27 +1614,36 @@ hppa_frame_cache (struct frame_info *nex
      GCC code.  */
   {
     int final_iteration = 0;
-    CORE_ADDR pc;
-    CORE_ADDR end_pc;
+    CORE_ADDR pc, end_pc;
     int looking_for_sp = u->Save_SP;
     int looking_for_rp = u->Save_RP;
     int fp_loc = -1;
-    end_pc = skip_prologue_using_sal (frame_func_unwind (next_frame));
-    if (end_pc == 0)
-      end_pc = frame_pc_unwind (next_frame);
+
+    /* We have to use hppa_skip_prologue instead of just 
+       skip_prologue_using_sal, in case we stepped into a function without
+       symbol information.  hppa_skip_prologue also bounds the returned
+       pc by the passed in pc, so it will not return a pc in the next
+       function.  */
+    prologue_end = hppa_skip_prologue (frame_func_unwind (next_frame));
+    end_pc = frame_pc_unwind (next_frame);
+
+    if (prologue_end != 0 && end_pc > prologue_end)
+      end_pc = prologue_end;
+
     frame_size = 0;
+
     for (pc = frame_func_unwind (next_frame);
 	 ((saved_gr_mask || saved_fr_mask
 	   || looking_for_sp || looking_for_rp
 	   || frame_size < (u->Total_frame_size << 3))
-	  && pc <= end_pc);
+	  && pc < end_pc);
 	 pc += 4)
       {
 	int reg;
 	char buf4[4];
 	long status = target_read_memory (pc, buf4, sizeof buf4);
 	long inst = extract_unsigned_integer (buf4, sizeof buf4);
-	
+
 	/* Note the interesting effects of this instruction.  */
 	frame_size += prologue_inst_adjust_sp (inst);
 	
@@ -1720,26 +1753,74 @@ hppa_frame_cache (struct frame_info *nex
        the current function (and is thus equivalent to the "saved"
        stack pointer.  */
     CORE_ADDR this_sp = frame_unwind_register_unsigned (next_frame, HPPA_SP_REGNUM);
-    /* FIXME: cagney/2004-02-22: This assumes that the frame has been
-       created.  If it hasn't everything will be out-of-wack.  */
-    if (u->Save_SP && trad_frame_addr_p (cache->saved_regs, HPPA_SP_REGNUM))
-      /* Both we're expecting the SP to be saved and the SP has been
-	 saved.  The entry SP value is saved at this frame's SP
-	 address.  */
-      cache->base = read_memory_integer (this_sp, TARGET_PTR_BIT / 8);
+
+    if (hppa_debug)
+      fprintf_unfiltered (gdb_stdlog, " (this_sp=0x%s, pc=0x%s, "
+		          "prologue_end=0x%s) ",
+		          paddr_nz (this_sp),
+			  paddr_nz (frame_pc_unwind (next_frame)),
+			  paddr_nz (prologue_end));
+
+    if (frame_pc_unwind (next_frame) >= prologue_end)
+      {
+        if (u->Save_SP && trad_frame_addr_p (cache->saved_regs, HPPA_SP_REGNUM))
+          {
+            /* Both we're expecting the SP to be saved and the SP has been
+	       saved.  The entry SP value is saved at this frame's SP
+	       address.  */
+            cache->base = read_memory_integer (this_sp, TARGET_PTR_BIT / 8);
+
+	    if (hppa_debug)
+	      fprintf_unfiltered (gdb_stdlog, " (base=0x%s) [saved] }",
+			          paddr_nz (cache->base));
+          }
+        else
+          {
+            /* The prologue has been slowly allocating stack space.  Adjust
+	     the SP back.  */
+            cache->base = this_sp - frame_size;
+	    if (hppa_debug)
+	      fprintf_unfiltered (gdb_stdlog, " (base=0x%s) [unwind adjust] } ",
+			          paddr_nz (cache->base));
+
+          }
+      }
     else
-      /* The prologue has been slowly allocating stack space.  Adjust
-	 the SP back.  */
-      cache->base = this_sp - frame_size;
+      {
+	/* This frame has not yet been created. */
+        cache->base = this_sp;
+
+	if (hppa_debug)
+	  fprintf_unfiltered (gdb_stdlog, " (base=0x%s) [before prologue] } ",
+			      paddr_nz (cache->base));
+
+      }
+
     trad_frame_set_value (cache->saved_regs, HPPA_SP_REGNUM, cache->base);
   }
 
   /* The PC is found in the "return register", "Millicode" uses "r31"
      as the return register while normal code uses "rp".  */
   if (u->Millicode)
-    cache->saved_regs[PCOQ_HEAD_REGNUM] = cache->saved_regs[31];
+    {
+      if (trad_frame_addr_p (cache->saved_regs, RP_REGNUM))
+        cache->saved_regs[PCOQ_HEAD_REGNUM] = cache->saved_regs[31];
+      else
+	{
+	  ULONGEST r31 = frame_unwind_register_unsigned (next_frame, 31);
+	  trad_frame_set_value (cache->saved_regs, PCOQ_HEAD_REGNUM, r31);
+        }
+    }
   else
-    cache->saved_regs[PCOQ_HEAD_REGNUM] = cache->saved_regs[RP_REGNUM];
+    {
+      if (trad_frame_addr_p (cache->saved_regs, RP_REGNUM))
+        cache->saved_regs[PCOQ_HEAD_REGNUM] = cache->saved_regs[RP_REGNUM];
+      else
+	{
+	  ULONGEST rp = frame_unwind_register_unsigned (next_frame, RP_REGNUM);
+	  trad_frame_set_value (cache->saved_regs, PCOQ_HEAD_REGNUM, rp);
+	}
+    }
 
   {
     /* Convert all the offsets into addresses.  */

-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch] Fix unwind handling for hppa

[Daniel Jacobowitz]

On Thu, Apr 22, 2004 at 08:13:01PM -0700, Randolph Chung wrote:
> > Ok, can you post the revised change (with comments explaining all this)?
> 
> Here goes. This fixes the end_pc bug with the earlier patch too. We
> really want to compare against prologue_end and not end_pc....
> 
> 2004-04-22  Randolph Chung  <tausq@debian.org>
> 
> 	* hppa-tdep.c (hppa_frame_cache): Handle the case when frame unwind
> 	starts at a pc before the frame is created.

Hmm... final result is interesting.  I think I'll give the
skip_prologue approach a try in the MIPS .pdr code also.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [patch] Fix unwind handling for hppa

[Andrew Cagney]

Yep, ok, nice.  Now where where we with the others (I maxed out)?

>>Ok, can you post the revised change (with comments explaining all this)?
> 
> 
> Here goes. This fixes the end_pc bug with the earlier patch too. We
> really want to compare against prologue_end and not end_pc....
> 
> 2004-04-22  Randolph Chung  <tausq@debian.org>
> 
> 	* hppa-tdep.c (hppa_frame_cache): Handle the case when frame unwind
> 	starts at a pc before the frame is created.
> 

Andrew