This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2 1/2] Introduce "gdb/configure.nat" (and delete "gdb/config/*/*.mh" files)
- From: Pedro Alves <palves at redhat dot com>
- To: Sergio Durigan Junior <sergiodj at redhat dot com>, GDB Patches <gdb-patches at sourceware dot org>
- Cc: Simon Marchi <simon dot marchi at polymtl dot ca>, John Baldwin <jhb at freebsd dot org>
- Date: Thu, 4 May 2017 17:16:35 +0100
- Subject: Re: [PATCH v2 1/2] Introduce "gdb/configure.nat" (and delete "gdb/config/*/*.mh" files)
- Authentication-results: sourceware.org; auth=none
- References: <20170425202309.15771-1-sergiodj@redhat.com> <20170503034931.4515-1-sergiodj@redhat.com> <20170503034931.4515-2-sergiodj@redhat.com>
On 05/03/2017 04:49 AM, Sergio Durigan Junior wrote:
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index b865b7c..e26079a 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -903,7 +903,16 @@ ALL_TARGET_OBS = \
> xtensa-tdep.o
>
> # Host-dependent makefile fragment comes in here.
Stale comment. Should say something about the variables being
defined in configure.nat.
> -@host_makefile_frag@
> +NAT_FILE = @NAT_FILE@
> +NATDEPFILES = @NATDEPFILES@
> +NAT_CDEPS = @NAT_CDEPS@
> +LOADLIBES = @LOADLIBES@
> +MH_CFLAGS = @MH_CFLAGS@
> +XM_CLIBS = @XM_CLIBS@
> +NAT_GENERATED_FILES = @NAT_GENERATED_FILES@
> +HAVE_NATIVE_GCORE_HOST = @HAVE_NATIVE_GCORE_HOST@
> +
> +@NAT_EXTRA_FRAGS_FILE@
And this last line here is where we source the
fragment, so that comment above should move here
with s/Host/Native-target/
I think it should be "FRAG", singular.
"makefile fragment file" is the term most often used.
Also, I think "host_makefile_frag" was lowercase because
that variable is not meant to be passed to make or any
tool make invokes.
> # End of host-dependent makefile fragment
Likewise, mind the comments.
>
> -# Copyright (C) 2013-2017 Free Software Foundation, Inc.
> -#
> diff --git a/gdb/config/i386/i386gnu.mh b/gdb/config/i386/i386gnu-extra.mh
> similarity index 58%
> rename from gdb/config/i386/i386gnu.mh
> rename to gdb/config/i386/i386gnu-extra.mh
Why the "extra" rename ? If anything, I'd expect i386gnu.mh -> i386gnu.mn?
> # Use our own user stubs for the msg rpcs, so we can make them time out, in
> # case the program is fucked, or we guess the wrong signal thread.
> -msg-MIGUFLAGS = -D'MSG_IMPORTS=waittime 1000;'
> +msg-MIGUFLAGS=-D'MSG_IMPORTS=waittime 1000;'
>
> # ick
> -MIGCOM = $(MIG) -cc cat - /dev/null
> +MIGCOM=$(MIG) -cc cat - /dev/null
Simon recently went through all variables making sure
there's a space around the "=". This goes in the other
direction.
>
> # Reply servers need special massaging of the code mig generates, to make
> # them work correctly for error returns in some cases.
> @@ -33,9 +21,4 @@ MIGCOM = $(MIG) -cc cat - /dev/null
> | $(MIGCOM) -sheader /dev/null -server /dev/null -user $*_U.c -header $*_U.h
>
> # MIG stubs are not yet ready for C++ compilation.
> -%_S.o %_U.o : COMPILE.post += -x c
> -
> -NAT_GENERATED_FILES = notify_S.h notify_S.c \
> - process_reply_S.h process_reply_S.c \
> - msg_reply_S.h msg_reply_S.c msg_U.h msg_U.c \
> - exc_request_U.h exc_request_U.c exc_request_S.h exc_request_S.c
> +%_S.o %_U.o : COMPILE.post +=-x c"
Was that quote char (") at the end added on purpose?
Looks like a typo.
Since most of the MIG related bits are left in the makefile
fragment, I wonder whether it really makes sense to move
the NAT_GENERATED_FILES setting far away from them.
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index 50f6f59..f9d8aac 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -2199,29 +2199,22 @@ if test "${host}" != "${target}"; then
> fi
> AC_SUBST(target_subdir)
>
> -frags=
> +# Importing nat definitions.
s/Importing/Import/
> +NAT_EXTRA_FRAGS_FILE=/dev/null
> if test "${gdb_native}" = "yes"; then
> - host_makefile_frag=${srcdir}/config/${gdb_host_cpu}/${gdb_host}.mh
> - if test ! -f ${host_makefile_frag}; then
> - AC_MSG_ERROR("*** Gdb does not support native target ${host}")
> - fi
> - frags="$frags $host_makefile_frag"
> -else
> - host_makefile_frag=/dev/null
> + . ${srcdir}/configure.nat
> + nativefile=$NAT_FILE
> fi
There's (at least) a related comment that would be good
to update:
# If nativefile (NAT_FILE) is not set in config/*/*.m[ht] files, we link
# to an empty version.
as NAT_FILE now comes from configure.nat, not the .mh files.
> +# Variables defined here:
> +#
> +# NAT_FILE - The header file with definitions for this native target.
> +#
> +# NATDEPFILES - Source files required for native debugging on this
> +# native target.
> +#
> +# NAT_CDEPS - Dynamic symbols to be exported for libthread_db.
> +#
> +# LOADLIBES - Libraries against which GDB will be linked for this
> +# native target.
> +#
> +# MH_CFLAGS - Additional CFLAGS for this native target.
At least MH_CFLAGS above is host-specific, not related to the
native target. See for example:
# doublest.c currently assumes some properties of FP arithmetic
# on the host which require this.
MH_CFLAGS = -mieee
# When compiled with cc, for debugging, this argument should be passed.
# We have no idea who our current compiler is though, so we skip it.
# MH_CFLAGS = -bnodelcsect
These are clearly always needed to build gdb on this host,
independently of whether native target is being built. I.e.,
when gdb is being built as a cross debugger, with
--target=something-else-not-the-host. Hence the MH_ prefix.
Some of those variables are ripe for a rename / normalization,
but that's something that can always be done after, and will
actually be simpler with your patchset in.
Thanks,
Pedro Alves