Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
@ 2004-02-07 22:38 Mark Kettenis
  2004-02-07 23:03 ` Daniel Jacobowitz
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Mark Kettenis @ 2004-02-07 22:38 UTC (permalink / raw)
  To: gdb-patches; +Cc: weigand

Here's my proposal for the per-architecture DWARF CFI register state
initialization hooks needed for S/390, and others.  This is a RFC,
since I'm not entirely confident whether my approach is acceptable.  I
chose to implement this using per-architecture data instead of adding
a function to the architecture vector.  I think it is cleaner since it
keeps things localized and modular, and the architecture vector is big
enough as it stands.  However you folks might think otherwise.

Ulrich, this should privide the hooks you need.  For S/390 you should
provide a function with the following signature:

void
s390_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
                            struct gdbarch dwarf2_frame_state_reg *reg);

and initialize REG according to your needs for the REGNUMs you care
about.  Note that REGNUM is the GDB register number.

Comments?

Mark


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>
 
	* dwarf2-frame.h (dwarf2_frame_set_init_reg): New prototype.
	* dwarf2-frame.c (dwarf2_frame_init_reg_data): New variable.
	(dwarf2_frame_default_init_reg): Renamed from
	dwarf2_frame_init_reg.  Tweak comments.
	(dwarf2_frame_init_reg_init, dwarf2_frame_set_init_reg,
	dwarf2_frame_init_reg): New functions.
	(_initialize_dwarf2_frame): Initialize dwarf2_frame_init_reg_data.

Index: dwarf2-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2-frame.c,v
retrieving revision 1.28
diff -u -p -r1.28 dwarf2-frame.c
--- dwarf2-frame.c 7 Feb 2004 14:44:50 -0000 1.28
+++ dwarf2-frame.c 7 Feb 2004 22:28:15 -0000
@@ -454,53 +454,94 @@ execute_cfa_program (unsigned char *insn
   dwarf2_frame_state_free_regs (fs->regs.prev);
   fs->regs.prev = NULL;
 }
+\f
 
-struct dwarf2_frame_cache
-{
-  /* DWARF Call Frame Address.  */
-  CORE_ADDR cfa;
+/* Per-architecture data keys.  */
 
-  /* Saved registers, indexed by GDB register number, not by DWARF
-     register number.  */
-  struct dwarf2_frame_state_reg *reg;
-};
+static struct gdbarch_data *dwarf2_frame_init_reg_data;
 
-/* Initialize the register state REG.  If we have a register that acts
-   as a program counter, mark it as a destination for the return
-   address.  If we have a register that serves as the stack pointer,
-   arrange for it to be filled with the call frame address (CFA).  The
-   other registers are marked as unspecified.
-
-   We copy the return address to the program counter, since many parts
-   in GDB assume that it is possible to get the return address by
-   unwind the program counter register.  However, on ISA's with a
-   dedicated return address register, the CFI usually only contains
-   information to unwind that return address register.
-
-   The reason we're treating the stack pointer special here is because
-   in many cases GCC doesn't emit CFI for the stack pointer and
-   implicitly assumes that it is equal to the CFA.  This makes some
-   sense since the DWARF specification (version 3, draft 8, p. 102)
-   says that:
-
-   "Typically, the CFA is defined to be the value of the stack pointer
-   at the call site in the previous frame (which may be different from
-   its value on entry to the current frame)."
-
-   However, this isn't true for all platforms supported by GCC
-   (e.g. IBM S/390 and zSeries).  For those targets we should override
-   the defaults given here.  */
+/* Default acrhitecture-specific register state initialization
+   function.  */
 
 static void
-dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
-		       struct dwarf2_frame_state_reg *reg)
+dwarf2_frame_default_init_reg (struct gdbarch *gdbarch, int regnum,
+			       struct dwarf2_frame_state_reg *reg)
 {
+  /* If we have a register that acts as a program counter, mark it as
+     a destination for the return address.  If we have a register that
+     serves as the stack pointer, arrange for it to be filled with the
+     call frame address (CFA).  The other registers are marked as
+     unspecified.
+
+     We copy the return address to the program counter, since many
+     parts in GDB assume that it is possible to get the return address
+     by unwinding the program counter register.  However, on ISA's
+     with a dedicated return address register, the CFI usually only
+     contains information to unwind that return address register.
+
+     The reason we're treating the stack pointer special here is
+     because in many cases GCC doesn't emit CFI for the stack pointer
+     and implicitly assumes that it is equal to the CFA.  This makes
+     some sense since the DWARF specification (version 3, draft 8,
+     p. 102) says that:
+
+     "Typically, the CFA is defined to be the value of the stack
+     pointer at the call site in the previous frame (which may be
+     different from its value on entry to the current frame)."
+
+     However, this isn't true for all platforms supported by GCC
+     (e.g. IBM S/390 and zSeries).  Those architectures should provide
+     their own architecture-specific initialization function.  */
+
   if (regnum == PC_REGNUM)
     reg->how = DWARF2_FRAME_REG_RA;
   else if (regnum == SP_REGNUM)
     reg->how = DWARF2_FRAME_REG_CFA;
 }
 
+/* Return a default for the architecture-specific register state
+   initialization function.  */
+
+static void *
+dwarf2_frame_init_reg_init (struct gdbarch *gdbarch)
+{
+  return dwarf2_frame_default_init_reg;
+}
+
+/* Set the architecture-specific register state initialization
+   function for GDBARCH to INIT_REG.  */
+
+void
+dwarf2_frame_set_init_reg (struct gdbarch *gdbarch,
+			   void (*init_reg) (struct gdbarch *, int,
+					     struct dwarf2_frame_state_reg *))
+{
+  set_gdbarch_data (gdbarch, dwarf2_frame_init_reg_data, init_reg);
+}
+
+/* Pre-initialize the register state REG for register REGNUM.  */
+
+static void
+dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
+		       struct dwarf2_frame_state_reg *reg)
+{
+  void (*init_reg) (struct gdbarch *, int, struct dwarf2_frame_state_reg *);
+
+  init_reg = gdbarch_data (gdbarch, dwarf2_frame_init_reg_data);
+  init_reg (gdbarch, regnum, reg);
+}
+\f
+
+struct dwarf2_frame_cache
+{
+  /* DWARF Call Frame Address.  */
+  CORE_ADDR cfa;
+
+  /* Saved registers, indexed by GDB register number, not by DWARF
+     register number.  */
+  struct dwarf2_frame_state_reg *reg;
+};
+
 static struct dwarf2_frame_cache *
 dwarf2_frame_cache (struct frame_info *next_frame, void **this_cache)
 {
@@ -1461,7 +1502,6 @@ decode_frame_entry (struct comp_unit *un
 
   return ret;
 }
-
 \f
 
 /* FIXME: kettenis/20030504: This still needs to be integrated with
@@ -1542,4 +1582,6 @@ void
 _initialize_dwarf2_frame (void)
 {
   dwarf2_frame_data = register_objfile_data ();
+  dwarf2_frame_init_reg_data =
+    register_gdbarch_data (dwarf2_frame_init_reg_init);
 }
Index: dwarf2-frame.h
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2-frame.h,v
retrieving revision 1.4
diff -u -p -r1.4 dwarf2-frame.h
--- dwarf2-frame.h 7 Feb 2004 14:44:50 -0000 1.4
+++ dwarf2-frame.h 7 Feb 2004 22:28:15 -0000
@@ -71,6 +71,13 @@ struct dwarf2_frame_state_reg
   enum dwarf2_frame_reg_rule how;
 };
 
+/* Set the architecture-specific register state initialization
+   function for GDBARCH to INIT_REG.  */
+
+extern void dwarf2_frame_set_init_reg (struct gdbarch *gdbarch,
+				       void (*init_reg) (struct gdbarch *, int,
+					     struct dwarf2_frame_state_reg *));
+
 /* Return the frame unwind methods for the function that contains PC,
    or NULL if it can't be handled by DWARF CFI frame unwinder.  */
 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
  2004-02-07 22:38 [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks Mark Kettenis
@ 2004-02-07 23:03 ` Daniel Jacobowitz
  2004-02-07 23:48 ` Andrew Cagney
  2004-02-08  1:01 ` Ulrich Weigand
  2 siblings, 0 replies; 20+ messages in thread
