[Ltrace-devel] [PATCH v3] add library path to stack trace output when using -w

Petr Machata pmachata at redhat.com
Mon Oct 7 06:01:48 UTC 2013


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

> For each stack frame prints the library path containing the code
> pointed by the IP.
> The output format is similar to the return value of backtrace_symbols
> function found in glibc, e.g.:
>  > /bin/ls(_init+0x19be) [0x40398e]
>  > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed) [0x7f50cbc3676d]
>  > /bin/ls(_init+0x25fd) [0x4045cd]
> ---
>  output.c |   34 ++++++++++++++++++++++++++++++----
>  1 file changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/output.c b/output.c
> index da8c419..f89b287 100644
> --- a/output.c
> +++ b/output.c
> @@ -548,6 +548,7 @@ free_stringp_cb(const char **stringp, void *data)
>  	free((char *)*stringp);
>  }
>  
> +
>  void
>  output_right(enum tof type, struct process *proc, struct library_symbol *libsym)
>  {
> @@ -670,16 +671,41 @@ again:
>  	    && proc->unwind_priv != NULL
>  	    && proc->unwind_as != NULL) {
>  		unw_cursor_t cursor;
> -		unw_word_t ip, sp;
> +		arch_addr_t ip;
> +		unw_word_t function_offset, sp;
> +		struct library *lib = NULL;
>  		int unwind_depth = options.bt_depth;
>  		char fn_name[100];
> +		const char *lib_name;
> +		size_t distance;
>  
>  		unw_init_remote(&cursor, proc->unwind_as, proc->unwind_priv);
>  		while (unwind_depth) {
> -			unw_get_reg(&cursor, UNW_REG_IP, &ip);
> +			unw_get_reg(&cursor, UNW_REG_IP, (unw_word_t *) &ip);

That cast is problematic.  unw_word_t is a uint64_t typedef, but
arch_addr_t is a void* typedef, and may be 32-bit.

>  			unw_get_reg(&cursor, UNW_REG_SP, &sp);
> -			unw_get_proc_name(&cursor, fn_name, 100, NULL);
> -			fprintf(options.output, "\t\t\t%s (ip = 0x%lx)\n", fn_name, (long) ip);
> +			unw_get_proc_name(&cursor, fn_name, sizeof(fn_name),
> +					&function_offset);
> +
> +			/* we are looking for the for the library with the
> +			 * closest base address to the current ip */

This should be /* We are looking for the library with the base address
closest to the current ip.  */ (Word order, duplicate words, capital
letter, period and two spaces at the end of sentence.)

> +			lib_name = NULL;
> +			distance = (size_t) -1;
> +			lib = proc->libraries;
> +			while (lib != NULL) {
> +				if ((ip > lib->base) &&
> +					    ((size_t)(ip - lib->base)
> +					    < distance)) {

That should still be ip >= lib->base (though it's still unlikely to make
difference in practice).

That cast to size_t will become problematic in the future.
Conceptually, arch_addr_t may be 64-bit even in a 32-bit tracer to allow
tracing 64-bit processes.  (Currently it never is, as it's a void*
typedef.)  I think it's fair to leave the code as-is now, but please
annotate it like this (or similar):

        /* N.B.: Assumes sizeof(size_t) == sizeof(arch_addr_t).
           Keyword: double cast.  */

The double cast bit is to tie this comment to a large bunch of other
code that was already annotated and will need attention when the switch
is done.

> +					distance = ip - lib->base;
> +					lib_name = lib->pathname;
> +				}
> +				lib = lib->next;
> +			}
> +
> +			if (!lib_name)

That should be if (lib_name == NULL).  In fact, just initialize lib_name
with "unmapped_area" and drop this if.

> +				lib_name = "unmapped_area";
> +			fprintf(options.output, " > %s(%s+0x%lx) [0x%lx]\n",
> +					lib_name, fn_name, function_offset, (long)ip);

Make this [%p] and remove the cast at IP.  That will do the right thing
now, and produce a warning in the future.

> +
>  			if (unw_step(&cursor) <= 0)
>  				break;
>  			unwind_depth--;

Thanks,
PM



More information about the Ltrace-devel mailing list