[RFC] Remove START_INFERIOR_TRAPS_EXPECTED macro from nm header

[RFC] Remove START_INFERIOR_TRAPS_EXPECTED macro from nm header

[Pierre Muller]

  This patch achieves the subject by
adding a new field to target_ops structure
called to_start_inferior_traps_expected.

  The changes are otherwise quite straightforward,
but once again, I have no way to even check that this
compiles for the different native GDB that it affects...

  Comments welcome,

Pierre Muller

2011-03-22  Pierre Muller  <muller@ics.u-strasbg.fr>

	Replace START_INFERIOR_TRAPS_EXPECTED macro by a new filed in
	target_ops struct named to_start_inferior_traps_expected.
	* config/alpha/alpha-osf3.mh: Set MH_CFLAGS to be able
	to set to_start_inferior_traps_expected correctly.
	config/alpha/nm-osf3.h: Remove START_INFERIOR_TRAPS_EXCEPTED macro.
	* target.h (struct target_ops): Add to_start_inferior_traps_expected
field.
	* target.c (update_current_target): Inherit field
	to_start_inferior_traps_expected for beneath and set default to 2.
	* alpha-nat.c (_initialize_alpha_nat): Set
	to_start_inferior_traps_expected to 3 if macro _STARTUP_INF_3_TRAPS.
	(darwin_ptrace_him): Use darwin_ops to_start_inferior_traps_expected
	field for startup_inferior parameter.
	* inf-ptrace.c (inf_ptrace_create_inferior): Adapt startup_inferior
	call using ops->toèstart_inferior_traps_expected.
	inf-ttrace.c (inf_ttrace_him): Ditto.
	* procfs.c (procfs_init_inferior): Ditto.

diff --git a/gdb/alpha-nat.c b/gdb/alpha-nat.c
index 1a55ea9..bb77035 100644
--- a/gdb/alpha-nat.c
+++ b/gdb/alpha-nat.c
@@ -197,6 +197,16 @@ _initialize_alpha_nat (void)
   struct target_ops *t;
 
   t = procfs_target ();
+
+  /* OSF-3 needs three traps.  */
+#ifdef _STARTUP_INF_3_TRAPS
+/* Number of traps that happen between exec'ing the shell
+   to run an inferior, and when we finally get to
+   the inferior code.  This is 2 on most implementations.  */
+/* was define START_INFERIOR_TRAPS_EXPECTED 3 */
+  t->to_start_inferior_traps_expected = 3;
+#endif
+
   add_target (t);
 
   deprecated_add_core_fns (&alpha_osf_core_fns);
diff --git a/gdb/config/alpha/alpha-osf3.mh b/gdb/config/alpha/alpha-osf3.mh
index 8dc6f80..1ef7195 100644
--- a/gdb/config/alpha/alpha-osf3.mh
+++ b/gdb/config/alpha/alpha-osf3.mh
@@ -1,5 +1,6 @@
 # Host: Little-endian Alpha running OSF/1-3.x and higher using procfs
 NAT_FILE= nm-osf3.h
+MH_CFLAGS=-D_STARTUP_INF_3_TRAPS
 NATDEPFILES= corelow.o alpha-nat.o fork-child.o \
 	solib-osf.o solib.o procfs.o proc-api.o proc-events.o proc-flags.o \
 	proc-why.o dec-thread.o
diff --git a/gdb/config/alpha/nm-osf3.h b/gdb/config/alpha/nm-osf3.h
index 626a497..56bf4d9 100644
--- a/gdb/config/alpha/nm-osf3.h
+++ b/gdb/config/alpha/nm-osf3.h
@@ -16,11 +16,6 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.
*/
 
