Re: [PATCH 12/15 V3] arm: implement bti injection

[Richard Earnshaw via Gcc-patches]

On 28/10/2022 17:40, Andrea Corallo via Gcc-patches wrote:
Hi all,

please find attached the third iteration of this patch addresing review
comments.

Thanks

   Andrea


@@ -23374,12 +23374,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
@@ -32992,6 +32986,61 @@ 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;
+}

See comment in earlier patch about the location of this function moving. 
 Can aarch_enable_bti take values other than 0 and 1?  If not, then 
writing aarch_enable_bti != 0 is slightly more robust, but perhaps this 
should be replaced by a macro anyway, much like a number of other 
predicates used by the backend.

+  return GET_CODE (pat) == UNSPEC_VOLATILE && XINT (pat, 1) == 
UNSPEC_BTI_NOP;

I'm not sure where this crept in, but UNSPEC and UNSPEC_VOLATILE have 
separate enums in the backend, so UNSPEC_BIT_NOP should really be 
VUNSPEC_BTI_NOP and defined in the enum "unspecv".

+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;
+    }
+

This will also need updating (see review on earlier patch) because 
PACBTI needs to be unspec_volatile, while PAC doesn't.

+/* The following two functions are for code compatibility with aarch64
+   code, this even if in arm we have only one bti instruction.  */
+

I'd just write
 /* Target specific mapping for aarch_gen_bti_c and aarch_gen_bti_j. 
For Arm, both of these map to a simple BTI instruction.  */


@@ -162,6 +162,7 @@ (define_c_enum "unspec" [
   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
 ])

BTI is an unspec volatile, so this should be in the "vunspec" enum and 
renamed accordingly (see above).

R.