* [PATCH 0/1] Support .gnu_debugdata for symbols in perf
@ 2025-02-13 19:05 Stephen Brennan
2025-02-13 19:05 ` [PATCH 1/1] tools: perf: support .gnu_debugdata for symbols Stephen Brennan
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Brennan @ 2025-02-13 19:05 UTC (permalink / raw)
To: Namhyung Kim, Ingo Molnar, Arnaldo Carvalho de Melo,
Peter Zijlstra
Cc: Stephen Brennan, linux-perf-users, Adrian Hunter, Liang, Kan,
James Clark, Athira Rajeev, linux-kernel, Chaitanya S Prakash,
Alexander Shishkin, Jiri Olsa, Mark Rutland, Ian Rogers
Hello all,
A while back, Fedora implemented a "MiniDebugInfo" feature[1]. This allows the
full symbol table of a binary to be compressed into a fake ELF file and embedded
within the ".gnu_debugdata" section. The standard RPMs thus include the full
symbol information, at least enough to provide symbol names for stack traces. As
part of the project, support for .gnu_debugdata was implemented in GDB. However,
perf does not yet have support for it.
For Fedora, RHEL, and all the derivatives, there is already the ".eh_frame"
section in ELF files, which allows perf to unwind stacks with "--call-graph
dwarf", and in some cases, there may be frame pointers to make it even easier
and more efficient to gather stack traces.
But once the data is recorded, perf may still be missing the symbol names for
many functions, because it doesn't have support for .gnu_debugdata.
This patch is a first stab at implementing that support. To demonstrate, here is
a diff between a sample output by "perf script" before and after the change:
@@ -16,9 +16,9 @@
55fdc45c837f rl_read_key+0x17f (/usr/bin/bash)
55fdc45a744b readline_internal_char+0x6b (/usr/bin/bash)
55fdc45a776c readline+0x4c (/usr/bin/bash)
- 55fdc4509dbe [unknown] (/usr/bin/bash)
- 55fdc450ce28 [unknown] (/usr/bin/bash)
- 55fdc450ebb9 [unknown] (/usr/bin/bash)
+ 55fdc4509dbe yy_readline_get+0xae (/usr/bin/bash)
+ 55fdc450ce28 shell_getc.lto_priv.0+0x278 (/usr/bin/bash)
+ 55fdc450ebb9 read_token.part.0+0x49 (/usr/bin/bash)
55fdc4512739 yyparse+0x3f9 (/usr/bin/bash)
55fdc4515bbc parse_command+0x3c (/usr/bin/bash)
55fdc4515d8c read_command+0x6c (/usr/bin/bash)
I believe this support will be even more important in the future, if and when
SFrame becomes supported: then we will have an efficient unwinder that doesn't
require debuginfo to be installed. But users won't be able to take full
advantage of their perf recordings without having the best symbol information
available.
My approach almost certainly can be improved: I'm not sure whether there is a
strong preference to decompress the ".gnu_debugdata" into a temporary file, or
if keeping it in-memory is good enough. I'd love feedback, since I know this
approach is a bit rough around the edges.
Thanks,
Stephen
[1]: https://fedoraproject.org/wiki/Features/MiniDebugInfo
Stephen Brennan (1):
tools: perf: support .gnu_debugdata for symbols
tools/perf/util/dso.c | 2 +
tools/perf/util/dso.h | 1 +
tools/perf/util/symbol-elf.c | 141 ++++++++++++++++++++++++++++++++++-
tools/perf/util/symbol.c | 2 +
tools/perf/util/symsrc.h | 1 +
5 files changed, 143 insertions(+), 4 deletions(-)
--
2.43.5
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/1] tools: perf: support .gnu_debugdata for symbols 2025-02-13 19:05 [PATCH 0/1] Support .gnu_debugdata for symbols in perf Stephen Brennan @ 2025-02-13 19:05 ` Stephen Brennan 2025-02-14 19:30 ` Andi Kleen 0 siblings, 1 reply; 6+ messages in thread From: Stephen Brennan @ 2025-02-13 19:05 UTC (permalink / raw) To: Namhyung Kim, Ingo Molnar, Arnaldo Carvalho de Melo, Peter Zijlstra Cc: Stephen Brennan, linux-perf-users, Adrian Hunter, Liang, Kan, James Clark, Athira Rajeev, linux-kernel, Chaitanya S Prakash, Alexander Shishkin, Jiri Olsa, Mark Rutland, Ian Rogers Fedora introduced a "MiniDebuginfo" feature, in which an LZMA-compressed ELF file is placed inside a section named ".gnu_debugdata". This file contains nothing but a symbol table, which can be used to supplement the .dynsym section which only contains required symbols for runtime. It is supported by GDB for stack traces, but it should be useful for tracing as well. Implement support for loading symbols from .gnu_debugdata. Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com> --- tools/perf/util/dso.c | 2 + tools/perf/util/dso.h | 1 + tools/perf/util/symbol-elf.c | 141 ++++++++++++++++++++++++++++++++++- tools/perf/util/symbol.c | 2 + tools/perf/util/symsrc.h | 1 + 5 files changed, 143 insertions(+), 4 deletions(-) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 5c6e85fdae0de..67c21999628d4 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -93,6 +93,7 @@ bool dso__is_object_file(const struct dso *dso) case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO: case DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO: case DSO_BINARY_TYPE__BUILDID_DEBUGINFO: + case DSO_BINARY_TYPE__GNU_DEBUGDATA: case DSO_BINARY_TYPE__SYSTEM_PATH_DSO: case DSO_BINARY_TYPE__GUEST_KMODULE: case DSO_BINARY_TYPE__GUEST_KMODULE_COMP: @@ -224,6 +225,7 @@ int dso__read_binary_type_filename(const struct dso *dso, case DSO_BINARY_TYPE__VMLINUX: case DSO_BINARY_TYPE__GUEST_VMLINUX: case DSO_BINARY_TYPE__SYSTEM_PATH_DSO: + case DSO_BINARY_TYPE__GNU_DEBUGDATA: __symbol__join_symfs(filename, size, dso__long_name(dso)); break; diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index bb8e8f444054d..84d5aac666aa1 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -33,6 +33,7 @@ enum dso_binary_type { DSO_BINARY_TYPE__UBUNTU_DEBUGINFO, DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO, DSO_BINARY_TYPE__BUILDID_DEBUGINFO, + DSO_BINARY_TYPE__GNU_DEBUGDATA, DSO_BINARY_TYPE__SYSTEM_PATH_DSO, DSO_BINARY_TYPE__GUEST_KMODULE, DSO_BINARY_TYPE__GUEST_KMODULE_COMP, diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c index 66fd1249660a3..e578b7d406a69 100644 --- a/tools/perf/util/symbol-elf.c +++ b/tools/perf/util/symbol-elf.c @@ -6,6 +6,7 @@ #include <string.h> #include <unistd.h> #include <inttypes.h> +#include <lzma.h> #include "dso.h" #include "map.h" @@ -1214,7 +1215,9 @@ void symsrc__destroy(struct symsrc *ss) { zfree(&ss->name); elf_end(ss->elf); - close(ss->fd); + free(ss->image); + if (ss->fd != -1) + close(ss->fd); } bool elf__needs_adjust_symbols(GElf_Ehdr ehdr) @@ -1228,12 +1231,115 @@ bool elf__needs_adjust_symbols(GElf_Ehdr ehdr) ehdr.e_type == ET_DYN; } +static Elf *read_gnu_debugdata(struct dso *dso, Elf *elf, const char *name, void **image_ret) +{ + Elf *elf_embedded; + GElf_Ehdr ehdr; + GElf_Shdr shdr; + Elf_Scn *scn; + Elf_Data *scn_data; + void *data = NULL; + size_t shndx, datasize, bytes_decoded; + lzma_stream stream = LZMA_STREAM_INIT; + lzma_ret ret; + + if (gelf_getehdr(elf, &ehdr) == NULL) { + pr_debug("%s: cannot read %s ELF file.\n", __func__, name); + *dso__load_errno(dso) = DSO_LOAD_ERRNO__INVALID_ELF; + return NULL; + } + + scn = elf_section_by_name(elf, &ehdr, &shdr, ".gnu_debugdata", &shndx); + if (!scn) { + pr_debug("%s: ELF file %s has no .gnu_debugdata\n", __func__, name); + *dso__load_errno(dso) = -ENOENT; + return NULL; + } + + if (shdr.sh_type == SHT_NOBITS) { + pr_debug("%s: .gnu_debugdata of ELF file %s has no data.\n", __func__, name); + *dso__load_errno(dso) = DSO_LOAD_ERRNO__INVALID_ELF; + return NULL; + } + + scn_data = elf_rawdata(scn, NULL); + if (!scn_data) { + pr_debug("%s: error reading .gnu_debugdata of %s: %s\n", __func__, + name, elf_errmsg(-1)); + *dso__load_errno(dso) = DSO_LOAD_ERRNO__INVALID_ELF; + return NULL; + } + + ret = lzma_stream_decoder(&stream, UINT64_MAX, 0); + if (ret != LZMA_OK) { + pr_debug("%s: error decompressing .gnu_debugdata of %s: %d\n", __func__, + name, ret); + *dso__load_errno(dso) = DSO_LOAD_ERRNO__INVALID_ELF; + lzma_end(&stream); + return NULL; + } + + stream.next_in = scn_data->d_buf; + stream.avail_in = scn_data->d_size; + + datasize = 2 * scn_data->d_size; + data = malloc(datasize); + + stream.next_out = data; + stream.avail_out = scn_data->d_size; + while (1) { + void *data_resized; + + ret = lzma_code(&stream, LZMA_RUN); + if (ret != LZMA_OK && ret != LZMA_STREAM_END) { + pr_debug("%s: error decompressing .gnu_debugdata of %s: %d\n", __func__, + name, ret); + *dso__load_errno(dso) = DSO_LOAD_ERRNO__INVALID_ELF; + free(data); + lzma_end(&stream); + return NULL; + } + bytes_decoded = (void *)stream.next_out - data; + if (ret == LZMA_STREAM_END) + datasize = bytes_decoded; + else + datasize *= 2; + data_resized = realloc(data, datasize); + if (!data_resized) { + pr_debug("%s: allocation error accessing .gnu_debugdata of %s\n", + __func__, name); + *dso__load_errno(dso) = -ENOMEM; + free(data); + lzma_end(&stream); + return NULL; + } + data = data_resized; + stream.next_out = data + bytes_decoded; + stream.avail_out = datasize - bytes_decoded; + if (ret == LZMA_STREAM_END) + break; + } + lzma_end(&stream); + + elf_embedded = elf_memory(data, bytes_decoded); + if (!elf_embedded) { + pr_debug("%s: error reading .gnu_debugdata of %s: %s\n", __func__, + name, elf_errmsg(-1)); + *dso__load_errno(dso) = DSO_LOAD_ERRNO__INVALID_ELF; + free(data); + return NULL; + } + *image_ret = data; + return elf_embedded; +} + int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name, enum dso_binary_type type) { GElf_Ehdr ehdr; Elf *elf; int fd; + ss->image = NULL; if (dso__needs_decompress(dso)) { fd = dso__decompress_kmodule_fd(dso, name); @@ -1256,6 +1362,18 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name, goto out_close; } + if (type == DSO_BINARY_TYPE__GNU_DEBUGDATA) { + Elf *embedded = read_gnu_debugdata(dso, elf, name, &ss->image); + + if (!embedded) + goto out_close; + + elf_end(elf); + close(fd); + fd = -1; + elf = embedded; + } + if (gelf_getehdr(elf, &ehdr) == NULL) { *dso__load_errno(dso) = DSO_LOAD_ERRNO__INVALID_ELF; pr_debug("%s: cannot get elf header.\n", __func__); @@ -1328,7 +1446,9 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name, out_elf_end: elf_end(elf); out_close: - close(fd); + if (fd != -1) + close(fd); + free(ss->image); return -1; } @@ -1854,10 +1974,23 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss, kmodule, 1); if (err < 0) return err; - err += nr; + nr += err; } - return err; + /* + * The .gnu_debugdata is a special situation: it contains a symbol + * table, but the runtime file may also contain dynsym entries which are + * not present there. We need to load both. + */ + if (syms_ss->type == DSO_BINARY_TYPE__GNU_DEBUGDATA && runtime_ss->dynsym) { + err = dso__load_sym_internal(dso, map, runtime_ss, runtime_ss, + kmodule, 1); + if (err < 0) + return err; + nr += err; + } + + return nr; } static int elf_read_maps(Elf *elf, bool exe, mapfn_t mapfn, void *data) diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 49b08adc6ee34..a0767762d4d73 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -84,6 +84,7 @@ static enum dso_binary_type binary_type_symtab[] = { DSO_BINARY_TYPE__FEDORA_DEBUGINFO, DSO_BINARY_TYPE__UBUNTU_DEBUGINFO, DSO_BINARY_TYPE__BUILDID_DEBUGINFO, + DSO_BINARY_TYPE__GNU_DEBUGDATA, DSO_BINARY_TYPE__SYSTEM_PATH_DSO, DSO_BINARY_TYPE__GUEST_KMODULE, DSO_BINARY_TYPE__GUEST_KMODULE_COMP, @@ -1716,6 +1717,7 @@ static bool dso__is_compatible_symtab_type(struct dso *dso, bool kmod, case DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO: case DSO_BINARY_TYPE__BUILDID_DEBUGINFO: case DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO: + case DSO_BINARY_TYPE__GNU_DEBUGDATA: return !kmod && dso__kernel(dso) == DSO_SPACE__USER; case DSO_BINARY_TYPE__KALLSYMS: diff --git a/tools/perf/util/symsrc.h b/tools/perf/util/symsrc.h index edf82028c9e62..38b628f12aaa5 100644 --- a/tools/perf/util/symsrc.h +++ b/tools/perf/util/symsrc.h @@ -15,6 +15,7 @@ struct symsrc { char *name; int fd; + void *image; enum dso_binary_type type; #ifdef HAVE_LIBELF_SUPPORT -- 2.43.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] tools: perf: support .gnu_debugdata for symbols 2025-02-13 19:05 ` [PATCH 1/1] tools: perf: support .gnu_debugdata for symbols Stephen Brennan @ 2025-02-14 19:30 ` Andi Kleen 2025-02-19 21:15 ` Namhyung Kim 0 siblings, 1 reply; 6+ messages in thread From: Andi Kleen @ 2025-02-14 19:30 UTC (permalink / raw) To: Stephen Brennan Cc: Namhyung Kim, Ingo Molnar, Arnaldo Carvalho de Melo, Peter Zijlstra, linux-perf-users, Adrian Hunter, Liang, Kan, James Clark, Athira Rajeev, linux-kernel, Chaitanya S Prakash, Alexander Shishkin, Jiri Olsa, Mark Rutland, Ian Rogers Stephen Brennan <stephen.s.brennan@oracle.com> writes: > DSO_BINARY_TYPE__GUEST_KMODULE_COMP, > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c > index 66fd1249660a3..e578b7d406a69 100644 > --- a/tools/perf/util/symbol-elf.c > +++ b/tools/perf/util/symbol-elf.c > @@ -6,6 +6,7 @@ > #include <string.h> > #include <unistd.h> > #include <inttypes.h> > +#include <lzma.h> This needs some ifdefs in case lzma.h is not available? -Andi ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] tools: perf: support .gnu_debugdata for symbols 2025-02-14 19:30 ` Andi Kleen @ 2025-02-19 21:15 ` Namhyung Kim 2025-02-19 21:57 ` Stephen Brennan 0 siblings, 1 reply; 6+ messages in thread From: Namhyung Kim @ 2025-02-19 21:15 UTC (permalink / raw) To: Andi Kleen Cc: Stephen Brennan, Ingo Molnar, Arnaldo Carvalho de Melo, Peter Zijlstra, linux-perf-users, Adrian Hunter, Liang, Kan, James Clark, Athira Rajeev, linux-kernel, Chaitanya S Prakash, Alexander Shishkin, Jiri Olsa, Mark Rutland, Ian Rogers Hello, On Fri, Feb 14, 2025 at 11:30:26AM -0800, Andi Kleen wrote: > Stephen Brennan <stephen.s.brennan@oracle.com> writes: > > DSO_BINARY_TYPE__GUEST_KMODULE_COMP, > > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c > > index 66fd1249660a3..e578b7d406a69 100644 > > --- a/tools/perf/util/symbol-elf.c > > +++ b/tools/perf/util/symbol-elf.c > > @@ -6,6 +6,7 @@ > > #include <string.h> > > #include <unistd.h> > > #include <inttypes.h> > > +#include <lzma.h> > > This needs some ifdefs in case lzma.h is not available? Right, should be guarded by HAVE_LZMA_SUPPORT. Thanks, Namhyung ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] tools: perf: support .gnu_debugdata for symbols 2025-02-19 21:15 ` Namhyung Kim @ 2025-02-19 21:57 ` Stephen Brennan 2025-02-19 22:14 ` Namhyung Kim 0 siblings, 1 reply; 6+ messages in thread From: Stephen Brennan @ 2025-02-19 21:57 UTC (permalink / raw) To: Namhyung Kim, Andi Kleen Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Peter Zijlstra, linux-perf-users, Adrian Hunter, Liang, Kan, James Clark, linux-kernel, Chaitanya S Prakash, Alexander Shishkin, Jiri Olsa, Mark Rutland, Ian Rogers Namhyung Kim <namhyung@kernel.org> writes: > Hello, > > On Fri, Feb 14, 2025 at 11:30:26AM -0800, Andi Kleen wrote: >> Stephen Brennan <stephen.s.brennan@oracle.com> writes: >> > DSO_BINARY_TYPE__GUEST_KMODULE_COMP, >> > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c >> > index 66fd1249660a3..e578b7d406a69 100644 >> > --- a/tools/perf/util/symbol-elf.c >> > +++ b/tools/perf/util/symbol-elf.c >> > @@ -6,6 +6,7 @@ >> > #include <string.h> >> > #include <unistd.h> >> > #include <inttypes.h> >> > +#include <lzma.h> >> >> This needs some ifdefs in case lzma.h is not available? > > Right, should be guarded by HAVE_LZMA_SUPPORT. Thank you both. Do you happen to have a preference on whether to do this inline with the HAVE_LZMA_SUPPORT guards, or whether to use a helper like lzma_decompress_to_file() with a temporary file (presumably one which is already unlinked...)? I suppose the more I ask the question, the more it feels obvious that an unlinked temporary file would require less new code. Both because I could use the existing helper, and also because there would be no need to free() the data which we decompressed into memory. So unless you have strong opinions I guess I'll switch it over :) Thanks, Stephen ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] tools: perf: support .gnu_debugdata for symbols 2025-02-19 21:57 ` Stephen Brennan @ 2025-02-19 22:14 ` Namhyung Kim 0 siblings, 0 replies; 6+ messages in thread From: Namhyung Kim @ 2025-02-19 22:14 UTC (permalink / raw) To: Stephen Brennan Cc: Andi Kleen, Ingo Molnar, Arnaldo Carvalho de Melo, Peter Zijlstra, linux-perf-users, Adrian Hunter, Liang, Kan, James Clark, linux-kernel, Chaitanya S Prakash, Alexander Shishkin, Jiri Olsa, Mark Rutland, Ian Rogers On Wed, Feb 19, 2025 at 01:57:25PM -0800, Stephen Brennan wrote: > Namhyung Kim <namhyung@kernel.org> writes: > > Hello, > > > > On Fri, Feb 14, 2025 at 11:30:26AM -0800, Andi Kleen wrote: > >> Stephen Brennan <stephen.s.brennan@oracle.com> writes: > >> > DSO_BINARY_TYPE__GUEST_KMODULE_COMP, > >> > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c > >> > index 66fd1249660a3..e578b7d406a69 100644 > >> > --- a/tools/perf/util/symbol-elf.c > >> > +++ b/tools/perf/util/symbol-elf.c > >> > @@ -6,6 +6,7 @@ > >> > #include <string.h> > >> > #include <unistd.h> > >> > #include <inttypes.h> > >> > +#include <lzma.h> > >> > >> This needs some ifdefs in case lzma.h is not available? > > > > Right, should be guarded by HAVE_LZMA_SUPPORT. > > Thank you both. Do you happen to have a preference on whether to do this > inline with the HAVE_LZMA_SUPPORT guards, or whether to use a helper > like lzma_decompress_to_file() with a temporary file (presumably one > which is already unlinked...)? > > I suppose the more I ask the question, the more it feels obvious that an > unlinked temporary file would require less new code. Both because I > could use the existing helper, and also because there would be no need > to free() the data which we decompressed into memory. So unless you have > strong opinions I guess I'll switch it over :) Should be ok to use the existing helper but I guess you still need to call close() once it's done with the data. Also I think it's better to add dummy functions in the compress.h when LZMA is not supported so that the symbol code can be unaware of the configurations. Thanks, Namhyung ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-02-19 22:14 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-02-13 19:05 [PATCH 0/1] Support .gnu_debugdata for symbols in perf Stephen Brennan 2025-02-13 19:05 ` [PATCH 1/1] tools: perf: support .gnu_debugdata for symbols Stephen Brennan 2025-02-14 19:30 ` Andi Kleen 2025-02-19 21:15 ` Namhyung Kim 2025-02-19 21:57 ` Stephen Brennan 2025-02-19 22:14 ` Namhyung Kim
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).