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

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

On Thu, 3 Aug 2023 19:31:06 +0200
Oleg Nesterov <oleg@redhat.com> wrote:

> Hi Masami,
> 
> On 08/01, Masami Hiramatsu wrote:
> >
> > Oleg, what do you think about accessing symbols in data section from
> > uprobes? Can we access it from user-land IP-relative address?
> 
> Sorry, I don't I understand... I don't even understand the context, can't
> find the whole thread on lore.kernel.org/lkml. Plus I forgot EVERYTHING
> I knew about tracing/uprobes anyway ;)

Oh, but thank you for replying :)

> 
> but when I look at traceprobe_parse_probe_arg() paths it seems to me that
> uprobe can fetch the IP-relative address, not sure about the syntax but
> perhaps something like xxx=+OFFSET(%ip). The question is how tools/perf
> can calculate this OFFSET. But I am sure you understand this all much
> better than me.

Yes, var=@+offset is the syntax. That will access the data in same file
(and maybe in the same section?)

> 
> > > > --- 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;
> 
> IIUC without this change @symbol will trigger the
> 
> 			/* uprobes don't support symbols */
> 			if (!(ctx->flags & TPARG_FL_KERNEL)) {
> 				trace_probe_log_err(ctx->offset, SYM_ON_UPROBE);
> 				return -EINVAL;
> 			}
> 
> in parse_probe_arg(), right?

Yes, that's right.

> 
> So FWIW the patch looks fine to me, but as you have mentioned tools/perf
> could probably (try to) turn @symbol into @+symbol_offset_in_file...

Yeah, that's a good point. Maybe I should try to find the data symbol
and find the offset. One thing I'm not sure is the address in the data section
maybe different from the address in the code section, and @+addr seems to
point the data in the code section because it calculate the offset from
the ip address.

> 
> In short, sorry for spam, I can't help ;)

No problem and thanks for your reply.

> 
> And just in case, I am on PTO till Aug 14, won't be able to read emails
> till then.
> 
> Oleg.
> 

Thank you!

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