public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Generate NT_PROCSTAT_{AUXV,VMMAP} in FreeBSD coredumps
@ 2018-07-11 12:35 Simon Ser
  2018-07-11 16:16 ` John Baldwin
  2018-07-16 13:39 ` [PATCH v2] Generate NT_PROCSTAT_{AUXV,VMMAP,PS_STRINGS} " Simon Ser
  0 siblings, 2 replies; 14+ messages in thread
From: Simon Ser @ 2018-07-11 12:35 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Ser, jhb

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;
+}
+
 /* 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;
 }
 
-- 
2.18.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Generate NT_PROCSTAT_{AUXV,VMMAP} in FreeBSD coredumps
  2018-07-11 12:35 [PATCH] Generate NT_PROCSTAT_{AUXV,VMMAP} in FreeBSD coredumps Simon Ser
@ 2018-07-11 16:16 ` John Baldwin
  2018-07-11 16:40   ` Simon Ser
  2018-07-16 13:39 ` [PATCH v2] Generate NT_PROCSTAT_{AUXV,VMMAP,PS_STRINGS} " Simon Ser
  1 sibling, 1 reply; 14+ messages in thread
From: John Baldwin @ 2018-07-11 16:16 UTC (permalink / raw)
  To: Simon Ser, gdb-patches

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.

-- 
John Baldwin

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Generate NT_PROCSTAT_{AUXV,VMMAP} in FreeBSD coredumps
  2018-07-11 16:16 ` John Baldwin
@ 2018-07-11 16:40   ` Simon Ser
  0 siblings, 0 replies; 14+ messages in thread
From: Simon Ser @ 2018-07-11 16:40 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2] Generate NT_PROCSTAT_{AUXV,VMMAP,PS_STRINGS} in FreeBSD coredumps
  2018-07-11 12:35 [PATCH] Generate NT_PROCSTAT_{AUXV,VMMAP} in FreeBSD coredumps Simon Ser
  2018-07-11 16:16 ` John Baldwin
@ 2018-07-16 13:39 ` Simon Ser
  2018-07-24  7:53   ` Simon Ser
                     ` (2 more replies)
  1 sibling, 3 replies; 14+ messages in thread
From: Simon Ser @ 2018-07-16 13:39 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Ser, jhb

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-16  Simon Ser  <contact@emersion.fr>
        * target.h (enum target_object): add FreeBSD-specific
        TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
        * fbsd-nat.c (fbsd_nat_target::xfer_partial): add support for
        TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
        * fbsd-tdep.c (fbsd_make_corefile_notes): write NT_PROCSTAT_AUXV,
        NT_PROCSTAT_VMMAP and NT_PROCSTAT_PS_STRINGS notes
---

This patch uses a different approach than the previous one: it adds two
FreeBSD-specific target objects. I chose this approach because there
were already some platform-specific target objects (e.g. for Darwin).
This should fix the issue John pointed out.

 gdb/fbsd-nat.c  | 17 ++++++++++++++-
 gdb/fbsd-tdep.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/target.h    |  4 ++++
 3 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 115deac0..2d056676 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -711,17 +711,32 @@ fbsd_nat_target::xfer_partial (enum target_object object,
       }
 #endif
     case TARGET_OBJECT_AUXV:
+    case TARGET_OBJECT_FREEBSD_VMMAP:
+    case TARGET_OBJECT_FREEBSD_PS_STRINGS:
       {
 	gdb::byte_vector buf_storage;
 	gdb_byte *buf;
 	size_t buflen;
 	int mib[4];
 
+        int proc_target;
+        switch (object) {
+        case TARGET_OBJECT_AUXV:
+          proc_target = KERN_PROC_AUXV;
+          break;
+        case TARGET_OBJECT_FREEBSD_VMMAP:
+          proc_target = KERN_PROC_VMMAP;
+          break;
+        case TARGET_OBJECT_FREEBSD_PS_STRINGS:
+          proc_target = KERN_PROC_PS_STRINGS;
+          break;
+        }
+
 	if (writebuf != NULL)
 	  return TARGET_XFER_E_IO;
 	mib[0] = CTL_KERN;
 	mib[1] = KERN_PROC;
-	mib[2] = KERN_PROC_AUXV;
+	mib[2] = proc_target;
 	mib[3] = pid;
 	if (offset == 0)
 	  {
diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index 9cea0098..2ea76e66 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,20 @@ fbsd_corefile_thread (struct thread_info *info,
      args->note_size, args->stop_signal);
 }
 
+static gdb::optional<gdb::byte_vector>
+fbsd_make_note_desc (enum target_object object, uint32_t structsize)
+{
+  gdb::optional<gdb::byte_vector> buf =
+    target_read_alloc (current_top_target (), object, NULL);
+  if (!buf || buf->empty ())
+    return {};
+
+  gdb::byte_vector desc (sizeof (structsize) + buf->size ());
+  memcpy (desc.data (), &structsize, sizeof (structsize));
+  memcpy (desc.data () + sizeof (structsize), buf->data (), buf->size ());
+  return desc;
+}
+
 /* Create appropriate note sections for a corefile, returning them in
    allocated memory.  */
 
@@ -586,6 +603,46 @@ 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.  */
+  uint32_t structsize = gdbarch_addr_bit (gdbarch) / 4; /* Elf_Auxinfo  */
+  gdb::optional<gdb::byte_vector> note_desc =
+    fbsd_make_note_desc (TARGET_OBJECT_AUXV, structsize);
+  if (note_desc && !note_desc->empty ())
+    {
+      note_data = elfcore_write_note (obfd, note_data, note_size,
+                                      "FreeBSD", NT_FREEBSD_PROCSTAT_AUXV,
+                                      note_desc->data (), note_desc->size ());
+      if (!note_data)
+        return NULL;
+    }
+  
+  /* File mappings  */
+  structsize = 0x488; /* struct kinfo_vmentry  */
+  note_desc = fbsd_make_note_desc (TARGET_OBJECT_FREEBSD_VMMAP, structsize);
+  if (note_desc && !note_desc->empty ())
+    {
+      note_data = elfcore_write_note (obfd, note_data, note_size,
+                                      "FreeBSD", NT_FREEBSD_PROCSTAT_VMMAP,
+                                      note_desc->data (), note_desc->size ());
+      if (!note_data)
+        return NULL;
+    }
+
+  structsize = gdbarch_addr_bit (gdbarch) / 8; /* void *  */
+  note_desc =
+    fbsd_make_note_desc (TARGET_OBJECT_FREEBSD_PS_STRINGS, structsize);
+  if (note_desc && !note_desc->empty ())
+    {
+      note_data = elfcore_write_note (obfd, note_data, note_size,
+                                      "FreeBSD",
+                                      NT_FREEBSD_PROCSTAT_PSSTRINGS,
+                                      note_desc->data (), note_desc->size ());
+      if (!note_data)
+        return NULL;
+    }
+
   return note_data;
 }
 
diff --git a/gdb/target.h b/gdb/target.h
index 18c4a84c..83f1172c 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -203,6 +203,10 @@ enum target_object
      of the process ID of the process in question, in hexadecimal
      format.  */
   TARGET_OBJECT_EXEC_FILE,
+  /* FreeBSD file mappings  */
+  TARGET_OBJECT_FREEBSD_VMMAP,
+  /* FreeBSD process strings  */
+  TARGET_OBJECT_FREEBSD_PS_STRINGS,
   /* Possible future objects: TARGET_OBJECT_FILE, ...  */
 };
 
