Re: RFC Patch: Fix bad eh_frame data for MIPS.

[Daniel Jacobowitz <drow at mvista dot com>]

On Thu, Sep 18, 2003 at 11:16:26AM -0700, David Daney wrote:
> Richard Henderson wrote:
> 
> >On Wed, Sep 17, 2003 at 02:31:30PM -0700, David Daney wrote:
> > 
> >
> >>	* gcc/config/mips/linux.h (MD_FALLBACK_FRAME_STATE_FOR): New,
> >>	needed for mips/libgcj signal handling.
> >>	* gcc/config/mips/mips.h 
> >>	(DWARF_FRAME_REGNUM_FOR_INITIAL_RETURN_SAVE): New.
> >>	* gcc/dwarf2out.c (DWARF_FRAME_REGNUM_FOR_INITIAL_RETURN_SAVE): New.
> >>	* gcc/dwarf2out.c (initial_return_save): Use
> >>	DWARF_FRAME_REGNUM_FOR_INITIAL_RETURN_SAVE instead of 
> >>	DWARF_FRAME_REGNUM
> >>   
> >>
> >
> >This is wrong.
> >
> After much though on the matter, I am starting to disagree.  It may not 
> be exactly correct, but it is close.
> 
> >
> >The existing DWARF_FRAME_REGNUM is completely wrong for mips.
> >
> ??
> 
> >After deleting that, you have a choice of:
> >
> >(1) Leaving DWARF_FRAME_RETURN_COLUMN alone, and adding a 
> >    REG_FRAME_RELATED_EXPR to the insn that saves r31 on the stack.
> >    The expression would mention (reg DWARF_FRAME_RETURN_COLUMN).
> >
> I beleve that this is already being done.
> 
> However it is not getting written to the CIE because 
> initial_return_save() uses DWARF_FRAME_REGNUM () to map the real 
> register back to the "dwarf frame regnum", which in the case of mips is 
> DWARF_FRAME_RETURN_COLUMN.  It then calls reg_save() which suppresses 
> the output because the registers are the same.
> 
> >
> >(2) Set
> >
> >	#define DWARF_FRAME_RETURN_COLUMN   GP_REG_FIRST+31
> >
> >    This will produce smaller CFI output.  I don't know if SGI's
> >    debugger can handle overlapping r31 with the return address.
> >    Gcc and gdb can.
> >
> Doing this will fail in the case of a leaf function where r31 is not 
> saved on the stack.
> 
> When unwinding through a signal handler we must restore all the 
> registers (including r31) however if we use r31 as the 
> DWARF_FRAME_RETURN_COLUMN then it must have the value of the return 
> address from the signal handler (i.e. the address of the faulting 
> instruction).  Unwinding then fails in the leaf function because r31 has 
> the wrong value.

Huh?  MD_FALLBACK_FRAME_STATE_FOR should be taking care of this.  When
we're unwinding in the leaf function the value in r31 should be
correct.

> Now what to do?  Here are my thoughts (and a psuedo-diff of 
> initial_return_save for context):
> 
> static void
> initial_return_save (rtl)
>     rtx rtl;
> {
>  unsigned int reg = (unsigned int) -1;
>  HOST_WIDE_INT offset = 0;
> 
>  switch (GET_CODE (rtl))
>    {
>    case REG:
>      /* RA is in a register.  */
> -      reg = DWARF_FRAME_REGNUM (REGNO (rtl));
> +      reg = REGNO (rtl);
>     break;
> 
> .
> .
> .
> This is what my original patch did (but only for MIPS).

No.  Absolutely not.  REGNO is a GCC internal register number. 
DWARF_FRAME_REGNUM is a property of the target's psABI, a defined
listing of registers. That's all it is.  The mapping should generally
be 1-1 and arbitrary.

> It seems that initial_return_save in the REG case is writing a mapping 
> of the "real return address register"-> DWARF_FRAME_RETURN_COLUMN into 
> the CIE.  If this is true, then reg must be the real register number, 
> not the DWARF_FRAME_REGNUM.

Every register number in any CIE or FDE is a DWARF_FRAME_REGNUM.  The
unwinder doesn't know anything about GCC's register numbers.

Have you read the DWARF specification?  If not, I recommend it highly.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer