All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf: Fix looking up dwarf unwind stack info
@ 2017-01-17 14:50 Matija Glavinic Pecotic
  2017-01-18 13:10 ` Jiri Olsa
  2017-01-26 15:25 ` [tip:perf/core] perf unwind: " tip-bot for Matija Glavinic Pecotic
  0 siblings, 2 replies; 3+ messages in thread
From: Matija Glavinic Pecotic @ 2017-01-17 14:50 UTC (permalink / raw)
  To: linux-kernel, namhyung, Jiri Olsa, acme, Masami Hiramatsu
  Cc: Sverdlin, Alexander (Nokia - DE/Ulm)

Using perf with call graph method dwarf fails to provide backtrace support
for stripped binary even though .gnu_debuglink points to *.dbg flavor with
properly populated debug symbols.

Problem is reproduced on ARM (v7, v8), kernels 3.14.y, 4.4.y and 4.10.rc3.
Perf is configured with libunwind, and unwind dwarf support [1]. Test code
(stress_bt.c) can be found on [2].

Running (explicitly disable other unwinding methods):

$ gcc -g -o stress_bt -fomit-frame-pointer -fno-unwind-tables \
	-fno-asynchronous-unwind-tables stress_bt.c
$ perf record -N --call-graph dwarf ./stress_bt
$ perf report

results in properly generated call graph. Stripping the binary and running
it results with missing call graph. Expected result is to have call graph:

$ gcc -g -o stress_bt -fomit-frame-pointer -fno-unwind-tables \
	-fno-asynchronous-unwind-tables stress_bt.c
$ objcopy --only-keep-debug stress_bt stress_bt.dbg
$ objcopy --strip-debug stress_bt
$ objcopy --add-gnu-debuglink=stress_bt.dbg stress_bt
$ perf record -N --call-graph dwarf ./stress_bt
$ perf report

Problem is that perf doesn't try to read symbols pointed by gnu debuglink.
Patch adds checking, and reading of the symbols from debuglink and symsrc.
Order of the check is to first check within dso, then check whether symsrc
is defined and try to read from it. Finally, debuglink is checked. Default
locations of debug files are discussed in [3] and [4]. Comments on RFC are
on [5].

[1] https://wiki.linaro.org/LEG/Engineering/TOOLS/perf-callstack-unwinding
[2] [1]#Backtrace_stress_application
[3] https://sourceware.org/gdb/onlinedocs/gdb/Separate-Debug-Files.html
[4] https://sourceware.org/binutils/docs/binutils/objcopy.html
[5] https://lkml.org/lkml/2016/8/22/473

Signed-off-by: Matija Glavinic Pecotic <matija.glavinic-pecotic.ext@nokia.com>
---
 tools/perf/util/dso.c                    | 48 +++++++++++++++++++++-------
 tools/perf/util/unwind-libunwind-local.c | 54 +++++++++++++++++++++++++++++---
 2 files changed, 86 insertions(+), 16 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index d2c6cdd..28d41e7 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -9,6 +9,13 @@
 #include "debug.h"
 #include "vdso.h"
 