-- 
2.18.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] Generate NT_PROCSTAT_{AUXV,VMMAP,PS_STRINGS} in FreeBSD coredumps
  2018-07-16 13:39 ` [PATCH v2] Generate NT_PROCSTAT_{AUXV,VMMAP,PS_STRINGS} " Simon Ser
@ 2018-07-24  7:53   ` Simon Ser
  2018-08-01 16:33     ` Simon Ser
  2018-08-01 18:16   ` John Baldwin
  2018-08-21 14:45   ` [PATCH v3] " Simon Ser
  2 siblings, 1 reply; 14+ messages in thread
From: Simon Ser @ 2018-07-24  7:53 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Ser, jhb

Hi John,

Do you think this approach is acceptable?

Thanks,

Simon Ser

On July 16, 2018 2:38 PM, Simon Ser <contact@emersion.fr> 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-16  Simon Ser  <contact@emersion.fr>
>         * target.h (enum target_object): add FreeBSD-specific
>         TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
>         * fbsd-nat.c (fbsd_nat_target::xfer_partial): add support for
>         TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
>         * fbsd-tdep.c (fbsd_make_corefile_notes): write NT_PROCSTAT_AUXV,
>         NT_PROCSTAT_VMMAP and NT_PROCSTAT_PS_STRINGS notes
> ---
>
> This patch uses a different approach than the previous one: it adds two
> FreeBSD-specific target objects. I chose this approach because there
> were already some platform-specific target objects (e.g. for Darwin).
> This should fix the issue John pointed out.
>
>  gdb/fbsd-nat.c  | 17 ++++++++++++++-
>  gdb/fbsd-tdep.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
>  gdb/target.h    |  4 ++++
>  3 files changed, 77 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
> index 115deac0..2d056676 100644
> --- a/gdb/fbsd-nat.c
> +++ b/gdb/fbsd-nat.c
> @@ -711,17 +711,32 @@ fbsd_nat_target::xfer_partial (enum target_object object,
>        }
>  #endif
>      case TARGET_OBJECT_AUXV:
> +    case TARGET_OBJECT_FREEBSD_VMMAP:
> +    case TARGET_OBJECT_FREEBSD_PS_STRINGS:
>        {
>  	gdb::byte_vector buf_storage;
>  	gdb_byte *buf;
>  	size_t buflen;
>  	int mib[4];
>
> +        int proc_target;
> +        switch (object) {
> +        case TARGET_OBJECT_AUXV:
> +          proc_target = KERN_PROC_AUXV;
> +          break;
> +        case TARGET_OBJECT_FREEBSD_VMMAP:
> +          proc_target = KERN_PROC_VMMAP;
> +          break;
> +        case TARGET_OBJECT_FREEBSD_PS_STRINGS:
> +          proc_target = KERN_PROC_PS_STRINGS;
> +          break;
> +        }
> +
>  	if (writebuf != NULL)
>  	  return TARGET_XFER_E_IO;
>  	mib[0] = CTL_KERN;
>  	mib[1] = KERN_PROC;
> -	mib[2] = KERN_PROC_AUXV;
> +	mib[2] = proc_target;
>  	mib[3] = pid;
>  	if (offset == 0)
>  	  {
> diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
> index 9cea0098..2ea76e66 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,20 @@ fbsd_corefile_thread (struct thread_info *info,
>       args->note_size, args->stop_signal);
>  }
>
> +static gdb::optional<gdb::byte_vector>
> +fbsd_make_note_desc (enum target_object object, uint32_t structsize)
> +{
> +  gdb::optional<gdb::byte_vector> buf =
> +    target_read_alloc (current_top_target (), object, NULL);
> +  if (!buf || buf->empty ())
> +    return {};
> +
> +  gdb::byte_vector desc (sizeof (structsize) + buf->size ());
> +  memcpy (desc.data (), &structsize, sizeof (structsize));
> +  memcpy (desc.data () + sizeof (structsize), buf->data (), buf->size ());
> +  return desc;
> +}
> +
>  /* Create appropriate note sections for a corefile, returning them in
>     allocated memory.  */
>
> @@ -586,6 +603,46 @@ 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.  */
> +  uint32_t structsize = gdbarch_addr_bit (gdbarch) / 4; /* Elf_Auxinfo  */
> +  gdb::optional<gdb::byte_vector> note_desc =
> +    fbsd_make_note_desc (TARGET_OBJECT_AUXV, structsize);
> +  if (note_desc && !note_desc->empty ())
> +    {
> +      note_data = elfcore_write_note (obfd, note_data, note_size,
> +                                      "FreeBSD", NT_FREEBSD_PROCSTAT_AUXV,
> +                                      note_desc->data (), note_desc->size ());
> +      if (!note_data)
> +        return NULL;
> +    }
> +
> +  /* File mappings  */
> +  structsize = 0x488; /* struct kinfo_vmentry  */
> +  note_desc = fbsd_make_note_desc (TARGET_OBJECT_FREEBSD_VMMAP, structsize);
> +  if (note_desc && !note_desc->empty ())
> +    {
> +      note_data = elfcore_write_note (obfd, note_data, note_size,
> +                                      "FreeBSD", NT_FREEBSD_PROCSTAT_VMMAP,
> +                                      note_desc->data (), note_desc->size ());
> +      if (!note_data)
> +        return NULL;
> +    }
> +
> +  structsize = gdbarch_addr_bit (gdbarch) / 8; /* void *  */
> +  note_desc =
> +    fbsd_make_note_desc (TARGET_OBJECT_FREEBSD_PS_STRINGS, structsize);
> +  if (note_desc && !note_desc->empty ())
> +    {
> +      note_data = elfcore_write_note (obfd, note_data, note_size,
> +                                      "FreeBSD",
> +                                      NT_FREEBSD_PROCSTAT_PSSTRINGS,
> +                                      note_desc->data (), note_desc->size ());
> +      if (!note_data)
> +        return NULL;
> +    }
> +
>    return note_data;
>  }
>
> diff --git a/gdb/target.h b/gdb/target.h
> index 18c4a84c..83f1172c 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -203,6 +203,10 @@ enum target_object
>       of the process ID of the process in question, in hexadecimal
>       format.  */
>    TARGET_OBJECT_EXEC_FILE,
> +  /* FreeBSD file mappings  */
> +  TARGET_OBJECT_FREEBSD_VMMAP,
> +  /* FreeBSD process strings  */
> +  TARGET_OBJECT_FREEBSD_PS_STRINGS,
>    /* Possible future objects: TARGET_OBJECT_FILE, ...  */
>  };
>
> --
> 2.18.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] Generate NT_PROCSTAT_{AUXV,VMMAP,PS_STRINGS} in FreeBSD coredumps
  2018-07-24  7:53   ` Simon Ser
