All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Tom Zanussi <tom.zanussi@linux.intel.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	linux-trace-users@vger.kernel.org,
	linux-kselftest@vger.kernel.org, shuah@kernel.org
Subject: Re: [PATCH v6 21/21] perf-probe: Add array argument support
Date: Thu, 22 Mar 2018 18:23:47 +0800	[thread overview]
Message-ID: <20180322182347.ca1aab9b77ab4c351100646b@kernel.org> (raw)
In-Reply-To: <8c02c538-4934-59c2-7795-9058ce57e625@linux.vnet.ibm.com>

On Mon, 19 Mar 2018 13:29:59 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> 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
> >
> >  <array-var>[<range>]
> > or
> >  <pointer-var>[<range>]
> >
> > where the <range> is <start>..<end>. 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 <mhiramat@kernel.org>

WARNING: multiple messages have this Message-ID (diff)
From: mhiramat at kernel.org (Masami Hiramatsu)
Subject: [PATCH v6 21/21] perf-probe: Add array argument support
Date: Thu, 22 Mar 2018 18:23:47 +0800	[thread overview]
Message-ID: <20180322182347.ca1aab9b77ab4c351100646b@kernel.org> (raw)
In-Reply-To: <8c02c538-4934-59c2-7795-9058ce57e625@linux.vnet.ibm.com>

On Mon, 19 Mar 2018 13:29:59 +0530
Ravi Bangoria <ravi.bangoria at linux.vnet.ibm.com> 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
> >
> >  <array-var>[<range>]
> > or
> >  <pointer-var>[<range>]
> >
> > where the <range> is <start>..<end>. 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 <mhiramat at kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: mhiramat@kernel.org (Masami Hiramatsu)
Subject: [PATCH v6 21/21] perf-probe: Add array argument support
Date: Thu, 22 Mar 2018 18:23:47 +0800	[thread overview]
Message-ID: <20180322182347.ca1aab9b77ab4c351100646b@kernel.org> (raw)
Message-ID: <20180322102347.Mv2GWMnoNl6VkZdzytaHtTR5cbTLYnjOKdeu3Z6JBVg@z> (raw)
In-Reply-To: <8c02c538-4934-59c2-7795-9058ce57e625@linux.vnet.ibm.com>

On Mon, 19 Mar 2018 13:29:59 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> 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
> >
> >  <array-var>[<range>]
> > or
> >  <pointer-var>[<range>]
> >
> > where the <range> is <start>..<end>. 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 <mhiramat at kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2018-03-22 10:23 UTC|newest]

