[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Libunwind-devel] [PATCH 20/27] ARM: fix non-signal-frame local unw_
From: |
Tommi Rantala |
Subject: |
Re: [Libunwind-devel] [PATCH 20/27] ARM: fix non-signal-frame local unw_resume() due to compiler optimization cleverness |
Date: |
Tue, 13 Nov 2012 11:41:18 +0200 |
2012/11/13 Ladislav Michl <address@hidden>:
> On Wed, Aug 22, 2012 at 02:28:46PM +0300, Tommi Rantala wrote:
>> When cross-compiling libunwind with optimizations (-O1 or higher),
>> gcc-4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5) optimizes away the memory
>> writes prior to the inline asm() statement in arm_local_resume() in the
>> non-signal-frame path, causing the `regs' array to be only allocated on
>> the stack, but not populated. This means that we are restoring garbage
>> to the registers.
>>
>> As suggested in the GCC docs, add a fixed size input memory constraint
>> for the array content. This is enough to get the desired code to be
>> generated.
>>
>> Adding __builtin_unreachable() to the point that we should never reach
>> was also in itself enough to inhibit the optimization. It also reduces
>> the function size by a few instructions.
>> ---
>> src/arm/Gresume.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/arm/Gresume.c b/src/arm/Gresume.c
>> index da45dec..4100d87 100644
>> --- a/src/arm/Gresume.c
>> +++ b/src/arm/Gresume.c
>> @@ -1,6 +1,7 @@
>> /* libunwind - a platform-independent unwind library
>> Copyright (C) 2008 CodeSourcery
>> Copyright 2011 Linaro Limited
>> + Copyright (C) 2012 Tommi Rantala <address@hidden>
>>
>> This file is part of libunwind.
>>
>> @@ -51,11 +52,16 @@ arm_local_resume (unw_addr_space_t as, unw_cursor_t
>> *cursor, void *arg)
>> regs[8] = uc->regs[13]; /* SP */
>> regs[9] = uc->regs[14]; /* LR */
>>
>> + struct regs_overlay {
>> + char x[sizeof(regs)];
>> + };
>> +
>> asm __volatile__ (
>> "ldmia %0, {r4-r12, lr}\n"
>> "mov sp, r12\n"
>> "bx lr\n"
>> - : : "r" (regs)
>> + : : "r" (regs),
>> + "m" (*(struct regs_overlay *)regs)
>> );
>> }
>> else
>> @@ -90,6 +96,7 @@ arm_local_resume (unw_addr_space_t as, unw_cursor_t
>> *cursor, void *arg)
>> : : "r" (c->sigcontext_sp), "r" (c->sigcontext_pc)
>> );
>> }
>> + __builtin_unreachable();
>> #else
>> printf ("%s: implement me\n", __FUNCTION__);
>> #endif
>
> This patch breaks building with older compilers (and the very same problem is
> with SH arch). __builtin_unreachable comes with GCC 4.5 and later, so what
> about this?
Hi, __builtin_unreachable is indeed GCC specific, and since someone
might want to use some other compiler, providing this check is IMO
reasonable.
> ---
> configure.ac | 11 +++++++++++
> include/libunwind_i.h | 6 ++++++
> src/arm/Gresume.c | 2 +-
> src/sh/Gresume.c | 2 +-
> 4 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index cffe19b..e3569ad 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -285,6 +285,17 @@ if test x$have__builtin___clear_cache = xyes; then
> fi
> AC_MSG_RESULT([$have__builtin___clear_cache])
>
> +AC_MSG_CHECKING([for __builtin_unreachable])
> +AC_LINK_IFELSE(
> + [AC_LANG_PROGRAM([[]], [[__builtin_unreachable(0, 0)]])],
__builtin_unreachable() does not take arguments, so please remove them
from here.
> + [have__builtin_unreachable=yes],
> + [have__builtin_unreachable=no])
> +if test x$have__builtin_unreachable = xyes; then
> + AC_DEFINE([HAVE__BUILTIN_UNREACHABLE], [1],
> + [Defined if __builtin_unreachable() is available])
> +fi
> +AC_MSG_RESULT([$have__builtin_unreachable])
> +
> AC_MSG_CHECKING([for __sync atomics])
> AC_LINK_IFELSE(
> [AC_LANG_PROGRAM([[]], [[
> diff --git a/include/libunwind_i.h b/include/libunwind_i.h
> index 23f615e..b9ff649 100644
> --- a/include/libunwind_i.h
> +++ b/include/libunwind_i.h
> @@ -72,6 +72,12 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> SOFTWARE. */
> # endif
> #endif
>
> +#if defined(HAVE__BUILTIN_UNREACHABLE)
> +# define unreachable() __builtin_unreachable()
> +#else
> +# define unreachable() do { for (;;) ; } while (0)
> +#endif
> +
I'm a bit surprised by the loop, I would expect something like a call
to abort(), so that if someone actually hits this they'll get a signal
and a core dump.
How about defining this only when we do _not_ have the GCC builtin
function available, something like:
#ifndef HAVE__BUILTIN_UNREACHABLE
# define __builtin_unreachable() ...
#endif
> #ifdef DEBUG
> # define UNW_DEBUG 1
> #else
> diff --git a/src/arm/Gresume.c b/src/arm/Gresume.c
> index 4100d87..2e5e9fd 100644
> --- a/src/arm/Gresume.c
> +++ b/src/arm/Gresume.c
> @@ -96,7 +96,7 @@ arm_local_resume (unw_addr_space_t as, unw_cursor_t
> *cursor, void *arg)
> : : "r" (c->sigcontext_sp), "r" (c->sigcontext_pc)
> );
> }
> - __builtin_unreachable();
> + unreachable();
> #else
> printf ("%s: implement me\n", __FUNCTION__);
> #endif
> diff --git a/src/sh/Gresume.c b/src/sh/Gresume.c
> index 6a76fe1..536670f 100644
> --- a/src/sh/Gresume.c
> +++ b/src/sh/Gresume.c
> @@ -109,7 +109,7 @@ sh_local_resume (unw_addr_space_t as, unw_cursor_t
> *cursor, void *arg)
> "r" (c->sigcontext_pc)
> );
> }
> - __builtin_unreachable();
> + unreachable();
> #endif
> return -UNW_EINVAL;
> }
- Re: [Libunwind-devel] [PATCH 20/27] ARM: fix non-signal-frame local unw_resume() due to compiler optimization cleverness, Ladislav Michl, 2012/11/13
- Re: [Libunwind-devel] [PATCH 20/27] ARM: fix non-signal-frame local unw_resume() due to compiler optimization cleverness,
Tommi Rantala <=
- Re: [Libunwind-devel] [PATCH 20/27] ARM: fix non-signal-frame local unw_resume() due to compiler optimization cleverness, Ladislav Michl, 2012/11/13
- Re: [Libunwind-devel] [PATCH 20/27] ARM: fix non-signal-frame local unw_resume() due to compiler optimization cleverness, Tommi Rantala, 2012/11/13
- Re: [Libunwind-devel] [PATCH 20/27] ARM: fix non-signal-frame local unw_resume() due to compiler optimization cleverness, Ladislav Michl, 2012/11/14
- Re: [Libunwind-devel] [PATCH 20/27] ARM: fix non-signal-frame local unw_resume() due to compiler optimization cleverness, Tommi Rantala, 2012/11/14
- Re: [Libunwind-devel] [PATCH 20/27] ARM: fix non-signal-frame local unw_resume() due to compiler optimization cleverness, Ladislav Michl, 2012/11/14
- Re: [Libunwind-devel] [PATCH 20/27] ARM: fix non-signal-frame local unw_resume() due to compiler optimization cleverness, Tommi Rantala, 2012/11/14
- Re: [Libunwind-devel] [PATCH 20/27] ARM: fix non-signal-frame local unw_resume() due to compiler optimization cleverness, Arun Sharma, 2012/11/14
- Re: [Libunwind-devel] [PATCH 20/27] ARM: fix non-signal-frame local unw_resume() due to compiler optimization cleverness, Arun Sharma, 2012/11/25