Re: [PATCH 2/9] Support fs_base and gs_base on FreeBSD/i386.

John Baldwin
2019-02-04T19:45:57Z

On 2/2/19 7:26 AM, Simon Marchi wrote:
> 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?

Correct.

>> 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 <jhb@FreeBSD.org>
>> +
>> + * 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 <jhb@FreeBSD.org>
>>
>> * 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.

Ok. I could also just perhaps collapse the other two #ifdef's instead of
using separate ones. FreeBSD always provides both if it provides one. None
of the other BSD's provide these currently. I suspect if they did they
would always provide both.

>> @@ -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.

I had originally just done this out of paranoia, but I checked and FreeBSD's
amd64 kernel will actually fail attempts to set the base address higher
than 4G with EINVAL rather than silently truncating.

--
John Baldwin

Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â