[PATCH] S/390 DWARF-2 CFI frame support

[PATCH] S/390 DWARF-2 CFI frame support

[Ulrich Weigand]

Hello,

this patch adds support for DWARF-2 frame handling to the s390 backend.

However, the DWARF-2 frame common code makes a couple of assumptions
that are not valid on s390:

- The return address column is not unwound into the specified register,
  only into the PC.  On s390, it should be unwound into both.

  The patch below fixes this by unwinding into both register
  and PC if the return column corresponds to a GDB register.


- Unwinding the return address column had this line of code:
 	  cache->reg[PC_REGNUM].loc.reg = reg;
  which stores a GDB register number into a place where DWARF-2 
  register numbers are expected.  This fails on s390 where the two
  numbering schemes diverge.

  This is fixed below by using a DWARF-2 register number as appropriate.


- Handling of unspecified registers made a couple of assumptions
  peculiar to i386 (or amd64); in particular that an unspecified
  stack pointer is supposed to be unwound to the CFA.  This is false
  on s390, where the CFA is biased relative to the stack pointer.
  Also, the presence of unspecified registers causes warnings to
  be emitted, which is somewhat awkward, as this is just the way
  GCC does it ...

  To fix this, I suggest the following.  What GCC assumes to happen
  when it leaves a register unspecified in the CFI depends on whether
  the register is call-saved or call-clobbered according to the 
  target's ABI.  If it is call-saved (and unspecified), the function
  doesn't save/restore it because it does not in fact ever modify it.
  Thus, in this case the debugger should copy the value from the 
  inner frame.  If it is call-clobbered (those will always be left
  unspecified), it should be assumed undefined.

  Now, the GDB common code doesn't know which registers are call-saved
  and which are call-clobbered.  Thus, I've extended the register
  group mechanism by providing two new system reggroups,
  call_saved_reggroup and call_clobbered_reggroup, which the target
  can define according to the platform ABI (which the target code 
  can certainly know).  The patch below uses those two groups to
  handle REG_UNSPECIFIED registers as either REG_SAME_VALUE or
  REG_UNDEFINED.

  I'm using two reggroups instead of one to leave the present 
  behaviour unchanged for existing targets, and also to allow
  a target to, say, place the stack pointer register neither in
  call_saved_reggroup nor call_clobbered_reggroup, in which case
  the current sp = CFA special case still triggers.

  I've also simply removed the 'complaint' about unspecified 
  registers because this is now no longer supposed to be a problem.