-/* Number of traps that happen between exec'ing the shell
-   to run an inferior, and when we finally get to
-   the inferior code.  This is 2 on most implementations.  */
-#define START_INFERIOR_TRAPS_EXPECTED 3
-
 /* Don't trace faults under OSF/1, rely on the posting of the appropriate
    signal if fault tracing is disabled.
    Tracing T_IFAULT under Alpha OSF/1 causes a `floating point enable'
diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index 7be85d5..74c8b5a 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1485,7 +1485,7 @@ darwin_ptrace_him (int pid)
 
   darwin_init_thread_list (inf);
 
-  startup_inferior (START_INFERIOR_TRAPS_EXPECTED);
+  startup_inferior (darwin_ops->to_start_inferior_traps_expected);
 }
 
 static void
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index b5e1744..0a3f776 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -142,7 +142,7 @@ inf_ptrace_create_inferior (struct target_ops *ops,
   /* START_INFERIOR_TRAPS_EXPECTED is defined in inferior.h, and will
      be 1 or 2 depending on whether we're starting without or with a
      shell.  */
-  startup_inferior (START_INFERIOR_TRAPS_EXPECTED);
+  startup_inferior (ops->to_start_inferior_traps_expected);
 
   /* On some targets, there must be some explicit actions taken after
      the inferior has been started up.  */
diff --git a/gdb/inf-ttrace.c b/gdb/inf-ttrace.c
index ab075db..7b9ee2d 100644
--- a/gdb/inf-ttrace.c
+++ b/gdb/inf-ttrace.c
@@ -630,7 +630,7 @@ inf_ttrace_him (struct target_ops *ops, int pid)
   /* START_INFERIOR_TRAPS_EXPECTED is defined in inferior.h, and will
      be 1 or 2 depending on whether we're starting without or with a
      shell.  */
-  startup_inferior (START_INFERIOR_TRAPS_EXPECTED);
+  startup_inferior (ops->to_start_inferior_traps_expected);
 
   /* On some targets, there must be some explicit actions taken after
      the inferior has been started up.  */
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 5d7cb23..467af2c 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -4710,7 +4710,7 @@ procfs_init_inferior (struct target_ops *ops, int pid)
 
   /* Typically two, one trap to exec the shell, one to exec the
      program being debugged.  Defined by "inferior.h".  */
-  startup_inferior (START_INFERIOR_TRAPS_EXPECTED);
+  startup_inferior (ops->to_start_inferior_traps_expected);
 
 #ifdef SYS_syssgi
   /* On mips-irix, we need to stop the inferior early enough during
diff --git a/gdb/target.c b/gdb/target.c
index 45259fd..3ab7248 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -677,6 +677,7 @@ update_current_target (void)
       INHERIT (to_static_tracepoint_marker_at, t);
       INHERIT (to_static_tracepoint_markers_by_strid, t);
       INHERIT (to_traceframe_info, t);
+      INHERIT (to_start_inferior_traps_expected, t);
       INHERIT (to_magic, t);
       /* Do not inherit to_memory_map.  */
       /* Do not inherit to_flash_erase.  */
@@ -886,6 +887,7 @@ update_current_target (void)
   de_fault (to_traceframe_info,
 	    (struct traceframe_info * (*) (void))
 	    tcomplain);
+  de_fault (to_start_inferior_traps_expected, 2);
 #undef de_fault
 
   /* Finally, position the target-stack beneath the squashed
diff --git a/gdb/target.h b/gdb/target.h
index 237d1aa..9842a0a 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -743,6 +743,9 @@ struct target_ops
        re-fetching when necessary.  */
     struct traceframe_info *(*to_traceframe_info) (void);
 
+    /* Number of traps expected at startup of target.  */
+    int to_start_inferior_traps_expected;
+
     int to_magic;
     /* Need sub-structure for target machine related rather than comm
related?
      */

Re: [RFC] Remove START_INFERIOR_TRAPS_EXPECTED macro from nm header

[Pedro Alves]

Why's this any better?  I claim that it's worse.
START_INFERIOR_TRAPS_EXPECTED isn't ever used by anything
outside of the native target backend implementation ---
it doesn't qualify in the set of macros that are evil for
a multi-arch gdb.  Then, nothing in common code _should_
know about how many internal traps are expected 
during startup_inferior, but you've just exposed that
detail with to_start_inferior_traps_expected.

-- 
Pedro Alves

