* [PATCH] [AArch64] Mark single precision pseudo registers unavailable if invalid
@ 2015-07-15 10:53 Pierre Langlois
2015-07-15 14:01 ` Yao Qi
0 siblings, 1 reply; 4+ messages in thread
From: Pierre Langlois @ 2015-07-15 10:53 UTC (permalink / raw)
To: gdb-patches; +Cc: Pierre Langlois
Hi all,
I noticed two failure in gdb.trace/mi-trace-frame-collected.exp:
FAIL: gdb.trace/mi-trace-frame-collected.exp: live:
-trace-frame-collected (register)
FAIL: gdb.trace/mi-trace-frame-collected.exp: tfile:
-trace-frame-collected (register)
In these cases, we are not collecting registers so the MI command
-trace-frame-collected should only give us the value of the PC.
However, it also gives us all of the single precision pseudo registers,
initialized with 0x0.
We can reproduce this error by simply issuing the
'maint print cooked-register' when no inferior is connected:
~~~
...
(gdb) maint print cooked-register
Name Nr Rel Offset Size Type Cooked value
x0 0 0 0 8 long <unavailable>
x1 1 1 8 8 long <unavailable>
...
d30 130 62 1540 8 *1 <unavailable>
d31 131 63 1548 8 *1 <unavailable>
s0 132 64 1556 4 *1 0x00000000
s1 133 65 1560 4 *1 0x00000000
s2 134 66 1564 4 *1 0x00000000
...
s28 160 92 1668 4 *1 0x00000000
s29 161 93 1672 4 *1 0x00000000
s30 162 94 1676 4 *1 0x00000000
s31 163 95 1680 4 *1 0x00000000
h0 164 96 1684 2 *1 <unavailable>
h1 165 97 1686 2 *1 <unavailable>
h2 166 98 1688 2 *1 <unavailable>
...
~~~
It turns out GDB does not check if S registers are valid before returning
a value for them. It should return <unavailable> in this case.
Thanks,
Pierre
gdb/ChangeLog:
* aarch64-tdep.c (aarch64_pseudo_read_value): Mark S register as
unavailable if invalid.
---
gdb/aarch64-tdep.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 2cecad0..cec4d3e 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2482,7 +2482,11 @@ aarch64_pseudo_read_value (struct gdbarch *gdbarch,
v_regnum = AARCH64_V0_REGNUM + regnum - AARCH64_S0_REGNUM;
status = regcache_raw_read (regcache, v_regnum, reg_buf);
- memcpy (buf, reg_buf, S_REGISTER_SIZE);
+ if (status != REG_VALID)
+ mark_value_bytes_unavailable (result_value, 0,
+ TYPE_LENGTH (value_type (result_value)));
+ else
+ memcpy (buf, reg_buf, S_REGISTER_SIZE);
return result_value;
}
--
2.1.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] [AArch64] Mark single precision pseudo registers unavailable if invalid
2015-07-15 10:53 [PATCH] [AArch64] Mark single precision pseudo registers unavailable if invalid Pierre Langlois
@ 2015-07-15 14:01 ` Yao Qi
2015-07-15 14:41 ` Yao Qi
0 siblings, 1 reply; 4+ messages in thread
From: Yao Qi @ 2015-07-15 14:01 UTC (permalink / raw)
To: Pierre Langlois; +Cc: gdb-patches
Pierre Langlois <pierre.langlois@arm.com> writes:
> It turns out GDB does not check if S registers are valid before returning
> a value for them. It should return <unavailable> in this case.
Yes, your fix is correct, however ...
>
> Thanks,
> Pierre
>
> gdb/ChangeLog:
>
> * aarch64-tdep.c (aarch64_pseudo_read_value): Mark S register as
> unavailable if invalid.
... when I read your patch, I am wondering why does aarch64 implement
gdbarch method pseudo_register_read_value rather than
pseudo_register_read. If we implement the pseudo_register_read, the
caller will mark the value unavailable according to its return value.
pseudo_register_read_value was added to handle partially available
registers by https://sourceware.org/ml/gdb-patches/2011-07/msg00351.html
but I don't think of a case that some aarch64 register is partially
available. Maybe, another fix to this problem is to implement
pseudo_register_read instead of pseudo_register_read_value.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] [AArch64] Mark single precision pseudo registers unavailable if invalid
2015-07-15 14:01 ` Yao Qi
@ 2015-07-15 14:41 ` Yao Qi
2015-07-16 9:09 ` Pierre Langlois
0 siblings, 1 reply; 4+ messages in thread
From: Yao Qi @ 2015-07-15 14:41 UTC (permalink / raw)
To: Pierre Langlois; +Cc: gdb-patches
On 15/07/15 15:00, Yao Qi wrote:
> ... when I read your patch, I am wondering why does aarch64 implement
> gdbarch method pseudo_register_read_value rather than
> pseudo_register_read. If we implement the pseudo_register_read, the
> caller will mark the value unavailable according to its return value.
> pseudo_register_read_value was added to handle partially available
> registers byhttps://sourceware.org/ml/gdb-patches/2011-07/msg00351.html
> but I don't think of a case that some aarch64 register is partially
> available. Maybe, another fix to this problem is to implement
> pseudo_register_read instead of pseudo_register_read_value.
To be clear, your patch can be pushed in, as it is correct and fixes
fails in tests. We can discuss and implement pseudo_register_read
rather than pseudo_register_read_value for aarch64 later in the
follow-up patch.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] [AArch64] Mark single precision pseudo registers unavailable if invalid
2015-07-15 14:41 ` Yao Qi
@ 2015-07-16 9:09 ` Pierre Langlois
0 siblings, 0 replies; 4+ messages in thread
From: Pierre Langlois @ 2015-07-16 9:09 UTC (permalink / raw)
To: Yao Qi; +Cc: pierre.langlois, gdb-patches
On 15/07/15 15:41, Yao Qi wrote:
> On 15/07/15 15:00, Yao Qi wrote:
>> ... when I read your patch, I am wondering why does aarch64 implement
>> gdbarch method pseudo_register_read_value rather than
>> pseudo_register_read. If we implement the pseudo_register_read, the
>> caller will mark the value unavailable according to its return value.
>> pseudo_register_read_value was added to handle partially available
>> registers byhttps://sourceware.org/ml/gdb-patches/2011-07/msg00351.html
>> but I don't think of a case that some aarch64 register is partially
>> available. Maybe, another fix to this problem is to implement
>> pseudo_register_read instead of pseudo_register_read_value.
Yes, it makes sense to me. I don't think we should have partially
available V registers in any cases. For example, reading the S0 register
just ignores the top-level bits of the Q0 register, but they are still
available to read through Q0. And writing to S0 will clear the top-level
bits of Q0. As it is implemented in aarch64_pseudo_write.
>
> To be clear, your patch can be pushed in, as it is correct and fixes
> fails in tests. We can discuss and implement pseudo_register_read
> rather than pseudo_register_read_value for aarch64 later in the
> follow-up patch.
>
OK, I'll push it in and make sure pseudo_register_read works as expected in
a follow-up patch.
Thanks,
Pierre
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-07-16 9:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-15 10:53 [PATCH] [AArch64] Mark single precision pseudo registers unavailable if invalid Pierre Langlois
2015-07-15 14:01 ` Yao Qi
2015-07-15 14:41 ` Yao Qi
2015-07-16 9:09 ` Pierre Langlois
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).