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

Ulrich Weigand
2003-12-04T20:09:17Z

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