From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16646 invoked by alias); 2 Feb 2019 15:26:24 -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 16638 invoked by uid 89); 2 Feb 2019 15:26:24 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=Couldnt, Couldn't, H*RU:Sendmail, Hx-spam-relays-external:Sendmail X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 02 Feb 2019 15:26:23 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id x12FQGgY018160 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Sat, 2 Feb 2019 10:26:21 -0500 Received: by simark.ca (Postfix, from userid 112) id 476B91E882; Sat, 2 Feb 2019 10:26:16 -0500 (EST) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id B06121E472; Sat, 2 Feb 2019 10:26:14 -0500 (EST) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Sat, 02 Feb 2019 15:26:00 -0000 From: Simon Marchi To: John Baldwin Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 2/9] Support fs_base and gs_base on FreeBSD/i386. In-Reply-To: References: Message-ID: X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.6 X-IsSubscribed: yes X-SW-Source: 2019-02/txt/msg00008.txt.bz2 On 2019-01-22 13:42, John Baldwin wrote: > The i386 BSD native target uses the same ptrace operations > (PT_[GS]ET[FG]SBASE) as the amd64 BSD native target to fetch and store > the registers. > > The amd64 BSD native now uses 'tdep->fsbase_regnum' instead of > hardcoding AMD64_FSBASE_REGNUM and AMD64_GSBASE_REGNUM to support > 32-bit targets. In addition, the store operations explicitly zero the > new register value before fetching it from the register cache to > ensure 32-bit values are zero-extended. To be clear, this happens when debugging a 32-bits process on a 64-bits OS? When debugging a 32-bits process on a 32-bits OS, the code in i386-bsd-nat.c would be used? > gdb/ChangeLog: > > * amd64-bsd-nat.c (amd64bsd_fetch_inferior_registers): Use > tdep->fsbase_regnum instead of constants for fs_base and gs_base. > (amd64bsd_store_inferior_registers): Likewise. > * amd64-fbsd-nat.c (amd64_fbsd_nat_target::read_description): > Enable segment base registers. > * i386-bsd-nat.c (i386bsd_fetch_inferior_registers): Use > PT_GETFSBASE and PT_GETGSBASE. > (i386bsd_store_inferior_registers): Use PT_SETFSBASE and PT_SETGSBASE. > * i386-fbsd-nat.c (i386_fbsd_nat_target::read_description): Enable > segment base registers. > * i386-fbsd-tdep.c (i386fbsd_core_read_description): Likewise. > --- > gdb/ChangeLog | 14 ++++++++++++ > gdb/amd64-bsd-nat.c | 26 ++++++++++++++------- > gdb/amd64-fbsd-nat.c | 4 ++-- > gdb/i386-bsd-nat.c | 54 ++++++++++++++++++++++++++++++++++++++++++++ > gdb/i386-fbsd-nat.c | 2 +- > gdb/i386-fbsd-tdep.c | 2 +- > 6 files changed, 90 insertions(+), 12 deletions(-) > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index 4afd5b664e..056a60fa23 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,3 +1,17 @@ > +2019-01-22 John Baldwin > + > + * amd64-bsd-nat.c (amd64bsd_fetch_inferior_registers): Use > + tdep->fsbase_regnum instead of constants for fs_base and gs_base. > + (amd64bsd_store_inferior_registers): Likewise. > + * amd64-fbsd-nat.c (amd64_fbsd_nat_target::read_description): > + Enable segment base registers. > + * i386-bsd-nat.c (i386bsd_fetch_inferior_registers): Use > + PT_GETFSBASE and PT_GETGSBASE. > + (i386bsd_store_inferior_registers): Use PT_SETFSBASE and > PT_SETGSBASE. > + * i386-fbsd-nat.c (i386_fbsd_nat_target::read_description): Enable > + segment base registers. > + * i386-fbsd-tdep.c (i386fbsd_core_read_description): Likewise. > + > 2019-01-22 John Baldwin > > * amd64-fbsd-nat.c (amd64_fbsd_nat_target::read_description): > diff --git a/gdb/amd64-bsd-nat.c b/gdb/amd64-bsd-nat.c > index a2a91abb91..0f47ff6c61 100644 > --- a/gdb/amd64-bsd-nat.c > +++ b/gdb/amd64-bsd-nat.c > @@ -43,6 +43,9 @@ amd64bsd_fetch_inferior_registers (struct regcache > *regcache, int regnum) > { > struct gdbarch *gdbarch = regcache->arch (); > pid_t pid = get_ptrace_pid (regcache->ptid ()); > +#ifdef PT_GETFSBASE > + const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); > +#endif TDEP is used in both #ifdef PT_GETFSBASE and #ifdef PT_GETGSBASE, but is only declared here in an #ifdef PT_GETFSBASE. I suppose it's not actually an issue because they will always be present together. But we might as well use "#if defined(PT_GETFSBASE) || defined(PT_GETGSBASE)" for the declaration. > > if (regnum == -1 || amd64_native_gregset_supplies_p (gdbarch, > regnum)) > { > @@ -57,27 +60,27 @@ amd64bsd_fetch_inferior_registers (struct regcache > *regcache, int regnum) > } > > #ifdef PT_GETFSBASE > - if (regnum == -1 || regnum == AMD64_FSBASE_REGNUM) > + if (regnum == -1 || regnum == tdep->fsbase_regnum) > { > register_t base; > > if (ptrace (PT_GETFSBASE, pid, (PTRACE_TYPE_ARG3) &base, 0) == > -1) > perror_with_name (_("Couldn't get segment register fs_base")); > > - regcache->raw_supply (AMD64_FSBASE_REGNUM, &base); > + regcache->raw_supply (tdep->fsbase_regnum, &base); > if (regnum != -1) > return; > } > #endif > #ifdef PT_GETGSBASE > - if (regnum == -1 || regnum == AMD64_GSBASE_REGNUM) > + if (regnum == -1 || regnum == tdep->fsbase_regnum + 1) > { > register_t base; > > if (ptrace (PT_GETGSBASE, pid, (PTRACE_TYPE_ARG3) &base, 0) == > -1) > perror_with_name (_("Couldn't get segment register gs_base")); > > - regcache->raw_supply (AMD64_GSBASE_REGNUM, &base); > + regcache->raw_supply (tdep->fsbase_regnum + 1, &base); > if (regnum != -1) > return; > } > @@ -116,6 +119,9 @@ amd64bsd_store_inferior_registers (struct regcache > *regcache, int regnum) > { > struct gdbarch *gdbarch = regcache->arch (); > pid_t pid = get_ptrace_pid (regcache->ptid ()); > +#ifdef PT_GETFSBASE > + const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); > +#endif Same here. > > if (regnum == -1 || amd64_native_gregset_supplies_p (gdbarch, > regnum)) > { > @@ -134,11 +140,13 @@ amd64bsd_store_inferior_registers (struct > regcache *regcache, int regnum) > } > > #ifdef PT_SETFSBASE > - if (regnum == -1 || regnum == AMD64_FSBASE_REGNUM) > + if (regnum == -1 || regnum == tdep->fsbase_regnum) > { > register_t base; > > - regcache->raw_collect (AMD64_FSBASE_REGNUM, &base); > + /* Clear the full base value to support 32-bit targets. */ > + base = 0; > + regcache->raw_collect (tdep->fsbase_regnum, &base); It's probably safer to clear the value to 0 as you did, so that's fine. But I would have thought that when debugging 32-bits processes, the high bits would be ignored at some point. The kernel would know that this is a 32 bits register, so it would just take the 32 low bits of what it receives, and it magically works whatever the original data type is because it's little endian. Otherwise, this LGTM. Simon