@ 2018-08-01 16:33     ` Simon Ser
  0 siblings, 0 replies; 14+ messages in thread
From: Simon Ser @ 2018-08-01 16:33 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Ser, jhb

Hi John,

What do you think of this patch?

Thanks!

Simon Ser

On July 24, 2018 8:17 AM, Simon Ser <contact@emersion.fr> wrote:
> Hi John,
>
> Do you think this approach is acceptable?
>
> Thanks,
>
> Simon Ser
>
> On July 16, 2018 2:38 PM, Simon Ser <contact@emersion.fr> 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-16  Simon Ser  <contact@emersion.fr>
> >         * target.h (enum target_object): add FreeBSD-specific
> >         TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
> >         * fbsd-nat.c (fbsd_nat_target::xfer_partial): add support for
> >         TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
> >         * fbsd-tdep.c (fbsd_make_corefile_notes): write NT_PROCSTAT_AUXV,
> >         NT_PROCSTAT_VMMAP and NT_PROCSTAT_PS_STRINGS notes
> > ---
> >
> > This patch uses a different approach than the previous one: it adds two
> > FreeBSD-specific target objects. I chose this approach because there
> > were already some platform-specific target objects (e.g. for Darwin).
> > This should fix the issue John pointed out.
> >
> >  gdb/fbsd-nat.c  | 17 ++++++++++++++-
> >  gdb/fbsd-tdep.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  gdb/target.h    |  4 ++++
> >  3 files changed, 77 insertions(+), 1 deletion(-)
> >
> > diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
> > index 115deac0..2d056676 100644
> > --- a/gdb/fbsd-nat.c
> > +++ b/gdb/fbsd-nat.c
> > @@ -711,17 +711,32 @@ fbsd_nat_target::xfer_partial (enum target_object object,
> >        }
> >  #endif
> >      case TARGET_OBJECT_AUXV:
> > +    case TARGET_OBJECT_FREEBSD_VMMAP:
> > +    case TARGET_OBJECT_FREEBSD_PS_STRINGS:
> >        {
> >  	gdb::byte_vector buf_storage;
> >  	gdb_byte *buf;
> >  	size_t buflen;
> >  	int mib[4];
> >
> > +        int proc_target;
> > +        switch (object) {
> > +        case TARGET_OBJECT_AUXV:
> > +          proc_target = KERN_PROC_AUXV;
> > +          break;
> > +        case TARGET_OBJECT_FREEBSD_VMMAP:
> > +          proc_target = KERN_PROC_VMMAP;
> > +          break;
> > +        case TARGET_OBJECT_FREEBSD_PS_STRINGS:
> > +          proc_target = KERN_PROC_PS_STRINGS;
> > +          break;
> > +        }
> > +
> >  	if (writebuf != NULL)
> >  	  return TARGET_XFER_E_IO;
> >  	mib[0] = CTL_KERN;
> >  	mib[1] = KERN_PROC;
> > -	mib[2] = KERN_PROC_AUXV;
> > +	mib[2] = proc_target;
> >  	mib[3] = pid;
> >  	if (offset == 0)
> >  	  {
> > diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
> > index 9cea0098..2ea76e66 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,20 @@ fbsd_corefile_thread (struct thread_info *info,
> >       args->note_size, args->stop_signal);
> >  }
> >
> > +static gdb::optional<gdb::byte_vector>
> > +fbsd_make_note_desc (enum target_object object, uint32_t structsize)
> > +{
> > +  gdb::optional<gdb::byte_vector> buf =
> > +    target_read_alloc (current_top_target (), object, NULL);
> > +  if (!buf || buf->empty ())
> > +    return {};
> > +
> > +  gdb::byte_vector desc (sizeof (structsize) + buf->size ());
> > +  memcpy (desc.data (), &structsize, sizeof (structsize));
> > +  memcpy (desc.data () + sizeof (structsize), buf->data (), buf->size ());
> > +  return desc;
> > +}
> > +
> >  /* Create appropriate note sections for a corefile, returning them in
> >     allocated memory.  */
> >
> > @@ -586,6 +603,46 @@ 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.  */
> > +  uint32_t structsize = gdbarch_addr_bit (gdbarch) / 4; /* Elf_Auxinfo  */
> > +  gdb::optional<gdb::byte_vector> note_desc =
> > +    fbsd_make_note_desc (TARGET_OBJECT_AUXV, structsize);
> > +  if (note_desc && !note_desc->empty ())
> > +    {
> > +      note_data = elfcore_write_note (obfd, note_data, note_size,
> > +                                      "FreeBSD", NT_FREEBSD_PROCSTAT_AUXV,
> > +                                      note_desc->data (), note_desc->size ());
> > +      if (!note_data)
> > +        return NULL;
> > +    }
> > +
> > +  /* File mappings  */
> > +  structsize = 0x488; /* struct kinfo_vmentry  */
> > +  note_desc = fbsd_make_note_desc (TARGET_OBJECT_FREEBSD_VMMAP, structsize);
> > +  if (note_desc && !note_desc->empty ())
> > +    {
> > +      note_data = elfcore_write_note (obfd, note_data, note_size,
> > +                                      "FreeBSD", NT_FREEBSD_PROCSTAT_VMMAP,
> > +                                      note_desc->data (), note_desc->size ());
> > +      if (!note_data)
> > +        return NULL;
> > +    }
> > +
> > +  structsize = gdbarch_addr_bit (gdbarch) / 8; /* void *  */
> > +  note_desc =
> > +    fbsd_make_note_desc (TARGET_OBJECT_FREEBSD_PS_STRINGS, structsize);
> > +  if (note_desc && !note_desc->empty ())
> > +    {
> > +      note_data = elfcore_write_note (obfd, note_data, note_size,
> > +                                      "FreeBSD",
> > +                                      NT_FREEBSD_PROCSTAT_PSSTRINGS,
> > +                                      note_desc->data (), note_desc->size ());
> > +      if (!note_data)
> > +        return NULL;
> > +    }
> > +
> >    return note_data;
> >  }
> >
> > diff --git a/gdb/target.h b/gdb/target.h
> > index 18c4a84c..83f1172c 100644
> > --- a/gdb/target.h
> > +++ b/gdb/target.h
> > @@ -203,6 +203,10 @@ enum target_object
> >       of the process ID of the process in question, in hexadecimal
> >       format.  */
> >    TARGET_OBJECT_EXEC_FILE,
> > +  /* FreeBSD file mappings  */
> > +  TARGET_OBJECT_FREEBSD_VMMAP,
> > +  /* FreeBSD process strings  */
> > +  TARGET_OBJECT_FREEBSD_PS_STRINGS,
> >    /* Possible future objects: TARGET_OBJECT_FILE, ...  */
> >  };
> >
> > --
> > 2.18.0

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] Generate NT_PROCSTAT_{AUXV,VMMAP,PS_STRINGS} in FreeBSD coredumps
  2018-07-16 13:39 ` [PATCH v2] Generate NT_PROCSTAT_{AUXV,VMMAP,PS_STRINGS} " Simon Ser
  2018-07-24  7:53   ` Simon Ser
@ 2018-08-01 18:16   ` John Baldwin
  2018-08-21 14:45   ` [PATCH v3] " Simon Ser
  2 siblings, 0 replies; 14+ messages in thread
From: John Baldwin @ 2018-08-01 18:16 UTC (permalink / raw)
  To: Simon Ser, gdb-patches

On 7/16/18 6:38 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-16  Simon Ser  <contact@emersion.fr>
>         * target.h (enum target_object): add FreeBSD-specific
>         TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
>         * fbsd-nat.c (fbsd_nat_target::xfer_partial): add support for
>         TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
>         * fbsd-tdep.c (fbsd_make_corefile_notes): write NT_PROCSTAT_AUXV,
>         NT_PROCSTAT_VMMAP and NT_PROCSTAT_PS_STRINGS notes
> ---
> 
> This patch uses a different approach than the previous one: it adds two
> FreeBSD-specific target objects. I chose this approach because there
> were already some platform-specific target objects (e.g. for Darwin).
> This should fix the issue John pointed out.
> 
>  gdb/fbsd-nat.c  | 17 ++++++++++++++-
>  gdb/fbsd-tdep.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
>  gdb/target.h    |  4 ++++
>  3 files changed, 77 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
> index 115deac0..2d056676 100644
> --- a/gdb/fbsd-nat.c
> +++ b/gdb/fbsd-nat.c
> @@ -711,17 +711,32 @@ fbsd_nat_target::xfer_partial (enum target_object object,
>        }
>  #endif
>      case TARGET_OBJECT_AUXV:
> +    case TARGET_OBJECT_FREEBSD_VMMAP:
> +    case TARGET_OBJECT_FREEBSD_PS_STRINGS:
>        {
>  	gdb::byte_vector buf_storage;
>  	gdb_byte *buf;
>  	size_t buflen;
>  	int mib[4];
>  
> +        int proc_target;
> +        switch (object) {
> +        case TARGET_OBJECT_AUXV:
> +          proc_target = KERN_PROC_AUXV;
> +          break;
> +        case TARGET_OBJECT_FREEBSD_VMMAP:
> +          proc_target = KERN_PROC_VMMAP;
> +          break;
> +        case TARGET_OBJECT_FREEBSD_PS_STRINGS:
> +          proc_target = KERN_PROC_PS_STRINGS;
> +          break;
> +        }
> +
>  	if (writebuf != NULL)
>  	  return TARGET_XFER_E_IO;
>  	mib[0] = CTL_KERN;
>  	mib[1] = KERN_PROC;
> -	mib[2] = KERN_PROC_AUXV;
> +	mib[2] = proc_target;
>  	mib[3] = pid;
>  	if (offset == 0)
>  	  {

This won't work correctly for VMMAP.  The sysctl returns a "packed" representation
whereas the core dump note uses an unpacked format.  src/lib/libutil/kinfo_getvmmap.c
in FreeBSD's source tree has an example of the unpacking.  This means that you will
have to use a separate case for VMMAP I think (and would be nice to use for FILE in
the future which has the same packing "feature")) where you read the sysctl into
a buffer and then unpack it into the destination.  You could size the temporary
buffer as 'offset + len' as the unpacking can only grow the buffer.

> diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
> index 9cea0098..2ea76e66 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,20 @@ fbsd_corefile_thread (struct thread_info *info,
>       args->note_size, args->stop_signal);
>  }
>  
> +static gdb::optional<gdb::byte_vector>
> +fbsd_make_note_desc (enum target_object object, uint32_t structsize)
> +{
> +  gdb::optional<gdb::byte_vector> buf =
> +    target_read_alloc (current_top_target (), object, NULL);
> +  if (!buf || buf->empty ())
> +    return {};
> +
> +  gdb::byte_vector desc (sizeof (structsize) + buf->size ());
> +  memcpy (desc.data (), &structsize, sizeof (structsize));
> +  memcpy (desc.data () + sizeof (structsize), buf->data (), buf->size ());
> +  return desc;
> +}
> +
>  /* Create appropriate note sections for a corefile, returning them in
>     allocated memory.  */
>  
> @@ -586,6 +603,46 @@ 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.  */

Typo here: Auxiliary.

> +  uint32_t structsize = gdbarch_addr_bit (gdbarch) / 4; /* Elf_Auxinfo  */

I think this should probably use gdbarch_pointer_bit rather than
gdbarch_addr_bit.

> +  gdb::optional<gdb::byte_vector> note_desc =
> +    fbsd_make_note_desc (TARGET_OBJECT_AUXV, structsize);
> +  if (note_desc && !note_desc->empty ())
> +    {
> +      note_data = elfcore_write_note (obfd, note_data, note_size,
> +                                      "FreeBSD", NT_FREEBSD_PROCSTAT_AUXV,
> +                                      note_desc->data (), note_desc->size ());
> +      if (!note_data)
> +        return NULL;
> +    }
> +  
> +  /* File mappings  */
> +  structsize = 0x488; /* struct kinfo_vmentry  */

A bare constant feels a bit gross.  I actually think it would be cleanest if
the TARGET_OBJECT_FREEBSD_* streams included the struct size in the data
stream just as the core dump notes do.  This provides flexibility in case
newer kernels extend the structure size in the future.

-- 
John Baldwin

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] Generate NT_PROCSTAT_{AUXV,VMMAP,PS_STRINGS} in FreeBSD coredumps
  2018-07-16 13:39 ` [PATCH v2] Generate NT_PROCSTAT_{AUXV,VMMAP,PS_STRINGS} " Simon Ser
  2018-07-24  7:53   ` Simon Ser
  2018-08-01 18:16   ` John Baldwin
