[v3] Fix PTRACE_GETREGSET failure for compat inferiors on arm64

Message ID 20170119210015.GA87383@beast
State New, archived
Headers

Commit Message

Kees Cook Jan. 19, 2017, 9 p.m. UTC
  When running a 32-bit ARM inferior with a 32-bit ARM GDB on a 64-bit
AArch64 host, only VFP registers (NT_ARM_VFP) are available. The FPA
registers (NT_PRFPREG) are not available so GDB must not request them, as
this will fail with -EINVAL.  This is most noticeably exposed 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-19  Kees Cook  <keescook@google.com>

    * gdb/arm-linux-nat.c: Skip soft-float registers when using hard-float.
---
v3:
- argh, actually drop needless other change.
v2:
- check have_fpa_registers instead, dropped needless other change: qiyaoltc
---
 gdb/arm-linux-nat.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
  

Comments

Kees Cook Jan. 26, 2017, 7:43 p.m. UTC | #1
On Thu, Jan 19, 2017 at 1:00 PM, Kees Cook <keescook@chromium.org> wrote:
> When running a 32-bit ARM inferior with a 32-bit ARM GDB on a 64-bit
> AArch64 host, only VFP registers (NT_ARM_VFP) are available. The FPA
> registers (NT_PRFPREG) are not available so GDB must not request them, as
> this will fail with -EINVAL.  This is most noticeably exposed 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-19  Kees Cook  <keescook@google.com>
>
>     * gdb/arm-linux-nat.c: Skip soft-float registers when using hard-float.

Hi! Friendly ping on this patch. Are there any corrections to be made,
or can someone commit it?

Thanks!

-Kees

> ---
> v3:
> - argh, actually drop needless other change.
> v2:
> - check have_fpa_registers instead, dropped needless other change: qiyaoltc
> ---
>  gdb/arm-linux-nat.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
> index d11bdc6..4fa5204 100644
> --- a/gdb/arm-linux-nat.c
> +++ b/gdb/arm-linux-nat.c
> @@ -384,13 +384,14 @@ arm_linux_fetch_inferior_registers (struct target_ops *ops,
>    if (-1 == regno)
>      {
>        fetch_regs (regcache);
> -      fetch_fpregs (regcache);
>        if (tdep->have_wmmx_registers)
>         fetch_wmmx_regs (regcache);
>        if (tdep->vfp_register_count > 0)
>         fetch_vfp_regs (regcache);
> +      if (tdep->have_fpa_registers)
> +       fetch_fpregs (regcache);
>      }
> -  else
> +  else
>      {
>        if (regno < ARM_F0_REGNUM || regno == ARM_PS_REGNUM)
>         fetch_regs (regcache);
> @@ -420,11 +421,12 @@ arm_linux_store_inferior_registers (struct target_ops *ops,
>    if (-1 == regno)
>      {
>        store_regs (regcache);
> -      store_fpregs (regcache);
>        if (tdep->have_wmmx_registers)
>         store_wmmx_regs (regcache);
>        if (tdep->vfp_register_count > 0)
>         store_vfp_regs (regcache);
> +      if (tdep->have_fpa_registers)
> +       store_fpregs (regcache);
>      }
>    else
>      {
> --
> 2.7.4
>
>
> --
> Kees Cook
> Nexus Security
  
Yao Qi Jan. 26, 2017, 11:23 p.m. UTC | #2
On Thu, Jan 19, 2017 at 9:00 PM, Kees Cook <keescook@chromium.org> wrote:
>

The patch is good to me, some nits below,

> gdb/ChangeLog:
>
> 2016-12-19  Kees Cook  <keescook@google.com>
>
>     * gdb/arm-linux-nat.c: Skip soft-float registers when using hard-float.

* gdb/arm-linux-nat.c (arm_linux_fetch_inferior_registers): Call fetch_fpregs
if target has fpa registers.
(arm_linux_store_inferior_registers): Call store_fpregs if target has
fpa registers.

>      }
> -  else
> +  else
>      {

Unnecessary change.  I'll push your patch in tomorrow.
  
Kees Cook Jan. 26, 2017, 11:37 p.m. UTC | #3
On Thu, Jan 26, 2017 at 3:23 PM, Yao Qi <qiyaoltc@gmail.com> wrote:
> On Thu, Jan 19, 2017 at 9:00 PM, Kees Cook <keescook@chromium.org> wrote:
>>
>
> The patch is good to me, some nits below,
>
>> gdb/ChangeLog:
>>
>> 2016-12-19  Kees Cook  <keescook@google.com>
>>
>>     * gdb/arm-linux-nat.c: Skip soft-float registers when using hard-float.
>
> * gdb/arm-linux-nat.c (arm_linux_fetch_inferior_registers): Call fetch_fpregs
> if target has fpa registers.
> (arm_linux_store_inferior_registers): Call store_fpregs if target has
> fpa registers.

Ah-ha, thanks!

>
>>      }
>> -  else
>> +  else
>>      {
>
> Unnecessary change.  I'll push your patch in tomorrow.

Okay, I wasn't sure if whitespace/code-style fixes should be merged as
they were found.

Thanks!

-Kees
  
Yao Qi Jan. 27, 2017, 11:28 a.m. UTC | #4
On 17-01-26 23:23:30, Yao Qi wrote:
> Unnecessary change.  I'll push your patch in tomorrow.
> 

Patch is pushed in!  Thanks for your contribution.
  
Kees Cook Jan. 27, 2017, 8:48 p.m. UTC | #5
On Fri, Jan 27, 2017 at 3:28 AM, Yao Qi <qiyaoltc@gmail.com> wrote:
> On 17-01-26 23:23:30, Yao Qi wrote:
>> Unnecessary change.  I'll push your patch in tomorrow.
>>
>
> Patch is pushed in!  Thanks for your contribution.

Great! Thanks for your help with it! :)

-Kees
  

Patch

diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index d11bdc6..4fa5204 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -384,13 +384,14 @@  arm_linux_fetch_inferior_registers (struct target_ops *ops,
   if (-1 == regno)
     {
       fetch_regs (regcache);
-      fetch_fpregs (regcache);
       if (tdep->have_wmmx_registers)
 	fetch_wmmx_regs (regcache);
       if (tdep->vfp_register_count > 0)
 	fetch_vfp_regs (regcache);
+      if (tdep->have_fpa_registers)
+	fetch_fpregs (regcache);
     }
-  else 
+  else
     {
       if (regno < ARM_F0_REGNUM || regno == ARM_PS_REGNUM)
 	fetch_regs (regcache);
@@ -420,11 +421,12 @@  arm_linux_store_inferior_registers (struct target_ops *ops,
   if (-1 == regno)
     {
       store_regs (regcache);
-      store_fpregs (regcache);
       if (tdep->have_wmmx_registers)
 	store_wmmx_regs (regcache);
       if (tdep->vfp_register_count > 0)
 	store_vfp_regs (regcache);
+      if (tdep->have_fpa_registers)
+	store_fpregs (regcache);
     }
   else
     {