Generate NT_PROCSTAT_{AUXV,VMMAP} in FreeBSD coredumps

Message ID iz0c1bAaNjgAACgCF3B9wWvubTieWo_FAMspoBDj72Th_JjQgzLhxnkAApDxTVICOTRuGk3F1EFNJDl96yeYU6oFRqZToAbjqKR-b61opzQ=@emersion.fr
State New, archived
Headers

Commit Message

Simon Ser July 11, 2018, 12:35 p.m. UTC
  gcore generates NT_AUXV and NT_FILE notes for Linux targets. On
FreeBSD auxv is stored in a NT_PROCSTAT_AUXV section, file mappings
are stored in a NT_PROCSTAT_VMMAP and both are prefixed with the
struct size.

2018-07-11  Simon Ser  <contact@emersion.fr>
        * fbsd-tdep.c (fbsd_make_corefile_notes): write NT_PROCSTAT_AUXV
        and NT_PROCSTAT_VMMAP notes

---

This is an improvement of my v2 patch [1]. Thanks John for your review!

Changes from v2 to v3:
- Use NT_FREEBSD_PROCSTAT_* enums instead or re-defining these
- Simplify Elf_Auxinfo struct size expression
- Also write NT_PROCSTAT_VMMAP notes
- Directly use sysctl as this will allow to support more notes in
  the future (all of these work in the same way)
- Use gdb::unique_xmalloc_ptr()

[1]: https://sourceware.org/ml/gdb-patches/2018-07/msg00267.html

 gdb/fbsd-tdep.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)
  

Comments

John Baldwin July 11, 2018, 4:16 p.m. UTC | #1
On 7/11/18 5:35 AM, Simon Ser wrote:
> gcore generates NT_AUXV and NT_FILE notes for Linux targets. On
> FreeBSD auxv is stored in a NT_PROCSTAT_AUXV section, file mappings
> are stored in a NT_PROCSTAT_VMMAP and both are prefixed with the
> struct size.
> 
> 2018-07-11  Simon Ser  <contact@emersion.fr>
>         * fbsd-tdep.c (fbsd_make_corefile_notes): write NT_PROCSTAT_AUXV
>         and NT_PROCSTAT_VMMAP notes
> 
> ---
> 
> This is an improvement of my v2 patch [1]. Thanks John for your review!
> 
> Changes from v2 to v3:
> - Use NT_FREEBSD_PROCSTAT_* enums instead or re-defining these
> - Simplify Elf_Auxinfo struct size expression
> - Also write NT_PROCSTAT_VMMAP notes
> - Directly use sysctl as this will allow to support more notes in
>   the future (all of these work in the same way)
> - Use gdb::unique_xmalloc_ptr()
> 
> [1]: https://sourceware.org/ml/gdb-patches/2018-07/msg00267.html
> 
>  gdb/fbsd-tdep.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
> index 9cea0098..e335ee6f 100644
> --- a/gdb/fbsd-tdep.c
> +++ b/gdb/fbsd-tdep.c
> @@ -25,6 +25,9 @@
>  #include "regset.h"
>  #include "gdbthread.h"
>  #include "xml-syscall.h"
> +#include <sys/user.h>
> +#include <sys/sysctl.h>
> +#include <sys/types.h>
>  
>  #include "elf-bfd.h"
>  #include "fbsd-tdep.h"
> @@ -512,6 +515,32 @@ fbsd_corefile_thread (struct thread_info *info,
>       args->note_size, args->stop_signal);
>  }
>  
> +static gdb::unique_xmalloc_ptr<char>
> +procstat_sysctl (pid_t pid, int what, size_t structsz, size_t *sizep)
> +{
> +  int name[4];
> +  name[0] = CTL_KERN;
> +  name[1] = KERN_PROC;
> +  name[2] = what;
> +  name[3] = pid;
> +  size_t len = 0;
> +  if (sysctl (name, 4, NULL, &len, NULL, 0) == -1)
> +    return NULL;
> + 
> +  int structsize = structsz;
> +  gdb::unique_xmalloc_ptr<char> buf
> +    ((char *) xmalloc (sizeof (structsize) + len));
> +  if (buf == NULL)
> +    return NULL;
> +  memcpy (buf.get (), &structsize, sizeof (structsize));
> +  void *p = buf.get () + sizeof (structsize);
> +  if (sysctl (name, 4, p, &len, NULL, 0) == -1)
> +    return NULL;
> +
> +  *sizep = sizeof (structsize) + len;
> +  return buf;
> +}
> +