@ 2018-08-21 14:45   ` Simon Ser
  2018-08-23 10:40     ` John Baldwin
  2018-08-23 14:02     ` [PATCH v4] " Simon Ser
  2 siblings, 2 replies; 14+ messages in thread
From: Simon Ser @ 2018-08-21 14:45 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Ser, jhb

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-08-21  Simon Ser  <contact@emersion.fr>
        * target.h (enum target_object): add FreeBSD-specific
        TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
        * fbsd-nat.c (fbsd_nat_target::xfer_partial): add support for
        TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
        * fbsd-tdep.c (fbsd_make_corefile_notes): write NT_PROCSTAT_AUXV,
        NT_PROCSTAT_VMMAP and NT_PROCSTAT_PS_STRINGS notes
---

Changes from v2 to v3:
- Remove constants, prepend struct size for FreeBSD-specific objects
- Use gdbarch_ptr_bit instead of gdbarch_addr_bit
- Fixed typo

This addresses all of John's comments except the VMMAP packed one.

John, you said coredumps use the unpacked format, but looking at the
`gcore` utility source code (in usr.bin/gcore/elfcore.c) I believe it
uses the packed format for coredumps. It also makes more sense to me
to use the packed format for coredumps because it allows us not to
store PATH_MAX zeros for each entry. I may be wrong though, let me know
what you think.

 gdb/fbsd-nat.c  | 50 ++++++++++++++++++++++++++++++++++++++++++
 gdb/fbsd-tdep.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/target.h    |  4 ++++
 3 files changed, 112 insertions(+)

diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 115deac0..7cd325e4 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -751,6 +751,56 @@ fbsd_nat_target::xfer_partial (enum target_object object,
 	  }
 	return TARGET_XFER_E_IO;
       }
+    case TARGET_OBJECT_FREEBSD_VMMAP:
+    case TARGET_OBJECT_FREEBSD_PS_STRINGS:
+      {
+	gdb::byte_vector buf_storage;
+	gdb_byte *buf;
+	size_t buflen;
+	int mib[4];
+
+        int proc_target;
+        uint32_t struct_size;
+        switch (object) {
+        case TARGET_OBJECT_FREEBSD_VMMAP:
+          proc_target = KERN_PROC_VMMAP;
+          struct_size = sizeof (struct kinfo_vmentry);
+          break;
+        case TARGET_OBJECT_FREEBSD_PS_STRINGS:
+          proc_target = KERN_PROC_PS_STRINGS;
+          struct_size = sizeof (void *);
+          break;
+        }
+
+	if (writebuf != NULL)
+	  return TARGET_XFER_E_IO;
+
+	mib[0] = CTL_KERN;
+	mib[1] = KERN_PROC;
+	mib[2] = proc_target;
+	mib[3] = pid;
+
+	buflen = sizeof (struct_size) + offset + len;
+	buf_storage.resize (buflen);
+	buf = buf_storage.data ();
+
+	memcpy (buf, &struct_size, sizeof (struct_size));
+
+	if (sysctl (mib, 4, buf + sizeof (struct_size), &buflen, NULL, 0) == 0)
+	  {
+	    buflen += sizeof (struct_size);
+	    if (buflen > offset)
+	      {
+		buflen -= offset;
+		memcpy (readbuf, buf + offset, buflen);
+	      }
+	    else
+	      buflen = 0;
+	    *xfered_len = buflen;
+	    return (buflen == 0) ? TARGET_XFER_EOF : TARGET_XFER_OK;
+	  }
+	return TARGET_XFER_E_IO;
+      }
     default:
       return inf_ptrace_target::xfer_partial (object, annex,
 					      readbuf, writebuf, offset,
diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index 9cea0098..659e64b6 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,23 @@ fbsd_corefile_thread (struct thread_info *info,
      args->note_size, args->stop_signal);
 }
 
+static gdb::optional<gdb::byte_vector>
+fbsd_make_note_desc (enum target_object object, uint32_t structsize)
+{
+  gdb::optional<gdb::byte_vector> buf =
+    target_read_alloc (current_top_target (), object, NULL);
+  if (!buf || buf->empty ())
+    return {};
+
+  if (structsize == 0)
+    return buf;
+
+  gdb::byte_vector desc (sizeof (structsize) + buf->size ());
+  memcpy (desc.data (), &structsize, sizeof (structsize));
+  memcpy (desc.data () + sizeof (structsize), buf->data (), buf->size ());
+  return desc;
+}
+
 /* Create appropriate note sections for a corefile, returning them in
    allocated memory.  */
 
@@ -586,6 +606,44 @@ fbsd_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
 
   note_data = thread_args.note_data;
 
+  pid_t pid = inferior_ptid.pid ();
+
+  /* Auxiliary vector.  */
+  uint32_t structsize = gdbarch_ptr_bit (gdbarch) / 4; /* Elf_Auxinfo  */
+  gdb::optional<gdb::byte_vector> note_desc =
+    fbsd_make_note_desc (TARGET_OBJECT_AUXV, structsize);
+  if (note_desc && !note_desc->empty ())
+    {
+      note_data = elfcore_write_note (obfd, note_data, note_size,
+                                      "FreeBSD", NT_FREEBSD_PROCSTAT_AUXV,
+                                      note_desc->data (), note_desc->size ());
+      if (!note_data)
+        return NULL;
+    }
+  
+  /* File mappings  */
+  note_desc = fbsd_make_note_desc (TARGET_OBJECT_FREEBSD_VMMAP, 0);
+  if (note_desc && !note_desc->empty ())
+    {
+      note_data = elfcore_write_note (obfd, note_data, note_size,
+                                      "FreeBSD", NT_FREEBSD_PROCSTAT_VMMAP,
+                                      note_desc->data (), note_desc->size ());
+      if (!note_data)
+        return NULL;
+    }
+
+  note_desc =
+    fbsd_make_note_desc (TARGET_OBJECT_FREEBSD_PS_STRINGS, 0);
+  if (note_desc && !note_desc->empty ())
+    {
+      note_data = elfcore_write_note (obfd, note_data, note_size,
+                                      "FreeBSD",
+                                      NT_FREEBSD_PROCSTAT_PSSTRINGS,
+                                      note_desc->data (), note_desc->size ());
+      if (!note_data)
+        return NULL;
+    }
+
   return note_data;
 }
 
diff --git a/gdb/target.h b/gdb/target.h
index 18c4a84c..83f1172c 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -203,6 +203,10 @@ enum target_object
      of the process ID of the process in question, in hexadecimal
      format.  */
   TARGET_OBJECT_EXEC_FILE,
+  /* FreeBSD file mappings  */
+  TARGET_OBJECT_FREEBSD_VMMAP,
+  /* FreeBSD process strings  */
+  TARGET_OBJECT_FREEBSD_PS_STRINGS,
   /* Possible future objects: TARGET_OBJECT_FILE, ...  */
 };
 
-- 
2.18.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] Generate NT_PROCSTAT_{AUXV,VMMAP,PS_STRINGS} in FreeBSD coredumps
  2018-08-21 14:45   ` [PATCH v3] " Simon Ser
