When crash is compiled with gcc 4.3 and -O2, the __builtin_return_address()
causes crash to crash. See also [1] for a discussion about that. The gcc
documentation [2] says

__builtin_return_address()

On some machines it may be impossible to determine the return address of
any function other than the current one; in such cases, or when the top
of the stack has been reached, this function will return 0 or a random
value. In addition, __builtin_frame_address may be used to determine if
the top of the stack has been reached.

This function should only be used with a nonzero argument for debugging
purposes.

Even the __builtin_frame_address() does not work here. Instead of checking
if the crash is built with -O2 and introducing new preprocessor checks here,
I use the backtrace() function which is available via glibc. This works here
(tested without the other patch which brought my attention to this bug).

Since crash only runs on Linux (IIRC), the glibc dependency should not be
a problem.

Signed-off-by: Bernhard Walle <bwalle@suse.de>


[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=165992
[2] http://gcc.gnu.org/onlinedocs/gcc/Return-Address.html

---
defs.h | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)

--- a/defs.h
+++ b/defs.h
@@ -1803,15 +1803,7 @@ struct alias_data { /* c
static inline void
save_return_address(ulong *retaddr)
{
- retaddr[0] = (ulong) __builtin_return_address(0);
-#if defined(X86) || defined(PPC) || defined(X86_64) || defined(PPC64)
- if (__builtin_frame_address(1))
- retaddr[1] = (ulong) __builtin_return_address(1);
- if (__builtin_frame_address(2))
- retaddr[2] = (ulong) __builtin_return_address(2);
- if (__builtin_frame_address(3))
- retaddr[3] = (ulong) __builtin_return_address(3);
-#endif
+ backtrace(retaddr, 4);
}

#endif /* !GDB_COMMON */

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

* Dave Anderson [2008-05-13 15:16]:
>
> A few questions on this one...
>
> Do you know if this works OK on ia64, s390 and s390x?

I made a test program (attached) and attached the result on all
architectures. In fact, s390 and s390x produced even 100 % correct
backtraces when the program was optimised with -O2. All other
architectures leave out frames in that case, but I think that's
limitation when frame pointers are omitted that is impossible to work
around.

> Compiling with warnings turned on yields:
>
> defs.h:1804: warning: implicit declaration of function ‘backtrace’

I forgot the <execinfo.h> include.

> I haven't looked at the glibc sources, but I'm presuming it's
> a "void backtrace(int)".

Indeed, it's int backtrace(void **buffer, int size). In my patch I
ignored the return value. The new patch (attached) corrects that
mistake. I'm not sure if it's necessary to zero out the rest since the
memory already was initialised with 0, but to be on the save side I
think it's not bad to do it.

Since sizeof(unsigned long) == sizeof(void *) on all architectures
crash supports, the "wrong" type of buffer should be ok. In fact,
__builtin_return_address() returns also an address, i.e. void * and
not a long. Conforming strictly to C99, we should use uintptr_t. I
don't know what the crash programming guide lines say, i.e. how old the
compiler can be where crash is used. I think we should not introduce
a C99 requirement because of that small patch here.


A few words on the test program: I used backtrace_symbols_fd() to
print out the backtrace here because I was too lazy to "port" the crash
nm magic to the test program. However, the program needs to be compiled
with -rdynamic to make that work. That's also documented in
backtrace(3) manual page. So for crash, the nm magic is a big "ugly"
but it works better than the backtrace_symbols_fd() here. But for the
test program I was only interesting in the values backtrace() produces,
not in resolving, so I think that should be ok.


Bernhard
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

* Dave Anderson [2008-05-13 15:36]:
>
> Bernhard Walle wrote:
> > When crash is compiled with gcc 4.3 and -O2, the __builtin_return_address()
> > causes crash to crash. See also [1] for a discussion about that.
>
> It appears that [1] only describes a problem if you ask for more frames
> than exist. Given the usage in crash, it's impossible to have a frame count
> of less than 4. So it must be some other bug, no?

Yes. I only referenced to that bug to state that the behaviour is
undefined, and it's not a gcc bug per definition.

However, I tried to track down the problem a bit. In a test program, I
encountered that when __builtin_frame_address(i) is zero, then the
program crashes when __builtin_frame_address(i+1) is executed.

However, changing that in crash doesn't help. It really crashes in
__builtin_return_address(1). Tracking that down is out of my knowledge.
I used the same compiler flags in crash and the test program.



Bernhard

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Hi,

* 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).



Bernhard

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

* 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'.


Bernhard

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

* Dave Anderson [2008-05-14 11:11]:
>
> 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?

Did you compile with some optimisation? I think I remember that gcc
only inlines code with optimisation turned on.

I think it makes sense to use

`always_inline'
Generally, functions are not inlined unless optimization is
specified. For functions declared inline, this attribute inlines
the function even if no optimization level was specified.

here, i.e. __attribute__((always_inline)). I personally prefer inline
functions over macros.


Bernhard

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

* Dave Anderson <anderson@redhat.com> [2008-05-14 11:51]:
> Bernhard Walle wrote:
> >
> > * Dave Anderson [2008-05-14 11:11]:
> >> 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?
> >
> > Did you compile with some optimisation? I think I remember that gcc
> > only inlines code with optimisation turned on.
> >
>
> No -- the Makefile is used as is -- you're the one modifying things... ;-)

You distribute crash without optimisations in Fedora/RHEL? Well, our
build system complains in that case. :-|


Bernhard
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility