diff mbox

[AArch64] PR71951: Fix unwinding with -fomit-frame-pointer

Message ID DB6PR0801MB20536D7CA6F0A0FEBB7390BC838D0@DB6PR0801MB2053.eurprd08.prod.outlook.com
State New
Headers show

Commit Message

Wilco Dijkstra Aug. 15, 2017, 4:27 p.m. UTC
ping


From: Wilco Dijkstra
Sent: 31 July 2017 16:57
To: GCC Patches; James Greenhalgh
Cc: nd
Subject: [PATCH][AArch64] PR71951: Fix unwinding with -fomit-frame-pointer
    
As described in PR71951, if libgcc is built with -fomit-frame-pointer,
unwinding crashes, for example while doing a backtrace.  The underlying
reason is the Dwarf unwinder does not setup the frame pointer register
in the initialization code.  When later unwinding a function that uses
the frame pointer, it tries to read FP using _Unwind_GetGR, and this
crashes if has never restored FP.  To unwind correctly the first frame
must save and restore FP (it is unwound in a special way so that it
uses SP instead of FP).  This is done by adding -fno-omit-frame-pointer.

OK for commit and backport to GCC6/7?

ChangeLog:
2017-07-31  Wilco Dijkstra  <wdijkstr@arm.com>

        PR target/71951
        * config/aarch64/aarch64.h (LIBGCC2_UNWIND_ATTRIBUTE): Define.

--

Comments

James Greenhalgh Sept. 20, 2017, 2:57 p.m. UTC | #1
On Tue, Aug 15, 2017 at 05:27:37PM +0100, Wilco Dijkstra wrote:
> 
> ping

This seems like a bit of a theoretical issue as we would normally build
libgcc with -fno-omit-frame-pointer anyway, but it can't hurt to guarantee
this, so OK.

Thanks,
James

> From: Wilco Dijkstra
> Sent: 31 July 2017 16:57
> To: GCC Patches; James Greenhalgh
> Cc: nd
> Subject: [PATCH][AArch64] PR71951: Fix unwinding with -fomit-frame-pointer
>     
> As described in PR71951, if libgcc is built with -fomit-frame-pointer,
> unwinding crashes, for example while doing a backtrace.  The underlying
> reason is the Dwarf unwinder does not setup the frame pointer register
> in the initialization code.  When later unwinding a function that uses
> the frame pointer, it tries to read FP using _Unwind_GetGR, and this
> crashes if has never restored FP.  To unwind correctly the first frame
> must save and restore FP (it is unwound in a special way so that it
> uses SP instead of FP).  This is done by adding -fno-omit-frame-pointer.
> 
> OK for commit and backport to GCC6/7?
> 
> ChangeLog:
> 2017-07-31  Wilco Dijkstra  <wdijkstr@arm.com>
> 
>         PR target/71951
>         * config/aarch64/aarch64.h (LIBGCC2_UNWIND_ATTRIBUTE): Define.
> 
> --
> diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
> index 7f91edb5713d7e8eda2f0a024a0f97b4e111c4b0..03fd93046bdbdb03bd7d0c4573928f504640f7e1 100644
> --- a/gcc/config/aarch64/aarch64.h
> +++ b/gcc/config/aarch64/aarch64.h
> @@ -971,4 +971,12 @@ extern const char *host_detect_local_cpu (int argc, const char **argv);
>  extern tree aarch64_fp16_type_node;
>  extern tree aarch64_fp16_ptr_type_node;
>  
> +/* The generic unwind code in libgcc does not initialize the frame pointer.
> +   So in order to unwind a function using a frame pointer, the very first
> +   function that is unwound must save the frame pointer.  That way the frame
> +   pointer is restored and its value is now valid - otherwise _Unwind_GetGR
> +   crashes.  Libgcc can now be safely built with -fomit-frame-pointer.  */
> +#define LIBGCC2_UNWIND_ATTRIBUTE \
> +  __attribute__((optimize ("no-omit-frame-pointer")))
> +
>  #endif /* GCC_AARCH64_H */
>
Wilco Dijkstra Sept. 21, 2017, 12:02 p.m. UTC | #2
James Greenhalgh wrote:

> This seems like a bit of a theoretical issue as we would normally build
> libgcc with -fno-omit-frame-pointer anyway, but it can't hurt to guarantee
> this, so OK.

It's not theoretical since there were multiple users reporting unwinding issues,
so clearly doing CFLAGS="-fomit-frame-pointer" is popular. While this patch 
fixes AArch64, this bug affects all targets. Obviously -fno-omit-frame-pointer
should be an automatic default for the libgcc unwind code until the Dwarf
engine is fixed and it is made clear what the target unwind interface is.

Wilco
diff mbox

Patch

diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 7f91edb5713d7e8eda2f0a024a0f97b4e111c4b0..03fd93046bdbdb03bd7d0c4573928f504640f7e1 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -971,4 +971,12 @@  extern const char *host_detect_local_cpu (int argc, const char **argv);
 extern tree aarch64_fp16_type_node;
 extern tree aarch64_fp16_ptr_type_node;
 
+/* The generic unwind code in libgcc does not initialize the frame pointer.
+   So in order to unwind a function using a frame pointer, the very first
+   function that is unwound must save the frame pointer.  That way the frame
+   pointer is restored and its value is now valid - otherwise _Unwind_GetGR
+   crashes.  Libgcc can now be safely built with -fomit-frame-pointer.  */
+#define LIBGCC2_UNWIND_ATTRIBUTE \
+  __attribute__((optimize ("no-omit-frame-pointer")))
+
 #endif /* GCC_AARCH64_H */