@ 2018-08-23 10:40     ` John Baldwin
  2018-08-23 14:02     ` [PATCH v4] " Simon Ser
  1 sibling, 0 replies; 14+ messages in thread
From: John Baldwin @ 2018-08-23 10:40 UTC (permalink / raw)
  To: Simon Ser, gdb-patches

On 8/21/18 3:45 PM, 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-08-21  Simon Ser  <contact@emersion.fr>
>         * target.h (enum target_object): add FreeBSD-specific
>         TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
>         * fbsd-nat.c (fbsd_nat_target::xfer_partial): add support for
>         TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
>         * fbsd-tdep.c (fbsd_make_corefile_notes): write NT_PROCSTAT_AUXV,
>         NT_PROCSTAT_VMMAP and NT_PROCSTAT_PS_STRINGS notes
> ---
> 
> Changes from v2 to v3:
> - Remove constants, prepend struct size for FreeBSD-specific objects
> - Use gdbarch_ptr_bit instead of gdbarch_addr_bit
> - Fixed typo
> 
> This addresses all of John's comments except the VMMAP packed one.
> 
> John, you said coredumps use the unpacked format, but looking at the
> `gcore` utility source code (in usr.bin/gcore/elfcore.c) I believe it
> uses the packed format for coredumps. It also makes more sense to me
> to use the packed format for coredumps because it allows us not to
> store PATH_MAX zeros for each entry. I may be wrong though, let me know
> what you think.

Ah, yes.  We used to pad them, but now we pack them by default (the kernel
has knobs to control packing in core dumps that default to packing).

> 
>  gdb/fbsd-nat.c  | 50 ++++++++++++++++++++++++++++++++++++++++++
>  gdb/fbsd-tdep.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
>  gdb/target.h    |  4 ++++
>  3 files changed, 112 insertions(+)
> 
> diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
> index 115deac0..7cd325e4 100644
> --- a/gdb/fbsd-nat.c
> +++ b/gdb/fbsd-nat.c
> @@ -751,6 +751,56 @@ fbsd_nat_target::xfer_partial (enum target_object object,
>  	  }
>  	return TARGET_XFER_E_IO;
>        }
> +    case TARGET_OBJECT_FREEBSD_VMMAP:
> +    case TARGET_OBJECT_FREEBSD_PS_STRINGS:
> +      {
> +	gdb::byte_vector buf_storage;
> +	gdb_byte *buf;
> +	size_t buflen;
> +	int mib[4];
> +
> +        int proc_target;
> +        uint32_t struct_size;
> +        switch (object) {
> +        case TARGET_OBJECT_FREEBSD_VMMAP:
> +          proc_target = KERN_PROC_VMMAP;
> +          struct_size = sizeof (struct kinfo_vmentry);
> +          break;
> +        case TARGET_OBJECT_FREEBSD_PS_STRINGS:
> +          proc_target = KERN_PROC_PS_STRINGS;
> +          struct_size = sizeof (void *);
> +          break;
> +        }

The indentation of this block seems off, perhaps it's not using
tabs but only spaces?

> +
> +	if (writebuf != NULL)
> +	  return TARGET_XFER_E_IO;
> +
> +	mib[0] = CTL_KERN;
> +	mib[1] = KERN_PROC;
> +	mib[2] = proc_target;
> +	mib[3] = pid;
> +
> +	buflen = sizeof (struct_size) + offset + len;

Presumably this should just be 'offset + len' as 'struct_size' is
part of the logical stream described by 'offset + len', but see
below where I think we need to rework buf_storage and buf.

> +	buf_storage.resize (buflen);
> +	buf = buf_storage.data ();
> +
> +	memcpy (buf, &struct_size, sizeof (struct_size));
> +
> +	if (sysctl (mib, 4, buf + sizeof (struct_size), &buflen, NULL, 0) == 0)
> +	  {

Hmm, if the caller asks to read a subset of the "stream", then buflen will be
too short and this will fail with ENOMEM or the like.  (It seems I failed to
handle this properly in the auxv case as well *sigh*.)  Probably you need to
follow the pattern that gcore uses in procstat_sysctl() in elfcore.c where you
first request the length via a NULL buffer, then size buf_storage to buflen +
sizeof struct size, then do the sysctl to fetch the entire buffer and finally
do the memcpy out of buf into readbuf.

> diff --git a/gdb/target.h b/gdb/target.h
> index 18c4a84c..83f1172c 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -203,6 +203,10 @@ enum target_object
>       of the process ID of the process in question, in hexadecimal
>       format.  */
>    TARGET_OBJECT_EXEC_FILE,
> +  /* FreeBSD file mappings  */
> +  TARGET_OBJECT_FREEBSD_VMMAP,

Perhaps 'virtual memory mappings'

> +  /* FreeBSD process strings  */
> +  TARGET_OBJECT_FREEBSD_PS_STRINGS,
>    /* Possible future objects: TARGET_OBJECT_FILE, ...  */
>  };
>  
> 


-- 
John Baldwin

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4] Generate NT_PROCSTAT_{AUXV,VMMAP,PS_STRINGS} in FreeBSD coredumps
  2018-08-21 14:45   ` [PATCH v3] " Simon Ser
  2018-08-23 10:40     ` John Baldwin
