* [PATCH] Fix PTRACE_GETREGSET failure for compat inferiors on arm64
@ 2016-12-02 21:46 Kees Cook
2016-12-02 22:49 ` Yao Qi
0 siblings, 1 reply; 10+ messages in thread
From: Kees Cook @ 2016-12-02 21:46 UTC (permalink / raw)
To: gdb-patches; +Cc: brian.murray, matthias.klose
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
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>
* 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);
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);
else if (tdep->have_wmmx_registers
&& regno >= ARM_WR0_REGNUM && regno <= ARM_WCGR7_REGNUM)
@@ -420,17 +422,19 @@ 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);
+ else
+ store_fpregs (regcache);
}
else
{
if (regno < ARM_F0_REGNUM || regno == ARM_PS_REGNUM)
store_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))
store_fpregs (regcache);
else if (tdep->have_wmmx_registers
&& regno >= ARM_WR0_REGNUM && regno <= ARM_WCGR7_REGNUM)
--
2.7.4
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Fix PTRACE_GETREGSET failure for compat inferiors on arm64
2016-12-02 21:46 [PATCH] Fix PTRACE_GETREGSET failure for compat inferiors on arm64 Kees Cook
@ 2016-12-02 22:49 ` Yao Qi
2016-12-02 23:08 ` Kees Cook
2016-12-02 23:50 ` Doug Evans
0 siblings, 2 replies; 10+ messages in thread
From: Yao Qi @ 2016-12-02 22:49 UTC (permalink / raw)
To: Kees Cook; +Cc: gdb-patches, brian.murray, matthias.klose
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."
> 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.
>
> * 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.
> 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.
--
Yao
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Fix PTRACE_GETREGSET failure for compat inferiors on arm64
2016-12-02 22:49 ` Yao Qi
@ 2016-12-02 23:08 ` Kees Cook
2016-12-04 11:11 ` Yao Qi
2016-12-02 23:50 ` Doug Evans
1 sibling, 1 reply; 10+ messages in thread
From: Kees Cook @ 2016-12-02 23:08 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches, brian.murray, matthias.klose
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Fix PTRACE_GETREGSET failure for compat inferiors on arm64
2016-12-02 23:08 ` Kees Cook
@ 2016-12-04 11:11 ` Yao Qi
2016-12-04 19:30 ` Kees Cook
0 siblings, 1 reply; 10+ messages in thread
From: Yao Qi @ 2016-12-04 11:11 UTC (permalink / raw)
To: Kees Cook; +Cc: gdb-patches, Brian Murray, Matthias Klose
On Fri, Dec 2, 2016 at 11:08 PM, Kees Cook <keescook@chromium.org> wrote:
>>> 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.
>
What is I meant is that instead of calling fetch_fpregs unconditionally,
we call fetch_fpregs if tdep->have_fpa_registers is true, like this,
if (tdep->have_fpa_registers)
fetch_fpregs (regcache);
IOW, we only fetch FPA registers if we know FPA registers are available,
as described in target description.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Fix PTRACE_GETREGSET failure for compat inferiors on arm64
2016-12-04 11:11 ` Yao Qi
@ 2016-12-04 19:30 ` Kees Cook
2016-12-05 10:26 ` Yao Qi
0 siblings, 1 reply; 10+ messages in thread
From: Kees Cook @ 2016-12-04 19:30 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches, Brian Murray, Matthias Klose
On Sun, Dec 4, 2016 at 3:11 AM, Yao Qi <qiyaoltc@gmail.com> wrote:
> On Fri, Dec 2, 2016 at 11:08 PM, Kees Cook <keescook@chromium.org> wrote:
>>>> 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.
>>
>
> What is I meant is that instead of calling fetch_fpregs unconditionally,
> we call fetch_fpregs if tdep->have_fpa_registers is true, like this,
>
> if (tdep->have_fpa_registers)
> fetch_fpregs (regcache);
>
> IOW, we only fetch FPA registers if we know FPA registers are available,
> as described in target description.
Right, I was asking how the target description is built. I couldn't
determine where FPA was detected.
-Kees
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Fix PTRACE_GETREGSET failure for compat inferiors on arm64
2016-12-04 19:30 ` Kees Cook
@ 2016-12-05 10:26 ` Yao Qi
2016-12-05 16:06 ` Kees Cook
0 siblings, 1 reply; 10+ messages in thread
From: Yao Qi @ 2016-12-05 10:26 UTC (permalink / raw)
To: Kees Cook; +Cc: gdb-patches, Brian Murray, Matthias Klose
On 16-12-04 11:30:36, Kees Cook wrote:
> Right, I was asking how the target description is built. I couldn't
> determine where FPA was detected.
>
It is detected in arm-tdep.c:arm_gdbarch_init, which looks for feature
org.gnu.gdb.arm.fpa, and set have_fpa_registers to 1 if this feature is
found.
--
Yao
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Fix PTRACE_GETREGSET failure for compat inferiors on arm64
2016-12-05 10:26 ` Yao Qi
@ 2016-12-05 16:06 ` Kees Cook
2016-12-09 9:27 ` Yao Qi
0 siblings, 1 reply; 10+ messages in thread
From: Kees Cook @ 2016-12-05 16:06 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches, Brian Murray, Matthias Klose
On Mon, Dec 5, 2016 at 2:26 AM, Yao Qi <qiyaoltc@gmail.com> wrote:
> On 16-12-04 11:30:36, Kees Cook wrote:
>> Right, I was asking how the target description is built. I couldn't
>> determine where FPA was detected.
>>
>
> It is detected in arm-tdep.c:arm_gdbarch_init, which looks for feature
> org.gnu.gdb.arm.fpa, and set have_fpa_registers to 1 if this feature is
> found.
Yup, that part I found. What sets org.gnu.gdb.arm.fpa?
-Kees
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Fix PTRACE_GETREGSET failure for compat inferiors on arm64
2016-12-05 16:06 ` Kees Cook
@ 2016-12-09 9:27 ` Yao Qi
0 siblings, 0 replies; 10+ messages in thread
From: Yao Qi @ 2016-12-09 9:27 UTC (permalink / raw)
To: Kees Cook; +Cc: gdb-patches, Brian Murray, Matthias Klose
On 16-12-05 08:06:06, Kees Cook wrote:
> On Mon, Dec 5, 2016 at 2:26 AM, Yao Qi <qiyaoltc@gmail.com> wrote:
> > On 16-12-04 11:30:36, Kees Cook wrote:
> >> Right, I was asking how the target description is built. I couldn't
> >> determine where FPA was detected.
> >>
> >
> > It is detected in arm-tdep.c:arm_gdbarch_init, which looks for feature
> > org.gnu.gdb.arm.fpa, and set have_fpa_registers to 1 if this feature is
> > found.
>
> Yup, that part I found. What sets org.gnu.gdb.arm.fpa?
>
The remote stub may know the architecture support FPA, and reply the
target description with feature org.gnu.gdb.arm.fpa to GDB.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Fix PTRACE_GETREGSET failure for compat inferiors on arm64
2016-12-02 22:49 ` Yao Qi
2016-12-02 23:08 ` Kees Cook
@ 2016-12-02 23:50 ` Doug Evans
2016-12-04 10:31 ` Yao Qi
1 sibling, 1 reply; 10+ messages in thread
From: Doug Evans @ 2016-12-02 23:50 UTC (permalink / raw)
To: Yao Qi; +Cc: Kees Cook, gdb-patches, brian.murray, matthias.klose
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:
>> gdb/ChangeLog:
>>
>> 2016-12-02 Kees Cook <keescook@google.com>
>
> You don't have FSF copyright assignment.
Hi.
Google has a blanket copyright assignment, so this is ok in that regard.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Fix PTRACE_GETREGSET failure for compat inferiors on arm64
2016-12-02 23:50 ` Doug Evans
@ 2016-12-04 10:31 ` Yao Qi
0 siblings, 0 replies; 10+ messages in thread
From: Yao Qi @ 2016-12-04 10:31 UTC (permalink / raw)
To: Doug Evans; +Cc: Kees Cook, gdb-patches, Brian Murray, Matthias Klose
On Fri, Dec 2, 2016 at 11:49 PM, Doug Evans <dje@google.com> wrote:
> 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:
>>> gdb/ChangeLog:
>>>
>>> 2016-12-02 Kees Cook <keescook@google.com>
>>
>> You don't have FSF copyright assignment.
>
> Hi.
> Google has a blanket copyright assignment, so this is ok in that regard.
Hi Doug,
I am bad at parsing /gd/gnuorg/copyright.list :(, you are right. I find it now.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-12-09 9:27 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-02 21:46 [PATCH] Fix PTRACE_GETREGSET failure for compat inferiors on arm64 Kees Cook
2016-12-02 22:49 ` Yao Qi
2016-12-02 23:08 ` Kees Cook
2016-12-04 11:11 ` Yao Qi
2016-12-04 19:30 ` Kees Cook
2016-12-05 10:26 ` Yao Qi
2016-12-05 16:06 ` Kees Cook
2016-12-09 9:27 ` Yao Qi
2016-12-02 23:50 ` Doug Evans
2016-12-04 10:31 ` Yao Qi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox