RFA: Use _Unwind_Ptr (Was: Re: RFC / RFA: dwarf2 unwinding for targets with call-part-clobbered registers)

[Joern Rennecke <joern dot rennecke at superh dot com>]

> > So instead of hard-coding _Unwind_Word, I use __UNWIND_CFA_TYPE__ and
> > __UNWIND_RA_TYPE__ and define them to _Unwind_Word if they are not
> > already defined.
> 
> Does changing Unwind_Word to Unwind_Ptr work?

If you change it in _Unwind_GetGR, yes.  But then, you change the behaviour
of execute_stack_op too, which is probably not desired.
While searching for uses of _Unwind_GetGR, I also found this hunk of code:

  #ifdef EH_RETURN_STACKADJ_RTX
    {
      void *target_cfa;
  
      /* If the last frame records a saved stack pointer, use it.  */
      if (_Unwind_GetGRPtr (target, __builtin_dwarf_sp_column ()))
        target_cfa = (void *)(_Unwind_Ptr)
          _Unwind_GetGR (target, __builtin_dwarf_sp_column ());
      else
        target_cfa = target->cfa;
  
      /* We adjust SP by the difference between CURRENT and TARGET's CFA.  */
      if (STACK_GROWS_DOWNWARD)
        return target_cfa - current->cfa + target->args_size;
      else
        return current->cfa - target_cfa - target->args_size;
    }
  #else
    return 0;
  #endif

, which I think also needs changing; my testcase just happended not to
hit it.

We should probably make a new function _Unwind_GetPtr, which is like
_Unwind_GetGR, but reads an Unwind_Ptr, and use that function in all the
places where _Unwind_GetGR is now used, except execute_stack_op.

I think this makes a cleaner design.  I've implemented this in the attached
patch, which was bootstrapped / regtested on i686-pc-linux-gnu.
If accepted, this supercedes the previous unwind-dw2.c/sh.h patch
(but not the other unwind related patches).

For little endian targets, this change should not cause a change in
behaviour except for avoiding misaligned accesses.
Do we have any targets besides sh64-elf -mb -m5-32media which are
big endian, use dwarf2 unwinding, and have a wider _Unwind_Word than
_Unwind_Ptr ?

I can't see how you can make execute_stack_op do something useful for
varying size registers, unless you use dwarf_reg_size_table and
appropriate conversions.
I'll leave this for later, if / when somebody actually needs this
functionality.

> In the two places where you added UNWIND_CFA_TYPE * casts, it looks like 
> you added an extra dereference which looks wrong.

No, by replacing _Unwind_GetGR with _Unwind_GetGRPtr, I have postponed the
dereference.

> Also, in both of these places, it isn't clear why the existing 
> Unwind_Ptr cast doesn't work.

It doesn't work because the value is read as an Unwind_Word first, causing
possible unalignment traps, and if the process survived that, you still
get the wrong value for big endian when you use the lower 32 bits of
the accidentially 64-bit value, which is actually the concatenation of
the original 32 bit value with the following 32 bits - you get the latter.


2003-07-08  J"orn Rennecke <joern.rennecke@superh.com>

	* unwind.h (_Unwind_GetPtr): Declare.
	* unwind-dw2.c (_Unwind_GetPtr): New function.
	(uw_update_context_1): Use it.  Use _Unwind_Ptr for type of tmp_sp.
	(uw_update_context, uw_install_context_1): Use _Unwind_GetPtr.
	(uw_init_context_1): Likewise.  Use _Unwind_Ptr for type of sp_slot.

Index: unwind.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/unwind.h,v
retrieving revision 1.12
diff -p -r1.12 unwind.h
*** unwind.h	7 May 2003 22:11:34 -0000	1.12
--- unwind.h	8 Jul 2003 19:52:52 -0000
*************** extern _Unwind_Reason_Code _Unwind_Backt
*** 145,150 ****
--- 145,151 ----
     selected registers maybe manipulated.  */
  
  extern _Unwind_Word _Unwind_GetGR (struct _Unwind_Context *, int);
+ extern _Unwind_Ptr _Unwind_GetPtr (struct _Unwind_Context *, int);
  extern void _Unwind_SetGR (struct _Unwind_Context *, int, _Unwind_Word);
  
  extern _Unwind_Ptr _Unwind_GetIP (struct _Unwind_Context *);
