Re: [Libunwind-devel] [PATCH 20/27] ARM: fix non-signal-frame local unw_resume() due to compiler optimization cleverness

[Ladislav Michl]

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?
---
 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)]])],
+  [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
+
 #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;
 }