[PATCH 0/3] perf: Fix perf probe for Fedora34 glibc update

[PATCH 0/3] perf: Fix perf probe for Fedora34 glibc update

[Masami Hiramatsu]

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>

[PATCH 1/3] perf-probe: Fix debuginfo__new() to enable build-id based debuginfo

[Masami Hiramatsu]

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++) {

[PATCH 2/3] perf symbol-elf: Decode dynsym even if symtab exists

[Masami Hiramatsu]

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;

[PATCH 3/3] perf probe: Do not show @plt function by default

[Masami Hiramatsu]

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 */

Re: [PATCH 3/3] perf probe: Do not show @plt function by default

[Thomas Richter]

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

Re: [PATCH 3/3] perf probe: Do not show @plt function by default

[Arnaldo Carvalho de Melo]

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

Re: [PATCH 0/3] perf: Fix perf probe for Fedora34 glibc update

[Namhyung Kim]

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>

Re: [PATCH 0/3] perf: Fix perf probe for Fedora34 glibc update

[Masami Hiramatsu]

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>