Index: unwind-dw2.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/unwind-dw2.c,v
retrieving revision 1.32
diff -p -r1.32 unwind-dw2.c
*** unwind-dw2.c	13 May 2003 06:49:46 -0000	1.32
--- unwind-dw2.c	8 Jul 2003 19:52:52 -0000
*************** _Unwind_GetGR (struct _Unwind_Context *c
*** 175,180 ****
--- 175,190 ----
  
  /* Get the value of the CFA as saved in CONTEXT.  */
  
+ inline _Unwind_Ptr
+ _Unwind_GetPtr (struct _Unwind_Context *context, int index)
+ {
+   index = DWARF_REG_TO_UNWIND_COLUMN (index);
+   /* This will segfault if the register hasn't been saved.  */
+   return * (_Unwind_Ptr *) context->reg[index];
+ }
+ 
+ /* Get the value of the CFA as saved in CONTEXT.  */
+ 
  _Unwind_Word
  _Unwind_GetCFA (struct _Unwind_Context *context)
  {
*************** uw_update_context_1 (struct _Unwind_Cont
*** 1095,1101 ****
       Always zap the saved stack pointer value for the next frame; carrying
       the value over from one frame to another doesn't make sense.  */
  
!   _Unwind_Word tmp_sp;
  
    if (!_Unwind_GetGRPtr (&orig_context, __builtin_dwarf_sp_column ()))
      {
--- 1105,1111 ----
       Always zap the saved stack pointer value for the next frame; carrying
       the value over from one frame to another doesn't make sense.  */
  
!   _Unwind_Ptr tmp_sp;
  
    if (!_Unwind_GetGRPtr (&orig_context, __builtin_dwarf_sp_column ()))
      {
*************** uw_update_context_1 (struct _Unwind_Cont
*** 1109,1115 ****
    switch (fs->cfa_how)
      {
      case CFA_REG_OFFSET:
!       cfa = (void *) (_Unwind_Ptr) _Unwind_GetGR (&orig_context, fs->cfa_reg);
        cfa += fs->cfa_offset;
        break;
  
--- 1119,1125 ----
    switch (fs->cfa_how)
      {
      case CFA_REG_OFFSET:
!       cfa = (void *) _Unwind_GetPtr (&orig_context, fs->cfa_reg);
        cfa += fs->cfa_offset;
        break;
  
*************** uw_update_context (struct _Unwind_Contex
*** 1175,1181 ****
    /* Compute the return address now, since the return address column
       can change from frame to frame.  */
    context->ra = __builtin_extract_return_addr
!     ((void *) (_Unwind_Ptr) _Unwind_GetGR (context, fs->retaddr_column));
  }
  
  /* Fill in CONTEXT for top-of-stack.  The only valid registers at this
--- 1185,1191 ----
    /* Compute the return address now, since the return address column
       can change from frame to frame.  */
    context->ra = __builtin_extract_return_addr
!     ((void *) _Unwind_GetPtr (context, fs->retaddr_column));
  }
  
  /* Fill in CONTEXT for top-of-stack.  The only valid registers at this
*************** uw_init_context_1 (struct _Unwind_Contex
*** 1198,1204 ****
  {
    void *ra = __builtin_extract_return_addr (__builtin_return_address (0));
    _Unwind_FrameState fs;
!   _Unwind_Word sp_slot;
  
    memset (context, 0, sizeof (struct _Unwind_Context));
    context->ra = ra;
--- 1208,1214 ----
  {
    void *ra = __builtin_extract_return_addr (__builtin_return_address (0));
    _Unwind_FrameState fs;
!   _Unwind_Ptr sp_slot;
  
    memset (context, 0, sizeof (struct _Unwind_Context));
    context->ra = ra;
*************** uw_install_context_1 (struct _Unwind_Con
*** 1274,1281 ****
  
      /* If the last frame records a saved stack pointer, use it.  */
      if (_Unwind_GetGRPtr (target, __builtin_dwarf_sp_column ()))
!       target_cfa = (void *)(_Unwind_Ptr)
!         _Unwind_GetGR (target, __builtin_dwarf_sp_column ());
      else
        target_cfa = target->cfa;
  
--- 1284,1291 ----
  
      /* If the last frame records a saved stack pointer, use it.  */
      if (_Unwind_GetGRPtr (target, __builtin_dwarf_sp_column ()))
!       target_cfa = (void *)
!         _Unwind_GetPtr (target, __builtin_dwarf_sp_column ());
      else
        target_cfa = target->cfa;