You can't use a sysctl like this in a tdep.c file.  fbsd-tdep.c runs on
any OS, (so for example you could be running gdb with a FreeBSD binary
on Linux or OS X against a debug server (gdbserver or lldb-server or some
such) running on a remote FreeBSD host (or VM) and use 'gcore' on the
debugging host to generate a local core file.

Native code that would only run on a FreeBSD host would live in fbsd-nat.c,
and when I have thought about handling other procstat notes in FreeBSD cores
I've mostly thought about adding some kind of hook that fbsd-tdep.c would
invoke to write extra core notes and setting that hook only for native
targets in fbsd-nat.c for the native gdbarchs.

Your previous patch for AUXV still works because the target_foo function
you called previously will talk to either the debug server or the
native target to fetch the AUXV data, so I think your previous patch
for NT_PROCSTAT_AUXV is a better approach for that note.
  
Simon Ser July 11, 2018, 4:39 p.m. UTC | #2
On July 11, 2018 5:16 PM, John Baldwin <jhb@FreeBSD.org> wrote:
> You can't use a sysctl like this in a tdep.c file. fbsd-tdep.c runs on
> any OS, (so for example you could be running gdb with a FreeBSD binary
> on Linux or OS X against a debug server (gdbserver or lldb-server or some
> such) running on a remote FreeBSD host (or VM) and use 'gcore' on the
> debugging host to generate a local core file.

Ah, that's what I feared when choosing this approach.

> Native code that would only run on a FreeBSD host would live in fbsd-nat.c,
> and when I have thought about handling other procstat notes in FreeBSD cores
> I've mostly thought about adding some kind of hook that fbsd-tdep.c would
> invoke to write extra core notes and setting that hook only for native
> targets in fbsd-nat.c for the native gdbarchs.
>
> Your previous patch for AUXV still works because the target_foo function
> you called previously will talk to either the debug server or the
> native target to fetch the AUXV data, so I think your previous patch
> for NT_PROCSTAT_AUXV is a better approach for that note.

Hmm, I need those other notes too. Would it be possible to add some
FreeBSD-specific TARGET_OBJECT_* constants? If not, could you explain
how one would add this hook?

Thanks,

Simon
  

Patch

diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index 9cea0098..e335ee6f 100644
--- a/gdb/fbsd-tdep.c
+++ b/gdb/fbsd-tdep.c
@@ -25,6 +25,9 @@ 
 #include "regset.h"
 #include "gdbthread.h"
 #include "xml-syscall.h"
+#include <sys/user.h>
+#include <sys/sysctl.h>
+#include <sys/types.h>
 
 #include "elf-bfd.h"
 #include "fbsd-tdep.h"
@@ -512,6 +515,32 @@  fbsd_corefile_thread (struct thread_info *info,
      args->note_size, args->stop_signal);
 }
 
+static gdb::unique_xmalloc_ptr<char>
+procstat_sysctl (pid_t pid, int what, size_t structsz, size_t *sizep)
+{
+  int name[4];
+  name[0] = CTL_KERN;
+  name[1] = KERN_PROC;
+  name[2] = what;
+  name[3] = pid;
+  size_t len = 0;
+  if (sysctl (name, 4, NULL, &len, NULL, 0) == -1)
+    return NULL;
+ 
+  int structsize = structsz;
+  gdb::unique_xmalloc_ptr<char> buf
+    ((char *) xmalloc (sizeof (structsize) + len));
+  if (buf == NULL)
+    return NULL;
+  memcpy (buf.get (), &structsize, sizeof (structsize));
+  void *p = buf.get () + sizeof (structsize);
+  if (sysctl (name, 4, p, &len, NULL, 0) == -1)
+    return NULL;
+
+  *sizep = sizeof (structsize) + len;
+  return buf;
+}
+
 /* Create appropriate note sections for a corefile, returning them in
    allocated memory.  */
 
@@ -586,6 +615,35 @@  fbsd_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
 
   note_data = thread_args.note_data;
 
+  pid_t pid = inferior_ptid.pid ();
+
+  /* Auxillary vector.  */
+  size_t auxinfo_size = gdbarch_addr_bit (gdbarch) / 4; /* Elf_Auxinfo */
+  size_t note_desc_size;
+  gdb::unique_xmalloc_ptr<char> note_desc;
+  note_desc = procstat_sysctl (pid, KERN_PROC_AUXV, auxinfo_size,
+                               &note_desc_size);
+  if (note_desc != NULL)
+    {
+      note_data = elfcore_write_note (obfd, note_data, note_size,
+                                      "FreeBSD", NT_FREEBSD_PROCSTAT_AUXV,
+                                      note_desc.get (), note_desc_size);
+      if (!note_data)
+        return NULL;
+    }
+
+  /* File mappings */
+  note_desc = procstat_sysctl (pid, KERN_PROC_VMMAP,
+                               sizeof(struct kinfo_vmentry), &note_desc_size);
+  if (note_desc != NULL)
+    {
+      note_data = elfcore_write_note (obfd, note_data, note_size,
+                                      "FreeBSD", NT_FREEBSD_PROCSTAT_VMMAP,
+                                      note_desc.get (), note_desc_size);
+      if (!note_data)
+        return NULL;
+    }
+
   return note_data;
 }