@ 2018-08-23 14:02     ` Simon Ser
  2018-08-23 16:16       ` John Baldwin
  2018-08-23 17:11       ` [PATCH v5] " Simon Ser
  1 sibling, 2 replies; 14+ messages in thread
From: Simon Ser @ 2018-08-23 14:02 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Ser

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-08-23  Simon Ser  <contact@emersion.fr>
        * target.h (enum target_object): add FreeBSD-specific
        TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
        * fbsd-nat.c (fbsd_nat_target::xfer_partial): add support for
        TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
        * fbsd-tdep.c (fbsd_make_corefile_notes): write NT_PROCSTAT_AUXV,
        NT_PROCSTAT_VMMAP and NT_PROCSTAT_PS_STRINGS notes
---

Changes from v3 to v4:
- Fixed indentation
- Always read with a large enough buffer
- Remove unused includes in fbsd-tdep.c
- "File mappings" → "Virtual memory mappings"

Tried to fix all indentation issues, let me know if I forgot some. I
haven't found out the magic incantation to convince vim to use this
indentation style for this project (yet).

I also rewrote the sysctl code. I think fixing the auxv code belongs to
another patch.

 gdb/fbsd-nat.c  | 54 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/fbsd-tdep.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/target.h    |  4 ++++
 3 files changed, 112 insertions(+)

diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 115deac0..032c42b8 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -751,6 +751,60 @@ fbsd_nat_target::xfer_partial (enum target_object object,
 	  }
 	return TARGET_XFER_E_IO;
       }
+    case TARGET_OBJECT_FREEBSD_VMMAP:
+    case TARGET_OBJECT_FREEBSD_PS_STRINGS:
+      {
+	gdb::byte_vector buf_storage;
+	gdb_byte *buf;
+	size_t buflen;
+	int mib[4];
+
+	int proc_target;
+	uint32_t struct_size;
+	switch (object)
+	  {
+	  case TARGET_OBJECT_FREEBSD_VMMAP:
+	    proc_target = KERN_PROC_VMMAP;
+	    struct_size = sizeof (struct kinfo_vmentry);
+	    break;
+	  case TARGET_OBJECT_FREEBSD_PS_STRINGS:
+	    proc_target = KERN_PROC_PS_STRINGS;
+	    struct_size = sizeof (void *);
+	    break;
+	  }
+
+	if (writebuf != NULL)
+	  return TARGET_XFER_E_IO;
+
+	mib[0] = CTL_KERN;
+	mib[1] = KERN_PROC;
+	mib[2] = proc_target;
+	mib[3] = pid;
+
+	if (sysctl (mib, 4, NULL, &buflen, NULL, 0) != 0)
+	  return TARGET_XFER_E_IO;
+	buflen += sizeof (struct_size);
+
+	if (offset >= buflen)
+	  {
+	    *xfered_len = 0;
+	    return TARGET_XFER_EOF;
+	  }
+
+	buf_storage.resize (buflen);
+	buf = buf_storage.data ();
+
+	memcpy (buf, &struct_size, sizeof (struct_size));
+	if (sysctl (mib, 4, buf + sizeof (struct_size), &buflen, NULL, 0) != 0)
+	  return TARGET_XFER_E_IO;
+	buflen += sizeof (struct_size);
+
+	if (offset + len > buflen)
+	  len = buflen - offset;
+	memcpy (readbuf, buf + offset, len);
+	*xfered_len = len;
+	return TARGET_XFER_OK;
+      }
     default:
       return inf_ptrace_target::xfer_partial (object, annex,
 					      readbuf, writebuf, offset,
diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index 9cea0098..9242c244 100644
--- a/gdb/fbsd-tdep.c
+++ b/gdb/fbsd-tdep.c
@@ -512,6 +512,23 @@ fbsd_corefile_thread (struct thread_info *info,
      args->note_size, args->stop_signal);
 }
 
+static gdb::optional<gdb::byte_vector>
+fbsd_make_note_desc (enum target_object object, uint32_t structsize)
+{
+  gdb::optional<gdb::byte_vector> buf =
+    target_read_alloc (current_top_target (), object, NULL);
+  if (!buf || buf->empty ())
+    return {};
+
+  if (structsize == 0)
+    return buf;
+
+  gdb::byte_vector desc (sizeof (structsize) + buf->size ());
+  memcpy (desc.data (), &structsize, sizeof (structsize));
+  memcpy (desc.data () + sizeof (structsize), buf->data (), buf->size ());
+  return desc;
+}
+
 /* Create appropriate note sections for a corefile, returning them in
    allocated memory.  */
 
@@ -586,6 +603,43 @@ fbsd_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
 
   note_data = thread_args.note_data;
 
+  pid_t pid = inferior_ptid.pid ();
+
+  /* Auxiliary vector.  */
+  uint32_t structsize = gdbarch_ptr_bit (gdbarch) / 4; /* Elf_Auxinfo  */
+  gdb::optional<gdb::byte_vector> note_desc =
+    fbsd_make_note_desc (TARGET_OBJECT_AUXV, structsize);
+  if (note_desc && !note_desc->empty ())
+    {
+      note_data = elfcore_write_note (obfd, note_data, note_size,
+				      "FreeBSD", NT_FREEBSD_PROCSTAT_AUXV,
+				      note_desc->data (), note_desc->size ());
+      if (!note_data)
+	return NULL;
+    }
+
+  /* Virtual memory mappings  */
+  note_desc = fbsd_make_note_desc (TARGET_OBJECT_FREEBSD_VMMAP, 0);
+  if (note_desc && !note_desc->empty ())
+    {
+      note_data = elfcore_write_note (obfd, note_data, note_size,
+				      "FreeBSD", NT_FREEBSD_PROCSTAT_VMMAP,
+				      note_desc->data (), note_desc->size ());
+      if (!note_data)
+	return NULL;
+    }
+
+  note_desc =
+    fbsd_make_note_desc (TARGET_OBJECT_FREEBSD_PS_STRINGS, 0);
+  if (note_desc && !note_desc->empty ())
+    {
+      note_data = elfcore_write_note (obfd, note_data, note_size, "FreeBSD",
+				      NT_FREEBSD_PROCSTAT_PSSTRINGS,
+				      note_desc->data (), note_desc->size ());
+      if (!note_data)
+	return NULL;
+    }
+
   return note_data;
 }
 
diff --git a/gdb/target.h b/gdb/target.h
index 18c4a84c..c584af54 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -203,6 +203,10 @@ enum target_object
      of the process ID of the process in question, in hexadecimal
      format.  */
   TARGET_OBJECT_EXEC_FILE,
+  /* FreeBSD virtual memory mappings  */
+  TARGET_OBJECT_FREEBSD_VMMAP,
+  /* FreeBSD process strings  */
+  TARGET_OBJECT_FREEBSD_PS_STRINGS,
   /* Possible future objects: TARGET_OBJECT_FILE, ...  */
 };
 
-- 
2.18.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] Generate NT_PROCSTAT_{AUXV,VMMAP,PS_STRINGS} in FreeBSD coredumps
  2018-08-23 14:02     ` [PATCH v4] " Simon Ser
@ 2018-08-23 16:16       ` John Baldwin
  2018-08-23 17:11       ` [PATCH v5] " Simon Ser
  1 sibling, 0 replies; 14+ messages in thread
From: John Baldwin @ 2018-08-23 16:16 UTC (permalink / raw)
  To: Simon Ser, gdb-patches

On 8/23/18 3:02 PM, 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-08-23  Simon Ser  <contact@emersion.fr>
>         * target.h (enum target_object): add FreeBSD-specific
>         TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
>         * fbsd-nat.c (fbsd_nat_target::xfer_partial): add support for
>         TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
>         * fbsd-tdep.c (fbsd_make_corefile_notes): write NT_PROCSTAT_AUXV,
>         NT_PROCSTAT_VMMAP and NT_PROCSTAT_PS_STRINGS notes
> ---
> +
> +	memcpy (buf, &struct_size, sizeof (struct_size));
> +	if (sysctl (mib, 4, buf + sizeof (struct_size), &buflen, NULL, 0) != 0)

One nit here: buflen is too large on input.  You should do
'buflen -= sizeof(struct size)' before calling sysctl().

Agreed that auxv sysctl issues belong to a separate patch.

-- 
John Baldwin

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v5] Generate NT_PROCSTAT_{AUXV,VMMAP,PS_STRINGS} in FreeBSD coredumps
  2018-08-23 14:02     ` [PATCH v4] " Simon Ser
  2018-08-23 16:16       ` John Baldwin
@ 2018-08-23 17:11       ` Simon Ser
  2018-09-02 20:02         ` Simon Ser
  1 sibling, 1 reply; 14+ messages in thread
From: Simon Ser @ 2018-08-23 17:11 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Ser, John Baldwin

gcore generates NT_AUXV and NT_FILE notes for Linux targets. On
FreeBSD auxv is stored in a NT_PROCSTAT_AUXV section, virtual memory
mappings are stored in a NT_PROCSTAT_VMMAP and both are prefixed with
the struct size.

2018-08-23  Simon Ser  <contact@emersion.fr>
        * target.h (enum target_object): add FreeBSD-specific
        TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
        * fbsd-nat.c (fbsd_nat_target::xfer_partial): add support for
        TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
        * fbsd-tdep.c (fbsd_make_corefile_notes): write NT_PROCSTAT_AUXV,
        NT_PROCSTAT_VMMAP and NT_PROCSTAT_PS_STRINGS notes
---

