On 20/01/17 08:41, Christophe Lyon wrote:
Hi Jiong,

On 19 January 2017 at 15:46, Jiong Wang <jiong.w...@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.w...@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

Reply via email to