Re: [PATCH v6 21/21] perf-probe: Add array argument support

[Masami Hiramatsu]

On Thu, 22 Mar 2018 16:19:46 +0530
Ravi Bangoria  wrote:

> Hi Masami :)
> 
> On 03/22/2018 03:53 PM, Masami Hiramatsu wrote:
> > On Mon, 19 Mar 2018 13:29:59 +0530
> > Ravi Bangoria  wrote:
> >
> >>
> >> Is it okay to allow user to specify array size with type field?
> > Fro this patch, yes.
> 
> So IIUC, perf _tool_ will allow user to record array either with "name[range]"
> or by "name:type[length]". Please correct me if that's wrong.

Yes, it is correct.

> And If perf tool will allow array length along with TYPE field, I guess we 
> should
> document that in man perf-probe?

Ah, right. OK, I'll add it.

Thanks!

> 
> Otherwise,
> 
> Acked-by: Ravi Bangoria 
> 
> Thanks,
> Ravi
> 
> >  The availability of type is checked only when
> > it is automatically generated.
> > IMO, it should be done in another patch, something like
> > "Validate user specified type casting" patch. Would you need it?
> >
> > Thank you,
> >
> 


-- 
Masami Hiramatsu 

Re: [PATCH v6 21/21] perf-probe: Add array argument support

[Masami Hiramatsu]

On Thu, 22 Mar 2018 16:19:46 +0530
Ravi Bangoria  wrote:

> Hi Masami :)
> 
> On 03/22/2018 03:53 PM, Masami Hiramatsu wrote:
> > On Mon, 19 Mar 2018 13:29:59 +0530
> > Ravi Bangoria  wrote:
> >
> >>
> >> Is it okay to allow user to specify array size with type field?
> > Fro this patch, yes.
> 
> So IIUC, perf _tool_ will allow user to record array either with "name[range]"
> or by "name:type[length]". Please correct me if that's wrong.

Yes, it is correct.

> And If perf tool will allow array length along with TYPE field, I guess we 
> should
> document that in man perf-probe?

Ah, right. OK, I'll add it.

Thanks!

> 
> Otherwise,
> 
> Acked-by: Ravi Bangoria 
> 
> Thanks,
> Ravi
> 
> >  The availability of type is checked only when
> > it is automatically generated.
> > IMO, it should be done in another patch, something like
> > "Validate user specified type casting" patch. Would you need it?
> >
> > Thank you,
> >
> 


-- 
Masami Hiramatsu 

Re: [PATCH v6 21/21] perf-probe: Add array argument support

[Ravi Bangoria]

Hi Masami :)

On 03/22/2018 03:53 PM, Masami Hiramatsu wrote:
> On Mon, 19 Mar 2018 13:29:59 +0530
> Ravi Bangoria  wrote:
>
>>
>> Is it okay to allow user to specify array size with type field?
> Fro this patch, yes.

So IIUC, perf _tool_ will allow user to record array either with "name[range]"
or by "name:type[length]". Please correct me if that's wrong.

And If perf tool will allow array length along with TYPE field, I guess we 
should
document that in man perf-probe?

Otherwise,

Acked-by: Ravi Bangoria 

Thanks,
Ravi

>  The availability of type is checked only when
> it is automatically generated.
> IMO, it should be done in another patch, something like
> "Validate user specified type casting" patch. Would you need it?
>
> Thank you,
>

Re: [PATCH v6 21/21] perf-probe: Add array argument support

[Ravi Bangoria]

Hi Masami :)

On 03/22/2018 03:53 PM, Masami Hiramatsu wrote:
> On Mon, 19 Mar 2018 13:29:59 +0530
> Ravi Bangoria  wrote:
>
>>
>> Is it okay to allow user to specify array size with type field?
> Fro this patch, yes.

So IIUC, perf _tool_ will allow user to record array either with "name[range]"
or by "name:type[length]". Please correct me if that's wrong.

And If perf tool will allow array length along with TYPE field, I guess we 
should
document that in man perf-probe?

Otherwise,

Acked-by: Ravi Bangoria 

Thanks,
Ravi

>  The availability of type is checked only when
> it is automatically generated.
> IMO, it should be done in another patch, something like
> "Validate user specified type casting" patch. Would you need it?
>
> Thank you,
>

Re: [PATCH v6 21/21] perf-probe: Add array argument support

[Masami Hiramatsu]

On Mon, 19 Mar 2018 13:29:59 +0530
Ravi Bangoria  wrote:

> Hi Masami,
> 
> On 03/17/2018 06:23 PM, Masami Hiramatsu wrote:
> > Since kprobes events support an array argument, perf-probe
> > can also support dumping array now.
> > The syntax are
> >
> >  []
> > or
> >  []
> >
> > where the  is ... e.g. array[0..5].
> > This can also be available with string type. In this
> > case, the string array should be "char *array[]" or
> > "char **array_ptr".
> >
> > Note that this feature is only available on the kernel which
> > supports array type.
> 
> User can still do,
> 
> # perf probe -x ~/hello main:3 'a=a:x32[3]'
> 
> which will successfully be installed in uprobe_events. But for a
> perf tool, x32[3] is not a valid type. And it seems perf tool don't
> validate the 'type' field:
> 
>     static int parse_perf_probe_arg(char *str, struct perf_probe_arg *arg)
>     {
>     
>     tmp = strchr(str, ':');
>     if (tmp) {  /* Type setting */
>     *tmp = '\0';
>     arg->type = strdup(tmp + 1);
>     if (arg->type == NULL)
>     return -ENOMEM;
>     pr_debug("type:%s ", arg->type);
>     }
> 
> Is it okay to allow user to specify array size with type field?

Fro this patch, yes. The availability of type is checked only when
it is automatically generated.
IMO, it should be done in another patch, something like
"Validate user specified type casting" patch. Would you need it?

Thank you,

-- 
Masami Hiramatsu 

Re: [PATCH v6 21/21] perf-probe: Add array argument support

[Masami Hiramatsu]

On Mon, 19 Mar 2018 13:29:59 +0530
Ravi Bangoria  wrote:

> Hi Masami,
> 
> On 03/17/2018 06:23 PM, Masami Hiramatsu wrote:
> > Since kprobes events support an array argument, perf-probe
> > can also support dumping array now.
> > The syntax are
> >
> >  []
> > or
> >  []
> >
> > where the  is ... e.g. array[0..5].
> > This can also be available with string type. In this
> > case, the string array should be "char *array[]" or
> > "char **array_ptr".
> >
> > Note that this feature is only available on the kernel which
> > supports array type.
> 
> User can still do,
> 
> # perf probe -x ~/hello main:3 'a=a:x32[3]'
> 
> which will successfully be installed in uprobe_events. But for a
> perf tool, x32[3] is not a valid type. And it seems perf tool don't
> validate the 'type' field:
> 
>     static int parse_perf_probe_arg(char *str, struct perf_probe_arg *arg)
>     {
>     
>     tmp = strchr(str, ':');
>     if (tmp) {  /* Type setting */
>     *tmp = '\0';
>     arg->type = strdup(tmp + 1);
>     if (arg->type == NULL)
>     return -ENOMEM;
>     pr_debug("type:%s ", arg->type);
>     }
> 
> Is it okay to allow user to specify array size with type field?

Fro this patch, yes. The availability of type is checked only when
it is automatically generated.
IMO, it should be done in another patch, something like
"Validate user specified type casting" patch. Would you need it?

Thank you,

-- 
Masami Hiramatsu 

Re: [PATCH v6 21/21] perf-probe: Add array argument support

[Ravi Bangoria]

Hi Masami,

On 03/17/2018 06:23 PM, Masami Hiramatsu wrote:
> Since kprobes events support an array argument, perf-probe
> can also support dumping array now.
> The syntax are
>
>  []
> or
>  []
>
> where the  is ... e.g. array[0..5].
> This can also be available with string type. In this
> case, the string array should be "char *array[]" or
> "char **array_ptr".
>
> Note that this feature is only available on the kernel which
> supports array type.

User can still do,

# perf probe -x ~/hello main:3 'a=a:x32[3]'

which will successfully be installed in uprobe_events. But for a
perf tool, x32[3] is not a valid type. And it seems perf tool don't
validate the 'type' field:

    static int parse_perf_probe_arg(char *str, struct perf_probe_arg *arg)
    {
    
    tmp = strchr(str, ':');
    if (tmp) {  /* Type setting */
    *tmp = '\0';
    arg->type = strdup(tmp + 1);
    if (arg->type == NULL)
    return -ENOMEM;
    pr_debug("type:%s ", arg->type);
    }

Is it okay to allow user to specify array size with type field?

Thanks,
Ravi

Re: [PATCH v6 21/21] perf-probe: Add array argument support

[Ravi Bangoria]

Hi Masami,

On 03/17/2018 06:23 PM, Masami Hiramatsu wrote:
> Since kprobes events support an array argument, perf-probe
> can also support dumping array now.
> The syntax are
>
>  []
> or
>  []
>
> where the  is ... e.g. array[0..5].
> This can also be available with string type. In this
> case, the string array should be "char *array[]" or
> "char **array_ptr".
>
> Note that this feature is only available on the kernel which
> supports array type.

User can still do,

# perf probe -x ~/hello main:3 'a=a:x32[3]'

which will successfully be installed in uprobe_events. But for a
perf tool, x32[3] is not a valid type. And it seems perf tool don't
validate the 'type' field:

    static int parse_perf_probe_arg(char *str, struct perf_probe_arg *arg)
    {
    
    tmp = strchr(str, ':');
    if (tmp) {  /* Type setting */
    *tmp = '\0';
    arg->type = strdup(tmp + 1);
    if (arg->type == NULL)
    return -ENOMEM;
    pr_debug("type:%s ", arg->type);
    }

Is it okay to allow user to specify array size with type field?

Thanks,
Ravi

[PATCH v6 21/21] perf-probe: Add array argument support

[Masami Hiramatsu]

Since kprobes events support an array argument, perf-probe
can also support dumping array now.
The syntax are

 []
or
 []

where the  is ... e.g. array[0..5].
This can also be available with string type. In this
case, the string array should be "char *array[]" or
"char **array_ptr".

Note that this feature is only available on the kernel which
supports array type.

Signed-off-by: Masami Hiramatsu 
---
 tools/perf/Documentation/perf-probe.txt |2 -
 tools/perf/util/probe-event.c   |   20 ++-
 tools/perf/util/probe-event.h   |2 +
 tools/perf/util/probe-file.c|5 ++
 tools/perf/util/probe-file.h|1 
 tools/perf/util/probe-finder.c  |   95 ++-
 6 files changed, 84 insertions(+), 41 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt 
b/tools/perf/Documentation/perf-probe.txt
index b6866a05edd2..a68e423cac48 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -196,7 +196,7 @@ Each probe argument follows below syntax.
 
  [NAME=]LOCALVAR|$retval|%REG|@SYMBOL[:TYPE]
 
-'NAME' specifies the name of this argument (optional). You can use the name of 
local variable, local data structure member (e.g. var->field, var.field2), 
local array with fixed index (e.g. array[1], var->array[0], var->pointer[2]), 
or kprobe-tracer argument format (e.g. $retval, %ax, etc). Note that the name 
of this argument will be set as the last member name if you specify a local 
data structure member (e.g. field2 for 'var->field1.field2'.)
+'NAME' specifies the name of this argument (optional). You can use the name of 
local variable, local data structure member (e.g. var->field, var.field2), 
local array with fixed index (e.g. array[1], var->array[0], var->pointer[2]) or 
range (e.g. array[1..4], var->array[0..2]), or kprobe-tracer argument format 
(e.g. $retval, %ax, etc). Note that the name of this argument will be set as 
the last member name if you specify a local data structure member (e.g. field2 
for 'var->field1.field2'.)
 '$vars' and '$params' special arguments are also available for NAME, '$vars' 
is expanded to the local variables (including function parameters) which can 
access at given probe point. '$params' is expanded to only the function 
parameters.
 'TYPE' casts the type of this argument (optional). If omitted, perf probe 
automatically set the type based on debuginfo (*). Currently, basic types 
(u8/u16/u32/u64/s8/s16/s32/s64), hexadecimal integers (x/x8/x16/x32/x64), 
signedness casting (u/s), "string" and bitfield are supported. (see TYPES for 
detail)
 On x86 systems %REG is always the short form of the register: for example %AX. 
%RAX or %EAX is not valid.
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index e1dbc9821617..e07a1957c9cd 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1570,6 +1570,7 @@ static int parse_perf_probe_arg(char *str, struct 
perf_probe_arg *arg)
 {
char *tmp, *goodname;
struct perf_probe_arg_field **fieldp;
+   long end;
 
pr_debug("parsing arg: %s into ", str);
 
@@ -1617,7 +1618,18 @@ static int parse_perf_probe_arg(char *str, struct 
perf_probe_arg *arg)
str = tmp;
(*fieldp)->index = strtol(str + 1, , 0);
(*fieldp)->ref = true;
-   if (*tmp != ']' || tmp == str + 1) {
+   if (tmp[0] == '.' && tmp[1] == '.') { /* dump array */
+   end = strtol(tmp + 2, , 0);
+   if (end < (*fieldp)->index || *tmp != ']') {
+   semantic_error("Invalid array range\n");
+   return -EINVAL;
+   }
+   arg->length = end - (*fieldp)->index + 1;
+   if (tmp[1] != '\0') {
+   semantic_error("Array range must not 
follow anything.");
+   return -EINVAL;
+   }
+   } else if (*tmp != ']' || tmp == str + 1) {
semantic_error("Array index must be a"
" number.\n");
return -EINVAL;
@@ -2022,6 +2034,12 @@ static int synthesize_probe_trace_arg(struct 
probe_trace_arg *arg,
if (!err && arg->type)
err = strbuf_addf(buf, ":%s", arg->type);
 
+   if (!err && arg->length) {
+   if (!arg->type)
+   return -EINVAL;
+   err = strbuf_addf(buf, "[%d]", arg->length);
+   }
+
return err;
 }
 
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 45b14f020558..e5ca1bf33ee7 100644
--- a/tools/perf/util/probe-event.h

[PATCH v6 21/21] perf-probe: Add array argument support

[Masami Hiramatsu]

Since kprobes events support an array argument, perf-probe
can also support dumping array now.
The syntax are

 []
or
 []

where the  is ... e.g. array[0..5].
This can also be available with string type. In this
case, the string array should be "char *array[]" or
"char **array_ptr".

Note that this feature is only available on the kernel which
supports array type.

Signed-off-by: Masami Hiramatsu 
---
 tools/perf/Documentation/perf-probe.txt |2 -
 tools/perf/util/probe-event.c   |   20 ++-
 tools/perf/util/probe-event.h   |2 +
 tools/perf/util/probe-file.c|5 ++
 tools/perf/util/probe-file.h|1 
 tools/perf/util/probe-finder.c  |   95 ++-
 6 files changed, 84 insertions(+), 41 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt 
b/tools/perf/Documentation/perf-probe.txt
index b6866a05edd2..a68e423cac48 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -196,7 +196,7 @@ Each probe argument follows below syntax.
 
  [NAME=]LOCALVAR|$retval|%REG|@SYMBOL[:TYPE]
 
-'NAME' specifies the name of this argument (optional). You can use the name of 
local variable, local data structure member (e.g. var->field, var.field2), 
local array with fixed index (e.g. array[1], var->array[0], var->pointer[2]), 
or kprobe-tracer argument format (e.g. $retval, %ax, etc). Note that the name 
of this argument will be set as the last member name if you specify a local 
data structure member (e.g. field2 for 'var->field1.field2'.)
+'NAME' specifies the name of this argument (optional). You can use the name of 
local variable, local data structure member (e.g. var->field, var.field2), 
local array with fixed index (e.g. array[1], var->array[0], var->pointer[2]) or 
range (e.g. array[1..4], var->array[0..2]), or kprobe-tracer argument format 
(e.g. $retval, %ax, etc). Note that the name of this argument will be set as 
the last member name if you specify a local data structure member (e.g. field2 
for 'var->field1.field2'.)
 '$vars' and '$params' special arguments are also available for NAME, '$vars' 
is expanded to the local variables (including function parameters) which can 
access at given probe point. '$params' is expanded to only the function 
parameters.
 'TYPE' casts the type of this argument (optional). If omitted, perf probe 
automatically set the type based on debuginfo (*). Currently, basic types 
(u8/u16/u32/u64/s8/s16/s32/s64), hexadecimal integers (x/x8/x16/x32/x64), 
signedness casting (u/s), "string" and bitfield are supported. (see TYPES for 
detail)
 On x86 systems %REG is always the short form of the register: for example %AX. 
%RAX or %EAX is not valid.
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index e1dbc9821617..e07a1957c9cd 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1570,6 +1570,7 @@ static int parse_perf_probe_arg(char *str, struct 
perf_probe_arg *arg)
 {
char *tmp, *goodname;
struct perf_probe_arg_field **fieldp;
+   long end;
 
pr_debug("parsing arg: %s into ", str);
 
@@ -1617,7 +1618,18 @@ static int parse_perf_probe_arg(char *str, struct 
perf_probe_arg *arg)
str = tmp;
(*fieldp)->index = strtol(str + 1, , 0);
(*fieldp)->ref = true;
-   if (*tmp != ']' || tmp == str + 1) {
+   if (tmp[0] == '.' && tmp[1] == '.') { /* dump array */
+   end = strtol(tmp + 2, , 0);
+   if (end < (*fieldp)->index || *tmp != ']') {
+   semantic_error("Invalid array range\n");
+   return -EINVAL;
+   }
+   arg->length = end - (*fieldp)->index + 1;
+   if (tmp[1] != '\0') {
+   semantic_error("Array range must not 
follow anything.");
+   return -EINVAL;
+   }
+   } else if (*tmp != ']' || tmp == str + 1) {
semantic_error("Array index must be a"
" number.\n");
return -EINVAL;
@@ -2022,6 +2034,12 @@ static int synthesize_probe_trace_arg(struct 
probe_trace_arg *arg,
if (!err && arg->type)
err = strbuf_addf(buf, ":%s", arg->type);
 
+   if (!err && arg->length) {
+   if (!arg->type)
+   return -EINVAL;
+   err = strbuf_addf(buf, "[%d]", arg->length);
+   }
+
return err;
 }
 
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 45b14f020558..e5ca1bf33ee7 100644
--- a/tools/perf/util/probe-event.h
+++