This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Fix PTRACE_GETREGSET failure for compat inferiors on arm64
- From: Kees Cook <keescook at chromium dot org>
- To: Yao Qi <qiyaoltc at gmail dot com>
- Cc: gdb-patches at sourceware dot org, brian dot murray at canonical dot com, matthias dot klose at canonical dot com
- Date: Fri, 2 Dec 2016 15:08:00 -0800
- Subject: Re: [PATCH] Fix PTRACE_GETREGSET failure for compat inferiors on arm64
- Authentication-results: sourceware.org; auth=none
- References: <20161202214613.GA54717@beast> <20161202224952.panaxwmmrx4emord@localhost>
On Fri, Dec 2, 2016 at 2:49 PM, Yao Qi <qiyaoltc@gmail.com> wrote:
> On 16-12-02 13:46:13, Kees Cook wrote:
>> When running a 32-bit ARM inferior on a 64-bit ARM host, only the hardware
>> floating-point registers (NT_ARM_VFP) are available. If the inferior
>> uses hard-float, do not request soft-float registers (NT_PRFPREG) and
>> run the risk of failing with EINVAL. This is most noticeably exposed
>
> "soft-float" is not accurate. FPA is coprocessor. Both VFP and FPA is
> implemented in the combination of software and hardware. I'd like to
> rewrite the commit log like this,
>
> "When running a 32-bit ARM inferior with a 32-bit ARM GDB on 64-bit
> AArch64 host, only VFP registers (NT_ARM_VFP) are available. The FPA
> registers (NT_PRFPREG) is not available."
That would be fine by me. I was kind of scratching my head over the
naming of the types of floating-point registers. :) Whatever the case,
arm64 doesn't support FPA, so an inferior using FPA couldn't run there
to start with. :)
>> when running "generate-core-file":
>>
>> (gdb) generate-core-file myprog.core
>> Unable to fetch the floating point registers.: Invalid argument.
>>
>> ptrace(PTRACE_GETREGSET, 27642, NT_FPREGSET, 0xffcc67f0) = -1 EINVAL (Invalid argument)
>>
>> gdb/ChangeLog:
>>
>> 2016-12-02 Kees Cook <keescook@google.com>
>
> You don't have FSF copyright assignment.
Oh, hm, I thought there might be a Google-contributions-to-gdb one I
was already covered under. What's the best approach for me to take to
fix this?
>> * gdb/arm-linux-nat.c: Skip soft-float registers when using hard-float.
>>
>> ---
>> gdb/arm-linux-nat.c | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
>> index d11bdc6..2126cd7 100644
>> --- a/gdb/arm-linux-nat.c
>> +++ b/gdb/arm-linux-nat.c
>> @@ -384,17 +384,19 @@ arm_linux_fetch_inferior_registers (struct target_ops *ops,
>> if (-1 == regno)
>> {
>> fetch_regs (regcache);
>> - fetch_fpregs (regcache);
>
> We should only call fetch_fpregs if tdep->have_fpa_registers is true.
I couldn't determine how this was handled. What actually sets
org.gnu.gdb.arm.fpa in tdesc? I found gdb/features/arm/arm-fpa.xml and
seems to imply it's always included with arm? I wasn't able to follow,
but it seemed like _having_ VFP was a indicator that FPA wasn't used.
>
>> if (tdep->have_wmmx_registers)
>> fetch_wmmx_regs (regcache);
>> if (tdep->vfp_register_count > 0)
>> fetch_vfp_regs (regcache);
>> + else
>> + fetch_fpregs (regcache);
>> }
>> - else
>> + else
>> {
>> if (regno < ARM_F0_REGNUM || regno == ARM_PS_REGNUM)
>> fetch_regs (regcache);
>> - else if (regno >= ARM_F0_REGNUM && regno <= ARM_FPS_REGNUM)
>> + else if (tdep->vfp_register_count == 0
>> + && regno >= ARM_F0_REGNUM && regno <= ARM_FPS_REGNUM)
>> fetch_fpregs (regcache);
>
> Do we really need this change? If FPA registers are not available,
> REGNO can't fall in this range (ARM_F0_REGNUM, ARM_FPS_REGNUM).
>
> These two comments above also apply to store registers.
It seemed like a reasonable change to make, but I see your point.
-Kees
--
Kees Cook
Nexus Security