Re: [PATCH 11/12] aarch64: Make bti pass generic so it can be used by the arm backend

[Richard Sandiford <richard.sandiford@arm.com>]

Andrea Corallo via Gcc-patches <gcc-patches@gcc.gnu.org> 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 <rtx_insn *> (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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +INSERT_PASS_BEFORE (pass_shorten_branches, 1, pass_insert_bti);