[Ltrace-devel] [PATCH] check return value of unw_get_reg and unw_get_proc_name

[Petr Machata]

Luca Clementi <luca.clementi at gmail.com> writes:

> @@ -661,9 +661,11 @@ output_right(enum tof type, struct process *proc, struct library_symbol *libsym,
>  					!= sizeof(arch_addr_t))]);
>  		unw_init_remote(&cursor, proc->unwind_as, proc->unwind_priv);
>  		while (unwind_depth) {
> -			unw_get_reg(&cursor, UNW_REG_IP, (unw_word_t *) &ip);
> -			unw_get_proc_name(&cursor, fn_name, sizeof(fn_name),
> -					(unw_word_t *) &function_offset);
> +
> +			if (unw_get_reg(&cursor, UNW_REG_IP, (unw_word_t *) &ip)) {
> +				fprintf(options.output, " > stacktrace_error\n");
> +				continue;
> +			}

It seems better to print out the actual error:

int err = unw_get_reg(&cursor, UNW_REG_IP, (unw_word_t *) &ip);
if (err < 0) {
        fprintf(options.output, " > Error: %s\n", unw_strerror(err));
        continue;
}

About that continue, if this errors out, we never change the
unwind_depth, nor call unw_step.  I suspect that should end it endless
loop--doesn't it?

> diff --git a/proc.c b/proc.c
> index 11755b0..97bcb60 100644
> --- a/proc.c
> +++ b/proc.c
> @@ -29,11 +29,6 @@
>  #include <stdlib.h>
>  #include <string.h>
>  
> -#if defined(HAVE_LIBUNWIND)
> -#include <libunwind.h>
> -#include <libunwind-ptrace.h>
> -#endif /* defined(HAVE_LIBUNWIND) */
> -
>  #include "backend.h"
>  #include "breakpoint.h"
>  #include "debug.h"
> diff --git a/proc.h b/proc.h
> index 64f8fe2..35943c3 100644
> --- a/proc.h
> +++ b/proc.h
> @@ -30,6 +30,7 @@
>  
>  #if defined(HAVE_LIBUNWIND)
>  # include <libunwind.h>
> +# include <libunwind-ptrace.h>
>  #endif /* defined(HAVE_LIBUNWIND) */
>  
>  #include "ltrace.h"

Yeah, this probably makes sense, but looks like a separate change.

Thanks,
PM