[RFC] print stack trace after each syscall

Denys Vlasenko dvlasenk at redhat.com
Mon Jun 24 10:44:47 UTC 2013


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






More information about the Strace-devel mailing list