Re: [PATCH] perf probe: Show correct error about @symbol for uprobe

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

(To: Oleg)

On Tue, 1 Aug 2023 09:55:07 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Fri, Jul 28, 2023 at 11:19:30PM +0900, Masami Hiramatsu (Google) escreveu:
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > 
> > Since @symbol variable access is not supported by uprobe event, it must be
> > correctly warn user instead of kernel version update.
> 
> Thanks, less cryptic now, applied.
> 
> But is that just a matter of writing code to support reading global
> variables from an uprobe?

Hm, maybe we can use "@+offset" for accessing symboled data, as far as
it is loaded with the text section.
Oleg, what do you think about accessing symbols in data section from
uprobes? Can we access it from user-land IP-relative address?

Thank you,

> 
> - Arnaldo
> 
>  
> > Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> > Closes: https://lore.kernel.org/all/ZLWDEjvFjrrEJODp@kernel.org/
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > ---
> >  tools/perf/util/probe-event.c |   13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 6e2110d605fb..c1ded85fe859 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -2800,13 +2800,18 @@ static void warn_uprobe_event_compat(struct probe_trace_event *tev)
> >  	if (!tev->uprobes || tev->nargs == 0 || !buf)
> >  		goto out;
> >  
> > -	for (i = 0; i < tev->nargs; i++)
> > -		if (strglobmatch(tev->args[i].value, "[$@+-]*")) {
> > -			pr_warning("Please upgrade your kernel to at least "
> > -				   "3.14 to have access to feature %s\n",
> > +	for (i = 0; i < tev->nargs; i++) {
> > +		if (strchr(tev->args[i].value, '@')) {
> > +			pr_warning("%s accesses a variable by symbol name, but that is not supported for user application probe.\n",
> > +				   tev->args[i].value);
> > +			break;
> > +		}
> > +		if (strglobmatch(tev->args[i].value, "[$+-]*")) {
> > +			pr_warning("Please upgrade your kernel to at least 3.14 to have access to feature %s\n",
> >  				   tev->args[i].value);
> >  			break;
> >  		}
> > +	}
> >  out:
> >  	free(buf);
> >  }
> > 
> 
> -- 
> 
> - Arnaldo


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>