linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] perf: Fix perf probe for Fedora34 glibc update
@ 2021-07-03 15:35 Masami Hiramatsu
  2021-07-03 15:35 ` [PATCH 1/3] perf-probe: Fix debuginfo__new() to enable build-id based debuginfo Masami Hiramatsu
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Masami Hiramatsu @ 2021-07-03 15:35 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-perf-users, Masami Hiramatsu, Sven Schnelle,
	Heiko Carstens, Stefan Liebler, Thomas Richter, linux-kernel

Hi Arnaldo,

Here is a series of patches to fix the perf-probe error against the
Fedora34 glibc update, which moves most of symbols from .symtab to
.dynsym. The key is that the "most of" symbols moved, but it still
have some PLT symbols in .symtab. Thus the perf symbol-elf failes to
decode symbols.

Here is the original report from Thomas about this issue.

 https://lore.kernel.org/linux-perf-users/f6752514-eaf9-371e-f81b-0d9e41ebae0c@linux.ibm.com/

Thank you,

---

Masami Hiramatsu (3):
      perf-probe: Fix debuginfo__new() to enable build-id based debuginfo
      perf symbol-elf: Decode dynsym even if symtab exists
      perf probe: Do not show @plt function by default


 tools/perf/builtin-probe.c     |    2 -
 tools/perf/util/probe-finder.c |    5 ++
 tools/perf/util/symbol-elf.c   |   82 ++++++++++++++++++++++++++--------------
 3 files changed, 60 insertions(+), 29 deletions(-)

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] perf-probe: Fix debuginfo__new() to enable build-id based debuginfo
  2021-07-03 15:35 [PATCH 0/3] perf: Fix perf probe for Fedora34 glibc update Masami Hiramatsu
@ 2021-07-03 15:35 ` Masami Hiramatsu
  2021-07-03 15:35 ` [PATCH 2/3] perf symbol-elf: Decode dynsym even if symtab exists Masami Hiramatsu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Masami Hiramatsu @ 2021-07-03 15:35 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-perf-users, Masami Hiramatsu, Sven Schnelle,
	Heiko Carstens, Stefan Liebler, Thomas Richter, linux-kernel

From: Masami Hiramatsu <mhriamat@kernel.org>

Fix debuginfo__new() to set the build-id to dso before
dso__read_binary_type_filename() so that it can find
DSO_BINARY_TYPE__BUILDID_DEBUGINFO debuginfo correctly.
However, this may not change the result, because elfutils
(libdwfl) has its own debuginfo finder. With/without this patch,
the perf probe correctly find the debuginfo file.

This is just a failsafe and keep code's sanity (if you use
dso__read_binary_type_filename(), you must set the build-id
to the dso.)

Reported-by: Thomas Richter <tmricht@linux.ibm.com>
Signed-off-by: Masami Hiramatsu <mhriamat@kernel.org>
---
 tools/perf/util/probe-finder.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index b029c29ce227..02ef0d78053b 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -118,12 +118,17 @@ struct debuginfo *debuginfo__new(const char *path)
 	char buf[PATH_MAX], nil = '\0';
 	struct dso *dso;
 	struct debuginfo *dinfo = NULL;
+	struct build_id bid;
 
 	/* Try to open distro debuginfo files */
 	dso = dso__new(path);
 	if (!dso)
 		goto out;
 
+	/* Set the build id for DSO_BINARY_TYPE__BUILDID_DEBUGINFO */
+	if (is_regular_file(path) && filename__read_build_id(path, &bid) > 0)
+		dso__set_build_id(dso, &bid);
+
 	for (type = distro_dwarf_types;
 	     !dinfo && *type != DSO_BINARY_TYPE__NOT_FOUND;
 	     type++) {


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/3] perf symbol-elf: Decode dynsym even if symtab exists
  2021-07-03 15:35 [PATCH 0/3] perf: Fix perf probe for Fedora34 glibc update Masami Hiramatsu
  2021-07-03 15:35 ` [PATCH 1/3] perf-probe: Fix debuginfo__new() to enable build-id based debuginfo Masami Hiramatsu
