From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id DD84D385782C for ; Fri, 1 Jul 2022 16:04:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DD84D385782C Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id CAA52113E; Fri, 1 Jul 2022 09:04:30 -0700 (PDT) Received: from [10.2.78.56] (unknown [10.2.78.56]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B46B63F66F; Fri, 1 Jul 2022 09:04:29 -0700 (PDT) Message-ID: <259a7197-c488-f45d-abe9-6bcc824e5314@foss.arm.com> Date: Fri, 1 Jul 2022 17:04:28 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH 12/12 V2] arm: implement bti injection Content-Language: en-GB To: Andrea Corallo , Andrea Corallo via Gcc-patches Cc: Richard Earnshaw , nd References: From: Richard Earnshaw In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3496.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, KAM_SHORT, NICE_REPLY_A, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 01 Jul 2022 16:04:32 -0000 On 28/06/2022 10:21, Andrea Corallo via Gcc-patches wrote: > Hi all, > > second iteration of this patch enabling Branch Target Identification > Armv8.1-M Mechanism [1]. > > This is achieved by using the bti pass made common with Aarch64. > > The pass iterates through the instructions and adds the necessary BTI > instructions at the beginning of every function and at every landing > pads targeted by indirect jumps. > > Best Regards > > Andrea > > [1] > > > gcc/ChangeLog > > 2022-04-07 Andrea Corallo > > * config.gcc (arm*-*-*): Add 'aarch-bti-insert.o' object. > * config/arm/arm-protos.h: Update. > * config/arm/arm.cc (aarch_bti_enabled, aarch_bti_j_insn_p) > (aarch_pac_insn_p, aarch_gen_bti_c, aarch_gen_bti_j): New > functions. > * config/arm/arm.md (bti_nop): New insn. > * config/arm/t-arm (PASSES_EXTRA): Add 'arm-passes.def'. > (aarch-bti-insert.o): New target. > * config/arm/unspecs.md (UNSPEC_BTI_NOP): New unspec. > * config/arm/aarch-bti-insert.cc (rest_of_insert_bti): Update > to verify arch compatibility. > > gcc/testsuite/ChangeLog > > 2022-04-07 Andrea Corallo > > * gcc.target/arm/bti-1.c: New testcase. > * gcc.target/arm/bti-2.c: Likewise. > @@ -104,6 +105,14 @@ rest_of_insert_bti (void) rtx_insn *insn; basic_block bb; +#if defined (TARGET_32BIT) || defined (TARGET_THUMB1) See the comment about errors in response to patch 10. I'd simply expect the gate function to be disabled when we can't support PAC, so we should never get here. + if (!arm_arch8) + { + error ("This architecture does not support branch protection instructions"); + goto exit; + } +#endif + ... + +rtx aarch_gen_bti_c (void) +{ + return gen_bti_nop (); +} + +rtx aarch_gen_bti_j (void) +{ + return gen_bti_nop (); +} + Function names should start a new line... Thus: rtx aarch_gen_bti_c (void) etc. +(define_insn "bti_nop" + [(unspec_volatile [(const_int 0)] UNSPEC_BTI_NOP)] + "TARGET_THUMB2" This isn't quite right. We need v8-m.main as the baseline architecture for the NOPs to behave as NOPs. + "bti" + [(set_attr "type" "mov_reg")]) + How to deal with architectural NOPs is an interesting question. I think really, for the scheduler we need to describe each newly defined NOP separately, then in the scheduling descriptions we can handle all unimplemented NOPs by grouping them together for that architecture, whilst describing more accurately how to handle them on CPUs where they acquire a defined behaviour. diff --git a/gcc/config.gcc b/gcc/config.gcc index 2021bdf9d2f..004e1dfa8d8 100644 --- a/gcc/config.gcc +++ b/gcc/config.gcc @@ -353,7 +353,7 @@ arc*-*-*) ;; arm*-*-*) cpu_type=arm - extra_objs="arm-builtins.o arm-mve-builtins.o aarch-common.o" + extra_objs="arm-builtins.o arm-mve-builtins.o aarch-common.o aarch-bti-insert.o" --- a/gcc/config/arm/t-arm +++ b/gcc/config/arm/t-arm @@ -175,3 +175,13 @@ arm-d.o: $(srcdir)/config/arm/arm-d.cc arm-common.o: arm-cpu-cdata.h driver-arm.o: arm-native.h + +PASSES_EXTRA += $(srcdir)/config/arm/arm-passes.def See comment on patch 11. Perhaps the right thing to do is to move the hunk that adds arm-passes.def into this patch.