From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30842 invoked by alias); 5 Jun 2017 20:27:48 -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 30826 invoked by uid 89); 5 Jun 2017 20:27:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.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=clarity, reserved X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 05 Jun 2017 20:27:46 +0000 Received: by simark.ca (Postfix, from userid 33) id AAC2C1E4E8; Mon, 5 Jun 2017 16:27:48 -0400 (EDT) To: John Baldwin Subject: Re: [PATCH] Look for FIR in the last FreeBSD/mips floating-point register. X-PHP-Originating-Script: 33:rcube.php MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Mon, 05 Jun 2017 20:27:00 -0000 From: Simon Marchi Cc: gdb-patches@sourceware.org In-Reply-To: <20170531165803.50633-1-jhb@FreeBSD.org> References: <20170531165803.50633-1-jhb@FreeBSD.org> Message-ID: X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.5 X-IsSubscribed: yes X-SW-Source: 2017-06/txt/msg00137.txt.bz2 On 2017-05-31 18:58, John Baldwin wrote: > FreeBSD/mips kernels were recently changed to include the floating > point implementation revision register in the floating point register > set exported in process cores and via ptrace() (r318067). This change > will first ship in FreeBSD 12.0 when it is eventually released. The > space used to hold FIR was previously reserved in 'struct fpreg' as a > zero-filled dummy for padding, so 'struct fpreg' has not changed in > size. On the one hand this means that there is not an easy way to > detect > if if the FIR register is the zero-filled dummy or the true FIR value. > However, it also means that there is no need to deal with multiple > layouts of 'struct fpreg'. I've chosen to always treat the last > register > in 'struct fpreg' as the FIR. This means that process cores and > ptrace() on older kernels will report a FIR value of 0. However, > FreeBSD > doesn't currently ship a release image for FreeBSD/mips, and releases > prior to 12.0 assume soft-float, so I think this is a reasonable > tradeoff > for simplicity. The patch LGTM, I just have one question down there, really out of curiosity. > gdb/ChangeLog: > > * mips-fbsd-nat.c (getfpregs_supplies): Return true for FIR. > * mips-fbsd-tdep.c (mips_fbsd_supply_fpregs): Split supply of FSR > out of loop and add supply of FIR. > (mips_fbsd_collect_fpregs): Split collect of FSR out of loop and > add collect of FIR. > --- > gdb/ChangeLog | 8 ++++++++ > gdb/mips-fbsd-nat.c | 2 +- > gdb/mips-fbsd-tdep.c | 41 ++++++++++++++++++++++++++++------------- > 3 files changed, 37 insertions(+), 14 deletions(-) > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index 263d57ff66..0e2a7b5833 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,3 +1,11 @@ > +2017-05-31 John Baldwin > + > + * mips-fbsd-nat.c (getfpregs_supplies): Return true for FIR. > + * mips-fbsd-tdep.c (mips_fbsd_supply_fpregs): Split supply of FSR > + out of loop and add supply of FIR. > + (mips_fbsd_collect_fpregs): Split collect of FSR out of loop and > + add collect of FIR. > + > 2017-05-31 Simon Marchi > > * memattr.c (mem_info_command): Rename to ... > diff --git a/gdb/mips-fbsd-nat.c b/gdb/mips-fbsd-nat.c > index 53817d7cd7..c381186535 100644 > --- a/gdb/mips-fbsd-nat.c > +++ b/gdb/mips-fbsd-nat.c > @@ -46,7 +46,7 @@ static bool > getfpregs_supplies (struct gdbarch *gdbarch, int regnum) > { > return (regnum >= mips_regnum (gdbarch)->fp0 > - && regnum < mips_regnum (gdbarch)->fp_implementation_revision); > + && regnum <= mips_regnum (gdbarch)->fp_implementation_revision); > } > > /* Fetch register REGNUM from the inferior. If REGNUM is -1, do this > diff --git a/gdb/mips-fbsd-tdep.c b/gdb/mips-fbsd-tdep.c > index 44b960d4d2..34a19b0ead 100644 > --- a/gdb/mips-fbsd-tdep.c > +++ b/gdb/mips-fbsd-tdep.c > @@ -39,7 +39,8 @@ > > /* Number of registers in `struct fpreg' from . The > first 32 hold floating point registers. 33 holds the FSR. The > - 34th is a dummy for padding. */ > + 34th holds FIR on FreeBSD 12.0 and newer kernels. On older kernels > + it was a zero-filled dummy for padding. */ > #define MIPS_FBSD_NUM_FPREGS 34 > > /* Supply a single register. The register size might not match, so > use > @@ -72,14 +73,21 @@ mips_fbsd_supply_fpregs (struct regcache > *regcache, int regnum, > { > struct gdbarch *gdbarch = get_regcache_arch (regcache); > const gdb_byte *regs = (const gdb_byte *) fpregs; > - int i, fp0num, fsrnum; > + int i, fp0num; > > fp0num = mips_regnum (gdbarch)->fp0; > - fsrnum = mips_regnum (gdbarch)->fp_control_status; > - for (i = fp0num; i <= fsrnum; i++) > - if (regnum == i || regnum == -1) > - mips_fbsd_supply_reg (regcache, i, > - regs + (i - fp0num) * regsize, regsize); > + for (i = 0; i <= 32; i++) > + if (regnum == fp0num + i || regnum == -1) > + mips_fbsd_supply_reg (regcache, fp0num + i, > + regs + i * regsize, regsize); > + if (regnum == mips_regnum (gdbarch)->fp_control_status || regnum == > -1) > + mips_fbsd_supply_reg (regcache, mips_regnum > (gdbarch)->fp_control_status, > + regs + 32 * regsize, regsize); > + if (regnum == mips_regnum (gdbarch)->fp_implementation_revision > + || regnum == -1) > + mips_fbsd_supply_reg (regcache, > + mips_regnum (gdbarch)->fp_implementation_revision, > + regs + 33 * regsize, regsize); > } > > /* Supply the general-purpose registers stored in GREGS to REGCACHE. > @@ -109,14 +117,21 @@ mips_fbsd_collect_fpregs (const struct regcache > *regcache, int regnum, > { > struct gdbarch *gdbarch = get_regcache_arch (regcache); > gdb_byte *regs = (gdb_byte *) fpregs; > - int i, fp0num, fsrnum; > + int i, fp0num; > > fp0num = mips_regnum (gdbarch)->fp0; > - fsrnum = mips_regnum (gdbarch)->fp_control_status; > - for (i = fp0num; i <= fsrnum; i++) > - if (regnum == i || regnum == -1) > - mips_fbsd_collect_reg (regcache, i, > - regs + (i - fp0num) * regsize, regsize); > + for (i = 0; i < 32; i++) > + if (regnum == fp0num + i || regnum == -1) > + mips_fbsd_collect_reg (regcache, fp0num + i, > + regs + i * regsize, regsize); > + if (regnum == mips_regnum (gdbarch)->fp_control_status || regnum == > -1) > + mips_fbsd_collect_reg (regcache, mips_regnum > (gdbarch)->fp_control_status, > + regs + 32 * regsize, regsize); > + if (regnum == mips_regnum (gdbarch)->fp_implementation_revision > + || regnum == -1) > + mips_fbsd_collect_reg (regcache, > + mips_regnum (gdbarch)->fp_implementation_revision, > + regs + 33 * regsize, regsize); I'm just curious to know why you changed this. They previous code assumed that the 32 floating-point registers and the control register were contiguous their numbering. Is this no longer true with the new register, or is it just for clarity that you separately handle the different kinds of registers? > } > > /* Collect the general-purpose registers from REGCACHE and store them Thanks, Simon