This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Status of DWARF-2 frames for S/390


Hello Mark,

thanks for your recent work in making DWARF-2 CFI frame handling
more suitable for S/390.  Unfortunately, things still don't quite
work out yet; I see two problems remaining:

- Unspecified RA column.  If fs->retaddr_column names a register
  that is not specified in the CFI, things break on S/390.
  What should happen in this case (on s390) is that the value
  of the named retaddr_column register from the inner frame
  should be copied into the outer frame's PC register.

  The minimal fix to make this work on s390 is this patch:
  
*************** dwarf2_frame_cache (struct frame_info *n
*** 639,645 ****
        {
  	if (cache->reg[regnum].how == REG_RA)
  	  {
! 	    if (fs->retaddr_column < fs->regs.num_regs)
  	      cache->reg[regnum] = fs->regs.reg[fs->retaddr_column];
  	    else
  	      {
--- 632,639 ----
        {
  	if (cache->reg[regnum].how == REG_RA)
  	  {
! 	    if (fs->retaddr_column < fs->regs.num_regs
! 		&& fs->regs.reg[fs->retaddr_column].how != REG_UNSPECIFIED)
  	      cache->reg[regnum] = fs->regs.reg[fs->retaddr_column];
  	    else
  	      {

   However, in the spirit of allowing the target to decide such
   things, it would presumably be a better solution to simply
   not copy the unwind state directly from fs->regs.reg, but instead
   from the corresponding cache->reg entry, which can be pre-
   initialized by the target to cope with the case that the
   register is unspecified in the CFI.

   One more thing: a REG_SAME_VALUE state must not be blindly
   copied to a different register number, because 'same' doesn't
   mean the same thing any more; it needs to be replaced by a 
   REG_SAVED_REG instead.


- That annoying message.  The 'complaint' when finding an
  unspecified register in the CFI is still active.  This 
  is quite annoying for two reasons: it will trigger for
  every single debug session on s390, giving the user the
  impression that something is wrong when in fact everything
  is completely normal; even more annoying, the additional
  unexpected output will cause test cases to fail.

  Does this message really save any useful purpose?  Could
  it not at least be changed to trigger only in the cases
  where the target hasn't provided a meaningful default?
  Or even moved to dwarf2_frame_prev_register to trigger
  only on an actual attempt to *access* an unspecified
  value?


Finally, of course, the actual mechanism to allow the target
to install default values is still missing.  Have you already
decided on the interface?  (Any chance we can get this in
time for 6.1? ;-/  If you're busy otherwise, I would certainly
be willing to do the implementation myself; but I'd need some
guidance on the interface question.)

FYI, below my current patch to activate DWARF-2 frames on s390
(no regressions on s390-ibm-linux and s390x-ibm-linux), still 
using the new reggroups.  (Pre-supposes the port modernization
patches.)

Bye,
Ulrich


ChangeLog:

	* dwarf2-frame.c: Include "reggroups.h".
	(dwarf2_frame_cache): Use call_saved_reggroup and 
	call_clobbered_reggroup to determine behaviour of registers 
	unspecified in the CFI.  Do not emit complaint about unspecified 
	registers.  Handle unspecified return column correctly.
	* 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	Sun Jan 18 19:36:06 2004
--- gdb-head-new/gdb/Makefile.in	Sun Jan 18 18:25:05 2004
*************** dwarf2expr.o: dwarf2expr.c $(defs_h) $(s
*** 1733,1739 ****
  	$(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) \
--- 1733,1739 ----
  	$(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)
*** 2252,2258 ****
  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) \
  	$(s390_tdep_h)
  scm-exp.o: scm-exp.c $(defs_h) $(symtab_h) $(gdbtypes_h) $(expression_h) \
--- 2252,2258 ----
  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) \
  	$(s390_tdep_h)
  scm-exp.o: scm-exp.c $(defs_h) $(symtab_h) $(gdbtypes_h) $(expression_h) \
diff -c -p -r gdb-head/gdb/dwarf2-frame.c gdb-head-new/gdb/dwarf2-frame.c
*** gdb-head/gdb/dwarf2-frame.c	Sun Jan 18 19:36:06 2004
--- gdb-head-new/gdb/dwarf2-frame.c	Sun Jan 18 19:36:12 2004
***************
*** 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
*** 481,486 ****
--- 482,488 ----
  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
*** 583,589 ****
  
      for (regnum = 0; regnum < num_regs; regnum++)
        {
! 	if (regnum == PC_REGNUM)
  	  cache->reg[regnum].how = REG_RA;
  	else if (regnum == SP_REGNUM)
  	  cache->reg[regnum].how = REG_CFA;
--- 585,597 ----
  
      for (regnum = 0; regnum < num_regs; regnum++)
        {
! 	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;
! 	else if (regnum == PC_REGNUM)
  	  cache->reg[regnum].how = REG_RA;
  	else if (regnum == SP_REGNUM)
  	  cache->reg[regnum].how = REG_CFA;
*************** dwarf2_frame_cache (struct frame_info *n
*** 611,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));
! 	else
  	  cache->reg[regnum] = fs->regs.reg[column];
        }
    }
--- 619,625 ----
  	if (regnum < 0 || regnum >= num_regs)
  	  continue;
  
! 	if (fs->regs.reg[column].how != REG_UNSPECIFIED)
  	  cache->reg[regnum] = fs->regs.reg[column];
        }
    }
*************** dwarf2_frame_cache (struct frame_info *n
*** 639,645 ****
        {
  	if (cache->reg[regnum].how == REG_RA)
  	  {
! 	    if (fs->retaddr_column < fs->regs.num_regs)
  	      cache->reg[regnum] = fs->regs.reg[fs->retaddr_column];
  	    else
  	      {
--- 632,639 ----
        {
  	if (cache->reg[regnum].how == REG_RA)
  	  {
! 	    if (fs->retaddr_column < fs->regs.num_regs
! 		&& fs->regs.reg[fs->retaddr_column].how != REG_UNSPECIFIED)
  	      cache->reg[regnum] = fs->regs.reg[fs->retaddr_column];
  	    else
  	      {
diff -c -p -r gdb-head/gdb/reggroups.c gdb-head-new/gdb/reggroups.c
*** gdb-head/gdb/reggroups.c	Sun Jan 18 19:36:06 2004
--- gdb-head-new/gdb/reggroups.c	Sun Jan 18 18:25:05 2004
*************** 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	Sun Jan 18 19:36:06 2004
--- gdb-head-new/gdb/reggroups.h	Sun Jan 18 18:25:05 2004
*************** 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	Sun Jan 18 19:36:06 2004
--- gdb-head-new/gdb/s390-tdep.c	Sun Jan 18 18:25:05 2004
***************
*** 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
*** 361,366 ****
--- 362,395 ----
    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
*** 2910,2915 ****
--- 2939,2946 ----
  
    /* 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]