diff mbox

[PATCH/AARCH64/ILP32] Fix unwinding (libgcc)

Message ID CA+=Sn1kmx_eB4CEkGOMT-SYhV78bMN_eLL_Y7L0wL7cTpb7HZw@mail.gmail.com
State New
Headers show

Commit Message

Andrew Pinski April 27, 2016, 9:13 p.m. UTC
Hi,
  AARCH64 ILP32 is like x32 where UNITS_PER_WORD > sizeof(void*) so we
need to define REG_VALUE_IN_UNWIND_CONTEXT for ILP32.  This fixes
unwinding through the signal handler.  This is independent of the ABI
which Linux kernel uses to store the registers.

OK?  Bootstrapped and tested on aarch64 with no regressions.

Thanks,
Andrew Pinski

ChangeLog:
* config/aarch64/value-unwind.h: New file.
* config.host (aarch64*-*-*): Add aarch64/value-unwind.h to tm_file.

Comments

Andrew Pinski May 12, 2016, 10:04 p.m. UTC | #1
On Wed, Apr 27, 2016 at 2:13 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> Hi,
>   AARCH64 ILP32 is like x32 where UNITS_PER_WORD > sizeof(void*) so we
> need to define REG_VALUE_IN_UNWIND_CONTEXT for ILP32.  This fixes
> unwinding through the signal handler.  This is independent of the ABI
> which Linux kernel uses to store the registers.
>
> OK?  Bootstrapped and tested on aarch64 with no regressions.

Ping.

>
> Thanks,
> Andrew Pinski
>
> ChangeLog:
> * config/aarch64/value-unwind.h: New file.
> * config.host (aarch64*-*-*): Add aarch64/value-unwind.h to tm_file.
James Greenhalgh May 26, 2016, 9:50 a.m. UTC | #2
On Wed, Apr 27, 2016 at 02:13:21PM -0700, Andrew Pinski wrote:
> Hi,
>   AARCH64 ILP32 is like x32 where UNITS_PER_WORD > sizeof(void*) so we
> need to define REG_VALUE_IN_UNWIND_CONTEXT for ILP32.  This fixes
> unwinding through the signal handler.  This is independent of the ABI
> which Linux kernel uses to store the registers.
> 
> OK?  Bootstrapped and tested on aarch64 with no regressions.

I've read back through the threads around this issue [1][2] and it looks
like most of the discussion was to do with the machinery and enabling
compatability between unwinder libraries rather than the ABI implications
of turning on REG_VALUE_IN_UNWIND_CONTEXT.

You had a concern in the PR:

  "Does it matter for the propose of unwinding as all we care about
   is pointers to get back to frame before?  Your commit might change
   the ABI for some targets."

That basically sums up my concerns too, particularly as I'm not remotely
familiar with this area! I'm assuming you convinced yourself this wasn't
an issue for x32, and therefore won't be an issue for AArch64 ilp32?

Could you write a paragraph on why this is OK to set my mind at ease, then
give it another few days for Marcus and Richard to comment? I've got no
objections to the patch content if you can help me to understand why this
is correct and does not introduce/break ABI.

Thanks,
James

---
[1]: Bug 48007 - [x32] Unwind library doesn't work with
        UNITS_PER_WORD > sizeof (void *)
     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48007
[2]: PATCH [8/n]: Prepare x32: PR other/48007: Unwind library doesn't
        work with UNITS_PER_WORD > sizeof (void *)
     https://gcc.gnu.org/ml/gcc-patches/2011-06/msg01913.html

> ChangeLog:
> * config/aarch64/value-unwind.h: New file.
> * config.host (aarch64*-*-*): Add aarch64/value-unwind.h to tm_file.
diff mbox

Patch

Index: libgcc/config.host
===================================================================
--- libgcc/config.host	(revision 235529)
+++ libgcc/config.host	(working copy)
@@ -1385,4 +1385,8 @@  i[34567]86-*-linux* | x86_64-*-linux*)
 	fi
 	tm_file="${tm_file} i386/value-unwind.h"
 	;;
+aarch64*-*-*)
+	# ILP32 needs an extra header for unwinding
+	tm_file="${tm_file} aarch64/value-unwind.h"
+	;;
 esac
Index: libgcc/ChangeLog
===================================================================
--- libgcc/ChangeLog	(revision 235529)
+++ libgcc/ChangeLog	(working copy)
@@ -1,3 +1,9 @@ 
+2016-04-27  Andrew Pinski  <apinski@cavium.com>
+
+	* config/aarch64/value-unwind.h: New file.
+	* config.host (aarch64*-*-*): Add aarch64/value-unwind.h
+	to tm_file.
+
 2016-04-25  Nick Clifton  <nickc@redhat.com>
 
 	* config/msp430/cmpd.c (__mspabi_cmpf): Add prototype.
Index: libgcc/config/aarch64/value-unwind.h
===================================================================
--- libgcc/config/aarch64/value-unwind.h	(revision 0)
+++ libgcc/config/aarch64/value-unwind.h	(revision 0)
@@ -0,0 +1,25 @@ 
+/* Store register values as _Unwind_Word type in DWARF2 EH unwind context.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published
+   by the Free Software Foundation; either version 3, or (at your
+   option) any later version.
+
+   GCC is distributed in the hope that it will be useful, but WITHOUT
+   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+   or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
+   License for more details.
+
+   You should have received a copy of the GNU General Public License and
+   a copy of the GCC Runtime Library Exception along with this program;
+   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* Define this macro if the target stores register values as _Unwind_Word
+   type in unwind context.  Only enable it for ilp32.  */
+#if defined __aarch64__ && !defined __LP64__
+# define REG_VALUE_IN_UNWIND_CONTEXT
+#endif