@ 2021-07-03 15:35 ` Masami Hiramatsu
  2021-07-03 15:35 ` [PATCH 3/3] perf probe: Do not show @plt function by default Masami Hiramatsu
  2021-07-05 20:48 ` [PATCH 0/3] perf: Fix perf probe for Fedora34 glibc update Namhyung Kim
  3 siblings, 0 replies; 8+ messages in thread
From: Masami Hiramatsu @ 2021-07-03 15:35 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-perf-users, Masami Hiramatsu, Sven Schnelle,
	Heiko Carstens, Stefan Liebler, Thomas Richter, linux-kernel

In Fedora34, libc-2.33.so has both .dynsym and .symtab sections and
most of (not all) symbols moved to .dynsym. In this case, perf only
decode the symbols in .symtab, and perf probe can not list up the
functions in the library.

To fix this issue, decode both .symtab and .dynsym sections.

Without this fix,
  -----
  $ ./perf probe -x /usr/lib64/libc-2.33.so -F
  @plt
  @plt
  calloc@plt
  free@plt
  malloc@plt
  memalign@plt
  realloc@plt
  -----

With this fix.

  -----
  $ ./perf probe -x /usr/lib64/libc-2.33.so -F
  @plt
  @plt
  a64l
  abort
  abs
  accept
  accept4
  access
  acct
  addmntent
  -----

Reported-by: Thomas Richter <tmricht@linux.ibm.com>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/symbol-elf.c |   82 ++++++++++++++++++++++++++++--------------
 1 file changed, 54 insertions(+), 28 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index a73345730ba9..eb10b4e0d888 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1074,8 +1074,9 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
 	return 0;
 }
 
-int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
-		  struct symsrc *runtime_ss, int kmodule)
+static int
+dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
+		       struct symsrc *runtime_ss, int kmodule, int dynsym)
 {
 	struct kmap *kmap = dso->kernel ? map__kmap(map) : NULL;
 	struct maps *kmaps = kmap ? map__kmaps(map) : NULL;
@@ -1098,34 +1099,15 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
 	if (kmap && !kmaps)
 		return -1;
 
-	dso->symtab_type = syms_ss->type;
-	dso->is_64_bit = syms_ss->is_64_bit;
-	dso->rel = syms_ss->ehdr.e_type == ET_REL;
-
-	/*
-	 * Modules may already have symbols from kallsyms, but those symbols
-	 * have the wrong values for the dso maps, so remove them.
-	 */
-	if (kmodule && syms_ss->symtab)
-		symbols__delete(&dso->symbols);
-
-	if (!syms_ss->symtab) {
-		/*
-		 * If the vmlinux is stripped, fail so we will fall back
-		 * to using kallsyms. The vmlinux runtime symbols aren't
-		 * of much use.
-		 */
-		if (dso->kernel)
-			goto out_elf_end;
-
-		syms_ss->symtab  = syms_ss->dynsym;
-		syms_ss->symshdr = syms_ss->dynshdr;
-	}
-
 	elf = syms_ss->elf;
 	ehdr = syms_ss->ehdr;
-	sec = syms_ss->symtab;
-	shdr = syms_ss->symshdr;
+	if (dynsym) {
+		sec  = syms_ss->dynsym;
+		shdr = syms_ss->dynshdr;
+	} else {
+		sec =  syms_ss->symtab;
+		shdr = syms_ss->symshdr;
+	}
 
 	if (elf_section_by_name(runtime_ss->elf, &runtime_ss->ehdr, &tshdr,
 				".text", NULL))
@@ -1312,6 +1294,50 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
 	return err;
 }
 
+int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
+		  struct symsrc *runtime_ss, int kmodule)
+{
+	int nr = 0;
+	int err = -1;
+
+	dso->symtab_type = syms_ss->type;
+	dso->is_64_bit = syms_ss->is_64_bit;
+	dso->rel = syms_ss->ehdr.e_type == ET_REL;
+
+	/*
+	 * Modules may already have symbols from kallsyms, but those symbols
+	 * have the wrong values for the dso maps, so remove them.
+	 */
+	if (kmodule && syms_ss->symtab)
+		symbols__delete(&dso->symbols);
+
+	if (!syms_ss->symtab) {
+		/*
+		 * If the vmlinux is stripped, fail so we will fall back
+		 * to using kallsyms. The vmlinux runtime symbols aren't
+		 * of much use.
+		 */
+		if (dso->kernel)
+			return err;
+	} else  {
+		err = dso__load_sym_internal(dso, map, syms_ss, runtime_ss,
+					     kmodule, 0);
+		if (err < 0)
+			return err;
+		nr = err;
+	}
+
+	if (syms_ss->dynsym) {
+		err = dso__load_sym_internal(dso, map, syms_ss, runtime_ss,
+					     kmodule, 1);
+		if (err < 0)
+			return err;
+		err += nr;
+	}
+
+	return err;
+}
+
 static int elf_read_maps(Elf *elf, bool exe, mapfn_t mapfn, void *data)
 {
 	GElf_Phdr phdr;


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 3/3] perf probe: Do not show @plt function by default
  2021-07-03 15:35 [PATCH 0/3] perf: Fix perf probe for Fedora34 glibc update Masami Hiramatsu
  2021-07-03 15:35 ` [PATCH 1/3] perf-probe: Fix debuginfo__new() to enable build-id based debuginfo Masami Hiramatsu
  2021-07-03 15:35 ` [PATCH 2/3] perf symbol-elf: Decode dynsym even if symtab exists Masami Hiramatsu
@ 2021-07-03 15:35 ` Masami Hiramatsu
  2021-07-05 12:11   ` Thomas Richter
  2021-07-05 20:48 ` [PATCH 0/3] perf: Fix perf probe for Fedora34 glibc update Namhyung Kim
  3 siblings, 1 reply; 8+ messages in thread