On Tuesday 22 March 2011 00:07:55, Pierre Muller wrote:
>   This patch achieves the subject by
> adding a new field to target_ops structure
> called to_start_inferior_traps_expected.
> 
>   The changes are otherwise quite straightforward,
> but once again, I have no way to even check that this
> compiles for the different native GDB that it affects...
> 
>   Comments welcome,
> 
> Pierre Muller
> 
> 2011-03-22  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
> 	Replace START_INFERIOR_TRAPS_EXPECTED macro by a new filed in
> 	target_ops struct named to_start_inferior_traps_expected.
> 	* config/alpha/alpha-osf3.mh: Set MH_CFLAGS to be able
> 	to set to_start_inferior_traps_expected correctly.
> 	config/alpha/nm-osf3.h: Remove START_INFERIOR_TRAPS_EXCEPTED macro.
> 	* target.h (struct target_ops): Add to_start_inferior_traps_expected
> field.
> 	* target.c (update_current_target): Inherit field
> 	to_start_inferior_traps_expected for beneath and set default to 2.
> 	* alpha-nat.c (_initialize_alpha_nat): Set
> 	to_start_inferior_traps_expected to 3 if macro _STARTUP_INF_3_TRAPS.
> 	(darwin_ptrace_him): Use darwin_ops to_start_inferior_traps_expected
> 	field for startup_inferior parameter.
> 	* inf-ptrace.c (inf_ptrace_create_inferior): Adapt startup_inferior
> 	call using ops->toèstart_inferior_traps_expected.
> 	inf-ttrace.c (inf_ttrace_him): Ditto.
> 	* procfs.c (procfs_init_inferior): Ditto.
> 
> diff --git a/gdb/alpha-nat.c b/gdb/alpha-nat.c
> index 1a55ea9..bb77035 100644
> --- a/gdb/alpha-nat.c
> +++ b/gdb/alpha-nat.c
> @@ -197,6 +197,16 @@ _initialize_alpha_nat (void)
>    struct target_ops *t;
>  
>    t = procfs_target ();
> +
> +  /* OSF-3 needs three traps.  */
> +#ifdef _STARTUP_INF_3_TRAPS
> +/* Number of traps that happen between exec'ing the shell
> +   to run an inferior, and when we finally get to
> +   the inferior code.  This is 2 on most implementations.  */
> +/* was define START_INFERIOR_TRAPS_EXPECTED 3 */
> +  t->to_start_inferior_traps_expected = 3;
> +#endif
> +
>    add_target (t);
>  
>    deprecated_add_core_fns (&alpha_osf_core_fns);
> diff --git a/gdb/config/alpha/alpha-osf3.mh b/gdb/config/alpha/alpha-osf3.mh
> index 8dc6f80..1ef7195 100644
> --- a/gdb/config/alpha/alpha-osf3.mh
> +++ b/gdb/config/alpha/alpha-osf3.mh
> @@ -1,5 +1,6 @@
>  # Host: Little-endian Alpha running OSF/1-3.x and higher using procfs
>  NAT_FILE= nm-osf3.h
> +MH_CFLAGS=-D_STARTUP_INF_3_TRAPS
>  NATDEPFILES= corelow.o alpha-nat.o fork-child.o \
>  	solib-osf.o solib.o procfs.o proc-api.o proc-events.o proc-flags.o \
>  	proc-why.o dec-thread.o
> diff --git a/gdb/config/alpha/nm-osf3.h b/gdb/config/alpha/nm-osf3.h
> index 626a497..56bf4d9 100644
> --- a/gdb/config/alpha/nm-osf3.h
> +++ b/gdb/config/alpha/nm-osf3.h
> @@ -16,11 +16,6 @@
>     You should have received a copy of the GNU General Public License
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.
> */
>  
> -/* Number of traps that happen between exec'ing the shell
> -   to run an inferior, and when we finally get to
> -   the inferior code.  This is 2 on most implementations.  */
> -#define START_INFERIOR_TRAPS_EXPECTED 3
> -
>  /* Don't trace faults under OSF/1, rely on the posting of the appropriate
>     signal if fault tracing is disabled.
>     Tracing T_IFAULT under Alpha OSF/1 causes a `floating point enable'
> diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
> index 7be85d5..74c8b5a 100644
> --- a/gdb/darwin-nat.c
> +++ b/gdb/darwin-nat.c
> @@ -1485,7 +1485,7 @@ darwin_ptrace_him (int pid)
>  
>    darwin_init_thread_list (inf);
>  
> -  startup_inferior (START_INFERIOR_TRAPS_EXPECTED);
> +  startup_inferior (darwin_ops->to_start_inferior_traps_expected);
>  }
>  
>  static void
> diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
> index b5e1744..0a3f776 100644
> --- a/gdb/inf-ptrace.c
> +++ b/gdb/inf-ptrace.c
> @@ -142,7 +142,7 @@ inf_ptrace_create_inferior (struct target_ops *ops,
>    /* START_INFERIOR_TRAPS_EXPECTED is defined in inferior.h, and will
>       be 1 or 2 depending on whether we're starting without or with a
>       shell.  */
> -  startup_inferior (START_INFERIOR_TRAPS_EXPECTED);
> +  startup_inferior (ops->to_start_inferior_traps_expected);
>  
>    /* On some targets, there must be some explicit actions taken after
>       the inferior has been started up.  */
> diff --git a/gdb/inf-ttrace.c b/gdb/inf-ttrace.c
> index ab075db..7b9ee2d 100644
> --- a/gdb/inf-ttrace.c
> +++ b/gdb/inf-ttrace.c
> @@ -630,7 +630,7 @@ inf_ttrace_him (struct target_ops *ops, int pid)
>    /* START_INFERIOR_TRAPS_EXPECTED is defined in inferior.h, and will
>       be 1 or 2 depending on whether we're starting without or with a
>       shell.  */
> -  startup_inferior (START_INFERIOR_TRAPS_EXPECTED);
> +  startup_inferior (ops->to_start_inferior_traps_expected);
>  
>    /* On some targets, there must be some explicit actions taken after
>       the inferior has been started up.  */
> diff --git a/gdb/procfs.c b/gdb/procfs.c
> index 5d7cb23..467af2c 100644
> --- a/gdb/procfs.c
> +++ b/gdb/procfs.c
> @@ -4710,7 +4710,7 @@ procfs_init_inferior (struct target_ops *ops, int pid)
>  
>    /* Typically two, one trap to exec the shell, one to exec the
>       program being debugged.  Defined by "inferior.h".  */
> -  startup_inferior (START_INFERIOR_TRAPS_EXPECTED);
> +  startup_inferior (ops->to_start_inferior_traps_expected);
>  
>  #ifdef SYS_syssgi
>    /* On mips-irix, we need to stop the inferior early enough during
> diff --git a/gdb/target.c b/gdb/target.c
> index 45259fd..3ab7248 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -677,6 +677,7 @@ update_current_target (void)
>        INHERIT (to_static_tracepoint_marker_at, t);
>        INHERIT (to_static_tracepoint_markers_by_strid, t);
>        INHERIT (to_traceframe_info, t);
> +      INHERIT (to_start_inferior_traps_expected, t);
>        INHERIT (to_magic, t);
>        /* Do not inherit to_memory_map.  */
>        /* Do not inherit to_flash_erase.  */
> @@ -886,6 +887,7 @@ update_current_target (void)
>    de_fault (to_traceframe_info,
>  	    (struct traceframe_info * (*) (void))
>  	    tcomplain);
> +  de_fault (to_start_inferior_traps_expected, 2);
>  #undef de_fault
>  
>    /* Finally, position the target-stack beneath the squashed
> diff --git a/gdb/target.h b/gdb/target.h
> index 237d1aa..9842a0a 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -743,6 +743,9 @@ struct target_ops
>         re-fetching when necessary.  */
>      struct traceframe_info *(*to_traceframe_info) (void);
>  
> +    /* Number of traps expected at startup of target.  */
> +    int to_start_inferior_traps_expected;
> +
>      int to_magic;
>      /* Need sub-structure for target machine related rather than comm
> related?
>       */
> 
> 