Thread overview: 103+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-17 12:37 [PATCH v6 00/21] tracing: probeevent: Improve fetcharg features Masami Hiramatsu
2018-03-17 12:37 ` Masami Hiramatsu
2018-03-17 12:37 ` mhiramat
2018-03-17 12:38 ` [PATCH v6 01/21] [BUGFIX] tracing: probeevent: Fix to support minus offset from symbol Masami Hiramatsu
2018-03-17 12:38   ` Masami Hiramatsu
2018-03-17 12:38   ` mhiramat
2018-03-17 12:38 ` [PATCH v6 02/21] selftests: ftrace: Add probe event argument syntax testcase Masami Hiramatsu
2018-03-17 12:38   ` Masami Hiramatsu
2018-03-17 12:38   ` mhiramat
2018-03-17 12:39 ` [PATCH v6 03/21] selftests: ftrace: Add a testcase for string type with kprobe_event Masami Hiramatsu
2018-03-17 12:39   ` Masami Hiramatsu
2018-03-17 12:39   ` mhiramat
2018-03-17 12:40 ` [PATCH v6 04/21] selftests: ftrace: Add a testcase for probepoint Masami Hiramatsu
2018-03-17 12:40   ` Masami Hiramatsu
2018-03-17 12:40   ` mhiramat
2018-03-23 16:19   ` Steven Rostedt
2018-03-23 16:19     ` Steven Rostedt
2018-03-23 16:19     ` rostedt
2018-03-17 12:41 ` [PATCH v6 05/21] tracing: probeevent: Cleanup print argument functions Masami Hiramatsu
2018-03-17 12:41   ` Masami Hiramatsu
2018-03-17 12:41   ` mhiramat
2018-03-23 16:36   ` Steven Rostedt
2018-03-23 16:36     ` Steven Rostedt
2018-03-23 16:36     ` rostedt
2018-03-26  4:17     ` Masami Hiramatsu
2018-03-26  4:17       ` Masami Hiramatsu
2018-03-26  4:17       ` mhiramat
2018-03-26 17:28       ` Steven Rostedt
2018-03-26 17:28         ` Steven Rostedt
2018-03-26 17:28         ` rostedt
2018-03-28  4:21         ` Masami Hiramatsu
2018-03-28  4:21           ` Masami Hiramatsu
2018-03-28  4:21           ` mhiramat
2018-03-17 12:41 ` [PATCH v6 06/21] tracing: probeevent: Cleanup argument field definition Masami Hiramatsu
2018-03-17 12:41   ` Masami Hiramatsu
2018-03-17 12:41   ` mhiramat
2018-03-17 12:42 ` [PATCH v6 07/21] tracing: probeevent: Remove NOKPROBE_SYMBOL from print functions Masami Hiramatsu
2018-03-17 12:42   ` Masami Hiramatsu
2018-03-17 12:42   ` mhiramat
2018-03-17 12:43 ` [PATCH v6 08/21] tracing: probeevent: Introduce new argument fetching code Masami Hiramatsu
2018-03-17 12:43   ` Masami Hiramatsu
2018-03-17 12:43   ` mhiramat
2018-03-17 12:44 ` [PATCH v6 09/21] tracing: probeevent: Unify fetch type tables Masami Hiramatsu
2018-03-17 12:44   ` Masami Hiramatsu
2018-03-17 12:44   ` mhiramat
2018-03-17 12:44 ` [PATCH v6 10/21] tracing: probeevent: Return consumed bytes of dynamic area Masami Hiramatsu
2018-03-17 12:44   ` Masami Hiramatsu
2018-03-17 12:44   ` mhiramat
2018-04-02 20:02   ` Steven Rostedt
2018-04-02 20:02     ` Steven Rostedt
2018-04-02 20:02     ` rostedt
2018-04-03 14:41     ` Masami Hiramatsu
2018-04-03 14:41       ` Masami Hiramatsu
2018-04-03 14:41       ` mhiramat
2018-03-17 12:45 ` [PATCH v6 11/21] tracing: probeevent: Append traceprobe_ for exported function Masami Hiramatsu
2018-03-17 12:45   ` Masami Hiramatsu
2018-03-17 12:45   ` mhiramat
2018-03-17 12:46 ` [PATCH v6 12/21] tracing: probeevent: Unify fetch_insn processing common part Masami Hiramatsu
2018-03-17 12:46   ` Masami Hiramatsu
2018-03-17 12:46   ` mhiramat
2018-03-17 12:47 ` [PATCH v6 13/21] tracing: probeevent: Add symbol type Masami Hiramatsu
2018-03-17 12:47   ` Masami Hiramatsu
2018-03-17 12:47   ` mhiramat
2018-03-17 12:47 ` [PATCH v6 14/21] x86: ptrace: Add function argument access API Masami Hiramatsu
2018-03-17 12:47   ` Masami Hiramatsu
2018-03-17 12:47   ` mhiramat
2018-03-17 12:48 ` [PATCH v6 15/21] tracing: probeevent: Add $argN for accessing function args Masami Hiramatsu
2018-03-17 12:48   ` Masami Hiramatsu
2018-03-17 12:48   ` mhiramat
2018-03-17 12:49 ` [PATCH v6 16/21] tracing: probeevent: Add array type support Masami Hiramatsu
2018-03-17 12:49   ` Masami Hiramatsu
2018-03-17 12:49   ` mhiramat
2018-03-17 12:50 ` [PATCH v6 17/21] selftests: ftrace: Add a testcase for symbol type Masami Hiramatsu
2018-03-17 12:50   ` Masami Hiramatsu
2018-03-17 12:50   ` mhiramat
2018-03-17 12:50 ` [PATCH v6 18/21] selftests: ftrace: Add a testcase for $argN with kprobe_event Masami Hiramatsu
2018-03-17 12:50   ` Masami Hiramatsu
2018-03-17 12:50   ` mhiramat
2018-03-17 12:51 ` [PATCH v6 19/21] selftests: ftrace: Add a testcase for array type " Masami Hiramatsu
2018-03-17 12:51   ` Masami Hiramatsu
2018-03-17 12:51   ` mhiramat
2018-03-17 12:52 ` [PATCH v6 20/21] [RESEND] perf-probe: Fix to convert array type collectly Masami Hiramatsu
2018-03-17 12:52   ` Masami Hiramatsu
2018-03-17 12:52   ` mhiramat
2018-03-20  6:36   ` [tip:perf/core] perf probe: Use right type to access array elements tip-bot for Masami Hiramatsu
2018-03-17 12:53 ` [PATCH v6 21/21] perf-probe: Add array argument support Masami Hiramatsu
2018-03-17 12:53   ` Masami Hiramatsu
2018-03-17 12:53   ` mhiramat
2018-03-19  7:59   ` Ravi Bangoria
2018-03-19  7:59     ` Ravi Bangoria
2018-03-19  7:59     ` ravi.bangoria
2018-03-22 10:23     ` Masami Hiramatsu [this message]
2018-03-22 10:23       ` Masami Hiramatsu
2018-03-22 10:23       ` mhiramat
2018-03-22 10:49       ` Ravi Bangoria
2018-03-22 10:49         ` Ravi Bangoria
2018-03-22 10:49         ` ravi.bangoria
2018-03-26  3:53         ` Masami Hiramatsu
2018-03-26  3:53           ` Masami Hiramatsu
2018-03-26  3:53           ` mhiramat
2018-03-17 14:06 ` [PATCH v6 00/21] tracing: probeevent: Improve fetcharg features Masami Hiramatsu
2018-03-17 14:06   ` Masami Hiramatsu
2018-03-17 14:06   ` mhiramat

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180322182347.ca1aab9b77ab4c351100646b@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=acme@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-trace-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=ravi.bangoria@linux.vnet.ibm.com \
    --cc=rostedt@goodmis.org \
    --cc=shuah@kernel.org \
    --cc=tom.zanussi@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.