| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20241105183633.28048e908539d5c638ceb10a@kernel.org>
Date: Tue, 5 Nov 2024 18:36:33 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Masami Hiramatsu (Google) <mhiramat@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim
<namhyung@...nel.org>, Peter Zijlstra <peterz@...radead.org>, Ingo Molnar
<mingo@...hat.com>, Ian Rogers <irogers@...gle.com>, Dima Kogan
<dima@...retsauce.net>, Alexander Lobakin <aleksander.lobakin@...el.com>,
Przemek Kitszel <przemyslaw.kitszel@...el.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] perf-probe: Require '@' prefix for filename always
On Tue, 5 Nov 2024 18:28:30 +0900
Masami Hiramatsu (Google) <mhiramat@...nel.org> wrote:
> On Mon, 4 Nov 2024 17:10:41 -0300
> Arnaldo Carvalho de Melo <acme@...nel.org> wrote:
>
> > On Tue, Nov 05, 2024 at 01:17:26AM +0900, Masami Hiramatsu (Google) wrote:
> > > From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
> >
> > > Currently perf probe allows user to specify probing place without '@'
> > > prefix, for example, both `-V file:line` and `-V function:line` are
> > > allowed. But this makes a problem if a (demangled) function name is
> > > hard to be distinguished from a file name.
> >
> > > This changes the perf-probe to require '@' prefix for filename even
> > > without function name. For example, `-V @file:line` and
> > > `-V function:line` are acceptable.
> >
> > > With this change, users can specify filename or function correctly.
> >
> > Well, this will break scripts that use perf probe for a given file,
> > probably the right thing not to break backwards compatibility is to
> > continue accepting and when there is a real conflict, an ambiguity that
> > makes differentiating from file to function names, then refuse it,
> > stating that it is ambiguous, probably spelling out the names of the
> > files and functions that match so and stating that to make it
> > unambiguoius, prefix file names with @.
>
> The problem is that the ambiguous function name. For example, Go's
> main routine is `main.main`, and this is not likely to the C function
> name, so currently perf probe treats it as a filename and failed to
> find that.
>
> I think one possible solution is to run search loop twice internally
> (search it as file name, if fails, search it as function name) if it
> looks like a file name but it does not start from `@`.
> This takes costs a bit but can keep backward compatibility.
I found another good idea, support `@*` :)
For example, if the `main.main` is not a file, we can
perf probe -x execfile -L 'main.main@*'
Then it is clear that `main.main` is a function name.
Thank you!
>
> Thank you,
>
> >
> > - Arnaldo
> >
> > > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>
> > > ---
> > > tools/perf/util/probe-event.c | 31 +++++++++----------------------
> > > 1 file changed, 9 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > > index 665dcce482e1..913a27cbb5d9 100644
> > > --- a/tools/perf/util/probe-event.c
> > > +++ b/tools/perf/util/probe-event.c
> > > @@ -1341,7 +1341,7 @@ static bool is_c_func_name(const char *name)
> > > * Stuff 'lr' according to the line range described by 'arg'.
> > > * The line range syntax is described by:
> > > *
> > > - * SRC[:SLN[+NUM|-ELN]]
> > > + * @SRC[:SLN[+NUM|-ELN]]
> > > * FNC[@SRC][:SLN[+NUM|-ELN]]
> > > */
> > > int parse_line_range_desc(const char *arg, struct line_range *lr)
> > > @@ -1404,16 +1404,10 @@ int parse_line_range_desc(const char *arg, struct line_range *lr)
> > > err = -ENOMEM;
> > > goto err;
> > > }
> > > + if (*name != '\0')
> > > + lr->function = name;
> > > + } else
> > > lr->function = name;
> > > - } else if (strpbrk_esc(name, "/."))
> > > - lr->file = name;
> > > - else if (is_c_func_name(name))/* We reuse it for checking funcname */
> > > - lr->function = name;
> > > - else { /* Invalid name */
> > > - semantic_error("'%s' is not a valid function name.\n", name);
> > > - err = -EINVAL;
> > > - goto err;
> > > - }
> > >
> > > return 0;
> > > err:
> > > @@ -1463,7 +1457,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> > >
> > > /*
> > > * <Syntax>
> > > - * perf probe [GRP:][EVENT=]SRC[:LN|;PTN]
> > > + * perf probe [GRP:][EVENT=]@SRC[:LN|;PTN]
> > > * perf probe [GRP:][EVENT=]FUNC[@SRC][+OFFS|%return|:LN|;PAT]
> > > * perf probe %[GRP:]SDT_EVENT
> > > */
> > > @@ -1516,19 +1510,12 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> > > /*
> > > * Check arg is function or file name and copy it.
> > > *
> > > - * We consider arg to be a file spec if and only if it satisfies
> > > - * all of the below criteria::
> > > - * - it does not include any of "+@%",
> > > - * - it includes one of ":;", and
> > > - * - it has a period '.' in the name.
> > > - *
> > > + * We consider arg to be a file spec if it starts with '@'.
> > > * Otherwise, we consider arg to be a function specification.
> > > */
> > > - if (!strpbrk_esc(arg, "+@%")) {
> > > - ptr = strpbrk_esc(arg, ";:");
> > > - /* This is a file spec if it includes a '.' before ; or : */
> > > - if (ptr && memchr(arg, '.', ptr - arg))
> > > - file_spec = true;
> > > + if (*arg == '@') {
> > > + file_spec = true;
> > > + arg++;
> > > }
> > >
> > > ptr = strpbrk_esc(arg, ";:+@%");
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@...nel.org>
--
Masami Hiramatsu (Google) <mhiramat@...nel.org>
Powered by blists - more mailing lists