RE: [RFC] Remove START_INFERIOR_TRAPS_EXPECTED macro from nm header

[Pierre Muller]

> -----Message d'origine-----
> De : Pedro Alves [mailto:pedro@codesourcery.com]
> Envoyé : mardi 22 mars 2011 10:34
> À : gdb-patches@sourceware.org
> Cc : Pierre Muller
> Objet : Re: [RFC] Remove START_INFERIOR_TRAPS_EXPECTED macro from nm
> header
> 
> Why's this any better?  I claim that it's worse.
> START_INFERIOR_TRAPS_EXPECTED isn't ever used by anything
> outside of the native target backend implementation ---
> it doesn't qualify in the set of macros that are evil for
> a multi-arch gdb.  Then, nothing in common code _should_
> know about how many internal traps are expected
> during startup_inferior, but you've just exposed that
> detail with to_start_inferior_traps_expected.

  That shows that I still didn't understand what target_ops really is:

Is there only one single target_ops that this
only for the native configuration?
 If this is the case, I agree with your analysis, but
I am still not sure about that...

Pierre

Re: [RFC] Remove START_INFERIOR_TRAPS_EXPECTED macro from nm header

[Pedro Alves]

On Tuesday 22 March 2011 09:37:41, Pierre Muller wrote:
> Is there only one single target_ops that this
> only for the native configuration?

