* [PATCH] Extend handling of immediates on ARM's SystemTap SDT probe support
@ 2013-12-23 19:47 Sergio Durigan Junior
2013-12-24 10:05 ` Marcus Shawcroft
2013-12-28 3:41 ` Joel Brobecker
0 siblings, 2 replies; 6+ messages in thread
From: Sergio Durigan Junior @ 2013-12-23 19:47 UTC (permalink / raw)
To: GDB Patches; +Cc: Marcus Shawcroft, Yufeng Zhang
Hi there,
Continuing my series of fixes on the SystemTap SDT support for the
ARM/AArch64 architectures, this patch now extends how ARM's SDT specific
parser handles literal numbers (immediates).
Currently, it only accepts "#" as the prefix. However, according to
"info '(as) ARM-Chars'", expressions can also have "$" and nothing as a
prefix. This patch extends the parser to accept those options.
It is a rather trivial patch, and tests have proved that it works fine.
OK to apply?
--
Sergio
2013-12-23 Sergio Durigan Junior <sergiodj@redhat.com>
* arm-linux-tdep.c (arm_stap_is_single_operand): Accept "$" as a
literal prefix. Also accept no prefix at all.
(arm_stap_parse_special_token): Likewise.
(arm_linux_init_abi): Likewise.
diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
index 0284f69..df2b8c4 100644
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -1116,7 +1116,7 @@ arm_linux_displaced_step_copy_insn (struct gdbarch *gdbarch,
static int
arm_stap_is_single_operand (struct gdbarch *gdbarch, const char *s)
{
- return (*s == '#' /* Literal number. */
+ return (*s == '#' || *s == '$' || isdigit (*s) /* Literal number. */
|| *s == '[' /* Register indirection or
displacement. */
|| isalpha (*s)); /* Register value. */
@@ -1183,14 +1183,19 @@ arm_stap_parse_special_token (struct gdbarch *gdbarch,
++tmp;
tmp = skip_spaces_const (tmp);
- if (*tmp++ != '#')
- return 0;
+ if (*tmp == '#' || *tmp == '$')
+ ++tmp;
if (*tmp == '-')
{
++tmp;
got_minus = 1;
}
+ else if (*tmp == '+')
+ ++tmp;
+
+ if (!isdigit (*tmp))
+ return 0;
displacement = strtol (tmp, &endp, 10);
tmp = endp;
@@ -1235,7 +1240,7 @@ static void
arm_linux_init_abi (struct gdbarch_info info,
struct gdbarch *gdbarch)
{
- static const char *const stap_integer_prefixes[] = { "#", NULL };
+ static const char *const stap_integer_prefixes[] = { "#", "$", "", NULL };
static const char *const stap_register_prefixes[] = { "r", NULL };
static const char *const stap_register_indirection_prefixes[] = { "[",
NULL };
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Extend handling of immediates on ARM's SystemTap SDT probe support
2013-12-23 19:47 [PATCH] Extend handling of immediates on ARM's SystemTap SDT probe support Sergio Durigan Junior
@ 2013-12-24 10:05 ` Marcus Shawcroft
2013-12-24 15:32 ` Sergio Durigan Junior
2013-12-28 3:41 ` Joel Brobecker
1 sibling, 1 reply; 6+ messages in thread
From: Marcus Shawcroft @ 2013-12-24 10:05 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: GDB Patches, Marcus Shawcroft, Yufeng Zhang
On 23 December 2013 19:47, Sergio Durigan Junior <sergiodj@redhat.com> wrote:
> Hi there,
>
> Continuing my series of fixes on the SystemTap SDT support for the
> ARM/AArch64 architectures, this patch now extends how ARM's SDT specific
> parser handles literal numbers (immediates).
>
> Currently, it only accepts "#" as the prefix. However, according to
> "info '(as) ARM-Chars'", expressions can also have "$" and nothing as a
> prefix. This patch extends the parser to accept those options.
>
> It is a rather trivial patch, and tests have proved that it works fine.
> OK to apply?
Looks fine to me.
This is another example of where the ARM (A32/T32) syntax differs
slightly from A64.
Cheers
/Marcus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Extend handling of immediates on ARM's SystemTap SDT probe support
2013-12-24 10:05 ` Marcus Shawcroft
@ 2013-12-24 15:32 ` Sergio Durigan Junior
2013-12-24 17:28 ` Marcus Shawcroft
0 siblings, 1 reply; 6+ messages in thread
From: Sergio Durigan Junior @ 2013-12-24 15:32 UTC (permalink / raw)
To: Marcus Shawcroft; +Cc: GDB Patches, Marcus Shawcroft, Yufeng Zhang
On Tuesday, December 24 2013, Marcus Shawcroft wrote:
> Looks fine to me.
Thanks Marcus.
I don't see your name listed as an ARM maintainer. Are you? Just
confirming if I should wait for someone else to approve.
--
Sergio
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Extend handling of immediates on ARM's SystemTap SDT probe support
2013-12-24 15:32 ` Sergio Durigan Junior
@ 2013-12-24 17:28 ` Marcus Shawcroft
0 siblings, 0 replies; 6+ messages in thread
From: Marcus Shawcroft @ 2013-12-24 17:28 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: GDB Patches, Marcus Shawcroft, Yufeng Zhang
On 24 December 2013 15:32, Sergio Durigan Junior <sergiodj@redhat.com> wrote:
> I don't see your name listed as an ARM maintainer. Are you? Just
> confirming if I should wait for someone else to approve.
Last response bounced, trying again...
Hi. Sorry. It may look fine to me but I can't approve it. /Marcus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Extend handling of immediates on ARM's SystemTap SDT probe support
2013-12-23 19:47 [PATCH] Extend handling of immediates on ARM's SystemTap SDT probe support Sergio Durigan Junior
2013-12-24 10:05 ` Marcus Shawcroft
@ 2013-12-28 3:41 ` Joel Brobecker
2013-12-28 21:25 ` Sergio Durigan Junior
1 sibling, 1 reply; 6+ messages in thread
From: Joel Brobecker @ 2013-12-28 3:41 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: GDB Patches, Marcus Shawcroft, Yufeng Zhang
> Continuing my series of fixes on the SystemTap SDT support for the
> ARM/AArch64 architectures, this patch now extends how ARM's SDT specific
> parser handles literal numbers (immediates).
>
> Currently, it only accepts "#" as the prefix. However, according to
> "info '(as) ARM-Chars'", expressions can also have "$" and nothing as a
> prefix. This patch extends the parser to accept those options.
>
> It is a rather trivial patch, and tests have proved that it works fine.
> OK to apply?
>
> --
> Sergio
>
> 2013-12-23 Sergio Durigan Junior <sergiodj@redhat.com>
>
> * arm-linux-tdep.c (arm_stap_is_single_operand): Accept "$" as a
> literal prefix. Also accept no prefix at all.
> (arm_stap_parse_special_token): Likewise.
> (arm_linux_init_abi): Likewise.
The patch itself looks fine, except for one (correct) bit which caught
my attention. See below.
>
> diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
> index 0284f69..df2b8c4 100644
> --- a/gdb/arm-linux-tdep.c
> +++ b/gdb/arm-linux-tdep.c
> @@ -1116,7 +1116,7 @@ arm_linux_displaced_step_copy_insn (struct gdbarch *gdbarch,
> static int
> arm_stap_is_single_operand (struct gdbarch *gdbarch, const char *s)
> {
> - return (*s == '#' /* Literal number. */
> + return (*s == '#' || *s == '$' || isdigit (*s) /* Literal number. */
> || *s == '[' /* Register indirection or
> displacement. */
> || isalpha (*s)); /* Register value. */
> @@ -1183,14 +1183,19 @@ arm_stap_parse_special_token (struct gdbarch *gdbarch,
>
> ++tmp;
> tmp = skip_spaces_const (tmp);
> - if (*tmp++ != '#')
> - return 0;
> + if (*tmp == '#' || *tmp == '$')
> + ++tmp;
>
> if (*tmp == '-')
> {
> ++tmp;
> got_minus = 1;
> }
> + else if (*tmp == '+')
> + ++tmp;
> +
> + if (!isdigit (*tmp))
> + return 0;
>
I think you mixed in one change which is unrelated to this patch
(the handling of the '+' sign). Perhaps you had meant to have it
as part of the first patch?
[PATCH v2] Fix for PR tdep/15653: Implement SystemTap SDT probe
support for AArch64
http://www.sourceware.org/ml/gdb-patches/2013-12/msg00887.html
IIRC, this patch was born by extraction out of v1 of the patch above.
If I am correct, this patch is pre-approved with the '+' bit moved to
the correct patch.
Thank you,
--
Joel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Extend handling of immediates on ARM's SystemTap SDT probe support
2013-12-28 3:41 ` Joel Brobecker
@ 2013-12-28 21:25 ` Sergio Durigan Junior
0 siblings, 0 replies; 6+ messages in thread
From: Sergio Durigan Junior @ 2013-12-28 21:25 UTC (permalink / raw)
To: Joel Brobecker; +Cc: GDB Patches, Marcus Shawcroft, Yufeng Zhang
On Saturday, December 28 2013, Joel Brobecker wrote:
>> diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
>> index 0284f69..df2b8c4 100644
>> --- a/gdb/arm-linux-tdep.c
>> +++ b/gdb/arm-linux-tdep.c
>> @@ -1116,7 +1116,7 @@ arm_linux_displaced_step_copy_insn (struct gdbarch *gdbarch,
>> static int
>> arm_stap_is_single_operand (struct gdbarch *gdbarch, const char *s)
>> {
>> - return (*s == '#' /* Literal number. */
>> + return (*s == '#' || *s == '$' || isdigit (*s) /* Literal number. */
>> || *s == '[' /* Register indirection or
>> displacement. */
>> || isalpha (*s)); /* Register value. */
>> @@ -1183,14 +1183,19 @@ arm_stap_parse_special_token (struct gdbarch *gdbarch,
>>
>> ++tmp;
>> tmp = skip_spaces_const (tmp);
>> - if (*tmp++ != '#')
>> - return 0;
>> + if (*tmp == '#' || *tmp == '$')
>> + ++tmp;
>>
>> if (*tmp == '-')
>> {
>> ++tmp;
>> got_minus = 1;
>> }
>> + else if (*tmp == '+')
>> + ++tmp;
>> +
>> + if (!isdigit (*tmp))
>> + return 0;
>>
>
> I think you mixed in one change which is unrelated to this patch
> (the handling of the '+' sign). Perhaps you had meant to have it
> as part of the first patch?
>
> [PATCH v2] Fix for PR tdep/15653: Implement SystemTap SDT probe
> support for AArch64
> http://www.sourceware.org/ml/gdb-patches/2013-12/msg00887.html
>
> IIRC, this patch was born by extraction out of v1 of the patch above.
>
> If I am correct, this patch is pre-approved with the '+' bit moved to
> the correct patch.
You are indeed correct that this change doesn't belong to this patch,
but it also doesn't belong to the AArch64 patch (that's why I didn't
include it there). I will actually post a separate patch for this
change. In fact, there are some other places that could use this
improvement too, so this new patch will include those as well.
Having said that, I removed this piece from the patch and committed the
rest. Thank you a lot for the review!
<https://sourceware.org/ml/gdb-cvs/2013-12/msg00124.html>
--
Sergio
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-12-28 21:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-23 19:47 [PATCH] Extend handling of immediates on ARM's SystemTap SDT probe support Sergio Durigan Junior
2013-12-24 10:05 ` Marcus Shawcroft
2013-12-24 15:32 ` Sergio Durigan Junior
2013-12-24 17:28 ` Marcus Shawcroft
2013-12-28 3:41 ` Joel Brobecker
2013-12-28 21:25 ` Sergio Durigan Junior
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).