From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12232 invoked by alias); 3 Jan 2018 19:05:56 -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 12172 invoked by uid 89); 3 Jan 2018 19:05:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No 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=H*F:D*freebsd.org, sk:freebsd X-Spam-User: qpsmtpd, 2 recipients 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, 03 Jan 2018 19:05:54 +0000 Received: from ralph.baldwin.cx (astound-66-234-202-155.ca.astound.net [66.234.202.155]) by mail.baldwin.cx (Postfix) with ESMTPSA id AE04F10A8CC; Wed, 3 Jan 2018 14:05:52 -0500 (EST) From: John Baldwin To: Simon Marchi Cc: gdb-patches@sourceware.org, binutils@sourceware.org Subject: Re: [PATCH 2/4] Support 'info proc' for FreeBSD process core dumps. Date: Wed, 03 Jan 2018 19:05:00 -0000 Message-ID: <2227931.gyrPRxzbqy@ralph.baldwin.cx> User-Agent: KMail/4.14.10 (FreeBSD/11.1-STABLE; KDE/4.14.30; amd64; ; ) In-Reply-To: References: <20171222220513.54983-1-jhb@FreeBSD.org> <20171222220513.54983-3-jhb@FreeBSD.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-IsSubscribed: yes X-SW-Source: 2018-01/txt/msg00036.txt.bz2 On Tuesday, December 26, 2017 08:56:51 PM Simon Marchi wrote: > On 2017-12-22 05:05 PM, John Baldwin wrote: > > - Command line arguments are obtained from the pr_psargs[] array > > saved in the NT_PRPSINFO note. > > - The 'cwd' and 'exe' values are obtained from the per-process file > > descriptor table stored in the NT_PROCSTAT_FILES core note. > > - 'mappings' is implemented by walking the array of VM map entries > > stored in the NT_PROCSTAT_VMMAP core note. > > - 'stat' and 'status' output is generated by outputting fields from > > the first structure stored in the NT_PROCSTAT_PROC core note. > > > > diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c > > index f89b520c5f..454036dcac 100644 > > --- a/gdb/fbsd-tdep.c > > +++ b/gdb/fbsd-tdep.c > > + is not available from a core dump. Instead, the per-thread data > > + structures contain the value of these fields for individual > > + threads. */ > > + > > +struct kinfo_proc_layout > > struct definitions should not be indented, they should look like: > > struct kinfo_proc_layout > { > int ki_layout; > ... > } Fixed. > > + > > +struct kinfo_proc_layout kinfo_proc_layout_32 = > > I would suggest making these const. Fixed. > > +static void > > +fbsd_print_sigset (const char *descr, unsigned char *sigset) > > +{ > > + printf_filtered ("%s:\t", descr); > > + for (int i = 0; i < _SIG_WORDS; i++) > > _SIG_WORDS seems to be FreeBSD-specific, so shouldn't be used in the tdep file, > unless we redefine it. Oops, yes. I added a local constant. > > + printf_filtered ("Name:\t%.19s\n", descdata + kp->ki_comm); > > + printf_filtered ("Pid:\t%s\n", > > + pulongest(bfd_get_32 (core_bfd, descdata + kp->ki_pid))); > > Missing a few spaces before parentheses here and there. Fixed. > > + default: > > + error (_("Not supported on this architecture.")); > > If you mean "not supported for FreeBSD", I'm not sure architecture > is the right word, since architecture usually refers to CPU architecture. Mmm, yes. I think I was trying to say "not supported on this gdbarch" in effect. However, the core target doesn't output any error if there is no valid core_info_proc method, so perhaps it would be best to just not output any error at all. This also matches linux-tdep.c which doesn't output anything for an unsupported enum value. > > diff --git a/gdb/fbsd-tdep.h b/gdb/fbsd-tdep.h > > index ff2e207aae..0029e03d41 100644 > > --- a/gdb/fbsd-tdep.h > > +++ b/gdb/fbsd-tdep.h > > @@ -21,5 +21,6 @@ > > #define FBSD_TDEP_H > > > > extern void fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch); > > +extern const char *fbsd_vm_map_entry_flags (int kve_flags, int kve_protection); > > Can you please add doc for this new function? Fixed. -- John Baldwin