From: Daniel Jacobowitz @ 2004-02-07 23:03 UTC (permalink / raw)
  To: gdb-patches

On Sat, Feb 07, 2004 at 11:37:52PM +0100, Mark Kettenis wrote:
> Here's my proposal for the per-architecture DWARF CFI register state
> initialization hooks needed for S/390, and others.  This is a RFC,
> since I'm not entirely confident whether my approach is acceptable.  I
> chose to implement this using per-architecture data instead of adding
> a function to the architecture vector.  I think it is cleaner since it
> keeps things localized and modular, and the architecture vector is big
> enough as it stands.  However you folks might think otherwise.

Hmm, I do.  You're adding a per-architecture data item which is a
function pointer, and what amounts to the rest of what gdbarch.sh would
generate (wrapper functions, default initialization.  I'd rather you
just used gdbarch.sh.

> Ulrich, this should privide the hooks you need.  For S/390 you should
> provide a function with the following signature:
> 
> void
> s390_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>                             struct gdbarch dwarf2_frame_state_reg *reg);
> 
> and initialize REG according to your needs for the REGNUMs you care
> about.  Note that REGNUM is the GDB register number.
> 
> Comments?

I've no objection.  I would have done something like:

void
gdbarch_dwarf2_frame_init_reg (struct gdbarch *gdbarch,
			       struct dwarf3_frame_state_reg *reg);

  dwarf2_frame_default_init_reg (reg);
  gdbarch_dwarf2_frame_init_reg (gdbarch, reg);

