Luca Clementi <[email protected]> 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
_______________________________________________
Ltrace-devel mailing list
[email protected]
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/ltrace-devel
