Re: [PATCH] Extend handling of immediates on ARM's SystemTap SDT probe support

[Joel Brobecker <brobecker@adacore.com>]

> 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