Re: [patch] Fix unwind handling for hppa

[Andrew Cagney <cagney at gnu dot org>]

> > > 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