Changes from v4 to v5:
- Fixed buflen too large before sysctl

 gdb/fbsd-nat.c  | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/fbsd-tdep.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/target.h    |  4 ++++
 3 files changed, 113 insertions(+)

diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 115deac0..3c1904a1 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -751,6 +751,61 @@ fbsd_nat_target::xfer_partial (enum target_object object,
 	  }
 	return TARGET_XFER_E_IO;
       }
+    case TARGET_OBJECT_FREEBSD_VMMAP:
+    case TARGET_OBJECT_FREEBSD_PS_STRINGS:
+      {
+	gdb::byte_vector buf_storage;
+	gdb_byte *buf;
+	size_t buflen;
+	int mib[4];
+
+	int proc_target;
+	uint32_t struct_size;
+	switch (object)
+	  {
+	  case TARGET_OBJECT_FREEBSD_VMMAP:
+	    proc_target = KERN_PROC_VMMAP;
+	    struct_size = sizeof (struct kinfo_vmentry);
+	    break;
+	  case TARGET_OBJECT_FREEBSD_PS_STRINGS:
+	    proc_target = KERN_PROC_PS_STRINGS;
+	    struct_size = sizeof (void *);
+	    break;
+	  }
+
+	if (writebuf != NULL)
+	  return TARGET_XFER_E_IO;
+
+	mib[0] = CTL_KERN;
+	mib[1] = KERN_PROC;
+	mib[2] = proc_target;
+	mib[3] = pid;
+
+	if (sysctl (mib, 4, NULL, &buflen, NULL, 0) != 0)
+	  return TARGET_XFER_E_IO;
+	buflen += sizeof (struct_size);
+
+	if (offset >= buflen)
+	  {
+	    *xfered_len = 0;
+	    return TARGET_XFER_EOF;
+	  }
+
+	buf_storage.resize (buflen);
+	buf = buf_storage.data ();
+
+	memcpy (buf, &struct_size, sizeof (struct_size));
+	buflen -= sizeof (struct_size);
+	if (sysctl (mib, 4, buf + sizeof (struct_size), &buflen, NULL, 0) != 0)
+	  return TARGET_XFER_E_IO;
+	buflen += sizeof (struct_size);
+
+	if (offset + len > buflen)
+	  len = buflen - offset;
+	memcpy (readbuf, buf + offset, len);
+	*xfered_len = len;
+	return TARGET_XFER_OK;
+      }
     default:
       return inf_ptrace_target::xfer_partial (object, annex,
 					      readbuf, writebuf, offset,
diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index 9cea0098..9242c244 100644
--- a/gdb/fbsd-tdep.c
+++ b/gdb/fbsd-tdep.c
@@ -512,6 +512,23 @@ fbsd_corefile_thread (struct thread_info *info,
      args->note_size, args->stop_signal);
 }
 
+static gdb::optional<gdb::byte_vector>
+fbsd_make_note_desc (enum target_object object, uint32_t structsize)
+{
+  gdb::optional<gdb::byte_vector> buf =
+    target_read_alloc (current_top_target (), object, NULL);
+  if (!buf || buf->empty ())
+    return {};
+
+  if (structsize == 0)
+    return buf;
+
+  gdb::byte_vector desc (sizeof (structsize) + buf->size ());
+  memcpy (desc.data (), &structsize, sizeof (structsize));
+  memcpy (desc.data () + sizeof (structsize), buf->data (), buf->size ());
+  return desc;
+}
+
 /* Create appropriate note sections for a corefile, returning them in
    allocated memory.  */
 
@@ -586,6 +603,43 @@ fbsd_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
 
   note_data = thread_args.note_data;
 
+  pid_t pid = inferior_ptid.pid ();
+
+  /* Auxiliary vector.  */
+  uint32_t structsize = gdbarch_ptr_bit (gdbarch) / 4; /* Elf_Auxinfo  */
+  gdb::optional<gdb::byte_vector> note_desc =
+    fbsd_make_note_desc (TARGET_OBJECT_AUXV, structsize);
+  if (note_desc && !note_desc->empty ())
+    {
+      note_data = elfcore_write_note (obfd, note_data, note_size,
+				      "FreeBSD", NT_FREEBSD_PROCSTAT_AUXV,
+				      note_desc->data (), note_desc->size ());
+      if (!note_data)
+	return NULL;
+    }
+
+  /* Virtual memory mappings  */
+  note_desc = fbsd_make_note_desc (TARGET_OBJECT_FREEBSD_VMMAP, 0);
+  if (note_desc && !note_desc->empty ())
+    {
+      note_data = elfcore_write_note (obfd, note_data, note_size,
+				      "FreeBSD", NT_FREEBSD_PROCSTAT_VMMAP,
+				      note_desc->data (), note_desc->size ());
+      if (!note_data)
+	return NULL;
+    }
+
+  note_desc =
+    fbsd_make_note_desc (TARGET_OBJECT_FREEBSD_PS_STRINGS, 0);
+  if (note_desc && !note_desc->empty ())
+    {
+      note_data = elfcore_write_note (obfd, note_data, note_size, "FreeBSD",
+				      NT_FREEBSD_PROCSTAT_PSSTRINGS,
+				      note_desc->data (), note_desc->size ());
+      if (!note_data)
+	return NULL;
+    }
+
   return note_data;
 }
 
diff --git a/gdb/target.h b/gdb/target.h
index 18c4a84c..c584af54 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -203,6 +203,10 @@ enum target_object
      of the process ID of the process in question, in hexadecimal
      format.  */
   TARGET_OBJECT_EXEC_FILE,
+  /* FreeBSD virtual memory mappings  */
+  TARGET_OBJECT_FREEBSD_VMMAP,
+  /* FreeBSD process strings  */
+  TARGET_OBJECT_FREEBSD_PS_STRINGS,
   /* Possible future objects: TARGET_OBJECT_FILE, ...  */
 };
 
