diff mbox series

[12/12,V2] arm: implement bti injection

Message ID gkrczet9m8k.fsf_-_@arm.com
State Superseded
Headers
Series None |

Commit Message

Andrea Corallo June 28, 2022, 9:21 a.m. UTC 
  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]
<https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/armv8-1-m-pointer-authentication-and-branch-target-identification-extension>

gcc/ChangeLog

2022-04-07  Andrea Corallo  <andrea.corallo@arm.com>

	* 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  <andrea.corallo@arm.com>

	* gcc.target/arm/bti-1.c: New testcase.
	* gcc.target/arm/bti-2.c: Likewise.

Comments

Richard Earnshaw July 1, 2022, 4:04 p.m. UTC | #1 
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]
> <https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/armv8-1-m-pointer-authentication-and-branch-target-identification-extension>
> 
> gcc/ChangeLog
> 
> 2022-04-07  Andrea Corallo  <andrea.corallo@arm.com>
> 
> 	* 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  <andrea.corallo@arm.com>
> 
> 	* 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.
diff mbox series

Patch

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"
 	extra_headers="mmintrin.h arm_neon.h arm_acle.h arm_fp16.h arm_cmse.h arm_bf16.h arm_mve_types.h arm_mve.h arm_cde.h"
 	target_type_format_char='%'
 	c_target_objs="arm-c.o"
diff --git a/gcc/config/arm/aarch-bti-insert.cc b/gcc/config/arm/aarch-bti-insert.cc
index 2d1d2e334a9..38f5a58ef2f 100644
--- a/gcc/config/arm/aarch-bti-insert.cc
+++ b/gcc/config/arm/aarch-bti-insert.cc
@@ -41,6 +41,7 @@ 
 #include "cfgrtl.h"
 #include "tree-pass.h"
 #include "cgraph.h"
+#include "diagnostic-core.h"
 
 /* This pass enables the support for Branch Target Identification Mechanism for
    Arm/AArch64.  This is a security feature introduced in ARMv8.5-A
@@ -104,6 +105,14 @@  rest_of_insert_bti (void)
   rtx_insn *insn;
   basic_block bb;
 
+#if defined (TARGET_32BIT) || defined (TARGET_THUMB1)
+  if (!arm_arch8)
+    {
+      error ("This architecture does not support branch protection instructions");
+      goto exit;
+    }
+#endif
+
   bb = 0;
   FOR_EACH_BB_FN (bb, cfun)
     {
@@ -175,6 +184,7 @@  rest_of_insert_bti (void)
 	}
     }
 
+ exit:
   timevar_pop (TV_MACH_DEP);
   return 0;
 }
diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 84764bf27ce..6befb6c4445 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -24,6 +24,8 @@ 
 
 #include "sbitmap.h"
 
+rtl_opt_pass *make_pass_insert_bti (gcc::context *ctxt);
+
 extern enum unwind_info_type arm_except_unwind_info (struct gcc_options *);
 extern int use_return_insn (int, rtx);
 extern bool use_simple_return_p (void);
diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc
index 6760a01d68a..059075e2c95 100644
--- a/gcc/config/arm/arm.cc
+++ b/gcc/config/arm/arm.cc
@@ -23360,11 +23360,6 @@  output_probe_stack_range (rtx reg1, rtx reg2)
   return "";
 }
 
-static bool  aarch_bti_enabled ()
-{
-  return false;
-}
-
 /* Generate the prologue instructions for entry into an ARM or Thumb-2
    function.  */
 void
@@ -32980,6 +32975,56 @@  arm_current_function_pac_enabled_p (void)
 	&& !crtl->is_leaf);
 }
 
+/* Return TRUE if Branch Target Identification Mechanism is enabled.  */
+bool
+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) == UNSPEC_BTI_NOP;
+}
+
+/* Check if X (or any sub-rtx of X) is a PACIASP/PACIBSP instruction.  */
+bool
+aarch_pac_insn_p (rtx x)
+{
+  if (!x || !INSN_P (x))
+    return false;
+
+  rtx pat = PATTERN (x);
+
+  if (GET_CODE (pat) == SET)
+    {
+      rtx tmp = XEXP (pat, 1);
+      if (tmp
+	  && GET_CODE (tmp) == UNSPEC
+	  && (XINT (tmp, 1) == UNSPEC_PAC_NOP
+	      || XINT (tmp, 1) == UNSPEC_PACBTI_NOP))
+	return true;
+    }
+
+  return false;
+}
+
+rtx aarch_gen_bti_c (void)
+{
+  return gen_bti_nop ();
+}
+
+rtx aarch_gen_bti_j (void)
+{
+  return gen_bti_nop ();
+}
+
 /* Implement TARGET_SCHED_CAN_SPECULATE_INSN.  Return true if INSN can be
    scheduled for speculative execution.  Reject the long-running division
    and square-root instructions.  */
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index ae032e70a7d..b14c45638a8 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -12919,6 +12919,12 @@ 
   "aut\t%|ip, %|lr, %|sp"
   [(set_attr "length" "2")])
 
