From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32324 invoked by alias); 6 Mar 2015 19:49:35 -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 32302 invoked by uid 89); 6 Mar 2015 19:49:34 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 06 Mar 2015 19:49:33 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t26JnPVG028122 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Fri, 6 Mar 2015 14:49:26 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t26JnNh5016517; Fri, 6 Mar 2015 14:49:24 -0500 Message-ID: <54FA04C3.5030405@redhat.com> Date: Fri, 06 Mar 2015 19:49:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: John Baldwin , gdb-patches@sourceware.org Subject: Re: [PATCH] Use kinfo_getvmmap () on FreeBSD to enumerate memory regions. References: <1611123.gixn7rQRH9@ralph.baldwin.cx> In-Reply-To: <1611123.gixn7rQRH9@ralph.baldwin.cx> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-03/txt/msg00176.txt.bz2 Hi John, Looks good to me, with a few nits/suggestions below. On 02/26/2015 09:44 PM, John Baldwin wrote: > Use kinfo_getvmmap () from libutil on FreeBSD to enumerate memory > regions in a running process instead of /proc//map. FreeBSD systems > do not mount procfs by default, but kinfo_getvmmap () uses a sysctl that > is always available. > > Skip memory regions for devices as well as regions an application has > requested to not be dumped via the MAP_NOCORE flag to mmap () or > MADV_NOCORE advice to madvise (). Note that GNU's coding conventions tell us to refer to functions by name without the ()'s. > > gdb/ChangeLog: > > * configure.ac: AC_CHECK_LIB(util, kinfo_getvmmap). > * configure: Regenerate. > * config.in: Regenerate. > * fbsd-nat.c (fbsd_find_memory_regions): Use kinfo_getvmmap to > enumerate memory regions if present. * fbsd-nat.c [!HAVE_KINFO_GETVMMAP] (fbsd_read_mapping): Don't define. (fbsd_find_memory_regions): Use kinfo_getvmmap to enumerate memory regions if present. > > --- > gdb/configure.ac | 5 +++++ > gdb/fbsd-nat.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 61 insertions(+) > > diff --git a/gdb/configure.ac b/gdb/configure.ac > index 6ac8adb..b094164 100644 > --- a/gdb/configure.ac > +++ b/gdb/configure.ac > @@ -532,6 +532,11 @@ AM_ZLIB > # On HP/UX we may need libxpdl for dlgetmodinfo (used by solib-pa64.c). > AC_SEARCH_LIBS(dlgetmodinfo, [dl xpdl]) > > +# On FreeBSD we may need libutil for kinfo_getvmmap (used by fbsd-nat.c). > +AC_CHECK_LIB(util, kinfo_getvmmap, > + [AC_DEFINE(HAVE_KINFO_GETVMMAP, 1, > + [Define to 1 if your system has the kinfo_getvmmap function. ])]) > + Isn't AC_SEARCH_LIBS(kinfo_getvmmap, [util]) pretty much the same? (Note: please make sure to use pristine GNU autoconf 2.64 when generating the files, to avoid spurious odd differences coming out. Some distros carry local patches that result in that, dunno about FreeBSD.) > AM_ICONV > > # GDB may fork/exec the iconv program to get the list of supported character > diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c > index 062eede..0369a0a 100644 > --- a/gdb/fbsd-nat.c > +++ b/gdb/fbsd-nat.c > @@ -26,6 +26,10 @@ > #include > #include > #include > +#ifdef HAVE_KINFO_GETVMMAP > +#include > +#include > +#endif > > #include "elf-bfd.h" > #include "fbsd-nat.h" > @@ -62,6 +66,7 @@ fbsd_pid_to_exec_file (struct target_ops *self, int pid) > return NULL; > } > > +#ifndef HAVE_KINFO_GETVMMAP > static int > fbsd_read_mapping (FILE *mapfile, unsigned long *start, unsigned long *end, > char *protection) > @@ -82,6 +87,7 @@ fbsd_read_mapping (FILE *mapfile, unsigned long *start, unsigned long *end, > > return (ret != 0 && ret != EOF); > } > +#endif > > /* Iterate over all the memory regions in the current inferior, > calling FUNC for each memory region. OBFD is passed as the last > @@ -91,6 +97,55 @@ int > fbsd_find_memory_regions (struct target_ops *self, > find_memory_region_ftype func, void *obfd) > { > +#ifdef HAVE_KINFO_GETVMMAP > + pid_t pid = ptid_get_pid (inferior_ptid); > + struct kinfo_vmentry *vmentl, *kve; > + uint64_t size; > + struct cleanup *cleanup; > + int i, nitems; > + > + vmentl = kinfo_getvmmap (pid, &nitems); > + if (vmentl == NULL) > + perror_with_name (_("Couldn't fetch VM map entries.")); > + cleanup = make_cleanup (free, vmentl); s/free/xfree/g. > + > + for (i = 0; i < nitems; i++) > + { > + kve = &vmentl[i]; > + > + /* Skip unreadable segments and those where MAP_NOCORE has been set. */ > + if (!(kve->kve_protection & KVME_PROT_READ) || > + kve->kve_flags & KVME_FLAG_NOCOREDUMP) > + continue; || goes at the beginning of the other line. > + > + /* Skip segments with an invalid type. */ > + if (kve->kve_type != KVME_TYPE_DEFAULT && > + kve->kve_type != KVME_TYPE_VNODE && > + kve->kve_type != KVME_TYPE_SWAP && > + kve->kve_type != KVME_TYPE_PHYS) > + continue; Likewise &&'s here. > + > + size = kve->kve_end - kve->kve_start; > + if (info_verbose) > + { > + fprintf_filtered (gdb_stdout, > + "Save segment, %ld bytes at %s (%c%c%c)\n", > + (long)size, Space after cast. > + paddress (target_gdbarch (), kve->kve_start), > + kve->kve_protection & KVME_PROT_READ ? 'r' : '-', > + kve->kve_protection & KVME_PROT_WRITE ? 'w' : '-', > + kve->kve_protection & KVME_PROT_EXEC ? 'x' : '-'); > + } > + > + /* Invoke the callback function to create the corefile segment. > + Pass MODIFIED as true, we do not know the real modification state. */ > + func (kve->kve_start, size, kve->kve_protection & KVME_PROT_READ, > + kve->kve_protection & KVME_PROT_WRITE, > + kve->kve_protection & KVME_PROT_EXEC, 1, obfd); > + } > + do_cleanups (cleanup); > + return 0; > +#else > pid_t pid = ptid_get_pid (inferior_ptid); > char *mapfilename; > FILE *mapfile; > @@ -136,4 +191,5 @@ fbsd_find_memory_regions (struct target_ops *self, > > do_cleanups (cleanup); > return 0; > +#endif > } I'd suggest splitting fbsd_find_memory_regions in two instead of the big #if/#else/#endif, but that's just personal preference. Thanks, Pedro Alves