Skip to contentSkip to conversation

perf: we can now read separate debug-info files based on a build ID

PATCH In review Dima Kogan 3 participants 6 messages Sep 8, 2015 – Oct 29, 2015 perf events

lore
Share & tools
lore

Dima Kogan

author
#0 lore raw

cover perf: we can now read separate debug-info files based on a build ID

Date: Mon, 7 Sep 2015 19:27:01 -0700
(Please Cc me when replying; I'm not subscribed)

Hi. Perf currently has trouble reading separate debug-info files when trying to look up symbols in a 'perf report'. According to the gdb documentation:

https://sourceware.org/gdb/onlinedocs/gdb/Separate-Debug-Files.html

separate debug-info files can live in any of

/usr/lib/debug/.build-id/ab/cdef1234.debug
/usr/bin/debuglink
/usr/bin/.debug/debuglink
/usr/lib/debug/usr/bin/debuglink

Prior to this patch series, perf reads the value of the debuglink from the .gnu_debuglink section, and tries to read a file of that name in the same directory as the stripped ELF file. For an executable, this would be "/usr/bin/debuglink" above. And that's it; the other paths are not checked.

The "normal" thing to do on Debian boxes these days is

/usr/lib/debug/.build-id/ab/cdef1234.debug

This patch series makes this work. The remaining two paths are still unchecked, but I don't know if they're "standard" anywhere

Dima Kogan (2):
perf: fixed type error when reading a build-id
perf: we can now read separate debug-info files based on a build ID

 tools/perf/util/symbol-minimal.c | 2 +-
 tools/perf/util/symbol.c         | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)
Signature · 1 line
2.1.4
Quoted 1 line

From b095f63f4bcfeb9c00f62a7627243c5231e4d666 Mon Sep 17 00:00:00 2001

From: Dima Kogan <[email protected]>
Date: Mon, 7 Sep 2015 17:30:28 -0700
Subject: [PATCH 1/2] perf: fixed type error when reading a build-id

This was benign, but wrong. The build-id should live in a char[], not a char*[]

---
 tools/perf/util/symbol-minimal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/symbol-minimal.c +1 -1
