From patchwork Thu Oct 27 09:55:19 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Andre Vieira (lists)" X-Patchwork-Id: 687522 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3t4MhY5PJzz9sxS for ; Thu, 27 Oct 2016 20:55:41 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=Pq6de5Ot; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=PhSTcWcIhTfKmxbMZ 5krkDPi+7lbSu3xKTQdFWyKQCmReKe3Zk5PjH8xfx7udLC8jrh+mVMcbldXcLCQj sb8dy5fHmEXD5Q6vAn6WOz9pWONhH/bbYVbfjcP4CU7Gz6eXYIGVA7WlC8wwo7k1 5Liy5xRaUyddICj9YthKWZ9bOA= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=dperrv5yYubchFTBPVvGisY ElHE=; b=Pq6de5OtucvtxL0BqdFXNfViE1ri1zCw1+Id3AdMuoiJZfIYAn5gPNg 5xSSV+uOb+H8z2dRxWqgbPzg5ivdz7LGVDzfodCGBgCZMxLJo03ekNt5+SnLDuva GP9Wb9rlUdf6Z73v0GbogFfKpEliahRqBRFBnoVca24Dlj7qmNC4= Received: (qmail 99527 invoked by alias); 27 Oct 2016 09:55:34 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 99517 invoked by uid 89); 27 Oct 2016 09:55:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.1 required=5.0 tests=BAYES_00, KAM_LOTSOFHASH, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=sk:TARGET, dgskipif, dg-skip-if, sk:!TARGET 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; Thu, 27 Oct 2016 09:55:22 +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 A70AC6D; Thu, 27 Oct 2016 02:55:21 -0700 (PDT) Received: from [10.2.206.246] (e107157-lin.cambridge.arm.com [10.2.206.246]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 372753F218; Thu, 27 Oct 2016 02:55:21 -0700 (PDT) Subject: Re: [PATCH 3/7, GCC, ARM, V8M] ARMv8-M Security Extension's cmse_nonsecure_entry: __acle_se label and bxns return To: Kyrill Tkachov , gcc-patches@gcc.gnu.org References: <5796116C.6010100@arm.com> <579612B8.7030401@arm.com> <580F8849.80001@arm.com> <58107F7B.3060507@foss.arm.com> From: "Andre Vieira (lists)" Message-ID: <5811CF07.8000501@arm.com> Date: Thu, 27 Oct 2016 10:55:19 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <58107F7B.3060507@foss.arm.com> X-IsSubscribed: yes On 26/10/16 11:03, Kyrill Tkachov wrote: > Hi Andre, > > On 25/10/16 17:28, Andre Vieira (lists) wrote: >> On 25/07/16 14:23, Andre Vieira (lists) wrote: >>> This patch extends support for the ARMv8-M Security Extensions >>> 'cmse_nonsecure_entry' attribute in two ways: >>> >>> 1) Generate two labels for the function, the regular function name and >>> one with the function's name appended to '__acle_se_', this will trigger >>> the linker to create a secure gateway veneer for this entry function. >>> 2) Return from cmse_nonsecure_entry marked functions using bxns. >>> >>> See Section 5.4 of ARM®v8-M Security Extensions >>> (http://infocenter.arm.com/help/topic/com.arm.doc.ecm0359818/index.html). >>> >>> >>> >>> *** gcc/ChangeLog *** >>> 2016-07-25 Andre Vieira >>> Thomas Preud'homme >>> >>> * config/arm/arm.c (use_return_insn): Change to return with >>> bxns >>> when cmse_nonsecure_entry. >>> (output_return_instruction): Likewise. >>> (arm_output_function_prologue): Likewise. >>> (thumb_pop): Likewise. >>> (thumb_exit): Likewise. >>> (arm_function_ok_for_sibcall): Disable sibcall for entry >>> functions. >>> (arm_asm_declare_function_name): New. >>> * config/arm/arm-protos.h (arm_asm_declare_function_name): New. >>> * config/arm/elf.h (ASM_DECLARE_FUNCTION_NAME): Redefine to >>> use arm_asm_declare_function_name. >>> >>> *** gcc/testsuite/ChangeLog *** >>> 2016-07-25 Andre Vieira >>> Thomas Preud'homme >>> >>> * gcc.target/arm/cmse/cmse-2.c: New. >>> * gcc.target/arm/cmse/cmse-4.c: New. >>> >> Hi, >> >> Rebased previous patch on top of trunk as requested. No changes to >> ChangeLog. >> >> Cheers, >> Andre > > @@ -19919,6 +19932,42 @@ output_return_instruction (rtx operand, bool > really_return, bool reverse, > return ""; > } > > +/* Output in FILE asm statements needed to declare the NAME of the > function > + defined by its DECL node. */ > + > +void > +arm_asm_declare_function_name (FILE *file, const char *name, tree decl) > +{ > + size_t cmse_name_len; > + char *cmse_name = 0; > + char cmse_prefix[] = "__acle_se_"; > + > + if (use_cmse && lookup_attribute ("cmse_nonsecure_entry", > + DECL_ATTRIBUTES (decl))) > + { > + cmse_name_len = sizeof (cmse_prefix) + strlen (name); > + cmse_name = XALLOCAVEC (char, cmse_name_len); > + snprintf (cmse_name, cmse_name_len, "%s%s", cmse_prefix, name); > + targetm.asm_out.globalize_label (file, cmse_name); > + } > + > > I think this definitely warrants a quick comment explaining why you're > adding > __acle_se_ to the function label > > > /* Scan INSN just before assembler is output for it. > @@ -25247,6 +25301,12 @@ thumb2_expand_return (bool simple_return) > > if (!simple_return && saved_regs_mask) > { > + /* TODO: Verify that this path is never taken for > cmse_nonsecure_entry > + functions or adapt code to handle according to ACLE. This path > should > + not be reachable for cmse_nonsecure_entry functions though we prefer > + to guard it for now to ensure that future code changes do not > silently > + change this behavior. */ > > I think you mean s/guard/assert/ > > + gcc_assert (!IS_CMSE_ENTRY (arm_current_func_type ())); > if (num_regs == 1) > { > rtx par = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (2)); > > This is ok with those changes. > Thanks, > Kyrill > Hi, Reworked comments. Also got rid of a redundant 'if (cmse_name)' in 'arm_asm_declare_function_name'. No change to ChangeLogs. Cheers, Andre diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h index e7d9f824596a62f5c99000940f6190ab6aee9255..a9b8326c2770c1f9a9787743cb5faa549e6b7d02 100644 --- a/gcc/config/arm/arm-protos.h +++ b/gcc/config/arm/arm-protos.h @@ -31,6 +31,7 @@ extern int arm_volatile_func (void); extern void arm_expand_prologue (void); extern void arm_expand_epilogue (bool); extern void arm_declare_function_name (FILE *, const char *, tree); +extern void arm_asm_declare_function_name (FILE *, const char *, tree); extern void thumb2_expand_return (bool); extern const char *arm_strip_name_encoding (const char *); extern void arm_asm_output_labelref (FILE *, const char *); diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 40edd3c7fb9567fadfddc36be777fbf53cbb8e9f..fdbdd423236e7388802bc4bd568f260d95485bbe 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -3895,6 +3895,11 @@ use_return_insn (int iscond, rtx sibling) return 0; } + /* ARMv8-M nonsecure entry function need to use bxns to return and thus need + several instructions if anything needs to be popped. */ + if (saved_int_regs && IS_CMSE_ENTRY (func_type)) + return 0; + /* If there are saved registers but the LR isn't saved, then we need two instructions for the return. */ if (saved_int_regs && !(saved_int_regs & (1 << LR_REGNUM))) @@ -6933,6 +6938,11 @@ arm_function_ok_for_sibcall (tree decl, tree exp) if (IS_INTERRUPT (func_type)) return false; + /* ARMv8-M non-secure entry functions need to return with bxns which is only + generated for entry functions themselves. */ + if (IS_CMSE_ENTRY (arm_current_func_type ())) + return false; + if (!VOID_TYPE_P (TREE_TYPE (DECL_RESULT (cfun->decl)))) { /* Check that the return value locations are the same. For @@ -19768,6 +19778,7 @@ output_return_instruction (rtx operand, bool really_return, bool reverse, (e.g. interworking) then we can load the return address directly into the PC. Otherwise we must load it into LR. */ if (really_return + && !IS_CMSE_ENTRY (func_type) && (IS_INTERRUPT (func_type) || !TARGET_INTERWORK)) return_reg = reg_names[PC_REGNUM]; else @@ -19908,8 +19919,10 @@ output_return_instruction (rtx operand, bool really_return, bool reverse, break; default: + if (IS_CMSE_ENTRY (func_type)) + snprintf (instr, sizeof (instr), "bxns%s\t%%|lr", conditional); /* Use bx if it's available. */ - if (arm_arch5 || arm_arch4t) + else if (arm_arch5 || arm_arch4t) sprintf (instr, "bx%s\t%%|lr", conditional); else sprintf (instr, "mov%s\t%%|pc, %%|lr", conditional); @@ -19922,6 +19935,44 @@ output_return_instruction (rtx operand, bool really_return, bool reverse, return ""; } +/* Output in FILE asm statements needed to declare the NAME of the function + defined by its DECL node. */ + +void +arm_asm_declare_function_name (FILE *file, const char *name, tree decl) +{ + size_t cmse_name_len; + char *cmse_name = 0; + char cmse_prefix[] = "__acle_se_"; + + /* When compiling with ARMv8-M Security Extensions enabled, we should print an + extra function label for each function with the 'cmse_nonsecure_entry' + attribute. This extra function label should be prepended with + '__acle_se_', telling the linker that it needs to create secure gateway + veneers for this function. */ + if (use_cmse && lookup_attribute ("cmse_nonsecure_entry", + DECL_ATTRIBUTES (decl))) + { + cmse_name_len = sizeof (cmse_prefix) + strlen (name); + cmse_name = XALLOCAVEC (char, cmse_name_len); + snprintf (cmse_name, cmse_name_len, "%s%s", cmse_prefix, name); + targetm.asm_out.globalize_label (file, cmse_name); + + ARM_DECLARE_FUNCTION_NAME (file, cmse_name, decl); + ASM_OUTPUT_TYPE_DIRECTIVE (file, cmse_name, "function"); + } + + ARM_DECLARE_FUNCTION_NAME (file, name, decl); + ASM_OUTPUT_TYPE_DIRECTIVE (file, name, "function"); + ASM_DECLARE_RESULT (file, DECL_RESULT (decl)); + ASM_OUTPUT_LABEL (file, name); + + if (cmse_name) + ASM_OUTPUT_LABEL (file, cmse_name); + + ARM_OUTPUT_FN_UNWIND (file, TRUE); +} + /* Write the function name into the code section, directly preceding the function prologue. @@ -19971,10 +20022,6 @@ arm_output_function_prologue (FILE *f, HOST_WIDE_INT frame_size) { unsigned long func_type; - /* ??? Do we want to print some of the below anyway? */ - if (TARGET_THUMB1) - return; - /* Sanity check. */ gcc_assert (!arm_ccfsm_state && !arm_target_insn); @@ -20009,6 +20056,8 @@ arm_output_function_prologue (FILE *f, HOST_WIDE_INT frame_size) asm_fprintf (f, "\t%@ Nested: function declared inside another function.\n"); if (IS_STACKALIGN (func_type)) asm_fprintf (f, "\t%@ Stack Align: May be called with mis-aligned SP.\n"); + if (IS_CMSE_ENTRY (func_type)) + asm_fprintf (f, "\t%@ Non-secure entry function: called from non-secure code.\n"); asm_fprintf (f, "\t%@ args = %d, pretend = %d, frame = %wd\n", crtl->args.size, @@ -24075,8 +24124,8 @@ thumb_pop (FILE *f, unsigned long mask) if (mask & (1 << PC_REGNUM)) { /* Catch popping the PC. */ - if (TARGET_INTERWORK || TARGET_BACKTRACE - || crtl->calls_eh_return) + if (TARGET_INTERWORK || TARGET_BACKTRACE || crtl->calls_eh_return + || IS_CMSE_ENTRY (arm_current_func_type ())) { /* The PC is never poped directly, instead it is popped into r3 and then BX is used. */ @@ -24137,7 +24186,10 @@ thumb_exit (FILE *f, int reg_containing_return_addr) if (crtl->calls_eh_return) asm_fprintf (f, "\tadd\t%r, %r\n", SP_REGNUM, ARM_EH_STACKADJ_REGNUM); - asm_fprintf (f, "\tbx\t%r\n", reg_containing_return_addr); + if (IS_CMSE_ENTRY (arm_current_func_type ())) + asm_fprintf (f, "\tbxns\t%r\n", reg_containing_return_addr); + else + asm_fprintf (f, "\tbx\t%r\n", reg_containing_return_addr); return; } /* Otherwise if we are not supporting interworking and we have not created @@ -24146,7 +24198,8 @@ thumb_exit (FILE *f, int reg_containing_return_addr) else if (!TARGET_INTERWORK && !TARGET_BACKTRACE && !is_called_in_ARM_mode (current_function_decl) - && !crtl->calls_eh_return) + && !crtl->calls_eh_return + && !IS_CMSE_ENTRY (arm_current_func_type ())) { asm_fprintf (f, "\tpop\t{%r}\n", PC_REGNUM); return; @@ -24369,7 +24422,10 @@ thumb_exit (FILE *f, int reg_containing_return_addr) asm_fprintf (f, "\tadd\t%r, %r\n", SP_REGNUM, ARM_EH_STACKADJ_REGNUM); /* Return to caller. */ - asm_fprintf (f, "\tbx\t%r\n", reg_containing_return_addr); + if (IS_CMSE_ENTRY (arm_current_func_type ())) + asm_fprintf (f, "\tbxns\t%r\n", reg_containing_return_addr); + else + asm_fprintf (f, "\tbx\t%r\n", reg_containing_return_addr); } /* Scan INSN just before assembler is output for it. @@ -25250,6 +25306,12 @@ thumb2_expand_return (bool simple_return) if (!simple_return && saved_regs_mask) { + /* TODO: Verify that this path is never taken for cmse_nonsecure_entry + functions or adapt code to handle according to ACLE. This path should + not be reachable for cmse_nonsecure_entry functions though we prefer + to assert it for now to ensure that future code changes do not silently + change this behavior. */ + gcc_assert (!IS_CMSE_ENTRY (arm_current_func_type ())); if (num_regs == 1) { rtx par = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (2)); @@ -25667,6 +25729,7 @@ arm_expand_epilogue (bool really_return) if (ARM_FUNC_TYPE (func_type) != ARM_FT_INTERWORKED && (TARGET_ARM || ARM_FUNC_TYPE (func_type) == ARM_FT_NORMAL) + && !IS_CMSE_ENTRY (func_type) && !IS_STACKALIGN (func_type) && really_return && crtl->args.pretend_args_size == 0 diff --git a/gcc/config/arm/elf.h b/gcc/config/arm/elf.h index bc4eb86f1da5beabf32647637eb87a3fc17a6c6a..03931eee7390ab4d09522e7d8bea10449719ddba 100644 --- a/gcc/config/arm/elf.h +++ b/gcc/config/arm/elf.h @@ -75,16 +75,7 @@ /* We might need a ARM specific header to function declarations. */ #undef ASM_DECLARE_FUNCTION_NAME -#define ASM_DECLARE_FUNCTION_NAME(FILE, NAME, DECL) \ - do \ - { \ - ARM_DECLARE_FUNCTION_NAME (FILE, NAME, DECL); \ - ASM_OUTPUT_TYPE_DIRECTIVE (FILE, NAME, "function"); \ - ASM_DECLARE_RESULT (FILE, DECL_RESULT (DECL)); \ - ASM_OUTPUT_LABEL(FILE, NAME); \ - ARM_OUTPUT_FN_UNWIND (FILE, TRUE); \ - } \ - while (0) +#define ASM_DECLARE_FUNCTION_NAME arm_asm_declare_function_name /* We might need an ARM specific trailer for function declarations. */ #undef ASM_DECLARE_FUNCTION_SIZE diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-10.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-10.c new file mode 100644 index 0000000000000000000000000000000000000000..1a91ac39ee37ef20495e047b402d3f5edc60a613 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-10.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-mcmse" } */ + +void +foo (void) {} + +/* { dg-final { scan-assembler-not "bxns" } } */ +/* { dg-final { scan-assembler "foo:" } } */ +/* { dg-final { scan-assembler-not "__acle_se_foo:" } } */ diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-4.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-4.c new file mode 100644 index 0000000000000000000000000000000000000000..6f930ab04a1097c64097a4e003296bbe85733319 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-4.c @@ -0,0 +1,27 @@ +/* { dg-do compile } */ +/* { dg-options "-mcmse" } */ + +struct span { + int a, b; +}; + +extern int qux (void); + +void __attribute__ ((cmse_nonsecure_entry)) +foo (void) {} + +static void __attribute__ ((cmse_nonsecure_entry)) +bar (void) {} /* { dg-warning "has no effect on functions with static linkage" } */ + +int __attribute__ ((cmse_nonsecure_entry)) +baz (void) +{ + return qux (); +} + +/* { dg-final { scan-assembler-times "bxns" 2 } } */ +/* { dg-final { scan-assembler "foo:" } } */ +/* { dg-final { scan-assembler "__acle_se_foo:" } } */ +/* { dg-final { scan-assembler-not "__acle_se_bar:" } } */ +/* { dg-final { scan-assembler "baz:" } } */ +/* { dg-final { scan-assembler "__acle_se_baz:" } } */ diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-9.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-9.c new file mode 100644 index 0000000000000000000000000000000000000000..1d97f0e1a37209bd129ffe96ce92a86bc2e0d5d1 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-9.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-skip-if "Testing exclusion of -mcmse" { arm-*-* } { "-mcmse" } { "" } } */ + + +int __attribute__ ((cmse_nonsecure_entry)) +foo (int a) +{ /* { dg-warning "attribute ignored without -mcmse option" } */ + return a + 1; +} + +/* { dg-final { scan-assembler "foo:" } } */ +/* { dg-final { scan-assembler-not "__acle_se_foo:" } } */