[PATCH] systemtap: Fix parsing of literals in SDT_V3 operand for s390x and ia64

[PATCH] systemtap: Fix parsing of literals in SDT_V3 operand for s390x and ia64

[Athira Rajeev]

systemtap fails to parse literal arguments of the format 'N' ( for example in s390x and ia64 )
The regular expression to match literals is defined in function "visit_target_symbol_arg" as below:

^[i\\$#][-]?[0-9][0-9]*$

In other architectures,
x86_64 - It is of the form $N, which has $ before the literal.
powerpc - It is of the form iN which has i before the literal.

This fails in s390x where literal is of the form 'N' which doesnt have leading $ or i.

This can be reproduced using simple test code and stap script as below:
This test is for SDT_USE_VARIADIC where STAP_PROBEV macro is used.

========================
cat sdt_va_args.c
#define SDT_USE_VARIADIC
#include "sys/sdt.h"

int main()
{
    STAP_PROBEV(test, mark_z);
    STAP_PROBEV(test, mark_a, 10);
    return 0;
}

========================
cat sdt.stp
probe process(@1).mark("mark_z")
{
  printf("_\n");
}

probe process(@1).mark("mark_a")
{
  printf("%d\n", $arg1);
}

========================

Running with stap,

stap sdt.stp sdt_va_args.exe -c ./sdt_va_args.exe
WARNING: Can't parse SDT_V3 operand '10': identifier '$arg1' at sdt.stp:8:18
 source:   printf("%d\n", $arg1);
                          ^
semantic error: unable to find local 'arg1' near pc 0x8000050a  in  main sdt_va_args.c (): identifier '$arg1' at :8:18
        source:   printf("%d\n", $arg1);
                                 ^
Pass 2: analysis failed.  Try again with another '--vp 01' option.

Taking the objdump,

objdump -s -j .note.stapsdt ./sdt_va_args.exe

./sdt_va_args.exe:     file format elf64-s390

Contents of section .note.stapsdt:
 0000 00000008 00000025 00000003 73746170  .......%....stap
 0010 73647400 00000000 80000506 00000000  sdt.............
 0020 80000638 00000000 00000000 74657374  ...8........test
 0030 006d6172 6b5f7a00 00000000 00000008  .mark_z.........
 0040 0000002a 00000003 73746170 73647400  ...*....stapsdt.
 0050 00000000 8000050a 00000000 80000638  ...............8
 0060 00000000 00000000 74657374 006d6172  ........test.mar
 0070 6b5f6100 2d344031 30000000           k_a.-4@10...

Where the argument is of the format "-4@10"

Here stap fails to parse argument of this format ( when it is passed as value directly ).

Below patch in tapsets.cxx fixes this issue:
Here the regex is changed to match literals of form 'N'
Also changed the way it extracts the value as s390x arg format doesnt have leading $, i or # before literal.
This could happen in ia64 also as it has the same format. I have tested this in s390x only.

Signed-off-by: Athira Rajeev <atrajeev@in.ibm.com>
---

 tapsets.cxx |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/tapsets.cxx b/tapsets.cxx
index 71a4ad8..bdba7f3 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -5451,11 +5451,17 @@ sdt_uprobe_var_expanding_visitor::visit_target_symbol_arg (target_symbol *e)
       // anyway.  With -mregnames, we could, if gcc somehow
       // communicated to us the presence of that option, but alas it
       // doesn't.  http://gcc.gnu.org/PR44995.
-      rc = regexp_match (asmarg, "^[i\\$#][-]?[0-9][0-9]*$", matches);
+      rc = regexp_match (asmarg, "^[i\\$#]?[-]?[0-9][0-9]*$", matches);
       if (! rc)
         {
-	  string sn = matches[0].substr(1);
+	  int k=0;
 	  int64_t n;
+	  string sn;
+	  const char *tmp = matches[0].c_str();
+	  if ( tmp[k] == 'i' || tmp[k] == '$' || tmp[k] == '#')
+		k++;
+	  sn = matches[0].substr(k);
+
 	  try
 	    {
 	      // We have to pay attention to the size & sign, as gcc sometimes

Re: [PATCH] systemtap: Fix parsing of literals in SDT_V3 operand for s390x and ia64

[Frank Ch. Eigler]

atrajeev wrote:

> systemtap fails to parse literal arguments of the format 'N' ( for
> example in s390x and ia64 ) The regular expression to match literals
> is defined in function "visit_target_symbol_arg" as below:
>
> ^[i\\$#][-]?[0-9][0-9]*$
> [...]
> This fails in s390x where literal is of the form 'N' which doesnt
> have leading $ or i.

Can the s390 do the same hack done for powerpc and add the 'i' prefix in
sys/sdt.h?

#if defined __powerpc__ || defined __powerpc64__
# define _SDT_ARGTMPL(id)       %I[id]%[id]
#else
# define _SDT_ARGTMPL(id)       %[id]
#endif

> [...]
> -      rc = regexp_match (asmarg, "^[i\\$#][-]?[0-9][0-9]*$", matches);
> +      rc = regexp_match (asmarg, "^[i\\$#]?[-]?[0-9][0-9]*$", matches);

I would be worried about this change because of possible conflicts
with the other operand addressing-mode combinations.  (Recall that 'i'
is added for ppc in order to separate register-number and
literal-number cases.)

- FChE

Re: [PATCH] systemtap: Fix parsing of literals in SDT_V3 operand for s390x and ia64

[Suzuki K. Poulose]

On 10/09/2012 09:04 PM, Frank Ch. Eigler wrote:
>
> atrajeev wrote:
>
>> systemtap fails to parse literal arguments of the format 'N' ( for
>> example in s390x and ia64 ) The regular expression to match literals
>> is defined in function "visit_target_symbol_arg" as below:
>>
>> ^[i\\$#][-]?[0-9][0-9]*$
>> [...]
>> This fails in s390x where literal is of the form 'N' which doesnt
>> have leading $ or i.
>
> Can the s390 do the same hack done for powerpc and add the 'i' prefix in
> sys/sdt.h?
>
> #if defined __powerpc__ || defined __powerpc64__
> # define _SDT_ARGTMPL(id)       %I[id]%[id]
> #else
> # define _SDT_ARGTMPL(id)       %[id]
> #endif

Frank,

We will try this and let you know the results. I guess we could do this
for ia64 as well.

>
>> [...]
>> -      rc = regexp_match (asmarg, "^[i\\$#][-]?[0-9][0-9]*$", matches);
>> +      rc = regexp_match (asmarg, "^[i\\$#]?[-]?[0-9][0-9]*$", matches);
>
> I would be worried about this change because of possible conflicts
> with the other operand addressing-mode combinations.  (Recall that 'i'
> is added for ppc in order to separate register-number and
> literal-number cases.)

OK.

Thanks
Suzuki
>
> - FChE
>

Re: [PATCH] systemtap: Fix parsing of literals in SDT_V3 operand for s390x and ia64

[Frank Ch. Eigler]

"Suzuki K. Poulose" <suzuki@in.ibm.com> writes:

> [...]
>> #if defined __powerpc__ || defined __powerpc64__
>> # define _SDT_ARGTMPL(id)       %I[id]%[id]
>> #else
>> # define _SDT_ARGTMPL(id)       %[id]
>> #endif
>
> We will try this and let you know the results. I guess we could do this
> for ia64 as well.

It doesn't seem as though gcc supports %I for anything other than
powerpc (and there it's done because some of the assembler instruction
mnemonics are modified).  I opened PR14715 to track further work on this.


- FChE