Re: [PATCH kvm-unit-tests v5 1/5] lib: backtrace printing

[Peter Feiner <pfeiner@google.com>]

On Tue, Mar 08, 2016 at 11:24:45AM +0700, Andrew Jones wrote:
> On Mon, Mar 07, 2016 at 09:46:53AM -0800, Peter Feiner wrote:
> > +#ifndef HAVE_ARCH_BACKTRACE
> > +int backtrace(const void **return_addrs, int max_depth)
> > +{
> > +	static int walking;
> > +	int depth = 0;
> > +	void *addr;
> > +
> > +	if (walking) {
> > +		printf("RECURSIVE STACK WALK!!!\n");
> > +		return 0;
> > +	}
> > +	walking = 1;
> > +
> > +	/* __builtin_return_address requires a compile-time constant argument */
> > +#define GET_RETURN_ADDRESS(i)						\
> > +	if (max_depth == i)						\
> > +		goto done;						\
> > +	addr = __builtin_return_address(i + 1);				\
> 
> Is the +1 to skip a level, which means addr is an address two levels up?
> If we do that, then won't we skip the callers of backtrace, which may or
> may not be dump_stack?

Right and right. In v6 dump_stack now does the skipping.


> 
> 
> > +	if (!addr)							\
> > +		goto done;						\
> > +	return_addrs[i] = __builtin_extract_return_addr(addr);		\
> 
> So here we put the i+1th return address into return_addrs[i]. I find
> that a bit confusing. Maybe we should leave it to the callers of
> backtrace to do any level skipping they want, i.e. here we shouldn't
> skip anything, but dump_stack could start printing at i=1.

Agreed. Changed in v6.

Peter