Another remark regarding hppa-tdep frame code

Another remark regarding hppa-tdep frame code

[Joel Brobecker]

There is a confusion between r3 and the fp register, as the same
index is used for both. This leads to a bit of hackery to try to
know whether r3 is a normal register or used as the frame pointer.

This also leads to things like this:

  if (u->Save_SP && !trad_frame_addr_p (cache->saved_regs, HPPA_FP_REGNUM)
      && fp_in_r1)
    {
      ULONGEST r1 = frame_unwind_register_unsigned (next_frame, 1);
      trad_frame_set_value (cache->saved_regs, HPPA_FP_REGNUM, r1);
    }

I'm currently working off a gdb-6.3 baseline, so I don't have this
code in our tree, but the above seems incorrect. Although it probably
fixes the unwinding issue:

  /* If Save_SP is set, then we expect the frame pointer to be saved in the
     frame.  However, there is a one-insn window where we haven't saved it
     yet, but we've already clobbered it.  Detect this case and fix it up.

     The prologue sequence for frame-pointer functions is:
        0: stw %rp, -20(%sp)
        4: copy %r3, %r1
        8: copy %sp, %r3
        c: stw,ma %r1, XX(%sp)

     So if we are at offset c, the r3 value that we want is not yet saved
     on the stack, but it's been overwritten.  The prologue analyzer will
     set fp_in_r1 when it sees the copy insn so we know to get the value 
     from r1 instead.  */

It also breaks:

        (gdb) print $r3

in that frame, since you've replaced of r3 by the value of r1.
this may have also broken unwinding if the upper frame uses r3 as
the frame pointer and r3 wasn't saved on stack in that frame.

In my opinion, the way to go is to defined FP_REGNUM as a pseudo
register whose value would be invalid (is that possible?) if no
no frame pointer is in use, or its value otherwise.

I just assigned myself a winter project to rewrite the unwinder.
I may never find the time to do it, but I have some ideas:

  1. Merge the two prologue analyzers we have.
     One is used to find the prologue-end address, and the other
     is used to scan it and help doing unwinding.

     For this part, I think we can use the unwind record to tell
     us what we're looking for, and we then scan the prologue until
     we've found all the instructions providing just that. This would
     still be the fallback method if debugging info is not available.

  2. For the unwinder, if we see the PC being past the function prologue
     end, then we can compute the entire frame cache only using the
     unwind record.

     Assuming that being inside the function prologue is a rare event,
     we should be able to compute frames without doing too much code
     scanning.

  3. If still inside the prologue, then scan the code and build a fake
     unwind record. Then we're back to 2.

I think we can simplify the unwinder that way. Right now, I find the
mix/merge between using the unwind record and the prologue scan results
confusing...

-- 
Joel

Re: Another remark regarding hppa-tdep frame code

[Randolph Chung]

> I'm currently working off a gdb-6.3 baseline, so I don't have this
> code in our tree, but the above seems incorrect. Although it probably
> fixes the unwinding issue:
> 
>   /* If Save_SP is set, then we expect the frame pointer to be saved in the
>      frame.  However, there is a one-insn window where we haven't saved it
>      yet, but we've already clobbered it.  Detect this case and fix it up.
> 
>      The prologue sequence for frame-pointer functions is:
>         0: stw %rp, -20(%sp)
>         4: copy %r3, %r1
>         8: copy %sp, %r3
>         c: stw,ma %r1, XX(%sp)
> 
>      So if we are at offset c, the r3 value that we want is not yet saved
>      on the stack, but it's been overwritten.  The prologue analyzer will
>      set fp_in_r1 when it sees the copy insn so we know to get the value 
>      from r1 instead.  */
> 
> It also breaks:
> 
>         (gdb) print $r3
> 
> in that frame, since you've replaced of r3 by the value of r1.
> this may have also broken unwinding if the upper frame uses r3 as
> the frame pointer and r3 wasn't saved on stack in that frame.

I think I've tested this case, but maybe not. It sounds familiar.

> In my opinion, the way to go is to defined FP_REGNUM as a pseudo
> register whose value would be invalid (is that possible?) if no
> no frame pointer is in use, or its value otherwise.

Yes, that might not be a bad idea at all....

> I just assigned myself a winter project to rewrite the unwinder.
> I may never find the time to do it, but I have some ideas:
> 
>   1. Merge the two prologue analyzers we have.
>      One is used to find the prologue-end address, and the other
>      is used to scan it and help doing unwinding.
> 
>      For this part, I think we can use the unwind record to tell
>      us what we're looking for, and we then scan the prologue until
>      we've found all the instructions providing just that. This would
>      still be the fallback method if debugging info is not available.

I've looked at this once before but convinced myself that the two 
prologue analyzers do different things. Nonetheless I agree it's not a 
good idea to have two, and if you can come up with a clean API to do 
this, I think it's a good goal.

>   2. For the unwinder, if we see the PC being past the function prologue
>      end, then we can compute the entire frame cache only using the
>      unwind record.
> 
>      Assuming that being inside the function prologue is a rare event,
>      we should be able to compute frames without doing too much code
>      scanning.

I need to think about this. but iirc we already don't do code reading if 
pc is after the prologue.

>   3. If still inside the prologue, then scan the code and build a fake
>      unwind record. Then we're back to 2.
> 
> I think we can simplify the unwinder that way. Right now, I find the
> mix/merge between using the unwind record and the prologue scan results
> confusing...

This is an interesting idea. I'd love to see a patch :)

randolph