From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11096 invoked by alias); 10 Apr 2013 11:33:08 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 11086 invoked by uid 89); 10 Apr 2013 11:33:08 -0000 X-Spam-SWARE-Status: No, score=-8.9 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_OC autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Wed, 10 Apr 2013 11:33:07 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r3ABX6QL008302 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 10 Apr 2013 07:33:06 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r3ABX4ES014837; Wed, 10 Apr 2013 07:33:05 -0400 Message-ID: <51654DF0.3090101@redhat.com> Date: Wed, 10 Apr 2013 19:49:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 MIME-Version: 1.0 To: Jan Kratochvil CC: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [patch 2/2+rfc+doc] Install gcore by default (+new man page) References: <20130407185443.GB15389@host2.jankratochvil.net> <83r4ilawlx.fsf@gnu.org> <20130408172841.GA28868@host2.jankratochvil.net> <87hajgua2o.fsf@fleche.redhat.com> <516424FE.4050307@redhat.com> <20130409165525.GA29570@host2.jankratochvil.net> In-Reply-To: <20130409165525.GA29570@host2.jankratochvil.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-04/txt/msg00275.txt.bz2 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 Date: Mon Oct 26 18:30:39 2009 +0000 2009-10-26 Michael Snyder Hui Zhu * 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