+(define_insn "bti_nop"
+  [(unspec_volatile [(const_int 0)] UNSPEC_BTI_NOP)]
+  "TARGET_THUMB2"
+  "bti"
+  [(set_attr "type" "mov_reg")])
+
 ;; Vector bits common to IWMMXT, Neon and MVE
 (include "vec-common.md")
 ;; Load the Intel Wireless Multimedia Extension patterns
diff --git a/gcc/config/arm/t-arm b/gcc/config/arm/t-arm
index 041cc6ec045..683342cb528 100644
--- 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
+
+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
+	$(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) \
+		$(srcdir)/config/arm/aarch-bti-insert.cc
diff --git a/gcc/config/arm/unspecs.md b/gcc/config/arm/unspecs.md
index dbe243a03f6..78e723a4b3c 100644
--- a/gcc/config/arm/unspecs.md
+++ b/gcc/config/arm/unspecs.md
@@ -162,6 +162,7 @@ 
   UNSPEC_PAC_NOP	; Represents PAC signing LR
   UNSPEC_PACBTI_NOP	; Represents PAC signing LR + valid landing pad
   UNSPEC_AUT_NOP	; Represents PAC verifying LR
+  UNSPEC_BTI_NOP	; Represent BTI
 ])
 
 
diff --git a/gcc/testsuite/gcc.target/arm/bti-1.c b/gcc/testsuite/gcc.target/arm/bti-1.c
new file mode 100644
index 00000000000..230ebb5a1da
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/bti-1.c
@@ -0,0 +1,12 @@ 
+/* Check that GCC does bti instruction.  */
+/* { dg-do compile } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */
+/* { dg-options "-march=armv8.1-m.main -mthumb -mbranch-protection=bti --save-temps" } */
+
+int
+main (void)
+{
+  return 0;
+}
+
+/* { dg-final { scan-assembler "bti" } } */
diff --git a/gcc/testsuite/gcc.target/arm/bti-2.c b/gcc/testsuite/gcc.target/arm/bti-2.c
new file mode 100644
index 00000000000..35aef6992e2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/bti-2.c
@@ -0,0 +1,58 @@ 
+/* { dg-do compile } */
+/* -Os to create jump table.  */
+/* { dg-options "-Os" } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */
+/* { dg-options "-march=armv8.1-m.main -mthumb -mbranch-protection=bti --save-temps" } */
+
+extern int f1 (void);
+extern int f2 (void);
+extern int f3 (void);
+extern int f4 (void);
+extern int f5 (void);
+extern int f6 (void);
+extern int f7 (void);
+extern int f8 (void);
+extern int f9 (void);
+extern int f10 (void);
+
+int (*ptr) (void);
+
+int
+f_jump_table (int y, int n)
+{
+  int i;
+  for (i = 0; i < n ;i ++)
+  {
+    switch (y)
+      {
+      case 0 : ptr = f1; break;
+      case 1 : ptr = f2; break;
+      case 2 : ptr = f3; break;
+      case 3 : ptr = f4; break;
+      case 4 : ptr = f5; break;
+      case 5 : ptr = f6; break;
+      case 6 : ptr = f7; break;
+      case 7 : ptr = f8; break;
+      case 8 : ptr = f9; break;
+      case 9 : ptr = f10; break;
+      default: break;
+      }
+    y += ptr ();
+  }
+  return (y == 0)? y+1:4;
+}
+
+int
+f_label_address ()
+{
+  static void * addr = &&lab1;
+  goto *addr;
+lab1:
+  addr = &&lab2;
+  return 1;
+lab2:
+  addr = &&lab1;
+  return 2;
+}
+
+/* { dg-final { scan-assembler-times "bti" 15 } } */