[Crash-utility] [PATCH] Use backtrace() instead of __builtin_return_address()

Dave Anderson anderson at redhat.com
Wed May 14 15:11:38 UTC 2008


Bernhard Walle wrote:
> * Dave Anderson [2008-05-14 10:11]:
>> Bernhard Walle wrote:
>>> * Dave Anderson [2008-05-14 08:56]:
>>>> Thanks for digging into this.  I agree with you on all counts.
>>>>
>>>> One final question: does the remaining call to __builtin_return_address(0)
>>>> in tools.c:getbuf() fail in your configuration as well?
>>> Yes. __builtin_return_address(0) works in all configurations and is
>>> also guaranteed to work with gcc. Only __builtin_return_address(n) with
>>> n > 0 makes problems when the frame pointer is omitted (which is the
>>> default with -O2).
>>>
>> I'm confused -- you say it fails in your configuration, but then say that
>> passing an argument of 0 (like getbuf() does) works in all configurations.
> 
> Sorry, the 'yes' was wrong. I meant 'no'. :)
> 

OK, that's what I thought you probably meant...

Anyway, compiling with warnings shows:

   defs.h: In function ‘save_return_address’:
   defs.h:1809: warning: passing argument 1 of ‘backtrace’ from incompatible pointer type
   defs.h:1813: warning: assignment makes integer from pointer without a cast

which are easily addressed by casting retaddr to a (void **) and using
a 0 instead of NULL.

But now the backtrace shows save_return_address(), which is pretty useless.

For example, currently it shows this:

   crash> kmem -i
   ...
   813e041: OFFSET_verify+118
   80aba32: nr_blockdev_pages+881
   80aae6a: dump_kmeminfo+918
   80a1452: cmd_kmem+3273
   ...

But with the patch, it shows:

   crash> kmem -i
   ...
   813deaa: save_return_address+25
   813e03a: OFFSET_verify+118
   80aba46: nr_blockdev_pages+881
   80aae7e: dump_kmeminfo+918

I thought that perhaps the new code prevented save_return_address()
from being inlined.  But as it turns out, even the old way the
function never was inlined.

I suppose we could go with 5 instead of 4, and have dump_trace()
skip the first one, presuming that this anomoly is not architecture-
or compiler-dependent.  Or maybe make it macro?

Dave





More information about the Crash-utility mailing list