Re: [patch 2/2+rfc+doc] Install gcore by default (+new man page)

[Pedro Alves <palves@redhat.com>]

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