but this is just as good, and we're not _that_ pinched for startup
time.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
  2004-02-07 22:38 [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks Mark Kettenis
  2004-02-07 23:03 ` Daniel Jacobowitz
@ 2004-02-07 23:48 ` Andrew Cagney
  2004-02-15 15:31   ` Mark Kettenis
  2004-02-08  1:01 ` Ulrich Weigand
  2 siblings, 1 reply; 20+ messages in thread
From: Andrew Cagney @ 2004-02-07 23:48 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches, weigand

> Here's my proposal for the per-architecture DWARF CFI register state
> initialization hooks needed for S/390, and others.  This is a RFC,
> since I'm not entirely confident whether my approach is acceptable.  I
> chose to implement this using per-architecture data instead of adding
> a function to the architecture vector.  I think it is cleaner since it
> keeps things localized and modular, and the architecture vector is big
> enough as it stands.

Yes.  Technical nit though - I think it is still better to have a local 
data struct and store the value in there.

Andrew



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
  2004-02-07 22:38 [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks Mark Kettenis
  2004-02-07 23:03 ` Daniel Jacobowitz
  2004-02-07 23:48 ` Andrew Cagney
@ 2004-02-08  1:01 ` Ulrich Weigand
  2 siblings, 0 replies; 20+ messages in thread
From: Ulrich Weigand @ 2004-02-08  1:01 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches, weigand

Mark Kettenis wrote:

> Ulrich, this should privide the hooks you need.  For S/390 you should
> provide a function with the following signature:
> 
> void
> s390_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>                             struct gdbarch dwarf2_frame_state_reg *reg);
> 
> and initialize REG according to your needs for the REGNUMs you care
> about.  Note that REGNUM is the GDB register number.

This works for S/390; thanks for taking care of this problem!

With your patch and the one appended below, the test suite passes
without regressions; even after additionally switching off the
traditional frame handling in s390-tdep, the test suite still
passes with regressions only in asm-source (because the assembler
code lacks CFI instrumentation).

Bye,
Ulrich


ChangeLog:

	* s390-tdep.c: Include "dwarf2-frame.h".
	(s390_dwarf2_frame_init_reg): New function.
	(s390_gdbarch_init): Install dwarf2_frame_sniffer and
	dwarf2_frame_base_sniffer.  Call dwarf2_frame_set_init_reg.
	* Makefile.in (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	Sat Feb  7 23:57:09 2004
--- gdb-head-new/gdb/Makefile.in	Sat Feb  7 23:59:04 2004
*************** s390-nat.o: s390-nat.c $(defs_h) $(tm_h)
*** 2276,2282 ****
  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) \
--- 2276,2282 ----
  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/s390-tdep.c gdb-head-new/gdb/s390-tdep.c
*** gdb-head/gdb/s390-tdep.c	Sat Feb  7 23:57:09 2004
--- gdb-head-new/gdb/s390-tdep.c	Sun Feb  8 00:08:20 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_unwind_sp (struct gdbarch *gdbarch,
*** 2298,2303 ****
--- 2299,2351 ----
  }
  
  
+ /* DWARF-2 frame support.  */
+ 
+ static void
+ s390_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
+                             struct dwarf2_frame_state_reg *reg)
+ {
+   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+ 
+   switch (tdep->abi)
+     {
+     case ABI_LINUX_S390:
+       /* Call-saved registers.  */
+       if ((regnum >= S390_R6_REGNUM && regnum <= S390_R15_REGNUM)
+ 	  || regnum == S390_F4_REGNUM
+ 	  || regnum == S390_F6_REGNUM)
+ 	reg->how = DWARF2_FRAME_REG_SAME_VALUE;
+ 
+       /* Call-clobbered registers.  */
+       else if ((regnum >= S390_R0_REGNUM && regnum <= S390_R5_REGNUM)
+ 	       || (regnum >= S390_F0_REGNUM && regnum <= S390_F15_REGNUM
+ 		   && regnum != S390_F4_REGNUM && regnum != S390_F6_REGNUM))
+ 	reg->how = DWARF2_FRAME_REG_UNDEFINED;
+ 
+       /* The return address column.  */
+       else if (regnum == PC_REGNUM)
+ 	reg->how = DWARF2_FRAME_REG_RA;
+       break;
+ 
+     case ABI_LINUX_ZSERIES:
+       /* Call-saved registers.  */
+       if ((regnum >= S390_R6_REGNUM && regnum <= S390_R15_REGNUM)
+ 	  || (regnum >= S390_F8_REGNUM && regnum <= S390_F15_REGNUM))
+ 	reg->how = DWARF2_FRAME_REG_SAME_VALUE;
+ 
+       /* Call-clobbered registers.  */
+       else if ((regnum >= S390_R0_REGNUM && regnum <= S390_R5_REGNUM)
+ 	       || (regnum >= S390_F0_REGNUM && regnum <= S390_F7_REGNUM))
+ 	reg->how = DWARF2_FRAME_REG_UNDEFINED;
+ 
+       /* The return address column.  */
+       else if (regnum == PC_REGNUM)
+ 	reg->how = DWARF2_FRAME_REG_RA;
+       break;
+     }
+ }
+ 
+ 
  /* Dummy function calls.  */
  
  /* Return non-zero if TYPE is an integer-like type, zero otherwise.
*************** s390_gdbarch_init (struct gdbarch_info i
*** 2910,2915 ****
--- 2958,2966 ----
  
    /* Frame handling.  */
    set_gdbarch_in_solib_call_trampoline (gdbarch, in_plt_section);
+   dwarf2_frame_set_init_reg (gdbarch, s390_dwarf2_frame_init_reg);
+   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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
  2004-02-07 23:48 ` Andrew Cagney
@ 2004-02-15 15:31   ` Mark Kettenis
  2004-02-15 16:04     ` Andrew Cagney
  0 siblings, 1 reply; 20+ messages in thread
From: Mark Kettenis @ 2004-02-15 15:31 UTC (permalink / raw)
  To: cagney, drow; +Cc: gdb-patches

   Date: Sat, 07 Feb 2004 18:48:47 -0500
   From: Andrew Cagney <cagney@gnu.org>

   > Here's my proposal for the per-architecture DWARF CFI register state
   > initialization hooks needed for S/390, and others.  This is a RFC,
   > since I'm not entirely confident whether my approach is acceptable.  I
   > chose to implement this using per-architecture data instead of adding
   > a function to the architecture vector.  I think it is cleaner since it
   > keeps things localized and modular, and the architecture vector is big
   > enough as it stands.

   Yes.  Technical nit though - I think it is still better to have a local 
   data struct and store the value in there.

I'm not sure what your idea is here.  Is it that you want me to use a
data structure that would be allocated by the dwarf2-frame.c module
such that I'd only need a single per-arch data key for the entire
dwarf2-frame.c module?  Or do you want the architecture to allocate
and initialize the structure?  The latter would mean more work for the
architecture; if you want to override a single member of the structure
you'd have to fill in all the details.  I don't really like that.

   From: Daniel Jacobowitz <drow@mvista.com>
   Date: Sat, 7 Feb 2004 18:03:30 -0500

   Hmm, I do.  You're adding a per-architecture data item which is a
   function pointer, and what amounts to the rest of what gdbarch.sh would
   generate (wrapper functions, default initialization.  I'd rather you
   just used gdbarch.sh.


What about Daniels objections that I'm hand-coding much what
gdbarch.sh already does?  I'm feeling that the modularity is worth it,
but how do you feel about that?

Mark


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
  2004-02-15 15:31   ` Mark Kettenis
@ 2004-02-15 16:04     ` Andrew Cagney
  2004-02-15 18:09       ` Daniel Jacobowitz
  2004-02-15 21:31       ` Mark Kettenis
  0 siblings, 2 replies; 20+ messages in thread
From: Andrew Cagney @ 2004-02-15 16:04 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: drow, gdb-patches

>    Date: Sat, 07 Feb 2004 18:48:47 -0500
>    From: Andrew Cagney <cagney@gnu.org>
> 
>    > Here's my proposal for the per-architecture DWARF CFI register state
>    > initialization hooks needed for S/390, and others.  This is a RFC,
>    > since I'm not entirely confident whether my approach is acceptable.  I
>    > chose to implement this using per-architecture data instead of adding
>    > a function to the architecture vector.  I think it is cleaner since it
>    > keeps things localized and modular, and the architecture vector is big
>    > enough as it stands.
> 
>    Yes.  Technical nit though - I think it is still better to have a local 
>    data struct and store the value in there.
> 
> I'm not sure what your idea is here.  Is it that you want me to use a
> data structure that would be allocated by the dwarf2-frame.c module
> such that I'd only need a single per-arch data key for the entire
> dwarf2-frame.c module?

Yes, just this:

> static struct gdbarch_data *frame_base_data;
> 
> struct frame_base_table
> {
>   frame_base_sniffer_ftype **sniffer;
....
> };
> 
> static struct frame_base_table *
> frame_base_table (struct gdbarch *gdbarch)
> {
>   struct frame_base_table *table = gdbarch_data (gdbarch, frame_base_data);
>   ...
>   return table;
> }

i.e, put the function inside a struct, instead of storing the function 
directly in that per-arch data pointer.

It's more consistent with:
http://sources.redhat.com/gdb/current/onlinedocs/gdbint_13.html#SEC114
but hmm, that's out of date, sigh.  Instead of free, memory is obtained 
using gdbarch_obstack_zalloc.

 > Or do you want the architecture to allocate
> and initialize the structure?  The latter would mean more work for the
> architecture; if you want to override a single member of the structure
> you'd have to fill in all the details.  I don't really like that.
> 
>    From: Daniel Jacobowitz <drow@mvista.com>
>    Date: Sat, 7 Feb 2004 18:03:30 -0500
> 
>    Hmm, I do.  You're adding a per-architecture data item which is a
>    function pointer, and what amounts to the rest of what gdbarch.sh would
>    generate (wrapper functions, default initialization.  I'd rather you
>    just used gdbarch.sh.
> 
> 
> What about Daniels objections that I'm hand-coding much what
> gdbarch.sh already does?  I'm feeling that the modularity is worth it,
> but how do you feel about that?

No. Yes.  Using gdbarch, and loosing that modularity, is far too high a 
price to pay.

Andrew



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
  2004-02-15 16:04     ` Andrew Cagney
@ 2004-02-15 18:09       ` Daniel Jacobowitz
  2004-02-15 19:49         ` Andrew Cagney
  2004-02-15 21:31       ` Mark Kettenis
  1 sibling, 1 reply; 20+ messages in thread
From: Daniel Jacobowitz @ 2004-02-15 18:09 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: Mark Kettenis, gdb-patches

On Sun, Feb 15, 2004 at 11:04:26AM -0500, Andrew Cagney wrote:
> > Or do you want the architecture to allocate
> >and initialize the structure?  The latter would mean more work for the
> >architecture; if you want to override a single member of the structure
> >you'd have to fill in all the details.  I don't really like that.
> >
> >   From: Daniel Jacobowitz <drow@mvista.com>
> >   Date: Sat, 7 Feb 2004 18:03:30 -0500
> >
> >   Hmm, I do.  You're adding a per-architecture data item which is a
> >   function pointer, and what amounts to the rest of what gdbarch.sh would
> >   generate (wrapper functions, default initialization.  I'd rather you
> >   just used gdbarch.sh.
> >
> >
> >What about Daniels objections that I'm hand-coding much what
> >gdbarch.sh already does?  I'm feeling that the modularity is worth it,
> >but how do you feel about that?
> 
> No. Yes.  Using gdbarch, and loosing that modularity, is far too high a 
> price to pay.

Since I am obviously not getting it, could someone explain to me what
the modularity advantage is?

All I see is a function pointer, with a default value or overridden by
the architecture initialization, used to parametrize a module's
behavior.  That is the same niche as every existing member of the
gdbarch vector.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
  2004-02-15 18:09       ` Daniel Jacobowitz
@ 2004-02-15 19:49         ` Andrew Cagney
  2004-02-15 20:37           ` Daniel Jacobowitz
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Cagney @ 2004-02-15 19:49 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Mark Kettenis, gdb-patches

> 
> Since I am obviously not getting it, could someone explain to me what
> the modularity advantage is?

Are you asking why modularity, in general, is advantage, or why here 
specifically this is more modula and hence an advantage?

The dwarf2-frame is able to locally, and opaquely (to other components) 
implement the per-architecture mechanisms that it needs.  No need to 
bloat that architecture vector with yet another global interface that 
nothing, other than dwarf2-frame requires.  No need to publish anything 
other than what is specificly relevant to dwarf2-frame's clients - the 
dwarf2 initialize routine.

> All I see is a function pointer, with a default value or overridden by
> the architecture initialization, used to parametrize a module's
> behavior.  That is the same niche as every existing member of the
> gdbarch vector.

Andrew



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
  2004-02-15 19:49         ` Andrew Cagney
@ 2004-02-15 20:37           ` Daniel Jacobowitz
  2004-02-15 21:37             ` Andrew Cagney
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Jacobowitz @ 2004-02-15 20:37 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: Mark Kettenis, gdb-patches

On Sun, Feb 15, 2004 at 02:49:16PM -0500, Andrew Cagney wrote:
> >
> >Since I am obviously not getting it, could someone explain to me what
> >the modularity advantage is?
> 
> Are you asking why modularity, in general, is advantage, or why here 
> specifically this is more modula and hence an advantage?

The latter, of course.  With a nod towards the former, see below.

> The dwarf2-frame is able to locally, and opaquely (to other components) 
> implement the per-architecture mechanisms that it needs.  No need to 
> bloat that architecture vector with yet another global interface that 
> nothing, other than dwarf2-frame requires.  No need to publish anything 
> other than what is specificly relevant to dwarf2-frame's clients - the 
> dwarf2 initialize routine.

This is what I don't understand.  Almost every architecture supported
by GDB will eventually support dwarf2-frame.  It is to the
architecture's advantage to supply this method, and in fact my
understanding was that many architectures will want to use it to
indicate which registers are considered call-clobbered and thus no
longer available (barring the issue of optimizations which change
calling convention).  It's no more a marginal method than (to pick an
example at random) PC_IN_SIGTRAMP.

Why is this different?  Are you saying that it would be preferable for
many of the methods currently in the architecture vector to move
outside of it into more modular pieces, e.g. function calling,
type-related, et cetera?  When you have that many pieces, I'm not sure
that you've gained any clarity.

So the architecture initialization functions will change into a lot of
set_dwarf2_frame_foo_func, set_infcall_bar_func instead of
set_gdbarch_foo_func and set_infcall_bar_func.

> 
> >All I see is a function pointer, with a default value or overridden by
> >the architecture initialization, used to parametrize a module's
> >behavior.  That is the same niche as every existing member of the
> >gdbarch vector.
> 
> Andrew
> 
> 
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
  2004-02-15 16:04     ` Andrew Cagney
  2004-02-15 18:09       ` Daniel Jacobowitz
@ 2004-02-15 21:31       ` Mark Kettenis
  1 sibling, 0 replies; 20+ messages in thread
From: Mark Kettenis @ 2004-02-15 21:31 UTC (permalink / raw)
  To: cagney; +Cc: drow, gdb-patches

Thanks for the info.  Based on what you wrote, I committed the
attached patch.  It should now be possible to integrate the IBM S/390
and zSeries stuff.

Mark


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* dwarf2-frame.h (dwarf2_frame_set_init_reg): New prototype.
	* dwarf2-frame.c (dwarf2_frame_data): New variable.
	(struct dwarf2_frame_ops): New.
	(dwarf2_frame_default_init_reg): New function, based on
	dwarf2_frame_init_reg.
	(dwarf2_frame_init, dwarf2_frame_set_init_reg): New function.
	(dwarf2_frame_init_reg): Call architecture-specific function.
	(dwarf2_frame_objfile_data): Renamed from dwarf2_frame_data.
	(dwarf2_frame_find_fde, add_fde): Use dwarf2_frame_objfile_data
	instead of dwarf2_frame_data.
	(_initialize_dwarf2_frame): Initailize new dwarf2_frame_data.
	Initialize dwarf2_frame_objfile instead of old dwarf2_frame_data.

Index: dwarf2-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2-frame.c,v
retrieving revision 1.29
diff -u -p -r1.29 dwarf2-frame.c
--- dwarf2-frame.c 7 Feb 2004 18:29:53 -0000 1.29
+++ dwarf2-frame.c 15 Feb 2004 21:27:19 -0000
@@ -454,53 +454,107 @@ execute_cfa_program (unsigned char *insn
   dwarf2_frame_state_free_regs (fs->regs.prev);
   fs->regs.prev = NULL;
 }
+\f
 
-struct dwarf2_frame_cache
-{
-  /* DWARF Call Frame Address.  */
-  CORE_ADDR cfa;
+/* Architecture-specific operations.  */
 
-  /* Saved registers, indexed by GDB register number, not by DWARF
-     register number.  */
-  struct dwarf2_frame_state_reg *reg;
+/* Per-architecture data key.  */
+static struct gdbarch_data *dwarf2_frame_data;
+
+struct dwarf2_frame_ops
+{
+  /* Pre-initialize the register state REG for register REGNUM.  */
+  void (*init_reg) (struct gdbarch *, int, struct dwarf2_frame_state_reg *);
 };
 
-/* Initialize the register state REG.  If we have a register that acts
-   as a program counter, mark it as a destination for the return
-   address.  If we have a register that serves as the stack pointer,
-   arrange for it to be filled with the call frame address (CFA).  The
-   other registers are marked as unspecified.
-
-   We copy the return address to the program counter, since many parts
-   in GDB assume that it is possible to get the return address by
-   unwind the program counter register.  However, on ISA's with a
-   dedicated return address register, the CFI usually only contains
-   information to unwind that return address register.
-
-   The reason we're treating the stack pointer special here is because
-   in many cases GCC doesn't emit CFI for the stack pointer and
-   implicitly assumes that it is equal to the CFA.  This makes some
-   sense since the DWARF specification (version 3, draft 8, p. 102)
-   says that:
-
-   "Typically, the CFA is defined to be the value of the stack pointer
-   at the call site in the previous frame (which may be different from
-   its value on entry to the current frame)."
-
-   However, this isn't true for all platforms supported by GCC
-   (e.g. IBM S/390 and zSeries).  For those targets we should override
-   the defaults given here.  */
+/* Default architecture-specific register state initialization
+   function.  */
 
 static void
-dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
-		       struct dwarf2_frame_state_reg *reg)
+dwarf2_frame_default_init_reg (struct gdbarch *gdbarch, int regnum,
+			       struct dwarf2_frame_state_reg *reg)
 {
+  /* If we have a register that acts as a program counter, mark it as
+     a destination for the return address.  If we have a register that
+     serves as the stack pointer, arrange for it to be filled with the
+     call frame address (CFA).  The other registers are marked as
+     unspecified.
+
+     We copy the return address to the program counter, since many
+     parts in GDB assume that it is possible to get the return address
+     by unwinding the program counter register.  However, on ISA's
+     with a dedicated return address register, the CFI usually only
+     contains information to unwind that return address register.
+
+     The reason we're treating the stack pointer special here is
+     because in many cases GCC doesn't emit CFI for the stack pointer
+     and implicitly assumes that it is equal to the CFA.  This makes
+     some sense since the DWARF specification (version 3, draft 8,
+     p. 102) says that:
+
+     "Typically, the CFA is defined to be the value of the stack
+     pointer at the call site in the previous frame (which may be
+     different from its value on entry to the current frame)."
+
+     However, this isn't true for all platforms supported by GCC
+     (e.g. IBM S/390 and zSeries).  Those architectures should provide
+     their own architecture-specific initialization function.  */
+
   if (regnum == PC_REGNUM)
     reg->how = DWARF2_FRAME_REG_RA;
   else if (regnum == SP_REGNUM)
     reg->how = DWARF2_FRAME_REG_CFA;
 }
 
+/* Return a default for the architecture-specific operations.  */
+
+static void *
+dwarf2_frame_init (struct gdbarch *gdbarch)
+{
+  struct dwarf2_frame_ops *ops;
+  
+  ops = GDBARCH_OBSTACK_ZALLOC (gdbarch, struct dwarf2_frame_ops);
+  ops->init_reg = dwarf2_frame_default_init_reg;
+  return ops;
+}
+
+/* Set the architecture-specific register state initialization
+   function for GDBARCH to INIT_REG.  */
+
+void
+dwarf2_frame_set_init_reg (struct gdbarch *gdbarch,
+			   void (*init_reg) (struct gdbarch *, int,
+					     struct dwarf2_frame_state_reg *))
+{
+  struct dwarf2_frame_ops *ops;
+
+  ops = gdbarch_data (gdbarch, dwarf2_frame_data);
+  ops->init_reg = init_reg;
+}
+
+/* Pre-initialize the register state REG for register REGNUM.  */
+
+static void
+dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
+		       struct dwarf2_frame_state_reg *reg)
+{
+  struct dwarf2_frame_ops *ops;
+
+  ops = gdbarch_data (gdbarch, dwarf2_frame_data);
+  ops->init_reg (gdbarch, regnum, reg);
+}
+\f
+
+struct dwarf2_frame_cache
+{
+  /* DWARF Call Frame Address.  */
+  CORE_ADDR cfa;
+
+  /* Saved registers, indexed by GDB register number, not by DWARF
+     register number.  */
+  struct dwarf2_frame_state_reg *reg;
+};
+
 static struct dwarf2_frame_cache *
 dwarf2_frame_cache (struct frame_info *next_frame, void **this_cache)
 {
@@ -851,7 +905,7 @@ struct comp_unit
   bfd_vma tbase;
 };
 
-const struct objfile_data *dwarf2_frame_data;
+const struct objfile_data *dwarf2_frame_objfile_data;
 
 static unsigned int
 read_1_byte (bfd *bfd, char *buf)
@@ -1111,7 +1165,7 @@ dwarf2_frame_find_fde (CORE_ADDR *pc)
       struct dwarf2_fde *fde;
       CORE_ADDR offset;
 
-      fde = objfile_data (objfile, dwarf2_frame_data);
+      fde = objfile_data (objfile, dwarf2_frame_objfile_data);
       if (fde == NULL)
 	continue;
 
@@ -1137,8 +1191,8 @@ dwarf2_frame_find_fde (CORE_ADDR *pc)
 static void
 add_fde (struct comp_unit *unit, struct dwarf2_fde *fde)
 {
-  fde->next = objfile_data (unit->objfile, dwarf2_frame_data);
-  set_objfile_data (unit->objfile, dwarf2_frame_data, fde);
+  fde->next = objfile_data (unit->objfile, dwarf2_frame_objfile_data);
+  set_objfile_data (unit->objfile, dwarf2_frame_objfile_data, fde);
 }
 
 #ifdef CC_HAS_LONG_LONG
@@ -1461,7 +1515,6 @@ decode_frame_entry (struct comp_unit *un
 
   return ret;
 }
-
 \f
 
 /* FIXME: kettenis/20030504: This still needs to be integrated with
@@ -1541,5 +1594,6 @@ void _initialize_dwarf2_frame (void);
 void
 _initialize_dwarf2_frame (void)
 {
-  dwarf2_frame_data = register_objfile_data ();
+  dwarf2_frame_data = register_gdbarch_data (dwarf2_frame_init);
+  dwarf2_frame_objfile_data = register_objfile_data ();
 }
Index: dwarf2-frame.h
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2-frame.h,v
retrieving revision 1.4
diff -u -p -r1.4 dwarf2-frame.h
--- dwarf2-frame.h 7 Feb 2004 14:44:50 -0000 1.4
+++ dwarf2-frame.h 15 Feb 2004 21:27:19 -0000
@@ -71,6 +71,13 @@ struct dwarf2_frame_state_reg
   enum dwarf2_frame_reg_rule how;
 };
 
+/* Set the architecture-specific register state initialization
+   function for GDBARCH to INIT_REG.  */
+
+extern void dwarf2_frame_set_init_reg (struct gdbarch *gdbarch,
+				       void (*init_reg) (struct gdbarch *, int,
+					     struct dwarf2_frame_state_reg *));
+
 /* Return the frame unwind methods for the function that contains PC,
    or NULL if it can't be handled by DWARF CFI frame unwinder.  */
 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
  2004-02-15 20:37           ` Daniel Jacobowitz
@ 2004-02-15 21:37             ` Andrew Cagney
  2004-02-15 22:54               ` Daniel Jacobowitz
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Cagney @ 2004-02-15 21:37 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Mark Kettenis, gdb-patches

> On Sun, Feb 15, 2004 at 02:49:16PM -0500, Andrew Cagney wrote:
> 
>> >
>> >Since I am obviously not getting it, could someone explain to me what
>> >the modularity advantage is?
> 
>> 
>> Are you asking why modularity, in general, is advantage, or why here 
>> specifically this is more modula and hence an advantage?
> 
> 
> The latter, of course.  With a nod towards the former, see below.
> 
> 
>> The dwarf2-frame is able to locally, and opaquely (to other components) 
>> implement the per-architecture mechanisms that it needs.  No need to 
>> bloat that architecture vector with yet another global interface that 
>> nothing, other than dwarf2-frame requires.  No need to publish anything 
>> other than what is specificly relevant to dwarf2-frame's clients - the 
>> dwarf2 initialize routine.
> 
> 
> This is what I don't understand.  Almost every architecture supported
> by GDB will eventually support dwarf2-frame.  It is to the
> architecture's advantage to supply this method, and in fact my
> understanding was that many architectures will want to use it to
> indicate which registers are considered call-clobbered and thus no
> longer available (barring the issue of optimizations which change
> calling convention).

But why should the architecture be supplying it to gdbarch, when it is 
the dwarf2 frame code that needs it?

 >  It's no more a marginal method than (to pick an
> example at random) PC_IN_SIGTRAMP.

Er: Use get_frame_type() == SIGTRAMP_FRAME instead of PC_IN_SIGTRAMP
http://sources.redhat.com/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gdb&pr=1159

> Why is this different?  Are you saying that it would be preferable for
> many of the methods currently in the architecture vector to move
> outside of it into more modular pieces, e.g. function calling,
> type-related, et cetera?  When you have that many pieces, I'm not sure
> that you've gained any clarity.

Um, actually, why is this case different?  Neither Mark nor I are doing 
anything new here.  frame-unwind, libunwind-frame, user-regs, 
frame-base, reggroups, regcache, v3abi, ... all, already use 
gdbarch_data.  The general trend of decomposing gdbarch into more 
digestable chunks has been going on for years.

While existing code continues to add to gdbarch.sh, new more modular 
code is using gdbarch_data.

> So the architecture initialization functions will change into a lot of
> set_dwarf2_frame_foo_func, set_infcall_bar_func instead of
> set_gdbarch_foo_func and set_infcall_bar_func.

MarkK is equally free to do something like replace

     frame-unwind::add_frame_sniffer
         (gdbarch, dwarf2-frame::dwarf2_frame_sniffer);

with:

     dwarf2-frame::add_dwarf2_frame_sniffer
         (gdbarch, <the arches optional CFI init function>);

In fact, Mark, I'd highly recommend it - it will screw down the 
interface further.

enjoy,
Andrew


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
  2004-02-15 21:37             ` Andrew Cagney
@ 2004-02-15 22:54               ` Daniel Jacobowitz
  0 siblings, 0 replies; 20+ messages in thread
From: Daniel Jacobowitz @ 2004-02-15 22:54 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: Mark Kettenis, gdb-patches

On Sun, Feb 15, 2004 at 04:36:50PM -0500, Andrew Cagney wrote:
> But why should the architecture be supplying it to gdbarch, when it is 
> the dwarf2 frame code that needs it?

By this explanation nothing belongs in gdbarch at all.  Gdbarch is not
a thing which can need other things.  It has producers and consumers
but is neither.

It sounds like we're moving away from gdbarch as the repository of
target-specific code in favor of each subsystem parametrizing itself. 
Is that a good summary?

> Um, actually, why is this case different?  Neither Mark nor I are doing 
> anything new here.  frame-unwind, libunwind-frame, user-regs, 
> frame-base, reggroups, regcache, v3abi, ... all, already use 
> gdbarch_data.  The general trend of decomposing gdbarch into more 
> digestable chunks has been going on for years.

Some of those at least are not good examples, because they are either
orthogonal to the OS/ABI - c++ ABI (v3abi) - or not one-to-one with the
ABI (frame unwind, frame base).  Whereas this method describes a
property of the ABI.  That's how it's different.

> While existing code continues to add to gdbarch.sh, new more modular 
> code is using gdbarch_data.

I guess the above is a good summary, then.  Should it be written down
somewhere?  I find the step from module-specific per-architecture data
(created by the module) to module-specific per-architecture
configuration hooks (provided, maybe optionally, by the architecture)
to be a bit confusing.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
  2004-02-18 16:59           ` Mark Kettenis
@ 2004-02-18 18:40             ` Andrew Cagney
  0 siblings, 0 replies; 20+ messages in thread
From: Andrew Cagney @ 2004-02-18 18:40 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: drow, weigand, gdb-patches

> Thanks Andrew.  Can't say I'm too happy with yet another function, but
> if that's the it's supposed to work...  Perhaps we should revisit the
> way these per-architecture keys work someday.

It's how it works.  Revising how it is supposed to work at some stage 
would be useful..

Andrew



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
  2004-02-16 20:55         ` Andrew Cagney
@ 2004-02-18 16:59           ` Mark Kettenis
  2004-02-18 18:40             ` Andrew Cagney
  0 siblings, 1 reply; 20+ messages in thread
From: Mark Kettenis @ 2004-02-18 16:59 UTC (permalink / raw)
  To: cagney; +Cc: cagney, drow, weigand, gdb-patches

   Date: Mon, 16 Feb 2004 15:55:52 -0500
   From: Andrew Cagney <cagney@gnu.org>

   > I've committed the attatched.  It follows the convention
   > described in that revised doco patch I posted:
   > http://sources.redhat.com/ml/gdb-patches/2004-02/msg00381.html
   > 
   > Andrew 

   with patch ...

   2004-02-16  Andrew Cagney  <cagney@redhat.com>

	   * dwarf2-frame.c (dwarf2_frame_ops): New function.
	   (dwarf2_frame_set_init_reg): Use, instead of gdbarch_data.
	   (dwarf2_frame_init_reg): Ditto.

Thanks Andrew.  Can't say I'm too happy with yet another function, but
if that's the it's supposed to work...  Perhaps we should revisit the
way these per-architecture keys work someday.

Mark


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
  2004-02-16 20:50       ` Andrew Cagney
@ 2004-02-16 20:55         ` Andrew Cagney
  2004-02-18 16:59           ` Mark Kettenis
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Cagney @ 2004-02-16 20:55 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: Daniel Jacobowitz, Ulrich Weigand, kettenis, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 673 bytes --]

> 
> Sorry, my analysis wasn't completely correct.  What actually happens
> is an init-order problem during the initialization of the gdbarch
> in initialize_current_architecture.  The problem is that I call
> dwarf2_frame_set_init_reg from within the s390_gdbarch_init routine
> (which I gather is the right place?), and at this point in time,
> the gdbarch_data call in dwarf2_frame_set_init_reg returns NULL
> because the gdbarch hasn't finished initialization yet:
> 
> I've committed the attatched.  It follows the convention described in that revised doco patch I posted:
> http://sources.redhat.com/ml/gdb-patches/2004-02/msg00381.html
> 
> Andrew 

with patch ...


[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 1408 bytes --]

2004-02-16  Andrew Cagney  <cagney@redhat.com>

	* dwarf2-frame.c (dwarf2_frame_ops): New function.
	(dwarf2_frame_set_init_reg): Use, instead of gdbarch_data.
	(dwarf2_frame_init_reg): Ditto.

Index: dwarf2-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2-frame.c,v
retrieving revision 1.30
diff -u -r1.30 dwarf2-frame.c
--- dwarf2-frame.c	15 Feb 2004 21:29:26 -0000	1.30
+++ dwarf2-frame.c	16 Feb 2004 20:28:01 -0000
@@ -518,6 +518,20 @@
   return ops;
 }
 
+static struct dwarf2_frame_ops *
+dwarf2_frame_ops (struct gdbarch *gdbarch)
+{
+  struct dwarf2_frame_ops *ops = gdbarch_data (gdbarch, dwarf2_frame_data);
+  if (ops == NULL)
+    {
+      /* ULGH, called during architecture initialization.  Patch
+         things up.  */
+      ops = dwarf2_frame_init (gdbarch);
+      set_gdbarch_data (gdbarch, dwarf2_frame_data, ops);
+    }
+  return ops;
+}
+
 /* Set the architecture-specific register state initialization
    function for GDBARCH to INIT_REG.  */
 
@@ -528,7 +542,7 @@
 {
   struct dwarf2_frame_ops *ops;
 
-  ops = gdbarch_data (gdbarch, dwarf2_frame_data);
+  ops = dwarf2_frame_ops (gdbarch);
   ops->init_reg = init_reg;
 }
 
@@ -540,7 +554,7 @@
 {
   struct dwarf2_frame_ops *ops;
 
-  ops = gdbarch_data (gdbarch, dwarf2_frame_data);
+  ops = dwarf2_frame_ops (gdbarch);
   ops->init_reg (gdbarch, regnum, reg);
 }
 \f

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
  2004-02-16 19:47     ` Daniel Jacobowitz
@ 2004-02-16 20:50       ` Andrew Cagney
  2004-02-16 20:55         ` Andrew Cagney
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Cagney @ 2004-02-16 20:50 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Ulrich Weigand, kettenis, gdb-patches


>> Sorry, my analysis wasn't completely correct.  What actually happens
>> is an init-order problem during the initialization of the gdbarch
>> in initialize_current_architecture.  The problem is that I call
>> dwarf2_frame_set_init_reg from within the s390_gdbarch_init routine
>> (which I gather is the right place?), and at this point in time,
>> the gdbarch_data call in dwarf2_frame_set_init_reg returns NULL
>> because the gdbarch hasn't finished initialization yet:

I've committed the attatched.  It follows the convention described in 
that revised doco patch I posted:
http://sources.redhat.com/ml/gdb-patches/2004-02/msg00381.html

Andrew

>> gdbarch_data (struct gdbarch *gdbarch, struct gdbarch_data *data)
>> {
>>   gdb_assert (data->index < gdbarch->nr_data);
>>   /* The data-pointer isn't initialized, call init() to get a value but
>>      only if the architecture initializaiton has completed.  Otherwise
>>      punt - hope that the caller knows what they are doing.  */
>>   if (gdbarch->data[data->index] == NULL
>>       && gdbarch->initialized_p)
> 
> 
> Oopsie.  Yes, now I can see what's going on - I'm not sure what should
> change here.  Andrew, do you recall if the initialized_p check was for
> any specific problem, or might we be able to remove it?
> 
> -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer 



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
  2004-02-16 13:01   ` Ulrich Weigand
@ 2004-02-16 19:47     ` Daniel Jacobowitz
  2004-02-16 20:50       ` Andrew Cagney
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Jacobowitz @ 2004-02-16 19:47 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: kettenis, gdb-patches, cagney

On Mon, Feb 16, 2004 at 02:01:29PM +0100, Ulrich Weigand wrote:
> Daniel Jacobowitz wrote:
> 
> > On Mon, Feb 16, 2004 at 02:27:24AM +0100, Ulrich Weigand wrote:
> > > +void
> > > +dwarf2_frame_set_init_reg (struct gdbarch *gdbarch,
> > > +                          void (*init_reg) (struct gdbarch *, int,
> > > +                                            struct dwarf2_frame_state_reg *))
> > 
> > > Unfortunately this now crashes on s390, because the 
> > > dwarf2_frame_init routine is called from within
> > > _initialize_dwarf2_frame, while the s390 backend calls
> > > dwarf2_frame_set_init_reg from within _initialize_s390_tdep.
> > 
> > Er, how?  You shouldn't have a gdbarch in _initialize_s390_tdep.  You
> > don't have one until s390_gdbarch_init.
> 
> Sorry, my analysis wasn't completely correct.  What actually happens
> is an init-order problem during the initialization of the gdbarch
> in initialize_current_architecture.  The problem is that I call
> dwarf2_frame_set_init_reg from within the s390_gdbarch_init routine
> (which I gather is the right place?), and at this point in time,
> the gdbarch_data call in dwarf2_frame_set_init_reg returns NULL
> because the gdbarch hasn't finished initialization yet:
> 
> gdbarch_data (struct gdbarch *gdbarch, struct gdbarch_data *data)
> {
>   gdb_assert (data->index < gdbarch->nr_data);
>   /* The data-pointer isn't initialized, call init() to get a value but
>      only if the architecture initializaiton has completed.  Otherwise
>      punt - hope that the caller knows what they are doing.  */
>   if (gdbarch->data[data->index] == NULL
>       && gdbarch->initialized_p)

Oopsie.  Yes, now I can see what's going on - I'm not sure what should
change here.  Andrew, do you recall if the initialized_p check was for
any specific problem, or might we be able to remove it?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
  2004-02-16  1:56 ` Daniel Jacobowitz
@ 2004-02-16 13:01   ` Ulrich Weigand
  2004-02-16 19:47     ` Daniel Jacobowitz
  0 siblings, 1 reply; 20+ messages in thread
From: Ulrich Weigand @ 2004-02-16 13:01 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Ulrich Weigand, kettenis, gdb-patches, cagney

Daniel Jacobowitz wrote:

> On Mon, Feb 16, 2004 at 02:27:24AM +0100, Ulrich Weigand wrote:
> > +void
> > +dwarf2_frame_set_init_reg (struct gdbarch *gdbarch,
> > +                          void (*init_reg) (struct gdbarch *, int,
> > +                                            struct dwarf2_frame_state_reg *))
> 
> > Unfortunately this now crashes on s390, because the 
> > dwarf2_frame_init routine is called from within
> > _initialize_dwarf2_frame, while the s390 backend calls
> > dwarf2_frame_set_init_reg from within _initialize_s390_tdep.
> 
> Er, how?  You shouldn't have a gdbarch in _initialize_s390_tdep.  You
> don't have one until s390_gdbarch_init.

Sorry, my analysis wasn't completely correct.  What actually happens
is an init-order problem during the initialization of the gdbarch
in initialize_current_architecture.  The problem is that I call
dwarf2_frame_set_init_reg from within the s390_gdbarch_init routine
(which I gather is the right place?), and at this point in time,
the gdbarch_data call in dwarf2_frame_set_init_reg returns NULL
because the gdbarch hasn't finished initialization yet:

gdbarch_data (struct gdbarch *gdbarch, struct gdbarch_data *data)
{
  gdb_assert (data->index < gdbarch->nr_data);
  /* The data-pointer isn't initialized, call init() to get a value but
     only if the architecture initializaiton has completed.  Otherwise
     punt - hope that the caller knows what they are doing.  */
  if (gdbarch->data[data->index] == NULL
      && gdbarch->initialized_p)

Bye,
Ulrich

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
  2004-02-16  1:28 Ulrich Weigand
@ 2004-02-16  1:56 ` Daniel Jacobowitz
  2004-02-16 13:01   ` Ulrich Weigand
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Jacobowitz @ 2004-02-16  1:56 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: kettenis, gdb-patches, cagney

On Mon, Feb 16, 2004 at 02:27:24AM +0100, Ulrich Weigand wrote:
> +void
> +dwarf2_frame_set_init_reg (struct gdbarch *gdbarch,
> +                          void (*init_reg) (struct gdbarch *, int,
> +                                            struct dwarf2_frame_state_reg *))

> Unfortunately this now crashes on s390, because the 
> dwarf2_frame_init routine is called from within
> _initialize_dwarf2_frame, while the s390 backend calls
> dwarf2_frame_set_init_reg from within _initialize_s390_tdep.

Er, how?  You shouldn't have a gdbarch in _initialize_s390_tdep.  You
don't have one until s390_gdbarch_init.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks
@ 2004-02-16  1:28 Ulrich Weigand
  2004-02-16  1:56 ` Daniel Jacobowitz
  0 siblings, 1 reply; 20+ messages in thread
From: Ulrich Weigand @ 2004-02-16  1:28 UTC (permalink / raw)
  To: kettenis; +Cc: gdb-patches, cagney

Mark Kettenis wrote:

+/* Return a default for the architecture-specific operations.  */
+
+static void *
+dwarf2_frame_init (struct gdbarch *gdbarch)
+{
+  struct dwarf2_frame_ops *ops;
+  
+  ops = GDBARCH_OBSTACK_ZALLOC (gdbarch, struct dwarf2_frame_ops);
+  ops->init_reg = dwarf2_frame_default_init_reg;
+  return ops;
+}
+
+/* Set the architecture-specific register state initialization
+   function for GDBARCH to INIT_REG.  */
+
+void
+dwarf2_frame_set_init_reg (struct gdbarch *gdbarch,
+                          void (*init_reg) (struct gdbarch *, int,
+                                            struct dwarf2_frame_state_reg *))
+{
+  struct dwarf2_frame_ops *ops;
+
+  ops = gdbarch_data (gdbarch, dwarf2_frame_data);
+  ops->init_reg = init_reg;
+}

Unfortunately this now crashes on s390, because the 
dwarf2_frame_init routine is called from within
_initialize_dwarf2_frame, while the s390 backend calls
dwarf2_frame_set_init_reg from within _initialize_s390_tdep.

Now, in the generated initialize_all_files routine, the
s390_tdep init routine is called *before* the dwarf2_frame
one, and hence dwarf2_frame_set_init_reg gets called before
dwarf2_frame_init.

Thus, the gdbarch_data call returns NULL, and the assignment
to ops->init_reg crashes.

Is there some usual way to solve this sort of init-order
issues with the gdb init sequence?

Bye,
Ulrich

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2004-02-18 18:40 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-02-07 22:38 [PATCH/RFC] Per-architecture DWARF CFI register state initialization hooks Mark Kettenis
2004-02-07 23:03 ` Daniel Jacobowitz
2004-02-07 23:48 ` Andrew Cagney
2004-02-15 15:31   ` Mark Kettenis
2004-02-15 16:04     ` Andrew Cagney
2004-02-15 18:09       ` Daniel Jacobowitz
2004-02-15 19:49         ` Andrew Cagney
2004-02-15 20:37           ` Daniel Jacobowitz
2004-02-15 21:37             ` Andrew Cagney
2004-02-15 22:54               ` Daniel Jacobowitz
2004-02-15 21:31       ` Mark Kettenis
2004-02-08  1:01 ` Ulrich Weigand
2004-02-16  1:28 Ulrich Weigand
2004-02-16  1:56 ` Daniel Jacobowitz
2004-02-16 13:01   ` Ulrich Weigand
2004-02-16 19:47     ` Daniel Jacobowitz
2004-02-16 20:50       ` Andrew Cagney
2004-02-16 20:55         ` Andrew Cagney
2004-02-18 16:59           ` Mark Kettenis
2004-02-18 18:40             ` Andrew Cagney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox