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