This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch 2/2+rfc+doc] Install gcore by default (+new man page)
- From: Pedro Alves <palves at redhat dot com>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Cc: Tom Tromey <tromey at redhat dot com>, gdb-patches at sourceware dot org
- Date: Wed, 10 Apr 2013 12:33:04 +0100
- Subject: Re: [patch 2/2+rfc+doc] Install gcore by default (+new man page)
- References: <20130407185443 dot GB15389 at host2 dot jankratochvil dot net> <83r4ilawlx dot fsf at gnu dot org> <20130408172841 dot GA28868 at host2 dot jankratochvil dot net> <87hajgua2o dot fsf at fleche dot redhat dot com> <516424FE dot 4050307 at redhat dot com> <20130409165525 dot GA29570 at host2 dot jankratochvil dot net>
On 04/09/2013 05:55 PM, Jan Kratochvil wrote:
> On Tue, 09 Apr 2013 16:26:06 +0200, Pedro Alves wrote:
>> - Should we install it on hosts/builds that don't support gcore
>> with the native target?
>
> I did not consider it worth the work... but here it is.
>
> I have found these files support the legacy to_find_memory_regions way
> amd64fbsd-nat gnu-nat i386fbsd-nat ppcfbsd-nat procfs sparc64fbsd-nat
>
> so I have enabled fcore for any *.mh files using one of those.
>
> The new set_gdbarch_find_memory_regions way uses only linux-tdep so I have put
> that in configure.ac, otherwise there would be a new variable in each
> configure.tgt rule using linux-tdep (23 cases) which would be IMO more prone
> to a future mistake.
Right, but adds the complication of having two ways of doing things.
It made the patch surprisingly harder to read to me than I was expecting, for
instance (e.g., I had to try it to convince myself that
"--host=x86_64-unknown-linux-gnu --target=arm-linux-gnu" or
"--host=x86_64-unknown-linux-gnu --target=arm-linux-gnu --enable-targest=x86_64-unknown-linux-gnu"
do the right thing).
'grep "^supply_gregset " *' gives a good approximation (if not exact) of
targets that do cores. I wonder whether instead adding a HAVE_NATIVE_GCORE(_HOST)
to each .mh corresponding to each of those wouldn't be easier on the long run.
It's very similar to what we used to do before:
commit c1180e35dc58a8ce9bb076eaeff249c83423f1bf
Author: Hui Zhu <teawater@gmail.com>
Date: Mon Oct 26 18:30:39 2009 +0000
2009-10-26 Michael Snyder <msnyder@vmware.com>
Hui Zhu <teawater@gmail.com>
* Makefile.in (SFILES): Add gcore.c.
(COMMON_OBS): Add gcore.o.
* config/alpha/alpha-linux.mh (NATDEPFILES): Delete gcore.o.
* config/alpha/fbsd.mh (NATDEPFILES): Ditto.
* config/arm/linux.mh (NATDEPFILES): Ditto.
* config/i386/fbsd.mh (NATDEPFILES): Ditto.
* config/i386/fbsd64.mh (NATDEPFILES): Ditto.
* config/i386/i386sol2.mh (NATDEPFILES): Ditto.
* config/i386/linux.mh (NATDEPFILES): Ditto.
* config/i386/linux64.mh (NATDEPFILES): Ditto.
* config/i386/sol2-64.mh (NATDEPFILES): Ditto.
* config/ia64/linux.mh (NATDEPFILES): Ditto.
* config/m32r/linux.mh (NATDEPFILES): Ditto.
* config/m68k/linux.mh (NATDEPFILES): Ditto.
* config/mips/linux.mh (NATDEPFILES): Ditto.
* config/pa/linux.mh (NATDEPFILES): Ditto.
* config/powerpc/linux.mh (NATDEPFILES): Ditto.
* config/powerpc/ppc64-linux.mh (NATDEPFILES): Ditto.
* config/s390/s390.mh (NATDEPFILES): Ditto.
* config/sparc/fbsd.mh (NATDEPFILES): Ditto.
* config/sparc/linux.mh (NATDEPFILES): Ditto.
* config/sparc/linux64.mh (NATDEPFILES): Ditto.
* config/sparc/sol2.mh (NATDEPFILES): Ditto.
* config/xtensa/linux.mh (NATDEPFILES): Ditto.
* target.c (dummy_find_memory_regions): Change output.
(dummy_make_corefile_notes): Ditto.
Except we'd set a flag, instead of adding gcore.o to NATDEPFILES.
> Contrary to my recent regression mishaps I have tested various make install
> cases this time (not out-of-src-tree but that is not applicable for this
> change).
With --enable-targets=all, I got:
$ make DESTDIR=/tmp/foo/ install
...
/usr/bin/install: cannot stat `gcore': No such file or directory
But I'm not sure that's related to the issue pointed out
above, or whether I hasn't applied the patch properly.
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -203,6 +203,7 @@ fi
>
> TARGET_OBS=
> all_targets=
> +HAVE_NATIVE_GCORE_TARGET=
>
> for targ_alias in `echo $target_alias $enable_targets | sed 's/,/ /g'`
> do
> @@ -236,6 +237,14 @@ do
> if test x${want64} = xfalse; then
> . ${srcdir}/../bfd/config.bfd
> fi
> +
> + # Check whether this target is native and supports gcore.
> + # Such target has to call set_gdbarch_find_memory_regions.
> + if test $gdb_native = yes -a "$targ_alias" = "$target_alias"; then
> + case " ${gdb_target_obs} " in
> + *" linux-tdep.o "*) HAVE_NATIVE_GCORE_TARGET=1 ;;
It's still better to push this into configure.tgt. E.g., keep the
if test line here, but have configure.tgt set $gdb_have_gcore, and
set HAVE_NATIVE_GCORE_TARGET accordingly. We can still do that
case " ${gdb_target_obs} " in
*" linux-tdep.o "*)
bit in configure.tgt instead of touching every target switch case.
The main difference is that #1 the linux-tdep.o knowledge
is localized, and #2, changes to configure.tgt don't
require configure regeneration (thinking of future changes
to that bit for other targets).
--
Pedro Alves