Use kinfo_getvmmap () on FreeBSD to enumerate memory regions.

  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 ().

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.

---
 gdb/configure.ac |  5 +++++
 gdb/fbsd-nat.c   | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)
  

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.

> 
> 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 <sys/types.h>
>  #include <sys/procfs.h>
>  #include <sys/sysctl.h>
> +#ifdef HAVE_KINFO_GETVMMAP
> +#include <sys/user.h>
> +#include <libutil.h>
> +#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
  

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?

 > @@ -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).
  

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