From: Masami Hiramatsu @ 2021-07-03 15:35 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-perf-users, Masami Hiramatsu, Sven Schnelle,
	Heiko Carstens, Stefan Liebler, Thomas Richter, linux-kernel

From: Masami Hiramatsu <mhriamat@kernel.org>

Fix the perf-probe --functions option do not show the PLT
stub symbols (*@plt) by default.

  -----
  $ ./perf probe -x /usr/lib64/libc-2.33.so -F | head
  a64l
  abort
  abs
  accept
  accept4
  access
  acct
  addmntent
  addseverity
  adjtime
  -----

Reported-by: Thomas Richter <tmricht@linux.ibm.com>
Signed-off-by: Masami Hiramatsu <mhriamat@kernel.org>
---
 tools/perf/builtin-probe.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 2bfd41df621c..e1dd51f2874b 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -31,7 +31,7 @@
 #include <linux/zalloc.h>
 
 #define DEFAULT_VAR_FILTER "!__k???tab_* & !__crc_*"
-#define DEFAULT_FUNC_FILTER "!_*"
+#define DEFAULT_FUNC_FILTER "!_* & !*@plt"
 #define DEFAULT_LIST_FILTER "*"
 
 /* Session management structure */


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] perf probe: Do not show @plt function by default
  2021-07-03 15:35 ` [PATCH 3/3] perf probe: Do not show @plt function by default Masami Hiramatsu
@ 2021-07-05 12:11   ` Thomas Richter
  2021-07-05 17:47     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Richter @ 2021-07-05 12:11 UTC (permalink / raw)
  To: Masami Hiramatsu, Arnaldo Carvalho de Melo
  Cc: linux-perf-users, Sven Schnelle, Heiko Carstens, Stefan Liebler,
	linux-kernel