-- 
2.18.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v5] Generate NT_PROCSTAT_{AUXV,VMMAP,PS_STRINGS} in FreeBSD coredumps
  2018-08-23 17:11       ` [PATCH v5] " Simon Ser
@ 2018-09-02 20:02         ` Simon Ser
  2018-09-06 22:12           ` John Baldwin
  0 siblings, 1 reply; 14+ messages in thread
From: Simon Ser @ 2018-09-02 20:02 UTC (permalink / raw)
  To: gdb-patches; +Cc: John Baldwin

Hi John,

Did you have time to have a look at this new version ?

Thanks,
---
Simon Ser
https://emersion.fr

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On 23 August 2018 7:10 PM, Simon Ser <contact@emersion.fr> wrote:

> gcore generates NT_AUXV and NT_FILE notes for Linux targets. On
> FreeBSD auxv is stored in a NT_PROCSTAT_AUXV section, virtual memory
> mappings are stored in a NT_PROCSTAT_VMMAP and both are prefixed with
> the struct size.
>
> 2018-08-23 Simon Ser contact@emersion.fr
>
>         * target.h (enum target_object): add FreeBSD-specific
>         TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
>         * fbsd-nat.c (fbsd_nat_target::xfer_partial): add support for
>         TARGET_OBJECT_FREEBSD_VMMAP and TARGET_OBJECT_FREEBSD_PS_STRINGS
>         * fbsd-tdep.c (fbsd_make_corefile_notes): write NT_PROCSTAT_AUXV,
>         NT_PROCSTAT_VMMAP and NT_PROCSTAT_PS_STRINGS notes
>
>
> Changes from v4 to v5:
>
> -   Fixed buflen too large before sysctl
>
>     gdb/fbsd-nat.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
>     gdb/fbsd-tdep.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
>     gdb/target.h | 4 ++++
>     3 files changed, 113 insertions(+)
>
>     diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
>     index 115deac0..3c1904a1 100644
>     --- a/gdb/fbsd-nat.c
>     +++ b/gdb/fbsd-nat.c
>     @@ -751,6 +751,61 @@ fbsd_nat_target::xfer_partial (enum target_object object,
>     }
>     return TARGET_XFER_E_IO;
>     }
>
>
> -   case TARGET_OBJECT_FREEBSD_VMMAP:
>
> -   case TARGET_OBJECT_FREEBSD_PS_STRINGS:
>
> -        {
>
>
> -   gdb::byte_vector buf_storage;
>
> -   gdb_byte *buf;
>
> -   size_t buflen;
>
> -   int mib[4];
>
> -
> -   int proc_target;
>
> -   uint32_t struct_size;
>
> -   switch (object)
>
> -       {
>
>
> -       case TARGET_OBJECT_FREEBSD_VMMAP:
>
>
> -         proc_target = KERN_PROC_VMMAP;
>
>
> -         struct_size = sizeof (struct kinfo_vmentry);
>
>
> -         break;
>
>
> -       case TARGET_OBJECT_FREEBSD_PS_STRINGS:
>
>
> -         proc_target = KERN_PROC_PS_STRINGS;
>
>
> -         struct_size = sizeof (void *);
>
>
> -         break;
>
>
> -       }
>
>
> -
> -   if (writebuf != NULL)
>
> -       return TARGET_XFER_E_IO;
>
>
> -
> -   mib[0] = CTL_KERN;
>
> -   mib[1] = KERN_PROC;
>
> -   mib[2] = proc_target;
>
> -   mib[3] = pid;
>
> -
> -   if (sysctl (mib, 4, NULL, &buflen, NULL, 0) != 0)
>
> -       return TARGET_XFER_E_IO;
>
>
> -   buflen += sizeof (struct_size);
>
> -
> -   if (offset >= buflen)
>
> -       {
>
>
> -         *xfered_len = 0;
>
>
> -         return TARGET_XFER_EOF;
>
>
> -       }
>
>
> -
> -   buf_storage.resize (buflen);
>
> -   buf = buf_storage.data ();
>
> -
> -   memcpy (buf, &struct_size, sizeof (struct_size));
>
> -   buflen -= sizeof (struct_size);
>
> -   if (sysctl (mib, 4, buf + sizeof (struct_size), &buflen, NULL, 0) != 0)
>
> -       return TARGET_XFER_E_IO;
>
>
> -   buflen += sizeof (struct_size);
>
> -
> -   if (offset + len > buflen)
>
> -       len = buflen - offset;
>
>
> -   memcpy (readbuf, buf + offset, len);
>
> -   *xfered_len = len;
>
> -   return TARGET_XFER_OK;
>
> -        }
>
>
>     default:
>     return inf_ptrace_target::xfer_partial (object, annex,
>     readbuf, writebuf, offset,
>     diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
>     index 9cea0098..9242c244 100644
>     --- a/gdb/fbsd-tdep.c
>     +++ b/gdb/fbsd-tdep.c
>     @@ -512,6 +512,23 @@ fbsd_corefile_thread (struct thread_info *info,
>     args->note_size, args->stop_signal);
>
>
> }
>
> +static gdb::optionalgdb::byte_vector
> +fbsd_make_note_desc (enum target_object object, uint32_t structsize)
> +{
>
> -   gdb::optionalgdb::byte_vector buf =
>
> -   target_read_alloc (current_top_target (), object, NULL);
>
> -   if (!buf || buf->empty ())
>
> -   return {};
>
> -
> -   if (structsize == 0)
>
> -   return buf;
>
> -
> -   gdb::byte_vector desc (sizeof (structsize) + buf->size ());
>
> -   memcpy (desc.data (), &structsize, sizeof (structsize));
>
> -   memcpy (desc.data () + sizeof (structsize), buf->data (), buf->size ());
>
> -   return desc;
>     +}
>
> -
>
> /* Create appropriate note sections for a corefile, returning them in
> allocated memory. */
>
> @@ -586,6 +603,43 @@ fbsd_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
>
> note_data = thread_args.note_data;
>
> -   pid_t pid = inferior_ptid.pid ();
>
> -
> -   /* Auxiliary vector. */
>
> -   uint32_t structsize = gdbarch_ptr_bit (gdbarch) / 4; /* Elf_Auxinfo */
>
> -   gdb::optionalgdb::byte_vector note_desc =
>
> -   fbsd_make_note_desc (TARGET_OBJECT_AUXV, structsize);
>
> -   if (note_desc && !note_desc->empty ())
>
> -   {
>
> -        note_data = elfcore_write_note (obfd, note_data, note_size,
>
>
> -         		      "FreeBSD", NT_FREEBSD_PROCSTAT_AUXV,
>
>
> -         		      note_desc->data (), note_desc->size ());
>
>
> -        if (!note_data)
>
>
> -   return NULL;
>
> -   }
>
> -
> -   /* Virtual memory mappings */
>
> -   note_desc = fbsd_make_note_desc (TARGET_OBJECT_FREEBSD_VMMAP, 0);
>
> -   if (note_desc && !note_desc->empty ())
>
> -   {
>
> -        note_data = elfcore_write_note (obfd, note_data, note_size,
>
>
> -         		      "FreeBSD", NT_FREEBSD_PROCSTAT_VMMAP,
>
>
> -         		      note_desc->data (), note_desc->size ());
>
>
> -        if (!note_data)
>
>
> -   return NULL;
>
> -   }
>
> -
> -   note_desc =
>
> -   fbsd_make_note_desc (TARGET_OBJECT_FREEBSD_PS_STRINGS, 0);
>
> -   if (note_desc && !note_desc->empty ())
>
> -   {
>
> -        note_data = elfcore_write_note (obfd, note_data, note_size, "FreeBSD",
>
>
> -         		      NT_FREEBSD_PROCSTAT_PSSTRINGS,
>
>
> -         		      note_desc->data (), note_desc->size ());
>
>
> -        if (!note_data)
>
>
> -   return NULL;
>
> -   }
>
> -   return note_data;
>     }
>
>     diff --git a/gdb/target.h b/gdb/target.h
>     index 18c4a84c..c584af54 100644
>     --- a/gdb/target.h
>     +++ b/gdb/target.h
>     @@ -203,6 +203,10 @@ enum target_object
>     of the process ID of the process in question, in hexadecimal
>     format. */
>     TARGET_OBJECT_EXEC_FILE,
>
> -   /* FreeBSD virtual memory mappings */
>
> -   TARGET_OBJECT_FREEBSD_VMMAP,
>
> -   /* FreeBSD process strings */
>
> -   TARGET_OBJECT_FREEBSD_PS_STRINGS,
>     /* Possible future objects: TARGET_OBJECT_FILE, ... */
>     };
>
>     --
>     2.18.0
>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v5] Generate NT_PROCSTAT_{AUXV,VMMAP,PS_STRINGS} in FreeBSD coredumps
  2018-09-02 20:02         ` Simon Ser
@ 2018-09-06 22:12           ` John Baldwin
  0 siblings, 0 replies; 14+ messages in thread
From: John Baldwin @ 2018-09-06 22:12 UTC (permalink / raw)
  To: Simon Ser, gdb-patches

On 9/2/18 1:01 PM, Simon Ser wrote:
> Hi John,
> 
> Did you have time to have a look at this new version ?

Yes, sorry for the delay.  I've fixed a few small nits (updating gdb/ChangeLog,
adding a reference to a PR that this change fixes by including auxv info in
gcore output, etc.) and pushed it.
-- 
John Baldwin

                                                                            

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-09-06 22:12 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-11 12:35 [PATCH] Generate NT_PROCSTAT_{AUXV,VMMAP} in FreeBSD coredumps Simon Ser
2018-07-11 16:16 ` John Baldwin
2018-07-11 16:40   ` Simon Ser
2018-07-16 13:39 ` [PATCH v2] Generate NT_PROCSTAT_{AUXV,VMMAP,PS_STRINGS} " Simon Ser
2018-07-24  7:53   ` Simon Ser
2018-08-01 16:33     ` Simon Ser
2018-08-01 18:16   ` John Baldwin
2018-08-21 14:45   ` [PATCH v3] " Simon Ser
2018-08-23 10:40     ` John Baldwin
2018-08-23 14:02     ` [PATCH v4] " Simon Ser
2018-08-23 16:16       ` John Baldwin
2018-08-23 17:11       ` [PATCH v5] " Simon Ser
2018-09-02 20:02         ` Simon Ser
2018-09-06 22:12           ` John Baldwin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).