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 5FB4A3856DE7 for ; Fri, 6 May 2022 08:23:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5FB4A3856DE7 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 DFB8D1063; Fri, 6 May 2022 01:23:25 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.37]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 2553B3FA27; Fri, 6 May 2022 01:23:25 -0700 (PDT) From: Richard Sandiford To: Andrea Corallo via Gcc-patches Mail-Followup-To: Andrea Corallo via Gcc-patches , Andrea Corallo , Richard Earnshaw , nd , richard.sandiford@arm.com Cc: Andrea Corallo , Richard Earnshaw , nd Subject: Re: [PATCH 11/12] aarch64: Make bti pass generic so it can be used by the arm backend References: Date: Fri, 06 May 2022 09:23:23 +0100 In-Reply-To: (Andrea Corallo via Gcc-patches's message of "Thu, 28 Apr 2022 11:51:43 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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, 06 May 2022 08:23:29 -0000 Andrea Corallo via Gcc-patches writes: > Hi all, > > this patch splits and restructures the aarch64 bti pass code in order > to have it usable by the arm backend as well. These changes have no > functional impact. > > Best Regards > > Andrea > > gcc/Changelog > > * config.gcc (aarch64*-*-*): Rename 'aarch64-bti-insert.o' into > 'aarch-bti-insert.o'. > * config/aarch64/aarch64-protos.h: Remove 'aarch64_bti_enabled' > proto. > * config/aarch64/aarch64.cc (aarch_bti_enabled): Rename. > (aarch_bti_j_insn_p, aarch_pac_insn_p): New functions. > (aarch64_output_mi_thunk) > (aarch64_print_patchable_function_entry) > (aarch64_file_end_indicate_exec_stack): Update renamed function > calls to renamed functions. > * config/aarch64/t-aarch64 (aarch-bti-insert.o): Update target. > * config/arm/aarch-bti-insert.cc: New file including and > generalizing code from aarch64-bti-insert.cc. > * config/arm/aarch-common-protos.h: Update. > * config/arm/arm-passes.def: New file. Looks good to me, thanks. Richard > diff --git a/gcc/config.gcc b/gcc/config.gcc > index 7b58e1314ff..2021bdf9d2f 100644 > --- a/gcc/config.gcc > +++ b/gcc/config.gcc > @@ -329,7 +329,7 @@ aarch64*-*-*) > c_target_objs="aarch64-c.o" > cxx_target_objs="aarch64-c.o" > d_target_objs="aarch64-d.o" > - extra_objs="aarch64-builtins.o aarch-common.o aarch64-sve-builtins.o aarch64-sve-builtins-shapes.o aarch64-sve-builtins-base.o aarch64-sve-builtins-sve2.o cortex-a57-fma-steering.o aarch64-speculation.o falkor-tag-collision-avoidance.o aarch64-bti-insert.o aarch64-cc-fusion.o" > + extra_objs="aarch64-builtins.o aarch-common.o aarch64-sve-builtins.o aarch64-sve-builtins-shapes.o aarch64-sve-builtins-base.o aarch64-sve-builtins-sve2.o cortex-a57-fma-steering.o aarch64-speculation.o falkor-tag-collision-avoidance.o aarch-bti-insert.o aarch64-cc-fusion.o" > target_gtfiles="\$(srcdir)/config/aarch64/aarch64-builtins.cc \$(srcdir)/config/aarch64/aarch64-sve-builtins.h \$(srcdir)/config/aarch64/aarch64-sve-builtins.cc" > target_has_targetm_common=yes > ;; > diff --git a/gcc/config/aarch64/aarch64-c.cc b/gcc/config/aarch64/aarch64-c.cc > index b0c5a4fd6b6..a9aad3abdc2 100644 > --- a/gcc/config/aarch64/aarch64-c.cc > +++ b/gcc/config/aarch64/aarch64-c.cc > @@ -179,7 +179,7 @@ aarch64_update_cpp_builtins (cpp_reader *pfile) > aarch64_def_or_undef (TARGET_RNG, "__ARM_FEATURE_RNG", pfile); > aarch64_def_or_undef (TARGET_MEMTAG, "__ARM_FEATURE_MEMORY_TAGGING", pfile); > > - aarch64_def_or_undef (aarch64_bti_enabled (), > + aarch64_def_or_undef (aarch_bti_enabled (), > "__ARM_FEATURE_BTI_DEFAULT", pfile); > > cpp_undef (pfile, "__ARM_FEATURE_PAC_DEFAULT"); > diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h > index fe2180e95ea..9fdf7f9cc9c 100644 > --- a/gcc/config/aarch64/aarch64-protos.h > +++ b/gcc/config/aarch64/aarch64-protos.h > @@ -891,7 +891,6 @@ void aarch64_register_pragmas (void); > void aarch64_relayout_simd_types (void); > void aarch64_reset_previous_fndecl (void); > bool aarch64_return_address_signing_enabled (void); > -bool aarch64_bti_enabled (void); > void aarch64_save_restore_target_globals (tree); > void aarch64_addti_scratch_regs (rtx, rtx, rtx *, > rtx *, rtx *, > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc > index eec743024c1..2f67f3872f6 100644 > --- a/gcc/config/aarch64/aarch64.cc > +++ b/gcc/config/aarch64/aarch64.cc > @@ -8534,11 +8534,61 @@ aarch64_return_address_signing_enabled (void) > > /* Return TRUE if Branch Target Identification Mechanism is enabled. */ > bool > -aarch64_bti_enabled (void) > +aarch_bti_enabled (void) > { > return (aarch_enable_bti == 1); > } > > +/* Check if INSN is a BTI J insn. */ > +bool > +aarch_bti_j_insn_p (rtx_insn *insn) > +{ > + if (!insn || !INSN_P (insn)) > + return false; > + > + rtx pat = PATTERN (insn); > + return GET_CODE (pat) == UNSPEC_VOLATILE && XINT (pat, 1) == UNSPECV_BTI_J; > +} > + > +/* Check if X (or any sub-rtx of X) is a PACIASP/PACIBSP instruction. */ > +bool > +aarch_pac_insn_p (rtx x) > +{ > + if (!INSN_P (x)) > + return false; > + > + subrtx_var_iterator::array_type array; > + FOR_EACH_SUBRTX_VAR (iter, array, PATTERN (x), ALL) > + { > + rtx sub = *iter; > + if (sub && GET_CODE (sub) == UNSPEC) > + { > + int unspec_val = XINT (sub, 1); > + switch (unspec_val) > + { > + case UNSPEC_PACIASP: > + case UNSPEC_PACIBSP: > + return true; > + > + default: > + return false; > + } > + iter.skip_subrtxes (); > + } > + } > + return false; > +} > + > +rtx aarch_gen_bti_c (void) > +{ > + return gen_bti_c (); > +} > + > +rtx aarch_gen_bti_j (void) > +{ > + return gen_bti_j (); > +} > + > /* The caller is going to use ST1D or LD1D to save or restore an SVE > register in mode MODE at BASE_RTX + OFFSET, where OFFSET is in > the range [1, 16] * GET_MODE_SIZE (MODE). Prepare for this by: > @@ -9918,7 +9968,7 @@ aarch64_output_mi_thunk (FILE *file, tree thunk ATTRIBUTE_UNUSED, > rtx_insn *insn; > const char *fnname = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (thunk)); > > - if (aarch64_bti_enabled ()) > + if (aarch_bti_enabled ()) > emit_insn (gen_bti_c()); > > reload_completed = 1; > @@ -22360,7 +22410,7 @@ aarch64_print_patchable_function_entry (FILE *file, > bool record_p) > { > if (cfun->machine->label_is_assembled > - && aarch64_bti_enabled () > + && aarch_bti_enabled () > && !cgraph_node::get (cfun->decl)->only_called_directly_p ()) > { > /* Remove the BTI that follows the patch area and insert a new BTI > @@ -26689,7 +26739,7 @@ aarch64_file_end_indicate_exec_stack () > file_end_indicate_exec_stack (); > > unsigned feature_1_and = 0; > - if (aarch64_bti_enabled ()) > + if (aarch_bti_enabled ()) > feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_BTI; > > if (aarch_ra_sign_scope != AARCH_FUNCTION_NONE) > diff --git a/gcc/config/aarch64/t-aarch64 b/gcc/config/aarch64/t-aarch64 > index 75b463d2f03..eb6ad3db955 100644 > --- a/gcc/config/aarch64/t-aarch64 > +++ b/gcc/config/aarch64/t-aarch64 > @@ -149,14 +149,14 @@ falkor-tag-collision-avoidance.o: \ > $(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) \ > $(srcdir)/config/aarch64/falkor-tag-collision-avoidance.cc > > -aarch64-bti-insert.o: $(srcdir)/config/aarch64/aarch64-bti-insert.cc \ > +aarch-bti-insert.o: $(srcdir)/config/arm/aarch-bti-insert.cc \ > $(CONFIG_H) $(SYSTEM_H) $(TM_H) $(REGS_H) insn-config.h $(RTL_BASE_H) \ > dominance.h cfg.h cfganal.h $(BASIC_BLOCK_H) $(INSN_ATTR_H) $(RECOG_H) \ > output.h hash-map.h $(DF_H) $(OBSTACK_H) $(TARGET_H) $(RTL_H) \ > $(CONTEXT_H) $(TREE_PASS_H) regrename.h \ > $(srcdir)/config/aarch64/aarch64-protos.h > $(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) \ > - $(srcdir)/config/aarch64/aarch64-bti-insert.cc > + $(srcdir)/config/arm/aarch-bti-insert.cc > > aarch64-cc-fusion.o: $(srcdir)/config/aarch64/aarch64-cc-fusion.cc \ > $(CONFIG_H) $(SYSTEM_H) $(CORETYPES_H) $(BACKEND_H) $(RTL_H) $(DF_H) \ > diff --git a/gcc/config/aarch64/aarch64-bti-insert.cc b/gcc/config/arm/aarch-bti-insert.cc > similarity index 80% > rename from gcc/config/aarch64/aarch64-bti-insert.cc > rename to gcc/config/arm/aarch-bti-insert.cc > index 7caed310bbc..2d1d2e334a9 100644 > --- a/gcc/config/aarch64/aarch64-bti-insert.cc > +++ b/gcc/config/arm/aarch-bti-insert.cc > @@ -42,10 +42,11 @@ > #include "tree-pass.h" > #include "cgraph.h" > > -/* This pass enables the support for Branch Target Identification Mechanism > - for AArch64. This is a new security feature introduced in ARMv8.5-A > - archtitecture. A BTI instruction is used to guard against the execution > - of instructions which are not the intended target of an indirect branch. > +/* This pass enables the support for Branch Target Identification Mechanism for > + Arm/AArch64. This is a security feature introduced in ARMv8.5-A > + architecture and ARMv8.1-M. A BTI instruction is used to guard against the > + execution of instructions which are not the intended target of an indirect > + branch. > > Outside of a guarded memory region, a BTI instruction executes as a NOP. > Within a guarded memory region any target of an indirect branch must be > @@ -53,7 +54,8 @@ > branch is triggered in a non-guarded memory region). An incompatibility > generates a Branch Target Exception. > > - The compatibility of the BTI instruction is as follows: > + The compatibility of the BTI instruction is as follows (AArch64 > + examples): > BTI j : Can be a target of any indirect jump (BR Xn). > BTI c : Can be a target of any indirect call (BLR Xn and BR X16/X17). > BTI jc: Can be a target of any indirect call or indirect jump. > @@ -90,47 +92,6 @@ const pass_data pass_data_insert_bti = > 0, /* todo_flags_finish. */ > }; > > -/* Check if X (or any sub-rtx of X) is a PACIASP/PACIBSP instruction. */ > -static bool > -aarch64_pac_insn_p (rtx x) > -{ > - if (!INSN_P (x)) > - return false; > - > - subrtx_var_iterator::array_type array; > - FOR_EACH_SUBRTX_VAR (iter, array, PATTERN (x), ALL) > - { > - rtx sub = *iter; > - if (sub && GET_CODE (sub) == UNSPEC) > - { > - int unspec_val = XINT (sub, 1); > - switch (unspec_val) > - { > - case UNSPEC_PACIASP: > - /* fall-through. */ > - case UNSPEC_PACIBSP: > - return true; > - > - default: > - return false; > - } > - iter.skip_subrtxes (); > - } > - } > - return false; > -} > - > -/* Check if INSN is a BTI J insn. */ > -static bool > -aarch64_bti_j_insn_p (rtx_insn *insn) > -{ > - if (!insn || !INSN_P (insn)) > - return false; > - > - rtx pat = PATTERN (insn); > - return GET_CODE (pat) == UNSPEC_VOLATILE && XINT (pat, 1) == UNSPECV_BTI_J; > -} > - > /* Insert the BTI instruction. */ > /* This is implemented as a late RTL pass that runs before branch > shortening and does the following. */ > @@ -155,7 +116,7 @@ rest_of_insert_bti (void) > && (LABEL_PRESERVE_P (insn) > || bb->flags & BB_NON_LOCAL_GOTO_TARGET)) > { > - bti_insn = gen_bti_j (); > + bti_insn = aarch_gen_bti_j (); > emit_insn_after (bti_insn, insn); > continue; > } > @@ -177,10 +138,10 @@ rest_of_insert_bti (void) > { > label = as_a (XEXP (RTVEC_ELT (vec, j), 0)); > rtx_insn *next = next_nonnote_nondebug_insn (label); > - if (aarch64_bti_j_insn_p (next)) > + if (aarch_bti_j_insn_p (next)) > continue; > > - bti_insn = gen_bti_j (); > + bti_insn = aarch_gen_bti_j (); > emit_insn_after (bti_insn, label); > } > } > @@ -191,7 +152,7 @@ rest_of_insert_bti (void) > will return. */ > if (CALL_P (insn) && (find_reg_note (insn, REG_SETJMP, NULL))) > { > - bti_insn = gen_bti_j (); > + bti_insn = aarch_gen_bti_j (); > emit_insn_after (bti_insn, insn); > continue; > } > @@ -207,9 +168,9 @@ rest_of_insert_bti (void) > { > bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb; > insn = BB_HEAD (bb); > - if (!aarch64_pac_insn_p (get_first_nonnote_insn ())) > + if (!aarch_pac_insn_p (get_first_nonnote_insn ())) > { > - bti_insn = gen_bti_c (); > + bti_insn = aarch_gen_bti_c (); > emit_insn_before (bti_insn, insn); > } > } > @@ -229,7 +190,7 @@ public: > /* opt_pass methods: */ > virtual bool gate (function *) > { > - return aarch64_bti_enabled (); > + return aarch_bti_enabled (); > } > > virtual unsigned int execute (function *) > diff --git a/gcc/config/arm/aarch-common-protos.h b/gcc/config/arm/aarch-common-protos.h > index 17a369f7e99..374982752ad 100644 > --- a/gcc/config/arm/aarch-common-protos.h > +++ b/gcc/config/arm/aarch-common-protos.h > @@ -42,6 +42,11 @@ extern int arm_no_early_alu_shift_value_dep (rtx, rtx); > extern int arm_no_early_mul_dep (rtx, rtx); > extern int arm_no_early_store_addr_dep (rtx, rtx); > extern bool arm_rtx_shift_left_p (rtx); > +extern bool aarch_bti_enabled (void); > +extern bool aarch_bti_j_insn_p (rtx_insn *); > +extern bool aarch_pac_insn_p (rtx); > +extern rtx aarch_gen_bti_c (void); > +extern rtx aarch_gen_bti_j (void); > > /* RTX cost table definitions. These are used when tuning for speed rather > than for size and should reflect the _additional_ cost over the cost > diff --git a/gcc/config/arm/arm-passes.def b/gcc/config/arm/arm-passes.def > new file mode 100644 > index 00000000000..beecd2b5455 > --- /dev/null > +++ b/gcc/config/arm/arm-passes.def > @@ -0,0 +1,21 @@ > +/* Arm-specific passes declarations. > + Copyright (C) 2021 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. > + > + You should have received a copy of the GNU General Public License > + along with GCC; see the file COPYING3. If not see > + . */ > + > +INSERT_PASS_BEFORE (pass_shorten_branches, 1, pass_insert_bti);