Tested on s390-ibm-linux and s390x-ibm-linux with no new regressions.
(In fact, even with prologue-based frame analysis switched off,
there are no new regressions, with the exception of the asm-source
case because the assembler test case doesn't provide CFI data.  
I've still left prologue analysis on for backward-compatibility 
with older systems that don't yet have DWARF-2 CFIs everywhere.)

Bye,
Ulrich


ChangeLog:

	* dwarf2-frame.c: Include "reggroups.h".
	(dwarf2_frame_cache): Do not skip the return address column.
	Copy return address to PC_REGNUM if different from RA column.
	Use call_saved_reggroup and call_clobbered_reggroup to 
	determine behaviour of registers unspecified in the CFI.
	Do not emit complaint about unspecified registers.
	* reggroups.c (call_saved_group, call_clobbered_group): Define.
	(call_saved_reggroup, call_clobbered_reggroup): Likewise.
	(_initialize_reggroup): Add these new groups.
	* reggroups.h (call_saved_reggroup, call_clobbered_reggroup): Declare.
	* s390-tdep.c: Include "dwarf2-frame.h".
	(s390_register_reggroup_p): Handle call_saved_reggroup and
	call_clobbered_reggroup groups.
	(s390_gdbarch_init): Install dwarf2_frame_sniffer and
	dwarf2_frame_base_sniffer.
	* Makefile.in (dwarf2-frame.o, s390-tdep.o): Update dependencies.

diff -c -p -r gdb-head/gdb/Makefile.in gdb-head-new/gdb/Makefile.in
*** gdb-head/gdb/Makefile.in	Wed Dec  3 14:50:26 2003
--- gdb-head-new/gdb/Makefile.in	Wed Dec  3 14:50:31 2003
*************** dwarf2expr.o: dwarf2expr.c $(defs_h) $(s
*** 1732,1738 ****
  	$(gdbcore_h) $(elf_dwarf2_h) $(dwarf2expr_h)
  dwarf2-frame.o: dwarf2-frame.c $(defs_h) $(dwarf2expr_h) $(elf_dwarf2_h) \
  	$(frame_h) $(frame_base_h) $(frame_unwind_h) $(gdbcore_h) \
! 	$(gdbtypes_h) $(symtab_h) $(objfiles_h) $(regcache_h) \
  	$(gdb_assert_h) $(gdb_string_h) $(complaints_h) $(dwarf2_frame_h)
  dwarf2loc.o: dwarf2loc.c $(defs_h) $(ui_out_h) $(value_h) $(frame_h) \
  	$(gdbcore_h) $(target_h) $(inferior_h) $(ax_h) $(ax_gdb_h) \
--- 1732,1738 ----
  	$(gdbcore_h) $(elf_dwarf2_h) $(dwarf2expr_h)
  dwarf2-frame.o: dwarf2-frame.c $(defs_h) $(dwarf2expr_h) $(elf_dwarf2_h) \
  	$(frame_h) $(frame_base_h) $(frame_unwind_h) $(gdbcore_h) \
! 	$(gdbtypes_h) $(symtab_h) $(objfiles_h) $(regcache_h) $(reggroups_h) \
  	$(gdb_assert_h) $(gdb_string_h) $(complaints_h) $(dwarf2_frame_h)
  dwarf2loc.o: dwarf2loc.c $(defs_h) $(ui_out_h) $(value_h) $(frame_h) \
  	$(gdbcore_h) $(target_h) $(inferior_h) $(ax_h) $(ax_gdb_h) \
*************** s390-nat.o: s390-nat.c $(defs_h) $(tm_h)
*** 2253,2259 ****
  s390-tdep.o: s390-tdep.c $(defs_h) $(arch_utils_h) $(frame_h) $(inferior_h) \
  	$(symtab_h) $(target_h) $(gdbcore_h) $(gdbcmd_h) $(symfile_h) \
  	$(objfiles_h) $(tm_h) $(__bfd_bfd_h) $(floatformat_h) $(regcache_h) \
! 	$(trad_frame_h) $(frame_base_h) $(frame_unwind_h) \
  	$(reggroups_h) $(regset_h) $(value_h) $(gdb_assert_h) $(dis_asm_h)
  scm-exp.o: scm-exp.c $(defs_h) $(symtab_h) $(gdbtypes_h) $(expression_h) \
  	$(parser_defs_h) $(language_h) $(value_h) $(c_lang_h) $(scm_lang_h) \
--- 2253,2259 ----
  s390-tdep.o: s390-tdep.c $(defs_h) $(arch_utils_h) $(frame_h) $(inferior_h) \
  	$(symtab_h) $(target_h) $(gdbcore_h) $(gdbcmd_h) $(symfile_h) \
  	$(objfiles_h) $(tm_h) $(__bfd_bfd_h) $(floatformat_h) $(regcache_h) \
! 	$(trad_frame_h) $(frame_base_h) $(frame_unwind_h) $(dwarf2_frame_h) \
  	$(reggroups_h) $(regset_h) $(value_h) $(gdb_assert_h) $(dis_asm_h)
  scm-exp.o: scm-exp.c $(defs_h) $(symtab_h) $(gdbtypes_h) $(expression_h) \
  	$(parser_defs_h) $(language_h) $(value_h) $(c_lang_h) $(scm_lang_h) \
diff -c -p -r gdb-head/gdb/dwarf2-frame.c gdb-head-new/gdb/dwarf2-frame.c
*** gdb-head/gdb/dwarf2-frame.c	Wed Dec  3 14:50:26 2003
--- gdb-head-new/gdb/dwarf2-frame.c	Wed Dec  3 14:50:31 2003
***************
*** 32,37 ****
--- 32,38 ----
  #include "symtab.h"
  #include "objfiles.h"
  #include "regcache.h"
+ #include "reggroups.h"
  
  #include "gdb_assert.h"
  #include "gdb_string.h"
*************** struct dwarf2_frame_cache
*** 475,480 ****
--- 476,482 ----
  static struct dwarf2_frame_cache *
  dwarf2_frame_cache (struct frame_info *next_frame, void **this_cache)
  {
+   struct gdbarch *gdbarch = get_frame_arch (next_frame);
    struct cleanup *old_chain;
    const int num_regs = NUM_REGS + NUM_PSEUDO_REGS;
    struct dwarf2_frame_cache *cache;
*************** dwarf2_frame_cache (struct frame_info *n
*** 565,582 ****
        {
  	int regnum;
  	
- 	/* Skip the return address column.  */
- 	if (column == fs->retaddr_column)
- 	  /* NOTE: cagney/2003-06-07: Is this right?  What if
- 	     RETADDR_COLUMN corresponds to a real register (and,
- 	     worse, that isn't the PC_REGNUM)?  I'm guessing that the
- 	     PC_REGNUM further down is trying to handle this.  That
- 	     can't be right though; PC_REGNUM may not be valid (it can
- 	     be negative).  I think, instead when RETADDR_COLUM isn't
- 	     a real register, it should map itself onto
- 	     frame_pc_unwind.  */
- 	  continue;
- 
  	/* Use the GDB register number as the destination index.  */
  	regnum = DWARF2_REG_TO_REGNUM (column);
  
--- 567,572 ----
*************** dwarf2_frame_cache (struct frame_info *n
*** 584,632 ****
  	if (regnum < 0 || regnum >= num_regs)
  	  continue;
  
- 	/* NOTE: cagney/2003-09-05: CFI should specify the disposition
- 	   of all debug info registers.  If it doesn't, complain (but
- 	   not too loudly).  It turns out that GCC assumes that an
- 	   unspecified register implies "same value" when CFI (draft
- 	   7) specifies nothing at all.  Such a register could equally
- 	   be interpreted as "undefined".  Also note that this check
- 	   isn't sufficient; it only checks that all registers in the
- 	   range [0 .. max column] are specified, and won't detect
- 	   problems when a debug info register falls outside of the
- 	   table.  We need a way of iterating through all the valid
- 	   DWARF2 register numbers.  */
- 	if (fs->regs.reg[column].how == REG_UNSPECIFIED)
- 	  complaint (&symfile_complaints,
- 		     "Incomplete CFI data; unspecified registers at 0x%s",
- 		     paddr (fs->pc));
- 
  	cache->reg[regnum] = fs->regs.reg[column];
        }
    }
  
    /* Store the location of the return addess.  If the return address
!      column (adjusted) is not the same as GDB's PC_REGNUM, then this
!      implies a copy from the return address column register.  */
!   if (fs->retaddr_column < fs->regs.num_regs
!       && fs->regs.reg[fs->retaddr_column].how != REG_UNDEFINED)
!     {
!       /* See comment above about a possibly negative PC_REGNUM.  If
!          this assertion fails, it's a problem with this code and not
!          the architecture.  */
!       gdb_assert (PC_REGNUM >= 0);
!       cache->reg[PC_REGNUM] = fs->regs.reg[fs->retaddr_column];
!     }
!   else
      {
        int reg = DWARF2_REG_TO_REGNUM (fs->retaddr_column);
!       if (reg != PC_REGNUM)
  	{
! 	  /* See comment above about PC_REGNUM being negative.  If
! 	     this assertion fails, it's a problem with this code and
! 	     not the architecture.  */
! 	  gdb_assert (PC_REGNUM >= 0);
! 	  cache->reg[PC_REGNUM].loc.reg = reg;
! 	  cache->reg[PC_REGNUM].how = REG_SAVED_REG;
  	}
      }
  
--- 574,619 ----
  	if (regnum < 0 || regnum >= num_regs)
  	  continue;
  
  	cache->reg[regnum] = fs->regs.reg[column];
        }
    }
  
+   /* Among the registers the CFI generated by GCC leaves unspecified,
+      those that are call-saved according to the target's ABI are presumed
+      to inherit their value from the next frame, while those that are
+      call-clobbered should be considered undefined.  */
+   {
+     int regnum;
+     for (regnum = 0; regnum < num_regs; regnum++)
+       if (cache->reg[regnum].how == REG_UNSPECIFIED)
+ 	{
+ 	  if (gdbarch_register_reggroup_p (gdbarch, regnum, 
+ 					   call_saved_reggroup))
+ 	    cache->reg[regnum].how = REG_SAME_VALUE;
+ 
+ 	  else if (gdbarch_register_reggroup_p (gdbarch, regnum, 
+ 						call_clobbered_reggroup))
+ 	    cache->reg[regnum].how = REG_UNDEFINED;
+ 	}
+   }
+ 
    /* Store the location of the return addess.  If the return address
!      column (adjusted) is not the same as gdb's PC_REGNUM, then this
!      implies a copy from the ra column register.  */
!   if (PC_REGNUM >= 0)
      {
        int reg = DWARF2_REG_TO_REGNUM (fs->retaddr_column);
!       if (reg != PC_REGNUM && reg >= 0 && reg < num_regs)
  	{
! 	  cache->reg[PC_REGNUM] = cache->reg[reg];
! 
! 	  /* 'Same value' in this case refers to the return address
! 	     register, not the PC register.  */
! 	  if (cache->reg[PC_REGNUM].how == REG_SAME_VALUE)
! 	    {
! 	      cache->reg[PC_REGNUM].loc.reg = fs->retaddr_column;
! 	      cache->reg[PC_REGNUM].how = REG_SAVED_REG;
! 	    }
  	}
      }
  
diff -c -p -r gdb-head/gdb/reggroups.c gdb-head-new/gdb/reggroups.c
*** gdb-head/gdb/reggroups.c	Wed Dec  3 14:50:26 2003
--- gdb-head-new/gdb/reggroups.c	Wed Dec  3 14:50:31 2003
*************** static struct reggroup vector_group = { 
*** 254,259 ****
--- 254,261 ----
  static struct reggroup all_group = { "all", USER_REGGROUP };
  static struct reggroup save_group = { "save", INTERNAL_REGGROUP };
  static struct reggroup restore_group = { "restore", INTERNAL_REGGROUP };
+ static struct reggroup call_saved_group = { "call-saved", INTERNAL_REGGROUP };
+ static struct reggroup call_clobbered_group = { "call-clobbered", INTERNAL_REGGROUP };
  
  struct reggroup *const general_reggroup = &general_group;
  struct reggroup *const float_reggroup = &float_group;
*************** struct reggroup *const vector_reggroup =
*** 262,267 ****
--- 264,271 ----
  struct reggroup *const all_reggroup = &all_group;
  struct reggroup *const save_reggroup = &save_group;
  struct reggroup *const restore_reggroup = &restore_group;
+ struct reggroup *const call_saved_reggroup = &call_saved_group;
+ struct reggroup *const call_clobbered_reggroup = &call_clobbered_group;
  
  extern initialize_file_ftype _initialize_reggroup; /* -Wmissing-prototypes */
  
*************** _initialize_reggroup (void)
*** 278,283 ****
--- 282,289 ----
    add_group (&default_groups, all_reggroup, XMALLOC (struct reggroup_el));
    add_group (&default_groups, save_reggroup, XMALLOC (struct reggroup_el));
    add_group (&default_groups, restore_reggroup, XMALLOC (struct reggroup_el));
+   add_group (&default_groups, call_saved_reggroup, XMALLOC (struct reggroup_el));
+   add_group (&default_groups, call_clobbered_reggroup, XMALLOC (struct reggroup_el));
  
    add_cmd ("reggroups", class_maintenance,
  	   maintenance_print_reggroups, "\
diff -c -p -r gdb-head/gdb/reggroups.h gdb-head-new/gdb/reggroups.h
*** gdb-head/gdb/reggroups.h	Wed Dec  3 14:50:26 2003
--- gdb-head-new/gdb/reggroups.h	Wed Dec  3 14:50:31 2003
*************** extern struct reggroup *const all_reggro
*** 39,44 ****
--- 39,46 ----
  /* Pre-defined, internal, register groups.  */
  extern struct reggroup *const save_reggroup;
  extern struct reggroup *const restore_reggroup;
+ extern struct reggroup *const call_saved_reggroup;
+ extern struct reggroup *const call_clobbered_reggroup;
  
  /* Create a new local register group.  */
  extern struct reggroup *reggroup_new (const char *name,
diff -c -p -r gdb-head/gdb/s390-tdep.c gdb-head-new/gdb/s390-tdep.c
*** gdb-head/gdb/s390-tdep.c	Wed Dec  3 14:50:26 2003
--- gdb-head-new/gdb/s390-tdep.c	Wed Dec  3 14:50:31 2003
***************
*** 39,44 ****
--- 39,45 ----
  #include "trad-frame.h"
  #include "frame-base.h"
  #include "frame-unwind.h"
+ #include "dwarf2-frame.h"
  #include "reggroups.h"
  #include "regset.h"
  #include "value.h"
*************** s390_register_reggroup_p (struct gdbarch
*** 435,440 ****
--- 436,469 ----
    if (group == save_reggroup || group == restore_reggroup)
      return regnum != S390_PSWM_REGNUM && regnum != S390_PSWA_REGNUM;
  
+   /* Call-saved registers.  */
+   if (group == call_saved_reggroup)
+     switch (tdep->abi)
+       {
+       case ABI_LINUX_S390:
+ 	return (regnum >= S390_R6_REGNUM && regnum <= S390_R15_REGNUM)
+ 	       || regnum == S390_F4_REGNUM
+ 	       || regnum == S390_F6_REGNUM;
+ 
+       case ABI_LINUX_ZSERIES:
+ 	return (regnum >= S390_R6_REGNUM && regnum <= S390_R15_REGNUM)
+ 	       || (regnum >= S390_F8_REGNUM && regnum <= S390_F15_REGNUM);
+       }
+ 
+   /* Call-clobbered registers.  */
+   if (group == call_clobbered_reggroup)
+     switch (tdep->abi)
+       {
+       case ABI_LINUX_S390:
+ 	return (regnum >= S390_R0_REGNUM && regnum <= S390_R5_REGNUM)
+ 	       || (regnum >= S390_F0_REGNUM && regnum <= S390_F15_REGNUM
+ 		   && regnum != S390_F4_REGNUM && regnum != S390_F6_REGNUM);
+ 
+       case ABI_LINUX_ZSERIES:
+ 	return (regnum >= S390_R0_REGNUM && regnum <= S390_R5_REGNUM)
+ 	       || (regnum >= S390_F0_REGNUM && regnum <= S390_F7_REGNUM);
+       }
+ 
    return default_register_reggroup_p (gdbarch, regnum, group);
  }
  
*************** s390_gdbarch_init (struct gdbarch_info i
*** 2992,2997 ****
--- 3021,3028 ----
  
    /* Frame handling.  */
    set_gdbarch_in_solib_call_trampoline (gdbarch, in_plt_section);
+   frame_unwind_append_sniffer (gdbarch, dwarf2_frame_sniffer);
+   frame_base_append_sniffer (gdbarch, dwarf2_frame_base_sniffer);
    frame_unwind_append_sniffer (gdbarch, s390_pltstub_frame_sniffer);
    frame_unwind_append_sniffer (gdbarch, s390_sigtramp_frame_sniffer);
    frame_unwind_append_sniffer (gdbarch, s390_frame_sniffer);
-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de

Re: [PATCH] S/390 DWARF-2 CFI frame support

[Jim Blandy]

Ulrich Weigand <weigand@i1.informatik.uni-erlangen.de> writes:
> this patch adds support for DWARF-2 frame handling to the s390
> backend.

The s390-tdep.c portion of this patch is approved.

Re: [PATCH] S/390 DWARF-2 CFI frame support

[Richard Henderson]

On Thu, Dec 04, 2003 at 09:09:12PM +0100, Ulrich Weigand wrote:
>   To fix this, I suggest the following.  What GCC assumes to happen
>   when it leaves a register unspecified in the CFI depends on whether
>   the register is call-saved or call-clobbered according to the 
>   target's ABI.  If it is call-saved (and unspecified), the function
>   doesn't save/restore it because it does not in fact ever modify it.
>   Thus, in this case the debugger should copy the value from the 
>   inner frame.  If it is call-clobbered (those will always be left
>   unspecified), it should be assumed undefined.

This is wrong.  The debugger should just assume *all* registers
that are not explicitly saved are preserved.  In the case of
call-clobbered registers, you just won't *know* that they are
actually dead.  But so what?  This is no worse than not having
location list information that tells you that a value is dead
after its register gets re-used for something else.

However, when I made this argument before, it wasn't good enough
for some people, and they added the annoying warning anyway.


r~

Re: [PATCH] S/390 DWARF-2 CFI frame support

[Andrew Cagney]

> On Thu, Dec 04, 2003 at 09:09:12PM +0100, Ulrich Weigand wrote:
> 
>>   To fix this, I suggest the following.  What GCC assumes to happen
>>   when it leaves a register unspecified in the CFI depends on whether
>>   the register is call-saved or call-clobbered according to the 
>>   target's ABI.  If it is call-saved (and unspecified), the function
>>   doesn't save/restore it because it does not in fact ever modify it.
>>   Thus, in this case the debugger should copy the value from the 
>>   inner frame.  If it is call-clobbered (those will always be left
>>   unspecified), it should be assumed undefined.
> 
> 
> This is wrong.  The debugger should just assume *all* registers
> that are not explicitly saved are preserved.  In the case of
> call-clobbered registers, you just won't *know* that they are
> actually dead.  But so what?  This is no worse than not having
> location list information that tells you that a value is dead
> after its register gets re-used for something else.
> 
> However, when I made this argument before, it wasn't good enough
> for some people, and they added the annoying warning anyway.

 From what I've seen of the PPC64, Ulrich's change and even more is 
likely needed.  PPC64, for instance, appears to point the return-address 
column and FPSCR and then forget to specify that it is initally found in LR.

Andrew

Re: [PATCH] S/390 DWARF-2 CFI frame support

[Richard Henderson]

On Thu, Dec 04, 2003 at 08:04:20PM -0500, Andrew Cagney wrote:
> From what I've seen of the PPC64, Ulrich's change and even more is 
> likely needed.

I just meant the but that deleted the warning, and introduced
all sorts of support machinery in the form of register groups.

Re: [PATCH] S/390 DWARF-2 CFI frame support

[Ulrich Weigand]

Richard Henderson wrote:

> On Thu, Dec 04, 2003 at 09:09:12PM +0100, Ulrich Weigand wrote:
> >   To fix this, I suggest the following.  What GCC assumes to happen
> >   when it leaves a register unspecified in the CFI depends on whether
> >   the register is call-saved or call-clobbered according to the 
> >   target's ABI.  If it is call-saved (and unspecified), the function
> >   doesn't save/restore it because it does not in fact ever modify it.
> >   Thus, in this case the debugger should copy the value from the 
> >   inner frame.  If it is call-clobbered (those will always be left
> >   unspecified), it should be assumed undefined.
> 
> This is wrong.  The debugger should just assume *all* registers
> that are not explicitly saved are preserved.  In the case of
> call-clobbered registers, you just won't *know* that they are
> actually dead.  But so what?  This is no worse than not having
> location list information that tells you that a value is dead
> after its register gets re-used for something else.

I have no strong opinion about that; if the gdb maintainers
accept treating all unspecified registers as preserved, that
is fine with me as well.   (I just don't want to see that
annoying warning all the time ;-/)

However, there is one point that requires special consideration
in any case: what if the stack pointer is unspecified?  On s390,
treating it as preserved from the inner frame is correct, but
on i386 and other platforms I guess this would be wrong -- it
needs to be set to the CFA there (which is wrong on s390 due
to the CFA bias we have).

I'm not familiar enough with gdb internals to decide what the
cleanest way of signaling that difference would be -- but there
needs to be *some* way or s390 will not work.   (My reggroup
suggestion would provide a way to solve this problem as well.)

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de

Re: [PATCH] S/390 DWARF-2 CFI frame support

[Richard Henderson]

On Fri, Dec 05, 2003 at 03:03:52AM +0100, Ulrich Weigand wrote:
> However, there is one point that requires special consideration
> in any case: what if the stack pointer is unspecified?  On s390,
> treating it as preserved from the inner frame is correct, but
> on i386 and other platforms I guess this would be wrong -- it
> needs to be set to the CFA there (which is wrong on s390 due
> to the CFA bias we have).

I thought s390 always provided a REG_SAVED for the stack pointer?


r~

Re: [PATCH] S/390 DWARF-2 CFI frame support

[Daniel Jacobowitz]

On Fri, Dec 05, 2003 at 03:03:52AM +0100, Ulrich Weigand wrote:
> However, there is one point that requires special consideration
> in any case: what if the stack pointer is unspecified?  On s390,
> treating it as preserved from the inner frame is correct, but
> on i386 and other platforms I guess this would be wrong -- it
> needs to be set to the CFA there (which is wrong on s390 due
> to the CFA bias we have).
> 
> I'm not familiar enough with gdb internals to decide what the
> cleanest way of signaling that difference would be -- but there
> needs to be *some* way or s390 will not work.   (My reggroup
> suggestion would provide a way to solve this problem as well.)

Just for my information...

Do you mean that the stack pointer always either:
  - is preserved, i.e. UNMODIFIED
  - or has explicit unwind information?

If so then yes, we definitely need to change something.  I'm also in
favor of killing the warning, as I said at the time - having used a GDB
with it in place for some time now, I find it quite annoying.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] S/390 DWARF-2 CFI frame support

[Ulrich Weigand]

Richard Henderson wrote:
> On Fri, Dec 05, 2003 at 03:03:52AM +0100, Ulrich Weigand wrote:
> > However, there is one point that requires special consideration
> > in any case: what if the stack pointer is unspecified?  On s390,
> > treating it as preserved from the inner frame is correct, but
> > on i386 and other platforms I guess this would be wrong -- it
> > needs to be set to the CFA there (which is wrong on s390 due
> > to the CFA bias we have).
> 
> I thought s390 always provided a REG_SAVED for the stack pointer?

Except in leaf functions ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de

Re: [PATCH] S/390 DWARF-2 CFI frame support

[Ulrich Weigand]

Daniel Jacobowitz wrote:

> Do you mean that the stack pointer always either:
>   - is preserved, i.e. UNMODIFIED
>   - or has explicit unwind information?

Yes, exactly.  (The former applies to leaf functions
without stack frame, the latter to all others.)

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de

Re: [PATCH] S/390 DWARF-2 CFI frame support

[Andrew Cagney]

> Hello,
> 
> this patch adds support for DWARF-2 frame handling to the s390 backend.
> 
> However, the DWARF-2 frame common code makes a couple of assumptions
> that are not valid on s390:
> 
> - The return address column is not unwound into the specified register,
>   only into the PC.  On s390, it should be unwound into both.
> 
>   The patch below fixes this by unwinding into both register
>   and PC if the return column corresponds to a GDB register.

What does the CFI output look like here?  (I'm mainly curious but it 
always helps to have concrete examples).

For a variant of the PPC64 GCC I found:

- return column pointed at the FSCR
(which is stupid)

- return column same-value as "LR" was implied
(which is contrary to the DWARF2's example, and what you also encountered)

this means that we've now got the following initial states all mapping 
onto "unspecified":

- same value
- unknown
- (for SP_REGNUM) it's the CFA (but is it one frame out?)
- (for return column) it's the LR

I'm wondering if it would be easier, here to "give up".  Throw the whole 
problem back at the architecture vis:

- initialize unwind table using per OSABI (?) method

- update the unwind table using the CFI initialize info

- update the unwind table using the PC's CFI unwind info

- if after all this a register is still unspecified, we complain
(btw, the compaint is only ment to appear once but apparently appears 
repeatedly?).

Thoughts for the moment on the theory?  Mark?

It should also open the way for the elimination of SP_REGNUM and 
PC_REGNUM from this file (ya!).

As for an interface, the <ARCH>_gdbarch_init code could something like:

	dwarf2_frame_add_sniffer (gdbarch, (*<ARCH>_dwarf2_cfi_init) (struct 
cfi_table *table));

(but with better names :-).

Andrew


> - Unwinding the return address column had this line of code:
>  	  cache->reg[PC_REGNUM].loc.reg = reg;
>   which stores a GDB register number into a place where DWARF-2 
>   register numbers are expected.  This fails on s390 where the two
>   numbering schemes diverge.
> 
>   This is fixed below by using a DWARF-2 register number as appropriate.
> 
> 
> - Handling of unspecified registers made a couple of assumptions
>   peculiar to i386 (or amd64); in particular that an unspecified
>   stack pointer is supposed to be unwound to the CFA.  This is false
>   on s390, where the CFA is biased relative to the stack pointer.
>   Also, the presence of unspecified registers causes warnings to
>   be emitted, which is somewhat awkward, as this is just the way
>   GCC does it ...
> 
>   To fix this, I suggest the following.  What GCC assumes to happen
>   when it leaves a register unspecified in the CFI depends on whether
>   the register is call-saved or call-clobbered according to the 
>   target's ABI.  If it is call-saved (and unspecified), the function
>   doesn't save/restore it because it does not in fact ever modify it.
>   Thus, in this case the debugger should copy the value from the 
>   inner frame.  If it is call-clobbered (those will always be left
>   unspecified), it should be assumed undefined.
> 
>   Now, the GDB common code doesn't know which registers are call-saved
>   and which are call-clobbered.  Thus, I've extended the register
>   group mechanism by providing two new system reggroups,
>   call_saved_reggroup and call_clobbered_reggroup, which the target
>   can define according to the platform ABI (which the target code 
>   can certainly know).  The patch below uses those two groups to
>   handle REG_UNSPECIFIED registers as either REG_SAME_VALUE or
>   REG_UNDEFINED.
> 
>   I'm using two reggroups instead of one to leave the present 
>   behaviour unchanged for existing targets, and also to allow
>   a target to, say, place the stack pointer register neither in
>   call_saved_reggroup nor call_clobbered_reggroup, in which case
>   the current sp = CFA special case still triggers.
> 
>   I've also simply removed the 'complaint' about unspecified 
>   registers because this is now no longer supposed to be a problem.
> 
> 
> Tested on s390-ibm-linux and s390x-ibm-linux with no new regressions.
> (In fact, even with prologue-based frame analysis switched off,
> there are no new regressions, with the exception of the asm-source
> case because the assembler test case doesn't provide CFI data.  
> I've still left prologue analysis on for backward-compatibility 
> with older systems that don't yet have DWARF-2 CFIs everywhere.)

>   	if (regnum < 0 || regnum >= num_regs)
>   	  continue;
>   
>   	cache->reg[regnum] = fs->regs.reg[column];
>         }
>     }
>   
> +   /* Among the registers the CFI generated by GCC leaves unspecified,
> +      those that are call-saved according to the target's ABI are presumed
> +      to inherit their value from the next frame, while those that are
> +      call-clobbered should be considered undefined.  */
> +   {
> +     int regnum;
> +     for (regnum = 0; regnum < num_regs; regnum++)
> +       if (cache->reg[regnum].how == REG_UNSPECIFIED)
> + 	{
> + 	  if (gdbarch_register_reggroup_p (gdbarch, regnum, 
> + 					   call_saved_reggroup))
> + 	    cache->reg[regnum].how = REG_SAME_VALUE;
> + 
> + 	  else if (gdbarch_register_reggroup_p (gdbarch, regnum, 
> + 						call_clobbered_reggroup))
> + 	    cache->reg[regnum].how = REG_UNDEFINED;
> + 	}
> +   }
> + 
>     /* Store the location of the return addess.  If the return address
> !      column (adjusted) is not the same as gdb's PC_REGNUM, then this
> !      implies a copy from the ra column register.  */
> !   if (PC_REGNUM >= 0)
>       {
>         int reg = DWARF2_REG_TO_REGNUM (fs->retaddr_column);
> !       if (reg != PC_REGNUM && reg >= 0 && reg < num_regs)
>   	{
> ! 	  cache->reg[PC_REGNUM] = cache->reg[reg];
> ! 
> ! 	  /* 'Same value' in this case refers to the return address
> ! 	     register, not the PC register.  */
> ! 	  if (cache->reg[PC_REGNUM].how == REG_SAME_VALUE)
> ! 	    {
> ! 	      cache->reg[PC_REGNUM].loc.reg = fs->retaddr_column;
> ! 	      cache->reg[PC_REGNUM].how = REG_SAVED_REG;
> ! 	    }
>   	}
>       }
>   
> diff -c -p -r gdb-head/gdb/reggroups.c gdb-head-new/gdb/reggroups.c
> *** gdb-head/gdb/reggroups.c	Wed Dec  3 14:50:26 2003
> --- gdb-head-new/gdb/reggroups.c	Wed Dec  3 14:50:31 2003
> *************** static struct reggroup vector_group = { 
> *** 254,259 ****
> --- 254,261 ----
>   static struct reggroup all_group = { "all", USER_REGGROUP };
>   static struct reggroup save_group = { "save", INTERNAL_REGGROUP };
>   static struct reggroup restore_group = { "restore", INTERNAL_REGGROUP };
> + static struct reggroup call_saved_group = { "call-saved", INTERNAL_REGGROUP };
> + static struct reggroup call_clobbered_group = { "call-clobbered", INTERNAL_REGGROUP };
>   
>   struct reggroup *const general_reggroup = &general_group;
>   struct reggroup *const float_reggroup = &float_group;
> *************** struct reggroup *const vector_reggroup =
> *** 262,267 ****
> --- 264,271 ----
>   struct reggroup *const all_reggroup = &all_group;
>   struct reggroup *const save_reggroup = &save_group;
>   struct reggroup *const restore_reggroup = &restore_group;
> + struct reggroup *const call_saved_reggroup = &call_saved_group;
> + struct reggroup *const call_clobbered_reggroup = &call_clobbered_group;
>   
>   extern initialize_file_ftype _initialize_reggroup; /* -Wmissing-prototypes */
>   
> *************** _initialize_reggroup (void)
> *** 278,283 ****
> --- 282,289 ----
>     add_group (&default_groups, all_reggroup, XMALLOC (struct reggroup_el));
>     add_group (&default_groups, save_reggroup, XMALLOC (struct reggroup_el));
>     add_group (&default_groups, restore_reggroup, XMALLOC (struct reggroup_el));
> +   add_group (&default_groups, call_saved_reggroup, XMALLOC (struct reggroup_el));
> +   add_group (&default_groups, call_clobbered_reggroup, XMALLOC (struct reggroup_el));
>   
>     add_cmd ("reggroups", class_maintenance,
>   	   maintenance_print_reggroups, "\
> diff -c -p -r gdb-head/gdb/reggroups.h gdb-head-new/gdb/reggroups.h
> *** gdb-head/gdb/reggroups.h	Wed Dec  3 14:50:26 2003
> --- gdb-head-new/gdb/reggroups.h	Wed Dec  3 14:50:31 2003
> *************** extern struct reggroup *const all_reggro
> *** 39,44 ****
> --- 39,46 ----
>   /* Pre-defined, internal, register groups.  */
>   extern struct reggroup *const save_reggroup;
>   extern struct reggroup *const restore_reggroup;
> + extern struct reggroup *const call_saved_reggroup;
> + extern struct reggroup *const call_clobbered_reggroup;
>   
>   /* Create a new local register group.  */
>   extern struct reggroup *reggroup_new (const char *name,
> diff -c -p -r gdb-head/gdb/s390-tdep.c gdb-head-new/gdb/s390-tdep.c
> *** gdb-head/gdb/s390-tdep.c	Wed Dec  3 14:50:26 2003
> --- gdb-head-new/gdb/s390-tdep.c	Wed Dec  3 14:50:31 2003
> ***************
> *** 39,44 ****
> --- 39,45 ----
>   #include "trad-frame.h"
>   #include "frame-base.h"
>   #include "frame-unwind.h"
> + #include "dwarf2-frame.h"
>   #include "reggroups.h"
>   #include "regset.h"
>   #include "value.h"
> *************** s390_register_reggroup_p (struct gdbarch
> *** 435,440 ****
> --- 436,469 ----
>     if (group == save_reggroup || group == restore_reggroup)
>       return regnum != S390_PSWM_REGNUM && regnum != S390_PSWA_REGNUM;
>   
> +   /* Call-saved registers.  */
> +   if (group == call_saved_reggroup)
> +     switch (tdep->abi)
> +       {
> +       case ABI_LINUX_S390:
> + 	return (regnum >= S390_R6_REGNUM && regnum <= S390_R15_REGNUM)
> + 	       || regnum == S390_F4_REGNUM
> + 	       || regnum == S390_F6_REGNUM;
> + 
> +       case ABI_LINUX_ZSERIES:
> + 	return (regnum >= S390_R6_REGNUM && regnum <= S390_R15_REGNUM)
> + 	       || (regnum >= S390_F8_REGNUM && regnum <= S390_F15_REGNUM);
> +       }
> + 
> +   /* Call-clobbered registers.  */
> +   if (group == call_clobbered_reggroup)
> +     switch (tdep->abi)
> +       {
> +       case ABI_LINUX_S390:
> + 	return (regnum >= S390_R0_REGNUM && regnum <= S390_R5_REGNUM)
> + 	       || (regnum >= S390_F0_REGNUM && regnum <= S390_F15_REGNUM
> + 		   && regnum != S390_F4_REGNUM && regnum != S390_F6_REGNUM);
> + 
> +       case ABI_LINUX_ZSERIES:
> + 	return (regnum >= S390_R0_REGNUM && regnum <= S390_R5_REGNUM)
> + 	       || (regnum >= S390_F0_REGNUM && regnum <= S390_F7_REGNUM);
> +       }
> + 
>     return default_register_reggroup_p (gdbarch, regnum, group);
>   }
>   

Re: [PATCH] S/390 DWARF-2 CFI frame support

[Ulrich Weigand]

Andrew Cagney wrote:

> What does the CFI output look like here?  (I'm mainly curious but it 
> always helps to have concrete examples).

First some information on our platform: we have a 'Program Status Word'
that contains amongst some other data the instruction address (PC).
We also have 16 general purpose registers %r0 .. %r15.  The ABI 
reserves register %r15 as stack pointer and register %r14 as function
call return address.

A function call (on 64-bit; 31-bit is similiar but a bit more tedious)
looks basically like this:

  brasl %r14, func

The BRANCH RELATIVE LONG AND SAVE (brasl) instruction saves the current
instruction address from the PSW into the specified register, in this
case %r14, and then loads the specified target address, in this case
the address of func, into the instruction address field of the PSW.
(The target address is actually encoded relative to the current PC,
but that doesn't matter here.)

The called function now typically looks like that:

  stmg %r6,%r15,48(%r15)       (STORE MULTIPLE)
  aghi %r15,-160               (ADD IMMEDIATE)

  ...

  lmg %r6,%r15,208(%r15)       (LOAD IMMEDIATE)
  br %r14                      (BRANCH REGISTER)

The prolog stores all call-saved registers, including the return address
%r14 and the old stack pointer %r15 into a register save area on the
stack (allocated but unused by the caller), and subsequently decrements
%r15 to allocate its own stack frame.

The function epilog restores all call-clobbered registers, including
the return register %r14 and the stack pointer %r15 (note that this
-as a side effect- pops the stack frame), and finally branches back
to the return address, which now again resides in register %r14.

The CFI looks about like this:

00000000 00000014 00000000 CIE
  Version:               1
  Augmentation:          "zR"
  Code alignment factor: 1
  Data alignment factor: -8
  Return address column: 14
  Augmentation data:     1b

  DW_CFA_def_cfa: r15 ofs 160
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop

00000018 0000001c 0000001c FDE cie=00000000 pc=000208c0..0002090e
  DW_CFA_advance_loc: 6 to 000208c6
  DW_CFA_offset: r15 at cfa-40
  DW_CFA_offset: r14 at cfa-48
  DW_CFA_offset: r13 at cfa-56
  DW_CFA_offset: r12 at cfa-64
  DW_CFA_offset: r11 at cfa-72
  DW_CFA_advance_loc: 8 to 000208ce
  DW_CFA_def_cfa_offset: 320

Note that we start out (at function entry) with a CFA of %r15 + 160,
i.e. the CFA points to the top of the save area that our caller has
allocated for us.  The CFI describes where the registers, including
%r14 and %r15 are saved.  After the AGHI, the CFA is now at %r15 + 
320.

To properly unwind this stack frame, we needs to restore all registers
that are described by DW_CFA_offset records.  Note that this includes
the stack pointer itself, and %r14.  To unwind the PC, however, we
need to copy the value in %r14 *after that was already unwound* into
the PC register (PSW address).  This is why we set the return address
column to 14 (which denotes %r14 according to the DWARF-2 register
numbering), which should have just that effect.


Now, in the case of a leaf function with no stack frame, things look
a bit different:

    ...

    br %r14

There is no prologue at all, and the epilogue consists simply of a 
branch to register %r14.  Such a function would have an empty CFI
record.  To unwind that, it is to be interpreted as follows: assume
all call-clobbered registers (*including* both return address (%r14)
and stack pointer (%r15)) as having the same value, and then move
the unwound (i.e. unchanged) value of %r14 into the PC register.

(There are other combinations possible: there might be saved registers
even in a leaf function, and these might include %r14.  Then, %r14
would need to be restored from the stack, while %r15 would still need
to be left unchanged.)


I hope this clarifies the sitation on our platform.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de

Re: [PATCH] S/390 DWARF-2 CFI frame support

[Andrew Cagney]

Mark,

This is really a question for you.  If you want I can hack up the code 
(although I don't know exactly when that would happen).

Andrew

> Hello,
> 
> this patch adds support for DWARF-2 frame handling to the s390 backend.
> 
> However, the DWARF-2 frame common code makes a couple of assumptions
> that are not valid on s390:
> 
> - The return address column is not unwound into the specified register,
>   only into the PC.  On s390, it should be unwound into both.
> 
>   The patch below fixes this by unwinding into both register
>   and PC if the return column corresponds to a GDB register.
> 
> What does the CFI output look like here?  (I'm mainly curious but it always helps to have concrete examples).
> 
> For a variant of the PPC64 GCC I found:
> 
> - return column pointed at the FSCR
> (which is stupid)
> 
> - return column same-value as "LR" was implied
> (which is contrary to the DWARF2's example, and what you also encountered)
> 
> this means that we've now got the following initial states all mapping onto "unspecified":
> 
> - same value
> - unknown
> - (for SP_REGNUM) it's the CFA (but is it one frame out?)
> - (for return column) it's the LR
> 
> I'm wondering if it would be easier, here to "give up".  Throw the whole problem back at the architecture vis:
> 
> - initialize unwind table using per OSABI (?) method
> 
> - update the unwind table using the CFI initialize info
> 
> - update the unwind table using the PC's CFI unwind info
> 
> - if after all this a register is still unspecified, we complain
> (btw, the compaint is only ment to appear once but apparently appears repeatedly?).
> 
> Thoughts for the moment on the theory?  Mark?
> 
> It should also open the way for the elimination of SP_REGNUM and PC_REGNUM from this file (ya!).
> 
> As for an interface, the <ARCH>_gdbarch_init code could something like:
> 
>     dwarf2_frame_add_sniffer (gdbarch, (*<ARCH>_dwarf2_cfi_init) (struct cfi_table *table));
> 
> (but with better names :-).
> 
> Andrew

Re: [PATCH] S/390 DWARF-2 CFI frame support

[Ulrich Weigand]

Andrew,

> > I'm wondering if it would be easier, here to "give up".  Throw the whole problem back at the architecture vis:
> > 
> > - initialize unwind table using per OSABI (?) method
> > 
> > - update the unwind table using the CFI initialize info
> > 
> > - update the unwind table using the PC's CFI unwind info
> > 
> > - if after all this a register is still unspecified, we complain
> > (btw, the compaint is only ment to appear once but apparently appears repeatedly?).
> > 
> > Thoughts for the moment on the theory?  Mark?

I don't quite see how to map the existing issues in this ABI:

- On s390, we need to copy %r14 *after it was unwound* to the PC
  register.  I see how to do that if an arch-specific routine is
  called after the CFI data is parsed, but not how to do it if
  the arch-specific routine is called *before*.

- On x86_64, you need to specific that the stack pointer is unwound
  to the CFA.  How should the arch-specific routine do that?
  (Again, if the arch-specific routine were called after the CFI
  is parsed, it could presumably copy the expression for the CFA
  as stack pointer unwind expression.  But before?)

However, this interface would solve the call-saved registers issue.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de

Re: [PATCH] S/390 DWARF-2 CFI frame support

[Mark Kettenis]

I've been thinking about this over the last few days, but I haven't
reached a conclusion yet.

I've considered per-architecture initialization of the unwind table
before.  However, the things Richard Henderson says about treating
uninitialized columns as "same value" make sense.  And as Ulrich
pointed out, it doesn't immediately solve our problems with the PC and
SP.

On the other hand, we should be able to introduce our own "rules" in
addition to the ones given by the DWARF2/3 specification.  We could
add the following two:

* REG_RETURN_ADDRESS: Set the particular register to the return
  address of the function.

* REG_CFA: Set the particular register to the call frame address.

We could even allow for an offset, such that we could specify rules
such as: set the ISA PC register to the return address plus an offset
of 8 bytes, or set the ISA SP register to the call frame address plus
minus an offset of 128 bytes.  I might need such a facility for SPARC.

How does that sound?

In the meantime, I'm going to try to remove some of the PC and
SP-related hacks in dwarf2-frame.c and see what happens.

Mark

Re: [PATCH] S/390 DWARF-2 CFI frame support

[Ulrich Weigand]

Mark Kettenis wrote:

> I've considered per-architecture initialization of the unwind table
> before.  However, the things Richard Henderson says about treating
> uninitialized columns as "same value" make sense.

However, I rather like to see 'value not available' instead of
an incorrect value in an 'info reg' display.  So if we do have
an arch-dependent callback, we might as well use ABI knowledge
to get this right.

> On the other hand, we should be able to introduce our own "rules" in
> addition to the ones given by the DWARF2/3 specification.  We could
> add the following two:
> 
> * REG_RETURN_ADDRESS: Set the particular register to the return
>   address of the function.
> 
> * REG_CFA: Set the particular register to the call frame address.
> 
> We could even allow for an offset, such that we could specify rules
> such as: set the ISA PC register to the return address plus an offset
> of 8 bytes, or set the ISA SP register to the call frame address plus
> minus an offset of 128 bytes.  I might need such a facility for SPARC.
> 
> How does that sound?

This would certainly solve the s390 situation.

> In the meantime, I'm going to try to remove some of the PC and
> SP-related hacks in dwarf2-frame.c and see what happens.

The only hack that cannot be replaced using the rules described
above (as far as I can see) is the 
  if (column == fs->retaddr_column)
    continue;
in dwarf2_frame_cache.  Does any platform rely on this behaviour?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de

Re: [PATCH] S/390 DWARF-2 CFI frame support

[Mark Kettenis]

   From: Ulrich Weigand <weigand@i1.informatik.uni-erlangen.de>
   Date: Sat, 13 Dec 2003 16:32:12 +0100 (CET)
   Cc: cagney@gnu.org, weigand@i1.informatik.uni-erlangen.de,
      gdb-patches@sources.redhat.com, uweigand@de.ibm.com
   Content-Type: text/plain; charset=us-ascii
   X-Spam-Level: 
   X-Spam-Checker-Version: SpamAssassin 2.60 (1.212-2003-09-23-exp) on 
	   elgar.kettenis.dyndns.org
   X-Spam-Status: No, hits=-4.9 required=5.0 tests=BAYES_00 autolearn=ham 
	   version=2.60

   Mark Kettenis wrote:

   > I've considered per-architecture initialization of the unwind table
   > before.  However, the things Richard Henderson says about treating
   > uninitialized columns as "same value" make sense.

   However, I rather like to see 'value not available' instead of
   an incorrect value in an 'info reg' display.  So if we do have
   an arch-dependent callback, we might as well use ABI knowledge
   to get this right.

It probably depends a bit on the context.  IMHO it is perfectly
all-right for a compiler to generate code that doesn't conform to the
ABI if it knows what it's doing.  Or for to user to ask the compiler
to generate code that doesn't conform to the ABI.  IIRC this happens
at various places in glibc.  It would be a bit unfortunately if we
made it hard or impossible for the user to view (valid) variables in
registers, just because we're strictly enforcing the ABI.  Of course
the real solution here would be to get GCC to emit CFI for all
registers, at least for .dwarf_frame (as opposed to .eh_frame)
sections.

   > In the meantime, I'm going to try to remove some of the PC and
   > SP-related hacks in dwarf2-frame.c and see what happens.

   The only hack that cannot be replaced using the rules described
   above (as far as I can see) is the 
     if (column == fs->retaddr_column)
       continue;
   in dwarf2_frame_cache.  Does any platform rely on this behaviour?

The reason why I added that hack in the first place is the case where
the return address column does not correspond to an actual register.
In that case we must make sure that we don't map it onto one of GDB's
(pseudo-)registers.  Assuming that the compiler has some freedom in
choosing the return address column number, and considering that the
DWARF-2 register numbers are largely undocumented for most targets, I
was worried that I couldn't guarantee this.

AFAICT there is no platform yet where GCC uses a return address column
number that would be mapped on the wrong GDB register, so I think we
can safely remove the code.  New targets that start using the DWARF-2
CFI stuff should make sure theur DWARF-2 register number mapping is
right.

Mark

Re: [PATCH] S/390 DWARF-2 CFI frame support

[Andrew Cagney]

>    From: Ulrich Weigand <weigand@i1.informatik.uni-erlangen.de>
>    Date: Sat, 13 Dec 2003 16:32:12 +0100 (CET)
>    Cc: cagney@gnu.org, weigand@i1.informatik.uni-erlangen.de,
>       gdb-patches@sources.redhat.com, uweigand@de.ibm.com
>    Content-Type: text/plain; charset=us-ascii
>    X-Spam-Level: 
>    X-Spam-Checker-Version: SpamAssassin 2.60 (1.212-2003-09-23-exp) on 
> 	   elgar.kettenis.dyndns.org
>    X-Spam-Status: No, hits=-4.9 required=5.0 tests=BAYES_00 autolearn=ham 
> 	   version=2.60
> 
>    Mark Kettenis wrote:
> 
>    > I've considered per-architecture initialization of the unwind table
>    > before.  However, the things Richard Henderson says about treating
>    > uninitialized columns as "same value" make sense.
> 
>    However, I rather like to see 'value not available' instead of
>    an incorrect value in an 'info reg' display.  So if we do have
>    an arch-dependent callback, we might as well use ABI knowledge
>    to get this right.
> 
> It probably depends a bit on the context.  IMHO it is perfectly
> all-right for a compiler to generate code that doesn't conform to the
> ABI if it knows what it's doing.  Or for to user to ask the compiler
> to generate code that doesn't conform to the ABI.  IIRC this happens
> at various places in glibc.  It would be a bit unfortunately if we
> made it hard or impossible for the user to view (valid) variables in
> registers, just because we're strictly enforcing the ABI.  Of course
> the real solution here would be to get GCC to emit CFI for all
> registers, at least for .dwarf_frame (as opposed to .eh_frame)
> sections.

Let me scare you a little ....

GCC is working towards to being able to do non-ABI local function calls 
(and knowing them a few global ones as well :-)

With that said, I think for values, GDB should print value-unknown, but 
for registers, print the likely wrong value (we could always add "info 
known-registers" :-).  But its a judgment call, anyone want to toss the 
coin?

>    > In the meantime, I'm going to try to remove some of the PC and
>    > SP-related hacks in dwarf2-frame.c and see what happens.
> 
>    The only hack that cannot be replaced using the rules described
>    above (as far as I can see) is the 
>      if (column == fs->retaddr_column)
>        continue;
>    in dwarf2_frame_cache.  Does any platform rely on this behaviour?
> 
> The reason why I added that hack in the first place is the case where
> the return address column does not correspond to an actual register.
> In that case we must make sure that we don't map it onto one of GDB's
> (pseudo-)registers.  Assuming that the compiler has some freedom in
> choosing the return address column number, and considering that the
> DWARF-2 register numbers are largely undocumented for most targets, I
> was worried that I couldn't guarantee this.
> 
> AFAICT there is no platform yet where GCC uses a return address column
> number that would be mapped on the wrong GDB register, so I think we
> can safely remove the code.  New targets that start using the DWARF-2
> CFI stuff should make sure theur DWARF-2 register number mapping is
> right.

Well, ... the PPC64 return-column, when I last looked, specified the 
dwarf2' floating-point status and control register number!  But let the 
person framifying the PPC64 sort that one out :-)

Anyway, with respect to your proposal, yes like it.

Andrew

Re: [PATCH] S/390 DWARF-2 CFI frame support

[Mark Kettenis]

   Date: Sun, 14 Dec 2003 11:40:50 -0500
   From: Andrew Cagney <cagney@gnu.org>

   > The reason why I added that hack in the first place is the case where
   > the return address column does not correspond to an actual register.
   > In that case we must make sure that we don't map it onto one of GDB's
   > (pseudo-)registers.  Assuming that the compiler has some freedom in
   > choosing the return address column number, and considering that the
   > DWARF-2 register numbers are largely undocumented for most targets, I
   > was worried that I couldn't guarantee this.
   > 
   > AFAICT there is no platform yet where GCC uses a return address column
   > number that would be mapped on the wrong GDB register, so I think we
   > can safely remove the code.  New targets that start using the DWARF-2
   > CFI stuff should make sure theur DWARF-2 register number mapping is
   > right.

   Well, ... the PPC64 return-column, when I last looked, specified the 
   dwarf2' floating-point status and control register number!  But let the 
   person framifying the PPC64 sort that one out :-)

Argh!  Someone teach GCC about the PPC64 DWARF register numbering
please!  Before it is too late!  Now it is using the PPC32 LR register
number, which just happens to be the PPC64 FPSCR register.

   Anyway, with respect to your proposal, yes like it.

I'll see whether I can implement it.

Mark