Yes.  You can't compile a gdb that natively runs/debugs
Windows and Linux and HPUX and Solaris and ...  You only
get _one_ native debugger backend.

>  If this is the case, I agree with your analysis, but
> I am still not sure about that...

-- 
Pedro Alves

Re: [RFC] Remove START_INFERIOR_TRAPS_EXPECTED macro from nm header

[Mark Kettenis]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Tue, 22 Mar 2011 09:34:26 +0000
> 
> Why's this any better?  I claim that it's worse.
> START_INFERIOR_TRAPS_EXPECTED isn't ever used by anything
> outside of the native target backend implementation ---
> it doesn't qualify in the set of macros that are evil for
> a multi-arch gdb.  Then, nothing in common code _should_
> know about how many internal traps are expected 
> during startup_inferior, but you've just exposed that
> detail with to_start_inferior_traps_expected.

I have to agree with Pedro here.

If you really think that getting rid of the nm header files is
something that has to be done (I'm not convinced it is), then perhaps
you should simply add:

MH_CFLAGS=-DSTART_INFERIOR_TRAPS_EXPECTED=3

to the appropriate Makefile fragments.  I'm not convinced that is an
improvmenet though.

Re: [RFC] Remove START_INFERIOR_TRAPS_EXPECTED macro from nm header

[Pedro Alves]

On Tuesday 22 March 2011 10:23:33, Mark Kettenis wrote:
> If you really think that getting rid of the nm header files is
> something that has to be done (I'm not convinced it is), then perhaps
> you should simply add:
> 
> MH_CFLAGS=-DSTART_INFERIOR_TRAPS_EXPECTED=3
> 
> to the appropriate Makefile fragments.  I'm not convinced that is an
> improvmenet though.

Yeah, but note that the H in MH_CFLAGS stands for _host_ --- macros
like START_INFERIOR_TRAPS_EXPECTED make sense for the native backend,
but not for a cross debugger built on that same host, so that isn't
super correct either.

Pierre, the macros that are evil and that we want to get rid of, are
those that affect common code.  For example, SOLIB_ADD:

breakpoint.c:#ifdef SOLIB_ADD
breakpoint.c:      SOLIB_ADD (NULL, 0, &current_target, auto_solib_add);
breakpoint.c:         step over a dlopen call and SOLIB_ADD is resetting the
infcmd.c:#ifdef SOLIB_ADD
infcmd.c:      SOLIB_ADD (NULL, 0, target, auto_solib_add);
infrun.c:#ifdef SOLIB_ADD
infrun.c:         SOLIB_ADD (NULL, 0, &current_target, auto_solib_add);
infrun.c:#if defined(SOLIB_ADD)
solib-frv.c:     SOLIB_ADD.  SOLIB_ADD eventually calls this this function,

A GDB build that defines SOLIB_ADD (AIX is the only one) will be useless
and quite broken as a cross debugger (e.g., "target remote" to debug with
a Windows gdbserver), because SOLIB_ADD will expand to something that only
makes (some, but not much!) sense for the _native_ backend.

-- 
Pedro Alves