| Message ID | a242b032-db02-244f-eafe-cd36ee608a86@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Jan 12, 2017 at 06:10:36PM +0000, Jiong Wang wrote: > On 06/01/17 11:47, Jiong Wang wrote: > >This is the update on libgcc unwinder support according to new > >DWARF proposal. > > > >As Joseph commented, duplication of unwind-dw2.c is not encouraged > >in libgcc, > >But from this patch, you can see there are a few places we need to > >modify for > >AArch64 in unwind-aarch64.c, so the file duplication approach is > >acceptable? > > > > > >libgcc/ > > > >2017-01-06 Jiong Wang <jiong.wang@arm.com> > > > > * config/aarch64/unwind-aarch64.c (DWARF_REGNUM_AARCH64_RA_STATE, > > RA_A_SIGNED_BIT): New macros. > > (execute_cfa_program): Multiplex DW_CFA_GNU_window_save on > >AArch64. > > (uw_frame_state_for): Clear bit[0] of > >DWARF_REGNUM_AARCH64_RA_STATE. > > (uw_update_context): Authenticate return address according to > > DWARF_REGNUM_AARCH64_RA_STATE. > > (uw_init_context_1): Strip signature of seed address. > > (uw_install_context): Re-authenticate EH handler's address. > > > Ping~ > > For comparision, I have also attached the patch using the target macros. Personally, I much prefer this approach. I haven't looked at your patch in detail, I'll leave that for after a libgcc maintainer has commented on whether these macros would be acceptable. Thanks, James
On 18/01/17 17:07, Jiong Wang wrote: > On 12/01/17 18:10, Jiong Wang wrote: >> On 06/01/17 11:47, Jiong Wang wrote: >>> This is the update on libgcc unwinder support according to new DWARF >>> proposal. >>> >>> As Joseph commented, duplication of unwind-dw2.c is not encouraged in >>> libgcc, >>> But from this patch, you can see there are a few places we need to >>> modify for >>> AArch64 in unwind-aarch64.c, so the file duplication approach is >>> acceptable? >>> >>> >>> libgcc/ >>> >>> 2017-01-06 Jiong Wang <jiong.wang@arm.com> >>> >>> * config/aarch64/unwind-aarch64.c >>> (DWARF_REGNUM_AARCH64_RA_STATE, >>> RA_A_SIGNED_BIT): New macros. >>> (execute_cfa_program): Multiplex DW_CFA_GNU_window_save on >>> AArch64. >>> (uw_frame_state_for): Clear bit[0] of >>> DWARF_REGNUM_AARCH64_RA_STATE. >>> (uw_update_context): Authenticate return address according to >>> DWARF_REGNUM_AARCH64_RA_STATE. >>> (uw_init_context_1): Strip signature of seed address. >>> (uw_install_context): Re-authenticate EH handler's address. >>> >> Ping~ >> >> For comparision, I have also attached the patch using the target macros. >> >> Four new target macros are introduced: >> >> MD_POST_EXTRACT_ROOT_ADDR >> MD_POST_EXTRACT_FRAME_ADDR >> MD_POST_FROB_EH_HANDLER_ADDR >> MD_POST_INIT_CONTEXT >> >> MD_POST_EXTRACT_ROOT_ADDR is to do target private post processing on >> the address >> inside _Unwind* functions, they are serving as root address to start the >> unwinding. MD_POST_EXTRACT_FRAME_ADDR is to do target private post >> processing >> on th address inside the real user program which throws the exceptions. >> >> MD_POST_FROB_EH_HANDLER_ADDR is to do target private frob on the EH >> handler's >> address before we install it into current context. >> >> MD_POST_INIT_CONTEXT it to do target private initialization on the >> context >> structure after common initialization. >> >> One "__aarch64__" macro check is needed to multiplex DW_CFA_window_save. > > Ping ~ > > Could global reviewers or libgcc maintainers please give a review on the > generic > part change? > > One small change is I removed MD_POST_INIT_CONTEXT as I found there is > MD_FROB_UPDATE_CONTEXT which serve the same purpose. I still need to > define > > MD_POST_EXTRACT_ROOT_ADDR > MD_POST_EXTRACT_FRAME_ADDR > MD_POST_FROB_EH_HANDLER_ADDR > > And do one __aarch64__ check to multiplexing DW_CFA_GNU_window_save. > > Thanks. > > libgcc/ChangeLog: > > 2017-01-18 Jiong Wang <jiong.wang@arm.com> > > * config/aarch64/aarch64-unwind.h: New file. > (DWARF_REGNUM_AARCH64_RA_STATE): Define. > (MD_POST_EXTRACT_ROOT_ADDR): Define. > (MD_POST_EXTRACT_FRAME_ADDR): Define. > (MD_POST_FROB_EH_HANDLER_ADDR): Define. > (MD_FROB_UPDATE_CONTEXT): Define. > (aarch64_post_extract_frame_addr): New function. > (aarch64_post_frob_eh_handler_addr): New function. > (aarch64_frob_update_context): New function. > * config/aarch64/linux-unwind.h: Include aarch64-unwind.h > * config.host (aarch64*-*-elf, aarch64*-*-rtems*, > aarch64*-*-freebsd*): > Initialize md_unwind_header to include aarch64-unwind.h. > * unwind-dw2.c (struct _Unwind_Context): Define "RA_A_SIGNED_BIT". > (execute_cfa_program): Multiplex DW_CFA_GNU_window_save for > __aarch64__. > (uw_update_context): Honor MD_POST_EXTRACT_FRAME_ADDR. > (uw_init_context_1): Honor MD_POST_EXTRACT_ROOT_ADDR. > (uw_frob_return_addr): New function. > (_Unwind_DebugHook): Use uw_frob_return_addr. > > Comments inline. > 1.patch > > > diff --git a/libgcc/unwind-dw2.c b/libgcc/unwind-dw2.c > index 8085a42ace15d53f4cb0c6681717012d906a6d47..cf640135275deb76b820f8209fa51eacfd64c4a2 100644 > --- a/libgcc/unwind-dw2.c > +++ b/libgcc/unwind-dw2.c > @@ -136,6 +136,8 @@ struct _Unwind_Context > #define SIGNAL_FRAME_BIT ((~(_Unwind_Word) 0 >> 1) + 1) > /* Context which has version/args_size/by_value fields. */ > #define EXTENDED_CONTEXT_BIT ((~(_Unwind_Word) 0 >> 2) + 1) > + /* Bit reserved on AArch64, return address has been signed with A key. */ > +#define RA_A_SIGNED_BIT ((~(_Unwind_Word) 0 >> 3) + 1) Why is this here? It appears to only be used within the AArch64-specific header file. > _Unwind_Word flags; > /* 0 for now, can be increased when further fields are added to > struct _Unwind_Context. */ > @@ -1185,6 +1187,11 @@ execute_cfa_program (const unsigned char *insn_ptr, > break; > > case DW_CFA_GNU_window_save: > +#ifdef __aarch64__ > + /* This CFA is multiplexed with Sparc. On AArch64 it's used to toggle > + return address signing status. */ > + fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset ^= 1; > +#else > /* ??? Hardcoded for SPARC register window configuration. */ > if (__LIBGCC_DWARF_FRAME_REGISTERS__ >= 32) > for (reg = 16; reg < 32; ++reg) > @@ -1192,6 +1199,7 @@ execute_cfa_program (const unsigned char *insn_ptr, > fs->regs.reg[reg].how = REG_SAVED_OFFSET; > fs->regs.reg[reg].loc.offset = (reg - 16) * sizeof (void *); > } > +#endif > break; > > case DW_CFA_GNU_args_size: > @@ -1513,10 +1521,15 @@ uw_update_context (struct _Unwind_Context *context, _Unwind_FrameState *fs) > stack frame. */ > context->ra = 0; > else > - /* Compute the return address now, since the return address column > - can change from frame to frame. */ > - context->ra = __builtin_extract_return_addr > - (_Unwind_GetPtr (context, fs->retaddr_column)); > + { > + /* Compute the return address now, since the return address column > + can change from frame to frame. */ > + context->ra = __builtin_extract_return_addr > + (_Unwind_GetPtr (context, fs->retaddr_column)); > +#ifdef MD_POST_EXTRACT_FRAME_ADDR > + context->ra = MD_POST_EXTRACT_FRAME_ADDR (context, fs, context->ra); > +#endif > + } > } > > static void > @@ -1550,6 +1563,9 @@ uw_init_context_1 (struct _Unwind_Context *context, > void *outer_cfa, void *outer_ra) > { > void *ra = __builtin_extract_return_addr (__builtin_return_address (0)); > +#ifdef MD_POST_EXTRACT_ROOT_ADDR > + ra = MD_POST_EXTRACT_ROOT_ADDR (ra); > +#endif > _Unwind_FrameState fs; > _Unwind_SpTmp sp_slot; > _Unwind_Reason_Code code; > @@ -1586,6 +1602,9 @@ uw_init_context_1 (struct _Unwind_Context *context, > initialization context, then we can't see it in the given > call frame data. So have the initialization context tell us. */ > context->ra = __builtin_extract_return_addr (outer_ra); > +#ifdef MD_POST_EXTRACT_ROOT_ADDR > + context->ra = MD_POST_EXTRACT_ROOT_ADDR (context->ra); > +#endif > } > > static void _Unwind_DebugHook (void *, void *) > @@ -1608,6 +1627,20 @@ _Unwind_DebugHook (void *cfa __attribute__ ((__unused__)), > #endif > } > > +/* Frob exception handler's address kept in TARGET before installing into > + CURRENT context. */ > + > +static void * > +uw_frob_return_addr (struct _Unwind_Context *current, > + struct _Unwind_Context *target) > +{ > + void *ret_addr = __builtin_frob_return_addr (target->ra); > +#ifdef MD_POST_FROB_EH_HANDLER_ADDR > + ret_addr = MD_POST_FROB_EH_HANDLER_ADDR (current, target, ret_addr); > +#endif > + return ret_addr; > +} > + I think this function should be marked inline. The optimizers would probably inline it anyway, but it seems wrong for us to rely on that. > /* Install TARGET into CURRENT so that we can return to it. This is a > macro because __builtin_eh_return must be invoked in the context of > our caller. */ > @@ -1616,7 +1649,7 @@ _Unwind_DebugHook (void *cfa __attribute__ ((__unused__)), > do \ > { \ > long offset = uw_install_context_1 ((CURRENT), (TARGET)); \ > - void *handler = __builtin_frob_return_addr ((TARGET)->ra); \ > + void *handler = uw_frob_return_addr ((CURRENT), (TARGET)); \ > _Unwind_DebugHook ((TARGET)->cfa, handler); \ > __builtin_eh_return (offset, handler); \ > } \ > diff --git a/libgcc/config.host b/libgcc/config.host > index 6f2e458e74e776a6b7a310919558bcca76389232..540bfa9635802adabb36a2d1b7cf3416462c59f3 100644 > --- a/libgcc/config.host > +++ b/libgcc/config.host > @@ -331,11 +331,13 @@ aarch64*-*-elf | aarch64*-*-rtems*) > extra_parts="$extra_parts crtfastmath.o" > tmake_file="${tmake_file} ${cpu_type}/t-aarch64" > tmake_file="${tmake_file} ${cpu_type}/t-softfp t-softfp t-crtfm" > + md_unwind_header=aarch64/aarch64-unwind.h > ;; > aarch64*-*-freebsd*) > extra_parts="$extra_parts crtfastmath.o" > tmake_file="${tmake_file} ${cpu_type}/t-aarch64" > tmake_file="${tmake_file} ${cpu_type}/t-softfp t-softfp t-crtfm" > + md_unwind_header=aarch64/aarch64-unwind.h > ;; > aarch64*-*-linux*) > extra_parts="$extra_parts crtfastmath.o" > diff --git a/libgcc/config/aarch64/aarch64-unwind.h b/libgcc/config/aarch64/aarch64-unwind.h > new file mode 100644 > index 0000000000000000000000000000000000000000..a57cae0ff54ea7e11c32362677c0b51501a9f727 > --- /dev/null > +++ b/libgcc/config/aarch64/aarch64-unwind.h > @@ -0,0 +1,87 @@ > +/* Copyright (C) 2017 Free Software Foundation, Inc. > + Contributed by ARM Ltd. > + > +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. > + > +Under Section 7 of GPL version 3, you are granted additional > +permissions described in the GCC Runtime Library Exception, version > +3.1, as published by the Free Software Foundation. > + > +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/>. */ > + > +#ifndef AARCH64_UNWIND_H > +#define AARCH64_UNWIND_H > + > +#define DWARF_REGNUM_AARCH64_RA_STATE 34 > + > +#define MD_POST_EXTRACT_ROOT_ADDR(addr) __builtin_aarch64_xpaclri (addr) > +#define MD_POST_EXTRACT_FRAME_ADDR(context, fs, addr) \ > + aarch64_post_extract_frame_addr (context, fs, addr) > +#define MD_POST_FROB_EH_HANDLER_ADDR(current, target, addr) \ > + aarch64_post_frob_eh_handler_addr (current, target, addr) > +#define MD_FROB_UPDATE_CONTEXT(context, fs) \ > + aarch64_frob_update_context (context, fs) > + > +/* Do AArch64 private extraction on ADDR based on context info CONTEXT and > + unwind frame info FS. If ADDR is signed, we do address authentication on it > + using CFA of current frame. */ > + All the functions in this HEADER file should be marked inline. > +static void * > +aarch64_post_extract_frame_addr (struct _Unwind_Context *context, > + _Unwind_FrameState *fs, void *addr) > +{ > + if (fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset & 0x1) > + { > + _Unwind_Word salt = (_Unwind_Word) context->cfa; > + return __builtin_aarch64_autia1716 (addr, salt); > + } > + else > + return addr; > +} > + > +/* Do AArch64 private frob on exception handler's address HANDLER_ADDR before > + installing it into current context CURRENT. TARGET is currently not used. > + We need to sign exception handler's address if CURRENT itself is signed. */ > + > +static void * > +aarch64_post_frob_eh_handler_addr (struct _Unwind_Context *current, > + struct _Unwind_Context *target > + ATTRIBUTE_UNUSED, > + void *handler_addr) > +{ > + if (current->flags & RA_A_SIGNED_BIT) > + return __builtin_aarch64_pacia1716 (handler_addr, > + (_Unwind_Word) current->cfa); > + else > + return handler_addr; > +} > + > +/* Do AArch64 private initialization on CONTEXT based on frame info FS. Mark > + CONTEXT as return address signed if bit 0 of DWARF_REGNUM_AARCH64_RA_STATE is > + set. */ > + > +static void > +aarch64_frob_update_context (struct _Unwind_Context *context, > + _Unwind_FrameState *fs) > +{ > + if (fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset & 0x1) > + /* The flag is used for re-authenticating EH handler's address. */ > + context->flags |= RA_A_SIGNED_BIT; > + > + return; > +} > + > +#endif /* defined AARCH64_UNWIND_H */ > diff --git a/libgcc/config/aarch64/linux-unwind.h b/libgcc/config/aarch64/linux-unwind.h > index 1256f010007ac964de8bb895379a7d893a446bd9..75774b30c080ad2361fb37c7208bcf3d3b57d77a 100644 > --- a/libgcc/config/aarch64/linux-unwind.h > +++ b/libgcc/config/aarch64/linux-unwind.h > @@ -24,6 +24,7 @@ > > #include <signal.h> > #include <sys/ucontext.h> > +#include "aarch64-unwind.h" > > > /* Since insns are always stored LE, on a BE system the opcodes will > R.
Thanks for the review. On 19/01/17 14:18, Richard Earnshaw (lists) wrote: > >> >> >> diff --git a/libgcc/unwind-dw2.c b/libgcc/unwind-dw2.c >> index 8085a42ace15d53f4cb0c6681717012d906a6d47..cf640135275deb76b820f8209fa51eacfd64c4a2 100644 >> --- a/libgcc/unwind-dw2.c >> +++ b/libgcc/unwind-dw2.c >> @@ -136,6 +136,8 @@ struct _Unwind_Context >> #define SIGNAL_FRAME_BIT ((~(_Unwind_Word) 0 >> 1) + 1) >> /* Context which has version/args_size/by_value fields. */ >> #define EXTENDED_CONTEXT_BIT ((~(_Unwind_Word) 0 >> 2) + 1) >> + /* Bit reserved on AArch64, return address has been signed with A key. */ >> +#define RA_A_SIGNED_BIT ((~(_Unwind_Word) 0 >> 3) + 1) > > Why is this here? It appears to only be used within the > AArch64-specific header file. I was putting it here so that when we allocate the next general purpose bit, we know clearly that bit 3 is allocated to AArch64 already, and the new general bit needs to go to the next one. This can avoid bit collision. > >> ... >> >> +/* Frob exception handler's address kept in TARGET before installing into >> + CURRENT context. */ >> + >> +static void * >> +uw_frob_return_addr (struct _Unwind_Context *current, >> + struct _Unwind_Context *target) >> +{ >> + void *ret_addr = __builtin_frob_return_addr (target->ra); >> +#ifdef MD_POST_FROB_EH_HANDLER_ADDR >> + ret_addr = MD_POST_FROB_EH_HANDLER_ADDR (current, target, ret_addr); >> +#endif >> + return ret_addr; >> +} >> + > > I think this function should be marked inline. The optimizers would > probably inline it anyway, but it seems wrong for us to rely on that. Thanks, fixed. Does the updated patch looks OK to you know? libgcc/ 2017-01-19 Jiong Wang <jiong.wang@arm.com> * config/aarch64/aarch64-unwind.h: New file. (DWARF_REGNUM_AARCH64_RA_STATE): Define. (MD_POST_EXTRACT_ROOT_ADDR): Define. (MD_POST_EXTRACT_FRAME_ADDR): Define. (MD_POST_FROB_EH_HANDLER_ADDR): Define. (MD_FROB_UPDATE_CONTEXT): Define. (aarch64_post_extract_frame_addr): New function. (aarch64_post_frob_eh_handler_addr): New function. (aarch64_frob_update_context): New function. * config/aarch64/linux-unwind.h: Include aarch64-unwind.h * config.host (aarch64*-*-elf, aarch64*-*-rtems*, aarch64*-*-freebsd*): Initialize md_unwind_header to include aarch64-unwind.h. * unwind-dw2.c (struct _Unwind_Context): Define "RA_A_SIGNED_BIT". (execute_cfa_program): Multiplex DW_CFA_GNU_window_save for __aarch64__. (uw_update_context): Honor MD_POST_EXTRACT_FRAME_ADDR. (uw_init_context_1): Honor MD_POST_EXTRACT_ROOT_ADDR. (uw_frob_return_addr): New function. (_Unwind_DebugHook): Use uw_frob_return_addr.diff --git a/libgcc/config.host b/libgcc/config.host index 6f2e458e74e776a6b7a310919558bcca76389232..540bfa9635802adabb36a2d1b7cf3416462c59f3 100644 --- a/libgcc/config.host +++ b/libgcc/config.host @@ -331,11 +331,13 @@ aarch64*-*-elf | aarch64*-*-rtems*) extra_parts="$extra_parts crtfastmath.o" tmake_file="${tmake_file} ${cpu_type}/t-aarch64" tmake_file="${tmake_file} ${cpu_type}/t-softfp t-softfp t-crtfm" + md_unwind_header=aarch64/aarch64-unwind.h ;; aarch64*-*-freebsd*) extra_parts="$extra_parts crtfastmath.o" tmake_file="${tmake_file} ${cpu_type}/t-aarch64" tmake_file="${tmake_file} ${cpu_type}/t-softfp t-softfp t-crtfm" + md_unwind_header=aarch64/aarch64-unwind.h ;; aarch64*-*-linux*) extra_parts="$extra_parts crtfastmath.o" diff --git a/libgcc/config/aarch64/aarch64-unwind.h b/libgcc/config/aarch64/aarch64-unwind.h new file mode 100644 index 0000000000000000000000000000000000000000..a43d965b358f3e830b85fc42c7bceacf7d41a671 --- /dev/null +++ b/libgcc/config/aarch64/aarch64-unwind.h @@ -0,0 +1,87 @@ +/* Copyright (C) 2017 Free Software Foundation, Inc. + Contributed by ARM Ltd. + +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. + +Under Section 7 of GPL version 3, you are granted additional +permissions described in the GCC Runtime Library Exception, version +3.1, as published by the Free Software Foundation. + +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/>. */ + +#ifndef AARCH64_UNWIND_H +#define AARCH64_UNWIND_H + +#define DWARF_REGNUM_AARCH64_RA_STATE 34 + +#define MD_POST_EXTRACT_ROOT_ADDR(addr) __builtin_aarch64_xpaclri (addr) +#define MD_POST_EXTRACT_FRAME_ADDR(context, fs, addr) \ + aarch64_post_extract_frame_addr (context, fs, addr) +#define MD_POST_FROB_EH_HANDLER_ADDR(current, target, addr) \ + aarch64_post_frob_eh_handler_addr (current, target, addr) +#define MD_FROB_UPDATE_CONTEXT(context, fs) \ + aarch64_frob_update_context (context, fs) + +/* Do AArch64 private extraction on ADDR based on context info CONTEXT and + unwind frame info FS. If ADDR is signed, we do address authentication on it + using CFA of current frame. */ + +static inline void * +aarch64_post_extract_frame_addr (struct _Unwind_Context *context, + _Unwind_FrameState *fs, void *addr) +{ + if (fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset & 0x1) + { + _Unwind_Word salt = (_Unwind_Word) context->cfa; + return __builtin_aarch64_autia1716 (addr, salt); + } + else + return addr; +} + +/* Do AArch64 private frob on exception handler's address HANDLER_ADDR before + installing it into current context CURRENT. TARGET is currently not used. + We need to sign exception handler's address if CURRENT itself is signed. */ + +static inline void * +aarch64_post_frob_eh_handler_addr (struct _Unwind_Context *current, + struct _Unwind_Context *target + ATTRIBUTE_UNUSED, + void *handler_addr) +{ + if (current->flags & RA_A_SIGNED_BIT) + return __builtin_aarch64_pacia1716 (handler_addr, + (_Unwind_Word) current->cfa); + else + return handler_addr; +} + +/* Do AArch64 private initialization on CONTEXT based on frame info FS. Mark + CONTEXT as return address signed if bit 0 of DWARF_REGNUM_AARCH64_RA_STATE is + set. */ + +static inline void +aarch64_frob_update_context (struct _Unwind_Context *context, + _Unwind_FrameState *fs) +{ + if (fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset & 0x1) + /* The flag is used for re-authenticating EH handler's address. */ + context->flags |= RA_A_SIGNED_BIT; + + return; +} + +#endif /* defined AARCH64_UNWIND_H */ diff --git a/libgcc/config/aarch64/linux-unwind.h b/libgcc/config/aarch64/linux-unwind.h index 1256f010007ac964de8bb895379a7d893a446bd9..75774b30c080ad2361fb37c7208bcf3d3b57d77a 100644 --- a/libgcc/config/aarch64/linux-unwind.h +++ b/libgcc/config/aarch64/linux-unwind.h @@ -24,6 +24,7 @@ #include <signal.h> #include <sys/ucontext.h> +#include "aarch64-unwind.h" /* Since insns are always stored LE, on a BE system the opcodes will diff --git a/libgcc/unwind-dw2.c b/libgcc/unwind-dw2.c index 8085a42ace15d53f4cb0c6681717012d906a6d47..f2e5ce832190ff3ee69f31e3b6d44b68395f68df 100644 --- a/libgcc/unwind-dw2.c +++ b/libgcc/unwind-dw2.c @@ -136,6 +136,8 @@ struct _Unwind_Context #define SIGNAL_FRAME_BIT ((~(_Unwind_Word) 0 >> 1) + 1) /* Context which has version/args_size/by_value fields. */ #define EXTENDED_CONTEXT_BIT ((~(_Unwind_Word) 0 >> 2) + 1) + /* Bit reserved on AArch64, return address has been signed with A key. */ +#define RA_A_SIGNED_BIT ((~(_Unwind_Word) 0 >> 3) + 1) _Unwind_Word flags; /* 0 for now, can be increased when further fields are added to struct _Unwind_Context. */ @@ -1185,6 +1187,11 @@ execute_cfa_program (const unsigned char *insn_ptr, break; case DW_CFA_GNU_window_save: +#ifdef __aarch64__ + /* This CFA is multiplexed with Sparc. On AArch64 it's used to toggle + return address signing status. */ + fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset ^= 1; +#else /* ??? Hardcoded for SPARC register window configuration. */ if (__LIBGCC_DWARF_FRAME_REGISTERS__ >= 32) for (reg = 16; reg < 32; ++reg) @@ -1192,6 +1199,7 @@ execute_cfa_program (const unsigned char *insn_ptr, fs->regs.reg[reg].how = REG_SAVED_OFFSET; fs->regs.reg[reg].loc.offset = (reg - 16) * sizeof (void *); } +#endif break; case DW_CFA_GNU_args_size: @@ -1513,10 +1521,15 @@ uw_update_context (struct _Unwind_Context *context, _Unwind_FrameState *fs) stack frame. */ context->ra = 0; else - /* Compute the return address now, since the return address column - can change from frame to frame. */ - context->ra = __builtin_extract_return_addr - (_Unwind_GetPtr (context, fs->retaddr_column)); + { + /* Compute the return address now, since the return address column + can change from frame to frame. */ + context->ra = __builtin_extract_return_addr + (_Unwind_GetPtr (context, fs->retaddr_column)); +#ifdef MD_POST_EXTRACT_FRAME_ADDR + context->ra = MD_POST_EXTRACT_FRAME_ADDR (context, fs, context->ra); +#endif + } } static void @@ -1550,6 +1563,9 @@ uw_init_context_1 (struct _Unwind_Context *context, void *outer_cfa, void *outer_ra) { void *ra = __builtin_extract_return_addr (__builtin_return_address (0)); +#ifdef MD_POST_EXTRACT_ROOT_ADDR + ra = MD_POST_EXTRACT_ROOT_ADDR (ra); +#endif _Unwind_FrameState fs; _Unwind_SpTmp sp_slot; _Unwind_Reason_Code code; @@ -1586,6 +1602,9 @@ uw_init_context_1 (struct _Unwind_Context *context, initialization context, then we can't see it in the given call frame data. So have the initialization context tell us. */ context->ra = __builtin_extract_return_addr (outer_ra); +#ifdef MD_POST_EXTRACT_ROOT_ADDR + context->ra = MD_POST_EXTRACT_ROOT_ADDR (context->ra); +#endif } static void _Unwind_DebugHook (void *, void *) @@ -1608,6 +1627,20 @@ _Unwind_DebugHook (void *cfa __attribute__ ((__unused__)), #endif } +/* Frob exception handler's address kept in TARGET before installing into + CURRENT context. */ + +static inline void * +uw_frob_return_addr (struct _Unwind_Context *current, + struct _Unwind_Context *target) +{ + void *ret_addr = __builtin_frob_return_addr (target->ra); +#ifdef MD_POST_FROB_EH_HANDLER_ADDR + ret_addr = MD_POST_FROB_EH_HANDLER_ADDR (current, target, ret_addr); +#endif + return ret_addr; +} + /* Install TARGET into CURRENT so that we can return to it. This is a macro because __builtin_eh_return must be invoked in the context of our caller. */ @@ -1616,7 +1649,7 @@ _Unwind_DebugHook (void *cfa __attribute__ ((__unused__)), do \ { \ long offset = uw_install_context_1 ((CURRENT), (TARGET)); \ - void *handler = __builtin_frob_return_addr ((TARGET)->ra); \ + void *handler = uw_frob_return_addr ((CURRENT), (TARGET)); \ _Unwind_DebugHook ((TARGET)->cfa, handler); \ __builtin_eh_return (offset, handler); \ } \
On 19/01/17 14:46, Jiong Wang wrote: > Thanks for the review. > > On 19/01/17 14:18, Richard Earnshaw (lists) wrote: >> >>> >>> >>> diff --git a/libgcc/unwind-dw2.c b/libgcc/unwind-dw2.c >>> index >>> 8085a42ace15d53f4cb0c6681717012d906a6d47..cf640135275deb76b820f8209fa51eacfd64c4a2 >>> 100644 >>> --- a/libgcc/unwind-dw2.c >>> +++ b/libgcc/unwind-dw2.c >>> @@ -136,6 +136,8 @@ struct _Unwind_Context >>> #define SIGNAL_FRAME_BIT ((~(_Unwind_Word) 0 >> 1) + 1) >>> /* Context which has version/args_size/by_value fields. */ >>> #define EXTENDED_CONTEXT_BIT ((~(_Unwind_Word) 0 >> 2) + 1) >>> + /* Bit reserved on AArch64, return address has been signed with A >>> key. */ >>> +#define RA_A_SIGNED_BIT ((~(_Unwind_Word) 0 >> 3) + 1) >> >> Why is this here? It appears to only be used within the >> AArch64-specific header file. > > I was putting it here so that when we allocate the next general purpose > bit, we > know clearly that bit 3 is allocated to AArch64 already, and the new > general bit > needs to go to the next one. This can avoid bit collision. > Fair enough. >> >>> ... >>> >>> +/* Frob exception handler's address kept in TARGET before installing >>> into >>> + CURRENT context. */ >>> + >>> +static void * >>> +uw_frob_return_addr (struct _Unwind_Context *current, >>> + struct _Unwind_Context *target) >>> +{ >>> + void *ret_addr = __builtin_frob_return_addr (target->ra); >>> +#ifdef MD_POST_FROB_EH_HANDLER_ADDR >>> + ret_addr = MD_POST_FROB_EH_HANDLER_ADDR (current, target, ret_addr); >>> +#endif >>> + return ret_addr; >>> +} >>> + >> >> I think this function should be marked inline. The optimizers would >> probably inline it anyway, but it seems wrong for us to rely on that. > > Thanks, fixed. > > Does the updated patch looks OK to you know? > > libgcc/ > > 2017-01-19 Jiong Wang <jiong.wang@arm.com> > > * config/aarch64/aarch64-unwind.h: New file. > (DWARF_REGNUM_AARCH64_RA_STATE): Define. > (MD_POST_EXTRACT_ROOT_ADDR): Define. > (MD_POST_EXTRACT_FRAME_ADDR): Define. > (MD_POST_FROB_EH_HANDLER_ADDR): Define. > (MD_FROB_UPDATE_CONTEXT): Define. > (aarch64_post_extract_frame_addr): New function. > (aarch64_post_frob_eh_handler_addr): New function. > (aarch64_frob_update_context): New function. > * config/aarch64/linux-unwind.h: Include aarch64-unwind.h > * config.host (aarch64*-*-elf, aarch64*-*-rtems*, > aarch64*-*-freebsd*): > Initialize md_unwind_header to include aarch64-unwind.h. > * unwind-dw2.c (struct _Unwind_Context): Define "RA_A_SIGNED_BIT". > (execute_cfa_program): Multiplex DW_CFA_GNU_window_save for > __aarch64__. > (uw_update_context): Honor MD_POST_EXTRACT_FRAME_ADDR. > (uw_init_context_1): Honor MD_POST_EXTRACT_ROOT_ADDR. > (uw_frob_return_addr): New function. > (_Unwind_DebugHook): Use uw_frob_return_addr. > OK. R.
Hi Jiong, On 19 January 2017 at 15:46, Jiong Wang <jiong.wang@foss.arm.com> wrote: > Thanks for the review. > > On 19/01/17 14:18, Richard Earnshaw (lists) wrote: >> >> >>> >>> >>> diff --git a/libgcc/unwind-dw2.c b/libgcc/unwind-dw2.c >>> index >>> 8085a42ace15d53f4cb0c6681717012d906a6d47..cf640135275deb76b820f8209fa51eacfd64c4a2 >>> 100644 >>> --- a/libgcc/unwind-dw2.c >>> +++ b/libgcc/unwind-dw2.c >>> @@ -136,6 +136,8 @@ struct _Unwind_Context >>> #define SIGNAL_FRAME_BIT ((~(_Unwind_Word) 0 >> 1) + 1) >>> /* Context which has version/args_size/by_value fields. */ >>> #define EXTENDED_CONTEXT_BIT ((~(_Unwind_Word) 0 >> 2) + 1) >>> + /* Bit reserved on AArch64, return address has been signed with A key. >>> */ >>> +#define RA_A_SIGNED_BIT ((~(_Unwind_Word) 0 >> 3) + 1) >> >> >> Why is this here? It appears to only be used within the >> AArch64-specific header file. > > > I was putting it here so that when we allocate the next general purpose bit, > we > know clearly that bit 3 is allocated to AArch64 already, and the new general > bit > needs to go to the next one. This can avoid bit collision. > >> >>> ... >>> >>> +/* Frob exception handler's address kept in TARGET before installing >>> into >>> + CURRENT context. */ >>> + >>> +static void * >>> +uw_frob_return_addr (struct _Unwind_Context *current, >>> + struct _Unwind_Context *target) >>> +{ >>> + void *ret_addr = __builtin_frob_return_addr (target->ra); >>> +#ifdef MD_POST_FROB_EH_HANDLER_ADDR >>> + ret_addr = MD_POST_FROB_EH_HANDLER_ADDR (current, target, ret_addr); >>> +#endif >>> + return ret_addr; >>> +} >>> + >> >> >> I think this function should be marked inline. The optimizers would >> probably inline it anyway, but it seems wrong for us to rely on that. > > > Thanks, fixed. > > Does the updated patch looks OK to you know? > > libgcc/ > > 2017-01-19 Jiong Wang <jiong.wang@arm.com> > > > * config/aarch64/aarch64-unwind.h: New file. > (DWARF_REGNUM_AARCH64_RA_STATE): Define. > (MD_POST_EXTRACT_ROOT_ADDR): Define. > (MD_POST_EXTRACT_FRAME_ADDR): Define. > (MD_POST_FROB_EH_HANDLER_ADDR): Define. > (MD_FROB_UPDATE_CONTEXT): Define. > (aarch64_post_extract_frame_addr): New function. > (aarch64_post_frob_eh_handler_addr): New function. > (aarch64_frob_update_context): New function. > * config/aarch64/linux-unwind.h: Include aarch64-unwind.h > * config.host (aarch64*-*-elf, aarch64*-*-rtems*, > aarch64*-*-freebsd*): > Initialize md_unwind_header to include aarch64-unwind.h. > * unwind-dw2.c (struct _Unwind_Context): Define "RA_A_SIGNED_BIT". > (execute_cfa_program): Multiplex DW_CFA_GNU_window_save for > __aarch64__. > (uw_update_context): Honor MD_POST_EXTRACT_FRAME_ADDR. > (uw_init_context_1): Honor MD_POST_EXTRACT_ROOT_ADDR. > (uw_frob_return_addr): New function. > (_Unwind_DebugHook): Use uw_frob_return_addr. > Since you committed this (r244673), GCC fails to build for AArch64: /tmp/8132498_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/unwind-dw2.c: In function 'execute_cfa_program': /tmp/8132498_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/unwind-dw2.c:1193:17: error: 'DWARF_REGNUM_AARCH64_RA_STATE' undeclared (first use in this function) fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset ^= 1; ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Christophe
On 20/01/17 08:41, Christophe Lyon wrote: > Hi Jiong, > > On 19 January 2017 at 15:46, Jiong Wang <jiong.wang@foss.arm.com> wrote: >> Thanks for the review. >> >> On 19/01/17 14:18, Richard Earnshaw (lists) wrote: >>> >>>> >>>> diff --git a/libgcc/unwind-dw2.c b/libgcc/unwind-dw2.c >>>> index >>>> 8085a42ace15d53f4cb0c6681717012d906a6d47..cf640135275deb76b820f8209fa51eacfd64c4a2 >>>> 100644 >>>> --- a/libgcc/unwind-dw2.c >>>> +++ b/libgcc/unwind-dw2.c >>>> @@ -136,6 +136,8 @@ struct _Unwind_Context >>>> #define SIGNAL_FRAME_BIT ((~(_Unwind_Word) 0 >> 1) + 1) >>>> /* Context which has version/args_size/by_value fields. */ >>>> #define EXTENDED_CONTEXT_BIT ((~(_Unwind_Word) 0 >> 2) + 1) >>>> + /* Bit reserved on AArch64, return address has been signed with A key. >>>> */ >>>> +#define RA_A_SIGNED_BIT ((~(_Unwind_Word) 0 >> 3) + 1) >>> >>> Why is this here? It appears to only be used within the >>> AArch64-specific header file. >> >> I was putting it here so that when we allocate the next general purpose bit, >> we >> know clearly that bit 3 is allocated to AArch64 already, and the new general >> bit >> needs to go to the next one. This can avoid bit collision. >> >>>> ... >>>> >>>> +/* Frob exception handler's address kept in TARGET before installing >>>> into >>>> + CURRENT context. */ >>>> + >>>> +static void * >>>> +uw_frob_return_addr (struct _Unwind_Context *current, >>>> + struct _Unwind_Context *target) >>>> +{ >>>> + void *ret_addr = __builtin_frob_return_addr (target->ra); >>>> +#ifdef MD_POST_FROB_EH_HANDLER_ADDR >>>> + ret_addr = MD_POST_FROB_EH_HANDLER_ADDR (current, target, ret_addr); >>>> +#endif >>>> + return ret_addr; >>>> +} >>>> + >>> >>> I think this function should be marked inline. The optimizers would >>> probably inline it anyway, but it seems wrong for us to rely on that. >> >> Thanks, fixed. >> >> Does the updated patch looks OK to you know? >> >> libgcc/ >> >> 2017-01-19 Jiong Wang <jiong.wang@arm.com> >> >> >> * config/aarch64/aarch64-unwind.h: New file. >> (DWARF_REGNUM_AARCH64_RA_STATE): Define. >> (MD_POST_EXTRACT_ROOT_ADDR): Define. >> (MD_POST_EXTRACT_FRAME_ADDR): Define. >> (MD_POST_FROB_EH_HANDLER_ADDR): Define. >> (MD_FROB_UPDATE_CONTEXT): Define. >> (aarch64_post_extract_frame_addr): New function. >> (aarch64_post_frob_eh_handler_addr): New function. >> (aarch64_frob_update_context): New function. >> * config/aarch64/linux-unwind.h: Include aarch64-unwind.h >> * config.host (aarch64*-*-elf, aarch64*-*-rtems*, >> aarch64*-*-freebsd*): >> Initialize md_unwind_header to include aarch64-unwind.h. >> * unwind-dw2.c (struct _Unwind_Context): Define "RA_A_SIGNED_BIT". >> (execute_cfa_program): Multiplex DW_CFA_GNU_window_save for >> __aarch64__. >> (uw_update_context): Honor MD_POST_EXTRACT_FRAME_ADDR. >> (uw_init_context_1): Honor MD_POST_EXTRACT_ROOT_ADDR. >> (uw_frob_return_addr): New function. >> (_Unwind_DebugHook): Use uw_frob_return_addr. >> > Since you committed this (r244673), GCC fails to build for AArch64: > /tmp/8132498_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/unwind-dw2.c: > In function 'execute_cfa_program': > /tmp/8132498_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/unwind-dw2.c:1193:17: > error: 'DWARF_REGNUM_AARCH64_RA_STATE' undeclared (first use in this > function) > fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset ^= 1; > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Hi Christophe, could you please confirm you svn revision please? I do have done bootstrap and regression on both x86 and aarch64 before commit this patch. I had forgotten to "svn add" one header file, but add it later. Thanks. > Christophe
On 20 January 2017 at 10:44, Jiong Wang <jiong.wang@foss.arm.com> wrote: > > > On 20/01/17 08:41, Christophe Lyon wrote: >> >> Hi Jiong, >> >> On 19 January 2017 at 15:46, Jiong Wang <jiong.wang@foss.arm.com> wrote: >>> >>> Thanks for the review. >>> >>> On 19/01/17 14:18, Richard Earnshaw (lists) wrote: >>>> >>>> >>>>> >>>>> diff --git a/libgcc/unwind-dw2.c b/libgcc/unwind-dw2.c >>>>> index >>>>> >>>>> 8085a42ace15d53f4cb0c6681717012d906a6d47..cf640135275deb76b820f8209fa51eacfd64c4a2 >>>>> 100644 >>>>> --- a/libgcc/unwind-dw2.c >>>>> +++ b/libgcc/unwind-dw2.c >>>>> @@ -136,6 +136,8 @@ struct _Unwind_Context >>>>> #define SIGNAL_FRAME_BIT ((~(_Unwind_Word) 0 >> 1) + 1) >>>>> /* Context which has version/args_size/by_value fields. */ >>>>> #define EXTENDED_CONTEXT_BIT ((~(_Unwind_Word) 0 >> 2) + 1) >>>>> + /* Bit reserved on AArch64, return address has been signed with A >>>>> key. >>>>> */ >>>>> +#define RA_A_SIGNED_BIT ((~(_Unwind_Word) 0 >> 3) + 1) >>>> >>>> >>>> Why is this here? It appears to only be used within the >>>> AArch64-specific header file. >>> >>> >>> I was putting it here so that when we allocate the next general purpose >>> bit, >>> we >>> know clearly that bit 3 is allocated to AArch64 already, and the new >>> general >>> bit >>> needs to go to the next one. This can avoid bit collision. >>> >>>>> ... >>>>> >>>>> +/* Frob exception handler's address kept in TARGET before installing >>>>> into >>>>> + CURRENT context. */ >>>>> + >>>>> +static void * >>>>> +uw_frob_return_addr (struct _Unwind_Context *current, >>>>> + struct _Unwind_Context *target) >>>>> +{ >>>>> + void *ret_addr = __builtin_frob_return_addr (target->ra); >>>>> +#ifdef MD_POST_FROB_EH_HANDLER_ADDR >>>>> + ret_addr = MD_POST_FROB_EH_HANDLER_ADDR (current, target, ret_addr); >>>>> +#endif >>>>> + return ret_addr; >>>>> +} >>>>> + >>>> >>>> >>>> I think this function should be marked inline. The optimizers would >>>> probably inline it anyway, but it seems wrong for us to rely on that. >>> >>> >>> Thanks, fixed. >>> >>> Does the updated patch looks OK to you know? >>> >>> libgcc/ >>> >>> 2017-01-19 Jiong Wang <jiong.wang@arm.com> >>> >>> >>> * config/aarch64/aarch64-unwind.h: New file. >>> (DWARF_REGNUM_AARCH64_RA_STATE): Define. >>> (MD_POST_EXTRACT_ROOT_ADDR): Define. >>> (MD_POST_EXTRACT_FRAME_ADDR): Define. >>> (MD_POST_FROB_EH_HANDLER_ADDR): Define. >>> (MD_FROB_UPDATE_CONTEXT): Define. >>> (aarch64_post_extract_frame_addr): New function. >>> (aarch64_post_frob_eh_handler_addr): New function. >>> (aarch64_frob_update_context): New function. >>> * config/aarch64/linux-unwind.h: Include aarch64-unwind.h >>> * config.host (aarch64*-*-elf, aarch64*-*-rtems*, >>> aarch64*-*-freebsd*): >>> Initialize md_unwind_header to include aarch64-unwind.h. >>> * unwind-dw2.c (struct _Unwind_Context): Define >>> "RA_A_SIGNED_BIT". >>> (execute_cfa_program): Multiplex DW_CFA_GNU_window_save for >>> __aarch64__. >>> (uw_update_context): Honor MD_POST_EXTRACT_FRAME_ADDR. >>> (uw_init_context_1): Honor MD_POST_EXTRACT_ROOT_ADDR. >>> (uw_frob_return_addr): New function. >>> (_Unwind_DebugHook): Use uw_frob_return_addr. >>> >> Since you committed this (r244673), GCC fails to build for AArch64: >> >> /tmp/8132498_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/unwind-dw2.c: >> In function 'execute_cfa_program': >> >> /tmp/8132498_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/unwind-dw2.c:1193:17: >> error: 'DWARF_REGNUM_AARCH64_RA_STATE' undeclared (first use in this >> function) >> fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset ^= 1; >> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > Hi Christophe, could you please confirm you svn revision please? > > I do have done bootstrap and regression on both x86 and aarch64 before > commit this patch. I had forgotten to "svn add" one header file, but add it > later. > The failures started with r244673, and are still present with r244687. When did you add the missing file? > Thanks. > >> Christophe > >
On 20/01/17 10:11, Christophe Lyon wrote: > >>> /tmp/8132498_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/unwind-dw2.c: >>> In function 'execute_cfa_program': >>> >>> /tmp/8132498_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/unwind-dw2.c:1193:17: >>> error: 'DWARF_REGNUM_AARCH64_RA_STATE' undeclared (first use in this >>> function) >>> fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset ^= 1; >>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> >> Hi Christophe, could you please confirm you svn revision please? >> >> I do have done bootstrap and regression on both x86 and aarch64 before >> commit this patch. I had forgotten to "svn add" one header file, but add it >> later. >> > The failures started with r244673, and are still present with r244687. > When did you add the missing file? It was r244674, https://gcc.gnu.org/ml/gcc-cvs/2017-01/msg00689.html, so should have been included in your code. The faliure looks strange to me then, I will svn up and re-start a fresh bootstrap on AArch64. > >> Thanks. >> >>> Christophe >>
On 20 January 2017 at 11:18, Jiong Wang <jiong.wang@foss.arm.com> wrote: > > > On 20/01/17 10:11, Christophe Lyon wrote: >> >> >>>> >>>> /tmp/8132498_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/unwind-dw2.c: >>>> In function 'execute_cfa_program': >>>> >>>> >>>> /tmp/8132498_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libgcc/unwind-dw2.c:1193:17: >>>> error: 'DWARF_REGNUM_AARCH64_RA_STATE' undeclared (first use in this >>>> function) >>>> fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset ^= 1; >>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>> >>> >>> Hi Christophe, could you please confirm you svn revision please? >>> >>> I do have done bootstrap and regression on both x86 and aarch64 before >>> commit this patch. I had forgotten to "svn add" one header file, but add >>> it >>> later. >>> >> The failures started with r244673, and are still present with r244687. >> When did you add the missing file? > > > It was r244674, https://gcc.gnu.org/ml/gcc-cvs/2017-01/msg00689.html, so > should have been included in your code. The faliure looks strange to me > then, I will svn up and re-start a fresh bootstrap on AArch64. > The file is present in my git clone. I'm not bootstrapping on AArch64, I'm building a cross-compiler on x86_64, but it shouldn't matter. >> >>> Thanks. >>> >>>> Christophe >>> >>> >
On 20/01/17 10:30, Christophe Lyon wrote: >>>>> error: 'DWARF_REGNUM_AARCH64_RA_STATE' undeclared (first use in this >>>>> function) >>>>> fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset ^= 1; >>>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>>> >>>> Hi Christophe, could you please confirm you svn revision please? >>>> >>>> I do have done bootstrap and regression on both x86 and aarch64 before >>>> commit this patch. I had forgotten to "svn add" one header file, but add >>>> it >>>> later. >>>> >>> The failures started with r244673, and are still present with r244687. >>> When did you add the missing file? >> >> It was r244674, https://gcc.gnu.org/ml/gcc-cvs/2017-01/msg00689.html, so >> should have been included in your code. The faliure looks strange to me >> then, I will svn up and re-start a fresh bootstrap on AArch64. >> > The file is present in my git clone. > I'm not bootstrapping on AArch64, I'm building a cross-compiler on x86_64, > but it shouldn't matter. Hi Christophe, Thanks, I reproduced this on cross linux environment, the reason is the header file is not included because of the inhabit_libc guard, while the unwinder header file should always be included. I will committed the attached patch as obvious, once I finished a fresh bootstrap, cross elf, cross linux. Thanks. libgcc/ 2017-01-20 Jiong Wang <jiong.wang@arm.com> * config/aarch64/linux-unwind.h: Always include aarch64-unwind.h.diff --git a/libgcc/config/aarch64/linux-unwind.h b/libgcc/config/aarch64/linux-unwind.h index a8fa1d5..70e5a8a 100644 --- a/libgcc/config/aarch64/linux-unwind.h +++ b/libgcc/config/aarch64/linux-unwind.h @@ -20,11 +20,13 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see <http://www.gnu.org/licenses/>. */ +/* Always include AArch64 unwinder header file. */ +#include "config/aarch64/aarch64-unwind.h" + #ifndef inhibit_libc #include <signal.h> #include <sys/ucontext.h> -#include "config/aarch64/aarch64-unwind.h" /* Since insns are always stored LE, on a BE system the opcodes will
On 20/01/17 11:54, Jiong Wang wrote: > On 20/01/17 10:30, Christophe Lyon wrote: >>>>>> error: 'DWARF_REGNUM_AARCH64_RA_STATE' undeclared (first use in this >>>>>> function) >>>>>> fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset ^= 1; >>>>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>>>> >>>>> Hi Christophe, could you please confirm you svn revision please? >>>>> >>>>> I do have done bootstrap and regression on both x86 and aarch64 before >>>>> commit this patch. I had forgotten to "svn add" one header file, >>>>> but add >>>>> it >>>>> later. >>>>> >>>> The failures started with r244673, and are still present with r244687. >>>> When did you add the missing file? >>> >>> It was r244674, >>> https://gcc.gnu.org/ml/gcc-cvs/2017-01/msg00689.html, so >>> should have been included in your code. The faliure looks strange to me >>> then, I will svn up and re-start a fresh bootstrap on AArch64. >>> >> The file is present in my git clone. >> I'm not bootstrapping on AArch64, I'm building a cross-compiler on >> x86_64, >> but it shouldn't matter. > > Hi Christophe, > > Thanks, I reproduced this on cross linux environment, the reason is > the header file is not included because of the inhabit_libc guard, while > the unwinder header file should always be included. > > I will committed the attached patch as obvious, once I finished a > fresh bootstrap, cross elf, cross linux. > If this survives a cross-build, please just commit it. It's very unlikely that a native build will throw up any problems. R. > Thanks. > > libgcc/ > > 2017-01-20 Jiong Wang <jiong.wang@arm.com> > > * config/aarch64/linux-unwind.h: Always include aarch64-unwind.h. > > > > k.patch > > > diff --git a/libgcc/config/aarch64/linux-unwind.h b/libgcc/config/aarch64/linux-unwind.h > index a8fa1d5..70e5a8a 100644 > --- a/libgcc/config/aarch64/linux-unwind.h > +++ b/libgcc/config/aarch64/linux-unwind.h > @@ -20,11 +20,13 @@ > see the files COPYING3 and COPYING.RUNTIME respectively. If not, see > <http://www.gnu.org/licenses/>. */ > > +/* Always include AArch64 unwinder header file. */ > +#include "config/aarch64/aarch64-unwind.h" > + > #ifndef inhibit_libc > > #include <signal.h> > #include <sys/ucontext.h> > -#include "config/aarch64/aarch64-unwind.h" > > > /* Since insns are always stored LE, on a BE system the opcodes will >
On 20 January 2017 at 12:54, Jiong Wang <jiong.wang@foss.arm.com> wrote: > On 20/01/17 10:30, Christophe Lyon wrote: >>>>>> >>>>>> error: 'DWARF_REGNUM_AARCH64_RA_STATE' undeclared (first use in this >>>>>> function) >>>>>> fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset ^= 1; >>>>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>>>> >>>>> >>>>> Hi Christophe, could you please confirm you svn revision please? >>>>> >>>>> I do have done bootstrap and regression on both x86 and aarch64 before >>>>> commit this patch. I had forgotten to "svn add" one header file, but >>>>> add >>>>> it >>>>> later. >>>>> >>>> The failures started with r244673, and are still present with r244687. >>>> When did you add the missing file? >>> >>> >>> It was r244674, https://gcc.gnu.org/ml/gcc-cvs/2017-01/msg00689.html, so >>> should have been included in your code. The faliure looks strange to me >>> then, I will svn up and re-start a fresh bootstrap on AArch64. >>> >> The file is present in my git clone. >> I'm not bootstrapping on AArch64, I'm building a cross-compiler on x86_64, >> but it shouldn't matter. > > > Hi Christophe, > > Thanks, I reproduced this on cross linux environment, the reason is the > header file is not included because of the inhabit_libc guard, while the > unwinder header file should always be included. > > I will committed the attached patch as obvious, once I finished a fresh > bootstrap, cross elf, cross linux. > After you committed this (r244710), my cross build for aarch64-linux-gnu now passes. I'm now left with the build failure reported by Andrew on aarch64(_be)-none-elf. Thanks for the fix. Christophe. > Thanks. > > libgcc/ > > 2017-01-20 Jiong Wang <jiong.wang@arm.com> > > * config/aarch64/linux-unwind.h: Always include aarch64-unwind.h. > >
diff --git a/libgcc/unwind-dw2.c b/libgcc/unwind-dw2.c index 8085a42ace15d53f4cb0c6681717012d906a6d47..35010a4065bb83f706701cb05392193f0ffa1f11 100644 --- a/libgcc/unwind-dw2.c +++ b/libgcc/unwind-dw2.c @@ -136,6 +136,8 @@ struct _Unwind_Context #define SIGNAL_FRAME_BIT ((~(_Unwind_Word) 0 >> 1) + 1) /* Context which has version/args_size/by_value fields. */ #define EXTENDED_CONTEXT_BIT ((~(_Unwind_Word) 0 >> 2) + 1) + /* Bit reserved on AArch64, return address has been signed with A key. */ +#define RA_A_SIGNED_BIT ((~(_Unwind_Word) 0 >> 3) + 1) _Unwind_Word flags; /* 0 for now, can be increased when further fields are added to struct _Unwind_Context. */ @@ -1185,6 +1187,11 @@ execute_cfa_program (const unsigned char *insn_ptr, break; case DW_CFA_GNU_window_save: +#ifdef __aarch64__ + /* This CFA is multiplexed with Sparc. On AArch64 it's used to toggle + return address signing status. */ + fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset ^= 1; +#else /* ??? Hardcoded for SPARC register window configuration. */ if (__LIBGCC_DWARF_FRAME_REGISTERS__ >= 32) for (reg = 16; reg < 32; ++reg) @@ -1192,6 +1199,7 @@ execute_cfa_program (const unsigned char *insn_ptr, fs->regs.reg[reg].how = REG_SAVED_OFFSET; fs->regs.reg[reg].loc.offset = (reg - 16) * sizeof (void *); } +#endif break; case DW_CFA_GNU_args_size: @@ -1513,10 +1521,15 @@ uw_update_context (struct _Unwind_Context *context, _Unwind_FrameState *fs) stack frame. */ context->ra = 0; else - /* Compute the return address now, since the return address column - can change from frame to frame. */ - context->ra = __builtin_extract_return_addr - (_Unwind_GetPtr (context, fs->retaddr_column)); + { + /* Compute the return address now, since the return address column + can change from frame to frame. */ + context->ra = __builtin_extract_return_addr + (_Unwind_GetPtr (context, fs->retaddr_column)); +#ifdef MD_POST_EXTRACT_FRAME_ADDR + context->ra = MD_POST_EXTRACT_FRAME_ADDR (context, fs, context->ra); +#endif + } } static void @@ -1550,6 +1563,9 @@ uw_init_context_1 (struct _Unwind_Context *context, void *outer_cfa, void *outer_ra) { void *ra = __builtin_extract_return_addr (__builtin_return_address (0)); +#ifdef MD_POST_EXTRACT_ROOT_ADDR + ra = MD_POST_EXTRACT_ROOT_ADDR (ra); +#endif _Unwind_FrameState fs; _Unwind_SpTmp sp_slot; _Unwind_Reason_Code code; @@ -1586,6 +1602,12 @@ uw_init_context_1 (struct _Unwind_Context *context, initialization context, then we can't see it in the given call frame data. So have the initialization context tell us. */ context->ra = __builtin_extract_return_addr (outer_ra); +#ifdef MD_POST_EXTRACT_ROOT_ADDR + context->ra = MD_POST_EXTRACT_ROOT_ADDR (context->ra); +#endif +#ifdef MD_POST_INIT_CONTEXT + MD_POST_INIT_CONTEXT (context, &fs); +#endif } static void _Unwind_DebugHook (void *, void *) @@ -1608,6 +1630,20 @@ _Unwind_DebugHook (void *cfa __attribute__ ((__unused__)), #endif } +/* Frob exception handler's address kept in TARGET before installing into + CURRENT context. */ + +static void * +uw_frob_return_addr (struct _Unwind_Context *current, + struct _Unwind_Context *target) +{ + void *ret_addr = __builtin_frob_return_addr (target->ra); +#ifdef MD_POST_FROB_EH_HANDLER_ADDR + ret_addr = MD_POST_FROB_EH_HANDLER_ADDR (current, target, ret_addr); +#endif + return ret_addr; +} + /* Install TARGET into CURRENT so that we can return to it. This is a macro because __builtin_eh_return must be invoked in the context of our caller. */ @@ -1616,7 +1652,7 @@ _Unwind_DebugHook (void *cfa __attribute__ ((__unused__)), do \ { \ long offset = uw_install_context_1 ((CURRENT), (TARGET)); \ - void *handler = __builtin_frob_return_addr ((TARGET)->ra); \ + void *handler = uw_frob_return_addr ((CURRENT), (TARGET)); \ _Unwind_DebugHook ((TARGET)->cfa, handler); \ __builtin_eh_return (offset, handler); \ } \ diff --git a/libgcc/config.host b/libgcc/config.host index 6f2e458e74e776a6b7a310919558bcca76389232..540bfa9635802adabb36a2d1b7cf3416462c59f3 100644 --- a/libgcc/config.host +++ b/libgcc/config.host @@ -331,11 +331,13 @@ aarch64*-*-elf | aarch64*-*-rtems*) extra_parts="$extra_parts crtfastmath.o" tmake_file="${tmake_file} ${cpu_type}/t-aarch64" tmake_file="${tmake_file} ${cpu_type}/t-softfp t-softfp t-crtfm" + md_unwind_header=aarch64/aarch64-unwind.h ;; aarch64*-*-freebsd*) extra_parts="$extra_parts crtfastmath.o" tmake_file="${tmake_file} ${cpu_type}/t-aarch64" tmake_file="${tmake_file} ${cpu_type}/t-softfp t-softfp t-crtfm" + md_unwind_header=aarch64/aarch64-unwind.h ;; aarch64*-*-linux*) extra_parts="$extra_parts crtfastmath.o" diff --git a/libgcc/config/aarch64/aarch64-unwind.h b/libgcc/config/aarch64/aarch64-unwind.h new file mode 100644 index 0000000000000000000000000000000000000000..fd20feb0ce061c82c9134609438c84bc161670f9 --- /dev/null +++ b/libgcc/config/aarch64/aarch64-unwind.h @@ -0,0 +1,87 @@ +/* Copyright (C) 2017 Free Software Foundation, Inc. + Contributed by ARM Ltd. + +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. + +Under Section 7 of GPL version 3, you are granted additional +permissions described in the GCC Runtime Library Exception, version +3.1, as published by the Free Software Foundation. + +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/>. */ + +#ifndef AARCH64_UNWIND_H +#define AARCH64_UNWIND_H + +#define DWARF_REGNUM_AARCH64_RA_STATE 34 + +#define MD_POST_EXTRACT_ROOT_ADDR(addr) __builtin_aarch64_xpaclri (addr) +#define MD_POST_EXTRACT_FRAME_ADDR(context, fs, addr) \ + aarch64_post_extract_frame_addr (context, fs, addr) +#define MD_POST_FROB_EH_HANDLER_ADDR(current, target, addr) \ + aarch64_post_frob_eh_handler_addr (current, target, addr) +#define MD_POST_INIT_CONTEXT(context, fs) \ + aarch64_post_init_context (context, fs) + +/* Do AArch64 private extraction on ADDR based on context info CONTEXT and + unwind frame info FS. If ADDR is signed, we do address authentication on it + using CFA of current frame. */ + +static void * +aarch64_post_extract_frame_addr (struct _Unwind_Context *context, + _Unwind_FrameState *fs, void *addr) +{ + if (fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset & 0x1) + { + _Unwind_Word salt = (_Unwind_Word) context->cfa; + return __builtin_aarch64_autia1716 (addr, salt); + } + else + return addr; +} + +/* Do AArch64 private frob on exception handler's address HANDLER_ADDR before + installing it into current context CURRENT. TARGET is currently not used. + We need to sign exception handler's address if CURRENT itself is signed. */ + +static void * +aarch64_post_frob_eh_handler_addr (struct _Unwind_Context *current, + struct _Unwind_Context *target + ATTRIBUTE_UNUSED, + void *handler_addr) +{ + if (current->flags & RA_A_SIGNED_BIT) + return __builtin_aarch64_pacia1716 (handler_addr, + (_Unwind_Word) current->cfa); + else + return handler_addr; +} + +/* Do AArch64 private initialization on CONTEXT based on frame info FS. Mark + CONTEXT as return address signed if bit 0 of DWARF_REGNUM_AARCH64_RA_STATE is + set. */ + +static void +aarch64_post_init_context (struct _Unwind_Context *context, + _Unwind_FrameState *fs) +{ + if (fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset & 0x1) + /* The flag is used for re-authenticating EH handler's address. */ + context->flags |= RA_A_SIGNED_BIT; + + return; +} + +#endif /* defined AARCH64_UNWIND_H */ diff --git a/libgcc/config/aarch64/linux-unwind.h b/libgcc/config/aarch64/linux-unwind.h index 1256f010007ac964de8bb895379a7d893a446bd9..75774b30c080ad2361fb37c7208bcf3d3b57d77a 100644 --- a/libgcc/config/aarch64/linux-unwind.h +++ b/libgcc/config/aarch64/linux-unwind.h @@ -24,6 +24,7 @@ #include <signal.h> #include <sys/ucontext.h> +#include "aarch64-unwind.h" /* Since insns are always stored LE, on a BE system the opcodes will