On 06/24/2013 09:18 AM, Luca Clementi wrote:
> This patch prints the stack trace of the traced process after
> each system call when using -k flag. It uses libunwind to
> unwind the stack and to obtain the function name pointed by
> the IP.
>
> Tested on Ubuntu 12.04 64 with the distribution version of
> libunwind (0.99-0.3ubuntu1) and on CentOS 6.3 with libunwind
> form the source code. On Ubuntu you need the libunwind and
> libunwind-dev package to compile the stack trace feature in
> the code.
>
> This is still a development patch, please let me know if you
> would consider pulling this patch and if so what do you think
> should be modified (I will also write man page and better
> autoconf script).
> +#ifdef LIB_UNWIND
> +#include <libunwind-ptrace.h>
> +#include <libunwind.h>
> +
> +/* keep a sorted array of cache entries, so that we can binary search
> + * through it
> + */
> +struct mmap_cache_t {
> + // example entry:
> + // 7fabbb09b000-7fabbb09f000 r--p 00179000 fc:00 1180246
> /lib/libc-2.11.1.so
> + //
> + // start_addr is 0x7fabbb09b000
> + // end_addr is 0x7fabbb09f000
> + // mmap_offset is 0x179000
> + // binary_filename is "/lib/libc-2.11.1.so"
> + unsigned long start_addr;
> + unsigned long end_addr;
> + unsigned long mmap_offset;
> + char* binary_filename;
> +};
> +#endif
> +
> /* Trace Control Block */
> struct tcb {
> int flags; /* See below for TCB_ values */
> @@ -417,6 +440,13 @@ struct tcb {
> struct timeval etime; /* Syscall entry time */
> /* Support for tracing forked processes: */
> long inst[2]; /* Saved clone args (badly named) */
> +
> +#ifdef LIB_UNWIND
> + // keep a cache of /proc/<pid>/mmap contents to avoid unnecessary file
> reads
> + struct mmap_cache_t* mmap_cache;
> + int mmap_cache_size;
> + struct UPT_info* libunwind_ui;
> +#endif
> };
>
> /* TCB flags */
> @@ -710,6 +740,14 @@ extern void tv_sub(struct timeval *, struct timeval *,
> struct timeval *);
> extern void tv_mul(struct timeval *, struct timeval *, int);
> extern void tv_div(struct timeval *, struct timeval *, int);
>
> +#ifdef LIB_UNWIND
> +/**
> + * print stack (-k flag) memory allocation and deallocation
> + */
> +extern void alloc_mmap_cache(struct tcb* tcp);
> +extern void delete_mmap_cache(struct tcb* tcp);
#else
# define alloc_mmap_cache(tcp) ((void)0)
# define delete_mmap_cache(tcp) ((void)0)
> +#endif
> print_mmap(struct tcb *tcp, long *u_arg, unsigned long long offset)
> {
> if (entering(tcp)) {
> +#ifdef LIB_UNWIND
> + /* clean the cache */
> + delete_mmap_cache(tcp);
> +#endif
If you add above defines, you can lose #ifdef here.
Comment is redundant - function name is good enough.
> +#ifdef LIB_UNWIND
> +/* if this is 1, then do the stack trace for every system call
> + */
> +int use_libunwind = 0;
Maybe bool?
> @@ -685,6 +698,15 @@ alloctcb(int pid)
> #if SUPPORTED_PERSONALITIES > 1
> tcp->currpers = current_personality;
> #endif
> +
> +#ifdef LIB_UNWIND
> + tcp->mmap_cache = NULL;
> + tcp->mmap_cache_size = 0;
tcp was memset to 0 a few lines above. These assignment are redundant.
> +#ifdef LIB_UNWIND
> + if (use_libunwind) {
> + libunwind_as = unw_create_addr_space (&_UPT_accessors, 0);
The rest of strace source uses a style with no space: "func(...)",
not "func (...)". Follow the style.
> + if (!libunwind_as) {
> + error_msg_and_die("Fatal error: unable to create
> address space for stack tracing\n");
Just call die_out_of_memory().
> +void alloc_mmap_cache(struct tcb* tcp) {
> +
> + if ( tcp->mmap_cache)
> + perror_msg_and_die("Memory map cache is empty");
> + if ( tcp->mmap_cache_size)
> + perror_msg_and_die("Memory map cache is empty");
?? please explain the code above.
> + /* start with a small dynamically-allocated array and then expand it */
> + int cur_array_size = 10;
> + struct mmap_cache_t* cache_head = malloc(cur_array_size *
> sizeof(*cache_head));
> + char filename[sizeof ("/proc/0123456789/maps")];
> +
> + sprintf(filename, "/proc/%d/maps", tcp->pid);
> +
> + FILE* f = fopen(filename, "r");
> + if ( ! f )
> + perror_msg_and_die("Unable to open %s", filename);
"Unable to" => "Can't". It's the same but shorter :)
> + char s[300];
> + while (fgets(s, sizeof(s), f) != NULL) {
> + unsigned long start_addr, end_addr, mmap_offset;
> + char binary_path[512];
> + binary_path[0] = '\0'; // 'reset' it just to be paranoid
> +
> + sscanf(s, "%lx-%lx %*c%*c%*c%*c %lx %*x:%*x %*d %[^\n]",
> &start_addr,
> + &end_addr, &mmap_offset, binary_path);
> +
> + /* ignore special 'fake files' like "[vdso]", "[heap]",
> "[stack]", */
> + if (binary_path[0] == '[' && binary_path[strlen(binary_path) -
> 1] == ']') {
> + continue;
> + }
I would drop the check for closing ']' - strlen is a bit expensive,
and all legal files are going to start with '/', not '[', anyway.
> + /* resize doubling its size */
> + if (tcp->mmap_cache_size >= cur_array_size) {
> + cur_array_size *= 2;
> + cache_head = realloc(cache_head, cur_array_size *
> sizeof(*cache_head));
Please, die_out_of_memory() if realloc returns NULL.
> +void delete_mmap_cache(struct tcb* tcp) {
The { should be on the next line, as the rest of the strace source does.
> +#ifdef LIB_UNWIND
> + extern int use_libunwind;
Move this extern to defs.h
> + if (use_libunwind) {
> + // caching for efficiency ...
> + if (!tcp->mmap_cache) {
> + alloc_mmap_cache(tcp);
> + }
> + // use libunwind to unwind the stack, which works even for code
> compiled
I don't understand what "which works even for code compiled"
tries to say.
> + print_libunwind_backtrace(tcp);
> + }
--
vda
------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:
Build for Windows Store.
http://p.sf.net/sfu/windows-dev2dev
_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel
