From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 36439 invoked by alias); 6 Mar 2015 21:46:11 -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 36421 invoked by uid 89); 6 Mar 2015 21:46:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.2 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=no version=3.3.2 X-HELO: bigwig.baldwin.cx Received: from bigwig.baldwin.cx (HELO bigwig.baldwin.cx) (96.47.65.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Fri, 06 Mar 2015 21:46:08 +0000 Received: from ralph.baldwin.cx (pool-173-54-116-245.nwrknj.fios.verizon.net [173.54.116.245]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 26AB3B915; Fri, 6 Mar 2015 16:46:06 -0500 (EST) From: John Baldwin To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Use kinfo_getvmmap () on FreeBSD to enumerate memory regions. Date: Fri, 06 Mar 2015 21:46:00 -0000 Message-ID: <1897208.OU0R2xD82y@ralph.baldwin.cx> User-Agent: KMail/4.14.2 (FreeBSD/10.1-STABLE; KDE/4.14.2; amd64; ; ) In-Reply-To: <54FA04C3.5030405@redhat.com> References: <1611123.gixn7rQRH9@ralph.baldwin.cx> <54FA04C3.5030405@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-SW-Source: 2015-03/txt/msg00186.txt.bz2 On Friday, March 06, 2015 07:49:23 PM Pedro Alves wrote: > 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. Ok. > > 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. Ok. > > --- > > > > 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? Might be, I wasn't sure from reading the autoconf docs that this would add the appropriate define. I'll certainly try it and if it works I'm happier to use the simpler form. > (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.) Yes, I had to download my own copy to use when I tested this locally. > > + 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. I wasn't sure about this one. kinfo_getvmmap() calls malloc() from libc internally (so it isn't using xmalloc() to allocate the memory that is returned). Is it still correct to use xfree() instead of free() in that case? > @@ -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. Do you mean an #if/#else/#endif around the entire function vs just the body or do you mean something else? If the former, I can easily do that (and collapse down to on #if/#else/#endif with the prior conditionally-defined function). -- John Baldwin