From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15104 invoked by alias); 1 Aug 2018 18:16:21 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 15007 invoked by uid 89); 1 Aug 2018 18:16:20 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy=feels, bare, gross X-HELO: mail.baldwin.cx Received: from bigwig.baldwin.cx (HELO mail.baldwin.cx) (96.47.65.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 01 Aug 2018 18:16:16 +0000 Received: from John-Baldwins-MacBook-Pro-2.local (ralph.baldwin.cx [66.234.199.215]) by mail.baldwin.cx (Postfix) with ESMTPSA id DD82810A87D; Wed, 1 Aug 2018 14:16:13 -0400 (EDT) Subject: Re: [PATCH v2] Generate NT_PROCSTAT_{AUXV,VMMAP,PS_STRINGS} in FreeBSD coredumps To: Simon Ser , gdb-patches@sourceware.org References: <_LRTDYZchE3m5uYvETytJBLgsrWRhqIDAXHnzDsU6kJBhItMetbpfxGe8OJD9a4K4b9brDxF9YvXWOzdDfTRBRfrZI-4XLl8mWwlrEpqmm0=@emersion.fr> From: John Baldwin Message-ID: Date: Wed, 01 Aug 2018 18:16:00 -0000 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <_LRTDYZchE3m5uYvETytJBLgsrWRhqIDAXHnzDsU6kJBhItMetbpfxGe8OJD9a4K4b9brDxF9YvXWOzdDfTRBRfrZI-4XLl8mWwlrEpqmm0=@emersion.fr> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-08/txt/msg00016.txt.bz2 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 > * 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 > +#include > +#include > > #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 > +fbsd_make_note_desc (enum target_object object, uint32_t structsize) > +{ > + gdb::optional 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 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