2020-07-09 08:08:32

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH 4/4] perf-probe: Warn if the target function is GNU Indirect function

Warn if the probe target function is GNU indirect function (GNU_IFUNC)
because it may not what the user want to probe.

The GNU indirect function ( https://sourceware.org/glibc/wiki/GNU_IFUNC )
is the dynamic solved symbol at runtime. IFUNC function is a selector
which is invoked from the elf loader, but the symbol address of the
function which will be modified by the IFUNC is same as the IFUNC in
the symbol table. This can confuse users who is trying to probe on
such functions.

For example, the memcpy is one of IFUNC.

# perf probe -x /lib64/libc-2.30.so -a memcpy
# perf probe -l
probe_libc:memcpy (on __new_memcpy_ifunc@x86_64/multiarch/memcpy.c in /usr/lib64/libc-2.30.so)

the probe is put on a IFUNC.

# perf record -e probe_libc:memcpy --call-graph dwarf -aR ./perf
# perf script
perf 1742 [000] 26201.715632: probe_libc:memcpy: (7fdaa53824c0)
7fdaa53824c0 __new_memcpy_ifunc+0x0 (inlined)
7fdaa5d4a980 elf_machine_rela+0x6c0 (inlined)
7fdaa5d4a980 elf_dynamic_do_Rela+0x6c0 (inlined)
7fdaa5d4a980 _dl_relocate_object+0x6c0 (/usr/lib64/ld-2.30.so)
7fdaa5d42155 dl_main+0x1cc5 (/usr/lib64/ld-2.30.so)
7fdaa5d5831a _dl_sysdep_start+0x54a (/usr/lib64/ld-2.30.so)
7fdaa5d3ffeb _dl_start_final+0x25b (inlined)
7fdaa5d3ffeb _dl_start+0x25b (/usr/lib64/ld-2.30.so)
7fdaa5d3f117 .annobin_rtld.c+0x7 (inlined)
...

And the event is invoked from the elf loader instead of the target
program's main code.


Moreover, at this moment, we can not probe on the function which will
be selected by the IFUNC, because it is determined at runtime. But
uprobe will be prepared before running the target binary.

Thus, I decided to warn user when the perf probe detects the probe point
is on the GNU IFUNC symbol. Someone who wants to probe an IFUNC symbol to
debug the IFUNC function, they can ignore this warning.


Reported-by: Andi Kleen <[email protected]>
Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/util/probe-event.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 1e95a336862c..671176d39569 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -379,6 +379,11 @@ static int find_alternative_probe_point(struct debuginfo *dinfo,
address = sym->start;
else
address = map->unmap_ip(map, sym->start) - map->reloc;
+ if (sym->type == STT_GNU_IFUNC) {
+ pr_warning("Warning: The probe address (0x%lx) is in a GNU indirect function.\n"
+ "This may not work as you expected unless you intend to probe the indirect function.\n",
+ (unsigned long)address);
+ }
break;
}
if (!address) {


2020-07-09 14:48:24

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH 4/4] perf-probe: Warn if the target function is GNU Indirect function

> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 1e95a336862c..671176d39569 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -379,6 +379,11 @@ static int find_alternative_probe_point(struct debuginfo *dinfo,
> address = sym->start;
> else
> address = map->unmap_ip(map, sym->start) - map->reloc;
> + if (sym->type == STT_GNU_IFUNC) {
> + pr_warning("Warning: The probe address (0x%lx) is in a GNU indirect function.\n"
> + "This may not work as you expected unless you intend to probe the indirect function.\n",

I would say something like this.

Consider identifying the final function used at run time and set the
probe directly on that.

I think that's more useful to the user.

-Andi

2020-07-10 03:30:59

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH 4/4] perf-probe: Warn if the target function is GNU Indirect function

On Thu, 9 Jul 2020 07:36:54 -0700
Andi Kleen <[email protected]> wrote:

> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 1e95a336862c..671176d39569 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -379,6 +379,11 @@ static int find_alternative_probe_point(struct debuginfo *dinfo,
> > address = sym->start;
> > else
> > address = map->unmap_ip(map, sym->start) - map->reloc;
> > + if (sym->type == STT_GNU_IFUNC) {
> > + pr_warning("Warning: The probe address (0x%lx) is in a GNU indirect function.\n"
> > + "This may not work as you expected unless you intend to probe the indirect function.\n",
>
> I would say something like this.
>
> Consider identifying the final function used at run time and set the
> probe directly on that.
>
> I think that's more useful to the user.

Hmm, would you mean the default function which may be used for the symbol?
Let me check how we can find it.

Thank you,

--
Masami Hiramatsu <[email protected]>

2020-07-10 11:16:04

by Srikar Dronamraju

[permalink] [raw]
Subject: Re: [PATCH 4/4] perf-probe: Warn if the target function is GNU Indirect function

* Masami Hiramatsu <[email protected]> [2020-07-09 17:07:31]:

> Reported-by: Andi Kleen <[email protected]>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> ---
> tools/perf/util/probe-event.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 1e95a336862c..671176d39569 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -379,6 +379,11 @@ static int find_alternative_probe_point(struct debuginfo *dinfo,
> address = sym->start;
> else
> address = map->unmap_ip(map, sym->start) - map->reloc;
> + if (sym->type == STT_GNU_IFUNC) {
> + pr_warning("Warning: The probe address (0x%lx) is in a GNU indirect function.\n"
> + "This may not work as you expected unless you intend to probe the indirect function.\n",
> + (unsigned long)address);
> + }

Are these GNU indirect functions possible in kernel? If not we could move
this warning under if (uprobes)

Also instead of printing the address, can we print the pp->function?

> break;
> }
> if (!address) {
>

--
Thanks and Regards
Srikar Dronamraju

2020-07-10 11:58:35

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 4/4] perf-probe: Warn if the target function is GNU Indirect function

Em Fri, Jul 10, 2020 at 12:30:08PM +0900, Masami Hiramatsu escreveu:
> On Thu, 9 Jul 2020 07:36:54 -0700
> Andi Kleen <[email protected]> wrote:
>
> > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > > index 1e95a336862c..671176d39569 100644
> > > --- a/tools/perf/util/probe-event.c
> > > +++ b/tools/perf/util/probe-event.c
> > > @@ -379,6 +379,11 @@ static int find_alternative_probe_point(struct debuginfo *dinfo,
> > > address = sym->start;
> > > else
> > > address = map->unmap_ip(map, sym->start) - map->reloc;
> > > + if (sym->type == STT_GNU_IFUNC) {
> > > + pr_warning("Warning: The probe address (0x%lx) is in a GNU indirect function.\n"
> > > + "This may not work as you expected unless you intend to probe the indirect function.\n",
> >
> > I would say something like this.
> >
> > Consider identifying the final function used at run time and set the
> > probe directly on that.
> >
> > I think that's more useful to the user.
>
> Hmm, would you mean the default function which may be used for the symbol?

Humm, I think he means that the user must somehow, knowing details
involved in picking the final function, probe that one instead of the
IFUNC one, right Andi?

- Arnaldo

> Let me check how we can find it.

2020-07-10 12:15:03

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH 4/4] perf-probe: Warn if the target function is GNU Indirect function

On Fri, 10 Jul 2020 16:45:12 +0530
Srikar Dronamraju <[email protected]> wrote:

> * Masami Hiramatsu <[email protected]> [2020-07-09 17:07:31]:
>
> > Reported-by: Andi Kleen <[email protected]>
> > Signed-off-by: Masami Hiramatsu <[email protected]>
> > ---
> > tools/perf/util/probe-event.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 1e95a336862c..671176d39569 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -379,6 +379,11 @@ static int find_alternative_probe_point(struct debuginfo *dinfo,
> > address = sym->start;
> > else
> > address = map->unmap_ip(map, sym->start) - map->reloc;
> > + if (sym->type == STT_GNU_IFUNC) {
> > + pr_warning("Warning: The probe address (0x%lx) is in a GNU indirect function.\n"
> > + "This may not work as you expected unless you intend to probe the indirect function.\n",
> > + (unsigned long)address);
> > + }
>
> Are these GNU indirect functions possible in kernel? If not we could move
> this warning under if (uprobes)

OK, I'll move it under if (uprobes).

>
> Also instead of printing the address, can we print the pp->function?

Hmm, OK. But it may not help user because the pp->function will the name
specified by user...

Thank you,


--
Masami Hiramatsu <[email protected]>

2020-07-10 12:57:05

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH 4/4] perf-probe: Warn if the target function is GNU Indirect function

On Fri, 10 Jul 2020 08:55:40 -0300
Arnaldo Carvalho de Melo <[email protected]> wrote:

> Em Fri, Jul 10, 2020 at 12:30:08PM +0900, Masami Hiramatsu escreveu:
> > On Thu, 9 Jul 2020 07:36:54 -0700
> > Andi Kleen <[email protected]> wrote:
> >
> > > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > > > index 1e95a336862c..671176d39569 100644
> > > > --- a/tools/perf/util/probe-event.c
> > > > +++ b/tools/perf/util/probe-event.c
> > > > @@ -379,6 +379,11 @@ static int find_alternative_probe_point(struct debuginfo *dinfo,
> > > > address = sym->start;
> > > > else
> > > > address = map->unmap_ip(map, sym->start) - map->reloc;
> > > > + if (sym->type == STT_GNU_IFUNC) {
> > > > + pr_warning("Warning: The probe address (0x%lx) is in a GNU indirect function.\n"
> > > > + "This may not work as you expected unless you intend to probe the indirect function.\n",
> > >
> > > I would say something like this.
> > >
> > > Consider identifying the final function used at run time and set the
> > > probe directly on that.
> > >
> > > I think that's more useful to the user.
> >
> > Hmm, would you mean the default function which may be used for the symbol?
>
> Humm, I think he means that the user must somehow, knowing details
> involved in picking the final function, probe that one instead of the
> IFUNC one, right Andi?

Ah, I got it. OK, I'll update the message :)

Thank you,


--
Masami Hiramatsu <[email protected]>