On 7/3/21 5:35 PM, Masami Hiramatsu wrote:
> From: Masami Hiramatsu <mhriamat@kernel.org>
> 
> Fix the perf-probe --functions option do not show the PLT
> stub symbols (*@plt) by default.
> 
>   -----
>   $ ./perf probe -x /usr/lib64/libc-2.33.so -F | head
>   a64l
>   abort
>   abs
>   accept
>   accept4
>   access
>   acct
>   addmntent
>   addseverity
>   adjtime
>   -----
> 
> Reported-by: Thomas Richter <tmricht@linux.ibm.com>
> Signed-off-by: Masami Hiramatsu <mhriamat@kernel.org>
> ---
>  tools/perf/builtin-probe.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 2bfd41df621c..e1dd51f2874b 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -31,7 +31,7 @@
>  #include <linux/zalloc.h>
>  
>  #define DEFAULT_VAR_FILTER "!__k???tab_* & !__crc_*"
> -#define DEFAULT_FUNC_FILTER "!_*"
> +#define DEFAULT_FUNC_FILTER "!_* & !*@plt"
>  #define DEFAULT_LIST_FILTER "*"
>  
>  /* Session management structure */
> 

Thanks, works again ...

-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] perf probe: Do not show @plt function by default
  2021-07-05 12:11   ` Thomas Richter
@ 2021-07-05 17:47     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-07-05 17:47 UTC (permalink / raw)
  To: Thomas Richter
  Cc: Masami Hiramatsu, linux-perf-users, Sven Schnelle,
	Heiko Carstens, Stefan Liebler, linux-kernel

Em Mon, Jul 05, 2021 at 02:11:11PM +0200, Thomas Richter escreveu:
> On 7/3/21 5:35 PM, Masami Hiramatsu wrote:
> > +++ b/tools/perf/builtin-probe.c
> > @@ -31,7 +31,7 @@
> >  #include <linux/zalloc.h>

> >  #define DEFAULT_VAR_FILTER "!__k???tab_* & !__crc_*"
> > -#define DEFAULT_FUNC_FILTER "!_*"
> > +#define DEFAULT_FUNC_FILTER "!_* & !*@plt"
> >  #define DEFAULT_LIST_FILTER "*"

> Thanks, works again ...

I took this as an Acked-by.

Thanks, applied.

- Arnaldo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] perf: Fix perf probe for Fedora34 glibc update
  2021-07-03 15:35 [PATCH 0/3] perf: Fix perf probe for Fedora34 glibc update Masami Hiramatsu
                   ` (2 preceding siblings ...)
  2021-07-03 15:35 ` [PATCH 3/3] perf probe: Do not show @plt function by default Masami Hiramatsu
@ 2021-07-05 20:48 ` Namhyung Kim
  2021-07-06  5:51   ` Masami Hiramatsu
  3 siblings, 1 reply; 8+ messages in thread
From: Namhyung Kim @ 2021-07-05 20:48 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Arnaldo Carvalho de Melo, linux-perf-users, Sven Schnelle,
	Heiko Carstens, Stefan Liebler, Thomas Richter, linux-kernel

On Sat, Jul 3, 2021 at 8:36 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> Hi Arnaldo,
>
> Here is a series of patches to fix the perf-probe error against the
> Fedora34 glibc update, which moves most of symbols from .symtab to
> .dynsym. The key is that the "most of" symbols moved, but it still
> have some PLT symbols in .symtab. Thus the perf symbol-elf failes to
> decode symbols.

Do you know what's the rationale of the move?
Is it a change from glibc or Fedora?

Thanks,
Namhyung


>
> Here is the original report from Thomas about this issue.
>
>  https://lore.kernel.org/linux-perf-users/f6752514-eaf9-371e-f81b-0d9e41ebae0c@linux.ibm.com/
>
> Thank you,
>
> ---
>
> Masami Hiramatsu (3):
>       perf-probe: Fix debuginfo__new() to enable build-id based debuginfo
>       perf symbol-elf: Decode dynsym even if symtab exists
>       perf probe: Do not show @plt function by default
>
>
>  tools/perf/builtin-probe.c     |    2 -
>  tools/perf/util/probe-finder.c |    5 ++
>  tools/perf/util/symbol-elf.c   |   82 ++++++++++++++++++++++++++--------------
>  3 files changed, 60 insertions(+), 29 deletions(-)
>
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] perf: Fix perf probe for Fedora34 glibc update
  2021-07-05 20:48 ` [PATCH 0/3] perf: Fix perf probe for Fedora34 glibc update Namhyung Kim
