From mboxrd@z Thu Jan  1 00:00:00 1970
From: Peter Feiner <pfeiner@google.com>
Subject: Re: [PATCH kvm-unit-tests v5 1/5] lib: backtrace printing
Date: Thu, 10 Mar 2016 16:31:25 -0800
Message-ID: <20160311003125.GA17549@google.com>
References: <cover.1457372594.git.pfeiner@google.com>
 <757ca48dec7f6c497948468cb1da89ff646f2e28.1457372594.git.pfeiner@google.com>
 <20160308042325.GB1638@localhost.redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org, pbonzini@redhat.com
To: Andrew Jones <drjones@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-pa0-f47.google.com ([209.85.220.47]:35956 "EHLO
	mail-pa0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S932975AbcCKAb1 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 10 Mar 2016 19:31:27 -0500
Received: by mail-pa0-f47.google.com with SMTP id tt10so79121628pab.3
        for <kvm@vger.kernel.org>; Thu, 10 Mar 2016 16:31:27 -0800 (PST)
Content-Disposition: inline
In-Reply-To: <20160308042325.GB1638@localhost.redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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