diff mbox

Fix exception handling for ILP32 aarch64

Message ID 1486509060.2866.57.camel@caviumnetworks.com
State New
Headers show

Commit Message

Steve Ellcey Feb. 7, 2017, 11:11 p.m. UTC
This patch was submitted last year by Andrew Pinski, this is a
resubmit/ping of that patch.

	https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01726.html

During the initial submittal James Greenhalgh asked if this was an ABI change.
I do not believe it is because while it is changing the size of the reg
structure in _Unwind_Context that structure is opaque to anything outside of
libunwind and so the change is not visible to user programs.  It is not
changing the type or size of _Unwind_Word which remains as 64 bits but it
fixes some places where those 64 bits are being truncated to 32 bits.  Any
ILP32 Aarch64 program that uses an unwind library without this change may
abort because of that truncation.

The other thing that this patch does (by setting REG_VALUE_IN_UNWIND_CONTEXT)
is change the value of ASSUME_EXTENDED_UNWIND_CONTEXT to 1.  That should not
matter because we are currently setting EXTENDED_CONTEXT_BIT in
uw_init_context_1 anyway and that basically does the same thing, saying that
we have the opaque extended unwind context structure which can be changed
without affecting the ABI.  I believe that only pre-G++ 3.0 objects would not
have the extended opaque context structure and there is no ILP32 Aarch64
support in a compiler that old.

I am a little confused about why, when ASSUME_EXTENDED_UNWIND_CONTEXT
is set in unwind-dw2.c, we don't still set the EXTENDED_CONTEXT_BIT in
uw_init_context_1.  I guess it doesn't matter because once we start assuming
the extended unwind context we will never go back and allowing a mix and match
with pre 3.0 unextended unwind contexts so it doesn't matter if the bit is set
or not.

I actually tested this patch without changing ASSUME_EXTENDED_UNWIND_CONTEXT
and it worked as well as this patch where ASSUME_EXTENDED_UNWIND_CONTEXT is
changed so that change could be left out out by setting it to 0 in the new
aarch64 value-unwind.h header file if we thought there was a reason to do
that.

Steve Ellcey
sellcey@cavium.com


2017-02-07  Andrew Pinski  <apinski@cavium.com>

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

Comments

Richard Earnshaw (lists) Feb. 14, 2017, 11:09 a.m. UTC | #1
On 07/02/17 23:11, Steve Ellcey wrote:
> This patch was submitted last year by Andrew Pinski, this is a
> resubmit/ping of that patch.
> 
> 	https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01726.html
> 
> During the initial submittal James Greenhalgh asked if this was an ABI change.
> I do not believe it is because while it is changing the size of the reg
> structure in _Unwind_Context that structure is opaque to anything outside of
> libunwind and so the change is not visible to user programs.  It is not
> changing the type or size of _Unwind_Word which remains as 64 bits but it
> fixes some places where those 64 bits are being truncated to 32 bits.  Any
> ILP32 Aarch64 program that uses an unwind library without this change may
> abort because of that truncation.
> 
> The other thing that this patch does (by setting REG_VALUE_IN_UNWIND_CONTEXT)
> is change the value of ASSUME_EXTENDED_UNWIND_CONTEXT to 1.  That should not
> matter because we are currently setting EXTENDED_CONTEXT_BIT in
> uw_init_context_1 anyway and that basically does the same thing, saying that
> we have the opaque extended unwind context structure which can be changed
> without affecting the ABI.  I believe that only pre-G++ 3.0 objects would not
> have the extended opaque context structure and there is no ILP32 Aarch64
> support in a compiler that old.
> 
> I am a little confused about why, when ASSUME_EXTENDED_UNWIND_CONTEXT
> is set in unwind-dw2.c, we don't still set the EXTENDED_CONTEXT_BIT in
> uw_init_context_1.  I guess it doesn't matter because once we start assuming
> the extended unwind context we will never go back and allowing a mix and match
> with pre 3.0 unextended unwind contexts so it doesn't matter if the bit is set
> or not.
> 
> I actually tested this patch without changing ASSUME_EXTENDED_UNWIND_CONTEXT
> and it worked as well as this patch where ASSUME_EXTENDED_UNWIND_CONTEXT is
> changed so that change could be left out out by setting it to 0 in the new
> aarch64 value-unwind.h header file if we thought there was a reason to do
> that.
> 
> Steve Ellcey
> sellcey@cavium.com
> 
> 
> 2017-02-07  Andrew Pinski  <apinski@cavium.com>
> 
> 	* config/aarch64/value-unwind.h: New file.
> 	* config.host (aarch64*-*-*): Add aarch64/value-unwind.h
> 	to tm_file.
> 

OK.

R.

> 
> unwind.patch
> 
> 
> diff --git a/libgcc/config.host b/libgcc/config.host
> index 9472a60..8bab369 100644
> --- a/libgcc/config.host
> +++ b/libgcc/config.host
> @@ -1379,4 +1379,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
> diff --git a/libgcc/config/aarch64/value-unwind.h b/libgcc/config/aarch64/value-unwind.h
> index e69de29..c79e832 100644
> --- a/libgcc/config/aarch64/value-unwind.h
> +++ b/libgcc/config/aarch64/value-unwind.h
> @@ -0,0 +1,25 @@
> +/* Store register values as _Unwind_Word type in DWARF2 EH unwind context.
> +   Copyright (C) 2017 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
>
diff mbox

Patch

diff --git a/libgcc/config.host b/libgcc/config.host
index 9472a60..8bab369 100644
--- a/libgcc/config.host
+++ b/libgcc/config.host
@@ -1379,4 +1379,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
diff --git a/libgcc/config/aarch64/value-unwind.h b/libgcc/config/aarch64/value-unwind.h
index e69de29..c79e832 100644
--- a/libgcc/config/aarch64/value-unwind.h
+++ b/libgcc/config/aarch64/value-unwind.h
@@ -0,0 +1,25 @@ 
+/* Store register values as _Unwind_Word type in DWARF2 EH unwind context.
+   Copyright (C) 2017 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