This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Use kinfo_getvmmap () on FreeBSD to enumerate memory regions.
- From: Pedro Alves <palves at redhat dot com>
- To: John Baldwin <jhb at freebsd dot org>
- Cc: gdb-patches at sourceware dot org
- Date: Fri, 06 Mar 2015 22:41:19 +0000
- Subject: Re: [PATCH] Use kinfo_getvmmap () on FreeBSD to enumerate memory regions.
- Authentication-results: sourceware.org; auth=none
- References: <1611123 dot gixn7rQRH9 at ralph dot baldwin dot cx> <54FA04C3 dot 5030405 at redhat dot com> <1897208 dot OU0R2xD82y at ralph dot baldwin dot cx>
On 03/06/2015 09:06 PM, John Baldwin wrote:
> 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/<pid>/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?
Ah, I just saw bare "free" and didn't realize that it was kinfo_getvmmap
that malloc the memory. Yeah, free is correct then. Sorry for the noise.
>
> > @@ -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).
Yeah, that.
Thanks,
Pedro Alves