index fd8477c..4890633 100644
@@ -337,7 +337,7 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
 		  symbol_filter_t filter __maybe_unused,
 		  int kmodule __maybe_unused)
 {
-	unsigned char *build_id[BUILD_ID_SIZE];
+	unsigned char build_id[BUILD_ID_SIZE];
 	int ret;
 
 	ret = fd__is_64_bit(ss->fd);
Signature · 1 line
2.1.4
Quoted 1 line

From 956877b14b0243868cbf381f0fc5607eb973f268 Mon Sep 17 00:00:00 2001

From: Dima Kogan <[email protected]>
Date: Mon, 7 Sep 2015 17:34:19 -0700
Subject: [PATCH 2/2] perf: we can now read separate debug-info files based on
a build ID

Recent GDB (at least on a vanilla Debian box) looks for debug information in

/usr/lib/debug/.build-id/nn/nnnnnnn

where nn/nnnnnn is the build-id of the stripped ELF binary. This is documented here:

https://sourceware.org/gdb/onlinedocs/gdb/Separate-Debug-Files.html

This was not working in perf because we didn't read the build id until AFTER we searched for the separate debug information file. This patch reads the build ID and THEN does the search.

---
 tools/perf/util/symbol.c | 9 +++++++++
 1 file changed, 9 insertions(+)
tools/perf/util/symbol.c +9 -0
index 1f97ffb..05c656b 100644
@@ -1406,6 +1406,7 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 	struct symsrc ss_[2];
 	struct symsrc *syms_ss = NULL, *runtime_ss = NULL;
 	bool kmod;
+	unsigned char build_id[BUILD_ID_SIZE];
 
 	pthread_mutex_lock(&dso->lock);
 
@@ -1461,6 +1462,14 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 		dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE ||
 		dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE_COMP;
 
+
+	/*
+	 * Read the build id if possible. This is required for
+	 * DSO_BINARY_TYPE__BUILDID_DEBUGINFO to work
+	 */
+	if (filename__read_build_id(dso->name, build_id, BUILD_ID_SIZE) > 0)
+		dso__set_build_id(dso, build_id);
+
 	/*
 	 * Iterate over candidate debug images.
 	 * Keep track of "interesting" ones (those which have a symtab, dynsym,
Signature · 1 line
2.1.4
author
#1 lore raw

Hi. This is a gentle ping. This patch fixes a real bug, and it'd be great if it was merged.

Here's a demo. This is on a recent Debian/sid box, tracing malloc() calls in emacs. The debug symbols for libc and emacs are split into separate (installed) packages, as is the norm.

    $ perf probe -x /lib/x86_64-linux-gnu/libc-2.19.so --add malloc                                                          
    $ perf record -g -eprobe_libc:malloc -p `pidof emacs`

Before patch:

    $ perf script | head -n 10
    emacs  7750 [001] 353845.297868: probe_libc:malloc: (7f4fbde59020)
                       7c020 malloc (/lib/x86_64-linux-gnu/libc-2.19.so)
                      144877 [unknown] (/usr/bin/emacs-snapshot-lucid)
                      144989 [unknown] (/usr/bin/emacs-snapshot-lucid)
                      144bcc [unknown] (/usr/bin/emacs-snapshot-lucid)
                      15a91b [unknown] (/usr/bin/emacs-snapshot-lucid)
                       99f25 [unknown] (/usr/bin/emacs-snapshot-lucid)
                      199211 [unknown] (/usr/bin/emacs-snapshot-lucid)
                      1a0c6b [unknown] (/usr/bin/emacs-snapshot-lucid)
                       1ed86 [unknown] (/usr/bin/emacs-snapshot-lucid)

After patch:

    $ perf script | head -n 10 
    emacs  7750 [001] 353845.297868: probe_libc:malloc: (7f4fbde59020)
                       7c020 malloc (/lib/x86_64-linux-gnu/libc-2.19.so)
                      144877 allocate_string_data (/usr/bin/emacs-snapshot-lucid)
                      144989 make_uninit_multibyte_string (/usr/bin/emacs-snapshot-lucid)
                      144bcc make_uninit_string.part.18 (/usr/bin/emacs-snapshot-lucid)
                      15a91b make_buffer_string_both (/usr/bin/emacs-snapshot-lucid)
                       99f25 decode_coding_object (/usr/bin/emacs-snapshot-lucid)
                      199211 read_process_output (/usr/bin/emacs-snapshot-lucid)
                      1a0c6b wait_reading_process_output (/usr/bin/emacs-snapshot-lucid)
                       1ed86 sit_for (/usr/bin/emacs-snapshot-lucid)
author
#2 lore raw

Hi. I'd like to get this merged before I forget about it and then I hit this bug again at some point in the future. Any comment would be great.

Thanks!

#3 lore raw

Em Tue, Oct 27, 2015 at 07:50:41PM -0700, Dima Kogan escreveu:

Quoted 1 line

Hi. I'd like to get this merged before I forget about it and then I hit

Hey, me too! But you forgot to ask me to :-) I.e. please CC whoever is listed in the MAINTAINERS file, also please next time use 'git format-patch' when sending patches, and don't attach the patches, so that I can use 'git am' on them.

Anyway, thanks for the patches, I am going thru them now.

- Arnaldo

Quoted 8 lines

this bug again at some point in the future. Any comment would be great.

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#4 lore raw

[tip:perf/core] perf symbols: Fix type error when reading a build-id

In reply to Dima Kogan
perf symbols: Fix type error when reading a build-id
Commit-ID:  f2f3096888569ff1fc15fa0187a39eef3a24b28e
Gitweb:     http://git.kernel.org/tip/f2f3096888569ff1fc15fa0187a39eef3a24b28e
Author:     Dima Kogan <[email protected]>
AuthorDate: Mon, 7 Sep 2015 17:30:28 -0700
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 28 Oct 2015 10:02:00 -0300

perf symbols: Fix type error when reading a build-id

This was benign, but wrong. The build-id should live in a char[], not a char*[]

---
 tools/perf/util/symbol-minimal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/symbol-minimal.c +1 -1
index fd8477c..4890633 100644
@@ -337,7 +337,7 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
 		  symbol_filter_t filter __maybe_unused,
 		  int kmodule __maybe_unused)
 {
-	unsigned char *build_id[BUILD_ID_SIZE];
+	unsigned char build_id[BUILD_ID_SIZE];
 	int ret;
 
 	ret = fd__is_64_bit(ss->fd);
#5 lore raw

[tip:perf/core] perf symbols: we can now read separate debug-info files based on a build ID

In reply to Dima Kogan
perf symbols: we can now read separate debug-info files based on a build ID
Commit-ID:  5baecbcd9c9a2f491afe1369fc22e7363f9c94d5
Gitweb:     http://git.kernel.org/tip/5baecbcd9c9a2f491afe1369fc22e7363f9c94d5
Author:     Dima Kogan <[email protected]>
AuthorDate: Mon, 7 Sep 2015 17:34:19 -0700
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 28 Oct 2015 10:04:27 -0300

perf symbols: we can now read separate debug-info files based on a build ID

Recent GDB (at least on a vanilla Debian box) looks for debug information in

/usr/lib/debug/.build-id/nn/nnnnnnn

where nn/nnnnnn is the build-id of the stripped ELF binary. This is documented here:

https://sourceware.org/gdb/onlinedocs/gdb/Separate-Debug-Files.html

This was not working in perf because we didn't read the build id until AFTER we searched for the separate debug information file. This patch reads the build ID and THEN does the search.

---
 tools/perf/util/symbol.c | 9 +++++++++
 1 file changed, 9 insertions(+)
tools/perf/util/symbol.c +9 -0
index e7bf0c4..71bf711 100644
@@ -1406,6 +1406,7 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 	struct symsrc ss_[2];
 	struct symsrc *syms_ss = NULL, *runtime_ss = NULL;
 	bool kmod;
+	unsigned char build_id[BUILD_ID_SIZE];
 
 	pthread_mutex_lock(&dso->lock);
 
@@ -1461,6 +1462,14 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 		dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE ||
 		dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE_COMP;
 
+
+	/*
+	 * Read the build id if possible. This is required for
+	 * DSO_BINARY_TYPE__BUILDID_DEBUGINFO to work
+	 */
+	if (filename__read_build_id(dso->name, build_id, BUILD_ID_SIZE) > 0)
+		dso__set_build_id(dso, build_id);
+
 	/*
 	 * Iterate over candidate debug images.
 	 * Keep track of "interesting" ones (those which have a symtab, dynsym,