From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 84188 invoked by alias); 20 Jan 2017 09:44:27 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 83839 invoked by uid 89); 20 Jan 2017 09:44:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 20 Jan 2017 09:44:12 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 7DCADC14; Fri, 20 Jan 2017 01:44:10 -0800 (PST) Received: from [10.2.206.198] (e104437-lin.cambridge.arm.com [10.2.206.198]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 530E03F3D6; Fri, 20 Jan 2017 01:44:09 -0800 (PST) Subject: Re: [Ping~]Re: [5/5][libgcc] Runtime support for AArch64 return address signing (needs new target macros) To: Christophe Lyon References: <4cf21d03-0a88-c6fa-df37-59ec4edf1d89@foss.arm.com> <60457276-8566-8bef-6073-4b00dd975759@foss.arm.com> <1e3f0dce-74c8-5a80-ed5a-7e6feeec1e63@foss.arm.com> <97fad7f9-cb42-ab1c-f039-ba42d5dc72c8@foss.arm.com> <2298ec38-cd83-9a37-9dbf-14406409d460@foss.arm.com> Cc: "Richard Earnshaw (lists)" , gcc-patches , James Greenhalgh , Ian Lance Taylor , Joseph Myers , Jakub Jelinek From: Jiong Wang Message-ID: <5ac6c28f-d09c-847f-ed9e-a87443582781@foss.arm.com> Date: Fri, 20 Jan 2017 09:46:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2017-01/txt/msg01557.txt.bz2 On 20/01/17 08:41, Christophe Lyon wrote: > Hi Jiong, > > On 19 January 2017 at 15: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. >> >>>> ... >>>> >>>> +/* 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 >> >> >> * 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