[igt-dev] [PATCH i-g-t] lib/core: Use libdw to decode stack trace with debugging symbols, v2.

[Petri Latvala]

On Wed, Aug 29, 2018 at 12:43:24PM +0200, Maarten Lankhorst wrote:
> elfutils is LGPLv3 or GPLv2, so it's ok to link against it.
> 
> Before:
> IGT-Version: 1.23-g8ae86abd419d (x86_64) (Linux: 4.16.0-1-amd64 x86_64)
> Starting subtest: fail-result
> (meta_test:29661) CRITICAL: Test assertion failure function test_result, file ../tests/meta_test.c:94:
> (meta_test:29661) CRITICAL: Failed assertion: result == 1
> (meta_test:29661) CRITICAL: error: 0 != 1
> Stack trace:
>   #0 [__igt_fail_assert+0x20a]
>   #1 [test_result+0x7a]
>   #2 [__real_main120+0x240]
>   #3 [main+0x4a]
>   #4 (../csu/libc-start.c) __libc_start_main:344
>   #5 [_start+0x2a]
> 
> After:
> IGT-Version: 1.23-g8ae86abd419d (x86_64) (Linux: 4.16.0-1-amd64 x86_64)
> Starting subtest: fail-result
> (meta_test:1357) CRITICAL: Test assertion failure function test_result, file ../tests/meta_test.c:94:
> (meta_test:1357) CRITICAL: Failed assertion: result == 1
> (meta_test:1357) CRITICAL: error: 0 != 1
> Stack trace:
>   #0 ../lib/igt_core.c:1467 __igt_fail_assert()
>   #1 ../tests/meta_test.c:95 test_result()
>   #2 ../tests/meta_test.c:137 __real_main120()
>   #3 ../tests/meta_test.c:120 main()
>   #4 ../csu/libc-start.c:344 __libc_start_main()
>   #5 [_start+0x2a]

These line numbers are off just a little bit (as discussed on
IRC). It's not a consistent off-by-one, could be that the used IP is
off by one instruction. Either way, we can brush that off as a TODO
bug, having a bit-off line numbers in stack traces is so much better
than not having them at all.



> 
> Changes since v1:
> - Add libdw dependency to readme.
> - Change backtrace format slightly.
> 
> Cc: Petri Latvala <petri.latvala at intel.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> ---
>  README          |  1 +
>  configure.ac    |  1 +
>  lib/Makefile.am |  2 ++
>  lib/igt_core.c  | 50 ++++++++++++++++++++++++++++++++++++++++++++-----
>  lib/meson.build |  1 +
>  meson.build     |  1 +
>  6 files changed, 51 insertions(+), 5 deletions(-)
> 
> diff --git a/README b/README
> index c71ecedd0e5e..f8ba5c710e81 100644
> --- a/README
> +++ b/README
> @@ -148,6 +148,7 @@ the default configuration (package names may vary):
>  	libpciaccess-dev
>  	libprocps-dev
>  	libunwind-dev
> +	libdw-dev
>  	python-docutils
>  	x11proto-dri2-dev
>  	xutils-dev
> diff --git a/configure.ac b/configure.ac
> index 72f359943779..c75ef284f3bc 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -128,6 +128,7 @@ PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
>  PKG_CHECK_MODULES(KMOD, [libkmod])
>  PKG_CHECK_MODULES(PROCPS, [libprocps])
>  PKG_CHECK_MODULES(LIBUNWIND, [libunwind])
> +PKG_CHECK_MODULES(LIBDW, [libdw])
>  PKG_CHECK_MODULES(SSL, [openssl])
>  PKG_CHECK_MODULES(VALGRIND, [valgrind], [have_valgrind=yes], [have_valgrind=no])
>  
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index 6251bdb85f67..388b8b00ae36 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -60,6 +60,7 @@ AM_CFLAGS = \
>  	    $(DRM_CFLAGS) \
>  	    $(PCIACCESS_CFLAGS) \
>  	    $(LIBUNWIND_CFLAGS) \
> +	    $(LIBDW_CFLAGS) \
>  	    $(GSL_CFLAGS) \
>  	    $(KMOD_CFLAGS) \
>  	    $(PROCPS_CFLAGS) \
> @@ -86,6 +87,7 @@ libintel_tools_la_LIBADD = \
>  	$(CAIRO_LIBS) \
>  	$(LIBUDEV_LIBS) \
>  	$(LIBUNWIND_LIBS) \
> +	$(LIBDW_LIBS) \
>  	$(TIMER_LIBS) \
>  	$(XMLRPC_LIBS) \
>  	$(LIBUDEV_LIBS) \
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index c52c081899c9..4776db93594e 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -73,6 +73,7 @@
>  
>  #define UNW_LOCAL_ONLY
>  #include <libunwind.h>
> +#include <elfutils/libdwfl.h>
>  
>  #ifdef HAVE_LIBGEN_H
>  #include <libgen.h>   /* for basename() on Solaris */
> @@ -1212,20 +1213,59 @@ static void print_backtrace(void)
>  	unw_context_t uc;
>  	int stack_num = 0;
>  
> +	Dwfl_Callbacks cbs = {
> +		.find_elf = dwfl_linux_proc_find_elf,
> +		.find_debuginfo = dwfl_standard_find_debuginfo,
> +	};
> +
> +	Dwfl *dwfl = dwfl_begin(&cbs);
> +
> +	if (dwfl_linux_proc_report(dwfl, getpid())) {
> +		dwfl_end(dwfl);
> +		dwfl = NULL;
> +	} else
> +		dwfl_report_end(dwfl, NULL, NULL);
> +
>  	igt_info("Stack trace:\n");
>  
>  	unw_getcontext(&uc);
>  	unw_init_local(&cursor, &uc);
>  	while (unw_step(&cursor) > 0) {
>  		char name[255];
> -		unw_word_t off;
> +		unw_word_t off, ip;
> +		Dwfl_Module *mod = NULL;
>  
> -		if (unw_get_proc_name(&cursor, name, 255, &off) < 0)
> -			strcpy(name, "<unknown>");
> +		unw_get_reg(&cursor, UNW_REG_IP, &ip);
> +
> +		if (dwfl)
> +			mod = dwfl_addrmodule(dwfl, ip);
>  
> -		igt_info("  #%d [%s+0x%x]\n", stack_num++, name,
> -			 (unsigned int) off);
> +		if (mod) {
> +			const char *src, *name;

Shadowed variable 'name' here.

With that fixed and an added note that libdw is a new mandatory
dependency in the commit message, this is


Reviewed-by: Petri Latvala <petri.latvala at intel.com>