+static const char * const debuglink_paths[] = {
+	"%.0s%s",
+	"%s/%s",
+	"%s/.debug/%s",
+	"/usr/lib/debug%s/%s"
+};
+
 char dso__symtab_origin(const struct dso *dso)
 {
 	static const char origin[] = {
@@ -44,24 +51,43 @@ int dso__read_binary_type_filename(const struct dso *dso,
 	size_t len;
 
 	switch (type) {
-	case DSO_BINARY_TYPE__DEBUGLINK: {
-		char *debuglink;
+	case DSO_BINARY_TYPE__DEBUGLINK:
+	{
+		const char *last_slash;
+		char dso_dir[PATH_MAX];
+		char symfile[PATH_MAX];
+		unsigned int i;
 
 		len = __symbol__join_symfs(filename, size, dso->long_name);
-		debuglink = filename + len;
-		while (debuglink != filename && *debuglink != '/')
-			debuglink--;
-		if (*debuglink == '/')
-			debuglink++;
+		last_slash = filename + len;
+		while (last_slash != filename && *last_slash != '/')
+			last_slash--;
 
-		ret = -1;
-		if (!is_regular_file(filename))
+		strncpy(dso_dir, filename, last_slash - filename);
+		dso_dir[last_slash-filename] = '\0';
+
+		if (!is_regular_file(filename)) {
+			ret = -1;
+			break;
+		}
+
+		ret = filename__read_debuglink(filename, symfile, PATH_MAX);
+		if (ret)
 			break;
 
-		ret = filename__read_debuglink(filename, debuglink,
-					       size - (debuglink - filename));
+		/* Check predefined locations where debug file might reside */
+		ret = -1;
+		for (i = 0; i < ARRAY_SIZE(debuglink_paths); i++) {
+			snprintf(filename, size,
+					debuglink_paths[i], dso_dir, symfile);
+			if (is_regular_file(filename)) {
+				ret = 0;
+				break;
+			}
 		}
+
 		break;
+	}
 	case DSO_BINARY_TYPE__BUILD_ID_CACHE:
 		if (dso__build_id_filename(dso, filename, size) == NULL)
 			ret = -1;
diff --git a/tools/perf/util/unwind-libunwind-local.c b/tools/perf/util/unwind-libunwind-local.c
index 6fec84d..bfb9b79 100644
--- a/tools/perf/util/unwind-libunwind-local.c
+++ b/tools/perf/util/unwind-libunwind-local.c
@@ -35,6 +35,7 @@
 #include "util.h"
 #include "debug.h"
 #include "asm/bug.h"
+#include "dso.h"
 
 extern int
 UNW_OBJ(dwarf_search_unwind_table) (unw_addr_space_t as,
@@ -297,15 +298,58 @@ static int read_unwind_spec_debug_frame(struct dso *dso,
 	int fd;
 	u64 ofs = dso->data.debug_frame_offset;
 
+	/* debug_frame can reside in:
+	 *  - dso
+	 *  - debug pointed by symsrc_filename
+	 *  - gnu_debuglink, which doesn't necessary
+	 *    has to be pointed by symsrc_filename
+	 */
 	if (ofs == 0) {
 		fd = dso__data_get_fd(dso, machine);
-		if (fd < 0)
-			return -EINVAL;
+		if (fd >= 0) {
+			ofs = elf_section_offset(fd, ".debug_frame");
+			dso__data_put_fd(dso);
+		}
+
+		if (ofs <= 0) {
+			fd = open(dso->symsrc_filename, O_RDONLY);
+			if (fd >= 0) {
+				ofs = elf_section_offset(fd, ".debug_frame");
+				close(fd);
+			}
+		}
+
+		if (ofs <= 0) {
+			char *debuglink = malloc(PATH_MAX);
+			int ret = 0;
+
+			ret = dso__read_binary_type_filename(
+				dso, DSO_BINARY_TYPE__DEBUGLINK,
+				machine->root_dir, debuglink, PATH_MAX);
+			if (!ret) {
+				fd = open(debuglink, O_RDONLY);
+				if (fd >= 0) {
+					ofs = elf_section_offset(fd,
+							".debug_frame");
+					close(fd);
+				}
+			}
+			if (ofs > 0) {
+				if (dso->symsrc_filename != NULL) {
+					pr_warning(
+						"%s: overwrite symsrc(%s,%s)\n",
+							__func__,
+							dso->symsrc_filename,
+							debuglink);
+					free(dso->symsrc_filename);
+				}
+				dso->symsrc_filename = debuglink;
+			} else {
+				free(debuglink);
+			}
+		}
 
-		/* Check the .debug_frame section for unwinding info */
-		ofs = elf_section_offset(fd, ".debug_frame");
 		dso->data.debug_frame_offset = ofs;
-		dso__data_put_fd(dso);
 	}
 
 	*offset = ofs;
-- 
2.1.4

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

* Re: [PATCH] perf: Fix looking up dwarf unwind stack info
  2017-01-17 14:50 [PATCH] perf: Fix looking up dwarf unwind stack info Matija Glavinic Pecotic
@ 2017-01-18 13:10 ` Jiri Olsa
  2017-01-26 15:25 ` [tip:perf/core] perf unwind: " tip-bot for Matija Glavinic Pecotic
  1 sibling, 0 replies; 3+ messages in thread
From: Jiri Olsa @ 2017-01-18 13:10 UTC (permalink / raw)
  To: Matija Glavinic Pecotic
  Cc: linux-kernel, namhyung, acme, Masami Hiramatsu, Sverdlin,
	Alexander (Nokia - DE/Ulm)

On Tue, Jan 17, 2017 at 03:50:35PM +0100, Matija Glavinic Pecotic wrote:

SNIP

>  extern int
>  UNW_OBJ(dwarf_search_unwind_table) (unw_addr_space_t as,
> @@ -297,15 +298,58 @@ static int read_unwind_spec_debug_frame(struct dso *dso,
>  	int fd;
>  	u64 ofs = dso->data.debug_frame_offset;
>  
> +	/* debug_frame can reside in:
> +	 *  - dso
> +	 *  - debug pointed by symsrc_filename
> +	 *  - gnu_debuglink, which doesn't necessary
> +	 *    has to be pointed by symsrc_filename
> +	 */

looks like we could use this in other parts,
like when __get_srcline and similar

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

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

* [tip:perf/core] perf unwind: Fix looking up dwarf unwind stack info
  2017-01-17 14:50 [PATCH] perf: Fix looking up dwarf unwind stack info Matija Glavinic Pecotic
  2017-01-18 13:10 ` Jiri Olsa
@ 2017-01-26 15:25 ` tip-bot for Matija Glavinic Pecotic
  1 sibling, 0 replies; 3+ messages in thread
From: tip-bot for Matija Glavinic Pecotic @ 2017-01-26 15:25 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, mhiramat, acme, tglx, mingo, jolsa, linux-kernel,
	matija.glavinic-pecotic.ext, alexander.sverdlin, namhyung

Commit-ID:  9343e45bf6cc4a05f6e271e9f8d06bc87875c604
Gitweb:     http://git.kernel.org/tip/9343e45bf6cc4a05f6e271e9f8d06bc87875c604
Author:     Matija Glavinic Pecotic <matija.glavinic-pecotic.ext@nokia.com>
AuthorDate: Tue, 17 Jan 2017 15:50:35 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 18 Jan 2017 12:29:52 -0300

perf unwind: Fix looking up dwarf unwind stack info

Using perf with call graph method dwarf fails to provide backtrace
support for stripped binary even though .gnu_debuglink points to *.dbg
flavor with properly populated debug symbols.

Problem is reproduced on ARM (v7, v8), kernels 3.14.y, 4.4.y and
4.10.rc3.  Perf is configured with libunwind, and unwind dwarf support
[1]. Test code (stress_bt.c) can be found on [2].

Running (explicitly disable other unwinding methods):

  $ gcc -g -o stress_bt -fomit-frame-pointer -fno-unwind-tables \
	-fno-asynchronous-unwind-tables stress_bt.c
  $ perf record -N --call-graph dwarf ./stress_bt
  $ perf report

results in properly generated call graph. Stripping the binary and running
it results with missing call graph. Expected result is to have call graph:

  $ gcc -g -o stress_bt -fomit-frame-pointer -fno-unwind-tables \
	  -fno-asynchronous-unwind-tables stress_bt.c
  $ objcopy --only-keep-debug stress_bt stress_bt.dbg
  $ objcopy --strip-debug stress_bt
  $ objcopy --add-gnu-debuglink=stress_bt.dbg stress_bt
  $ perf record -N --call-graph dwarf ./stress_bt
  $ perf report

Problem is that perf doesn't try to read symbols pointed by gnu
debuglink.  Patch adds checking, and reading of the symbols from
debuglink and symsrc.  Order of the check is to first check within dso,
then check whether symsrc is defined and try to read from it. Finally,
debuglink is checked. Default locations of debug files are discussed in
[3] and [4]. Comments on RFC are on [5].

[1] https://wiki.linaro.org/LEG/Engineering/TOOLS/perf-callstack-unwinding
[2] [1]#Backtrace_stress_application
[3] https://sourceware.org/gdb/onlinedocs/gdb/Separate-Debug-Files.html
[4] https://sourceware.org/binutils/docs/binutils/objcopy.html
[5] https://lkml.org/lkml/2016/8/22/473

Signed-off-by: Matija Glavinic Pecotic <matija.glavinic-pecotic.ext@nokia.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Sverdlin <alexander.sverdlin@nokia.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/d309d40a-463f-482b-68e1-1465326efdc1@nokia.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dso.c                    | 48 +++++++++++++++++++++-------
 tools/perf/util/unwind-libunwind-local.c | 54 +++++++++++++++++++++++++++++---
 2 files changed, 86 insertions(+), 16 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index d2c6cdd..28d41e7 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -9,6 +9,13 @@
 #include "debug.h"
 #include "vdso.h"
 
+static const char * const debuglink_paths[] = {
+	"%.0s%s",
+	"%s/%s",
+	"%s/.debug/%s",
+	"/usr/lib/debug%s/%s"
+};
+
 char dso__symtab_origin(const struct dso *dso)
 {
 	static const char origin[] = {
@@ -44,24 +51,43 @@ int dso__read_binary_type_filename(const struct dso *dso,
 	size_t len;
 
 	switch (type) {
-	case DSO_BINARY_TYPE__DEBUGLINK: {
-		char *debuglink;
+	case DSO_BINARY_TYPE__DEBUGLINK:
+	{
+		const char *last_slash;
+		char dso_dir[PATH_MAX];
+		char symfile[PATH_MAX];
+		unsigned int i;
 
 		len = __symbol__join_symfs(filename, size, dso->long_name);
-		debuglink = filename + len;
-		while (debuglink != filename && *debuglink != '/')
-			debuglink--;
-		if (*debuglink == '/')
-			debuglink++;
+		last_slash = filename + len;
+		while (last_slash != filename && *last_slash != '/')
+			last_slash--;
 
-		ret = -1;
-		if (!is_regular_file(filename))
+		strncpy(dso_dir, filename, last_slash - filename);
+		dso_dir[last_slash-filename] = '\0';
+
+		if (!is_regular_file(filename)) {
+			ret = -1;
+			break;
+		}
+
+		ret = filename__read_debuglink(filename, symfile, PATH_MAX);
+		if (ret)
 			break;
 
-		ret = filename__read_debuglink(filename, debuglink,
-					       size - (debuglink - filename));
+		/* Check predefined locations where debug file might reside */
+		ret = -1;
+		for (i = 0; i < ARRAY_SIZE(debuglink_paths); i++) {
+			snprintf(filename, size,
+					debuglink_paths[i], dso_dir, symfile);
+			if (is_regular_file(filename)) {
+				ret = 0;
+				break;
+			}
 		}
+
 		break;
+	}
 	case DSO_BINARY_TYPE__BUILD_ID_CACHE:
 		if (dso__build_id_filename(dso, filename, size) == NULL)
 			ret = -1;
diff --git a/tools/perf/util/unwind-libunwind-local.c b/tools/perf/util/unwind-libunwind-local.c
index 6fec84d..bfb9b79 100644
--- a/tools/perf/util/unwind-libunwind-local.c
+++ b/tools/perf/util/unwind-libunwind-local.c
@@ -35,6 +35,7 @@
 #include "util.h"
 #include "debug.h"
 #include "asm/bug.h"
+#include "dso.h"
 
 extern int
 UNW_OBJ(dwarf_search_unwind_table) (unw_addr_space_t as,
@@ -297,15 +298,58 @@ static int read_unwind_spec_debug_frame(struct dso *dso,
 	int fd;
 	u64 ofs = dso->data.debug_frame_offset;
 
+	/* debug_frame can reside in:
+	 *  - dso
+	 *  - debug pointed by symsrc_filename
+	 *  - gnu_debuglink, which doesn't necessary
+	 *    has to be pointed by symsrc_filename
+	 */
 	if (ofs == 0) {
 		fd = dso__data_get_fd(dso, machine);
-		if (fd < 0)
-			return -EINVAL;
+		if (fd >= 0) {
+			ofs = elf_section_offset(fd, ".debug_frame");
+			dso__data_put_fd(dso);
+		}
+
+		if (ofs <= 0) {
+			fd = open(dso->symsrc_filename, O_RDONLY);
+			if (fd >= 0) {
+				ofs = elf_section_offset(fd, ".debug_frame");
+				close(fd);
+			}
+		}
+
+		if (ofs <= 0) {
+			char *debuglink = malloc(PATH_MAX);
+			int ret = 0;
+
+			ret = dso__read_binary_type_filename(
+				dso, DSO_BINARY_TYPE__DEBUGLINK,
+				machine->root_dir, debuglink, PATH_MAX);
+			if (!ret) {
+				fd = open(debuglink, O_RDONLY);
+				if (fd >= 0) {
+					ofs = elf_section_offset(fd,
+							".debug_frame");
+					close(fd);
+				}
+			}
+			if (ofs > 0) {
+				if (dso->symsrc_filename != NULL) {
+					pr_warning(
+						"%s: overwrite symsrc(%s,%s)\n",
+							__func__,
+							dso->symsrc_filename,
+							debuglink);
+					free(dso->symsrc_filename);
+				}
+				dso->symsrc_filename = debuglink;
+			} else {
+				free(debuglink);
+			}
+		}
 
-		/* Check the .debug_frame section for unwinding info */
-		ofs = elf_section_offset(fd, ".debug_frame");
 		dso->data.debug_frame_offset = ofs;
-		dso__data_put_fd(dso);
 	}
 
 	*offset = ofs;

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

end of thread, other threads:[~2017-01-26 15:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-17 14:50 [PATCH] perf: Fix looking up dwarf unwind stack info Matija Glavinic Pecotic
2017-01-18 13:10 ` Jiri Olsa
2017-01-26 15:25 ` [tip:perf/core] perf unwind: " tip-bot for Matija Glavinic Pecotic

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.