@ 2021-07-06  5:51   ` Masami Hiramatsu
  0 siblings, 0 replies; 8+ messages in thread
From: Masami Hiramatsu @ 2021-07-06  5:51 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, linux-perf-users, Sven Schnelle,
	Heiko Carstens, Stefan Liebler, Thomas Richter, linux-kernel

On Mon, 5 Jul 2021 13:48:04 -0700
Namhyung Kim <namhyung@gmail.com> wrote:

> On Sat, Jul 3, 2021 at 8:36 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > Hi Arnaldo,
> >
> > Here is a series of patches to fix the perf-probe error against the
> > Fedora34 glibc update, which moves most of symbols from .symtab to
> > .dynsym. The key is that the "most of" symbols moved, but it still
> > have some PLT symbols in .symtab. Thus the perf symbol-elf failes to
> > decode symbols.
> 
> Do you know what's the rationale of the move?
> Is it a change from glibc or Fedora?

I don't know, but it seems that this happens when updating glibc package,
the version is same but revision is different. Also, in the Ubuntu, I saw
all symbols has been moved to .dynsym in older glibc.
Thus I guess that this depends on the build option or configuration.
Might Fedora change the packaging script?

Thank you,

> 
> Thanks,
> Namhyung
> 
> 
> >
> > Here is the original report from Thomas about this issue.
> >
> >  https://lore.kernel.org/linux-perf-users/f6752514-eaf9-371e-f81b-0d9e41ebae0c@linux.ibm.com/
> >
> > Thank you,
> >
> > ---
> >
> > Masami Hiramatsu (3):
> >       perf-probe: Fix debuginfo__new() to enable build-id based debuginfo
> >       perf symbol-elf: Decode dynsym even if symtab exists
> >       perf probe: Do not show @plt function by default
> >
> >
> >  tools/perf/builtin-probe.c     |    2 -
> >  tools/perf/util/probe-finder.c |    5 ++
> >  tools/perf/util/symbol-elf.c   |   82 ++++++++++++++++++++++++++--------------
> >  3 files changed, 60 insertions(+), 29 deletions(-)
> >
> > --
> > Masami Hiramatsu (Linaro) <mhiramat@kernel.org>


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-07-06  5:51 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-03 15:35 [PATCH 0/3] perf: Fix perf probe for Fedora34 glibc update Masami Hiramatsu
2021-07-03 15:35 ` [PATCH 1/3] perf-probe: Fix debuginfo__new() to enable build-id based debuginfo Masami Hiramatsu
2021-07-03 15:35 ` [PATCH 2/3] perf symbol-elf: Decode dynsym even if symtab exists Masami Hiramatsu
2021-07-03 15:35 ` [PATCH 3/3] perf probe: Do not show @plt function by default Masami Hiramatsu
2021-07-05 12:11   ` Thomas Richter
2021-07-05 17:47     ` Arnaldo Carvalho de Melo
2021-07-05 20:48 ` [PATCH 0/3] perf: Fix perf probe for Fedora34 glibc update Namhyung Kim
2021-07-06  5:51   ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).