[gcc(refs/vendors/ARM/heads/gcs-13)] aarch64, arm: Fix branch-protection= parsing

[gcc(refs/vendors/ARM/heads/gcs-13)] aarch64, arm: Fix branch-protection= parsing

[Szabolcs Nagy]

https://gcc.gnu.org/g:fbf3ad1ee99950e67cb28f822aaa69b34b81ebbe

commit fbf3ad1ee99950e67cb28f822aaa69b34b81ebbe
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Thu Jun 15 17:15:09 2023 +0100

    aarch64,arm: Fix branch-protection= parsing
    
    Refactor the parsing to have a single API and fix a few parsing issues:
    
    - Different handling of "bti+none" and "none+bti": these should be
      rejected because "none" can only appear alone.
    
    - Accepted empty strings such as "bti++pac-ret" or "bti+", this bug
      was caused by using strtok_r.
    
    - Memory got leaked (str_root was never freed). And two buffers got
      allocated when one is enough.
    
    The callbacks now have no failure mode, only parsing can fail and
    all failures are handled locally.  The "-mbranch-protection=" vs
    "target("branch-protection=")" difference in the error message is
    handled by a separate argument to aarch_validate_mbranch_protection.
    
    gcc/ChangeLog:
    
            * config/aarch64/aarch64.cc (aarch64_override_options): Update.
            (aarch64_handle_attr_branch_protection): Update.
            * config/arm/aarch-common-protos.h (aarch_parse_branch_protection):
            Remove.
            (aarch_validate_mbranch_protection): Add new argument.
            * config/arm/aarch-common.cc (aarch_handle_no_branch_protection):
            Update.
            (aarch_handle_standard_branch_protection): Update.
            (aarch_handle_pac_ret_protection): Update.
            (aarch_handle_pac_ret_leaf): Update.
            (aarch_handle_pac_ret_b_key): Update.
            (aarch_handle_bti_protection): Update.
            (aarch_parse_branch_protection): Remove.
            (next_tok): New.
            (aarch_validate_mbranch_protection): Rewrite.
            * config/arm/aarch-common.h (struct aarch_branch_protect_type):
            Add field "alone".
            * config/arm/arm.cc (arm_configure_build_target): Update.
    
    gcc/testsuite/ChangeLog:
    
            * gcc.target/aarch64/branch-protection-attr.c: Update.
            * gcc.target/aarch64/branch-protection-option.c: Update.
    
    (cherry picked from commit 321477fc3a0f8de18c4452f431309f896ae3a854)

Diff:
---
 gcc/config/aarch64/aarch64.cc                      |  37 +---
 gcc/config/arm/aarch-common-protos.h               |   5 +-
 gcc/config/arm/aarch-common.cc                     | 217 +++++++++------------
 gcc/config/arm/aarch-common.h                      |  14 +-
 gcc/config/arm/arm.cc                              |   3 +-
 .../gcc.target/aarch64/branch-protection-attr.c    |   6 +-
 .../gcc.target/aarch64/branch-protection-option.c  |   2 +-
 7 files changed, 116 insertions(+), 168 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 6c91d80dd2f9..9cafd55f9c08 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -18361,7 +18361,8 @@ aarch64_override_options (void)
     aarch64_validate_sls_mitigation (aarch64_harden_sls_string);
 
   if (aarch64_branch_protection_string)
-    aarch_validate_mbranch_protection (aarch64_branch_protection_string);
+    aarch_validate_mbranch_protection (aarch64_branch_protection_string,
+				       "-mbranch-protection=");
 
   /* -mcpu=CPU is shorthand for -march=ARCH_FOR_CPU, -mtune=CPU.
      If either of -march or -mtune is given, they override their
@@ -18735,34 +18736,12 @@ aarch64_handle_attr_cpu (const char *str)
 
 /* Handle the argument STR to the branch-protection= attribute.  */
 
- static bool
- aarch64_handle_attr_branch_protection (const char* str)
- {
-  char *err_str = (char *) xmalloc (strlen (str) + 1);
-  enum aarch_parse_opt_result res = aarch_parse_branch_protection (str,
-								   &err_str);
-  bool success = false;
-  switch (res)
-    {
-     case AARCH_PARSE_MISSING_ARG:
-       error ("missing argument to %<target(\"branch-protection=\")%> pragma or"
-	      " attribute");
-       break;
-     case AARCH_PARSE_INVALID_ARG:
-       error ("invalid protection type %qs in %<target(\"branch-protection"
-	      "=\")%> pragma or attribute", err_str);
-       break;
-     case AARCH_PARSE_OK:
-       success = true;
-      /* Fall through.  */
-     case AARCH_PARSE_INVALID_FEATURE:
-       break;
-     default:
-       gcc_unreachable ();
-    }
-  free (err_str);
-  return success;
- }
+static bool
+aarch64_handle_attr_branch_protection (const char* str)
+{
+  return aarch_validate_mbranch_protection (str,
+					    "target(\"branch-protection=\")");
+}
 
 /* Handle the argument STR to the tune= target attribute.  */
 
diff --git a/gcc/config/arm/aarch-common-protos.h b/gcc/config/arm/aarch-common-protos.h
index f8cb65620960..75ffdfbb0504 100644
--- a/gcc/config/arm/aarch-common-protos.h
+++ b/gcc/config/arm/aarch-common-protos.h
@@ -159,10 +159,7 @@ rtx_insn *arm_md_asm_adjust (vec<rtx> &outputs, vec<rtx> & /*inputs*/,
 			     vec<rtx> &clobbers, HARD_REG_SET &clobbered_regs,
 			     location_t loc);
 
-/* Parsing routine for branch-protection common to AArch64 and Arm.  */
-enum aarch_parse_opt_result aarch_parse_branch_protection (const char*, char**);
-
 /* Validation routine for branch-protection common to AArch64 and Arm.  */
-bool aarch_validate_mbranch_protection (const char *);
+bool aarch_validate_mbranch_protection (const char *, const char *);
 
 #endif /* GCC_AARCH_COMMON_PROTOS_H */
diff --git a/gcc/config/arm/aarch-common.cc b/gcc/config/arm/aarch-common.cc
index cbc7f68a8bf6..2dcd6b3e3b71 100644
--- a/gcc/config/arm/aarch-common.cc
+++ b/gcc/config/arm/aarch-common.cc
@@ -659,169 +659,146 @@ arm_md_asm_adjust (vec<rtx> &outputs, vec<rtx> & /*inputs*/,
   return saw_asm_flag ? seq : NULL;
 }
 
-static enum aarch_parse_opt_result
-aarch_handle_no_branch_protection (char* str, char* rest)
+static void
+aarch_handle_no_branch_protection (void)
 {
   aarch_ra_sign_scope = AARCH_FUNCTION_NONE;
   aarch_enable_bti = 0;
-  if (rest)
-    {
-      error ("unexpected %<%s%> after %<%s%>", rest, str);
-      return AARCH_PARSE_INVALID_FEATURE;
-    }
-  return AARCH_PARSE_OK;
 }
 
-static enum aarch_parse_opt_result
-aarch_handle_standard_branch_protection (char* str, char* rest)
+static void
+aarch_handle_standard_branch_protection (void)
 {
   aarch_ra_sign_scope = AARCH_FUNCTION_NON_LEAF;
   aarch_ra_sign_key = AARCH_KEY_A;
   aarch_enable_bti = 1;
-  if (rest)
-    {
-      error ("unexpected %<%s%> after %<%s%>", rest, str);
-      return AARCH_PARSE_INVALID_FEATURE;
-    }
-  return AARCH_PARSE_OK;
 }
 
-static enum aarch_parse_opt_result
-aarch_handle_pac_ret_protection (char* str ATTRIBUTE_UNUSED,
-				 char* rest ATTRIBUTE_UNUSED)
+static void
+aarch_handle_pac_ret_protection (void)
 {
   aarch_ra_sign_scope = AARCH_FUNCTION_NON_LEAF;
   aarch_ra_sign_key = AARCH_KEY_A;
-  return AARCH_PARSE_OK;
 }
 
-static enum aarch_parse_opt_result
-aarch_handle_pac_ret_leaf (char* str ATTRIBUTE_UNUSED,
-			   char* rest ATTRIBUTE_UNUSED)
+static void
+aarch_handle_pac_ret_leaf (void)
 {
   aarch_ra_sign_scope = AARCH_FUNCTION_ALL;
-  return AARCH_PARSE_OK;
 }
 
-static enum aarch_parse_opt_result
-aarch_handle_pac_ret_b_key (char* str ATTRIBUTE_UNUSED,
-			    char* rest ATTRIBUTE_UNUSED)
+static void
+aarch_handle_pac_ret_b_key (void)
 {
   aarch_ra_sign_key = AARCH_KEY_B;
-  return AARCH_PARSE_OK;
 }
 
-static enum aarch_parse_opt_result
-aarch_handle_bti_protection (char* str ATTRIBUTE_UNUSED,
-			     char* rest ATTRIBUTE_UNUSED)
+static void
+aarch_handle_bti_protection (void)
 {
   aarch_enable_bti = 1;
-  return AARCH_PARSE_OK;
 }
 
 static const struct aarch_branch_protect_type aarch_pac_ret_subtypes[] = {
-  { "leaf", aarch_handle_pac_ret_leaf, NULL, 0 },
-  { "b-key", aarch_handle_pac_ret_b_key, NULL, 0 },
-  { NULL, NULL, NULL, 0 }
+  { "leaf", false, aarch_handle_pac_ret_leaf, NULL, 0 },
+  { "b-key", false, aarch_handle_pac_ret_b_key, NULL, 0 },
+  { NULL, false, NULL, NULL, 0 }
 };
 
 static const struct aarch_branch_protect_type aarch_branch_protect_types[] = {
-  { "none", aarch_handle_no_branch_protection, NULL, 0 },
-  { "standard", aarch_handle_standard_branch_protection, NULL, 0 },
-  { "pac-ret", aarch_handle_pac_ret_protection, aarch_pac_ret_subtypes,
+  { "none", true, aarch_handle_no_branch_protection, NULL, 0 },
+  { "standard", true, aarch_handle_standard_branch_protection, NULL, 0 },
+  { "pac-ret", false, aarch_handle_pac_ret_protection, aarch_pac_ret_subtypes,
     ARRAY_SIZE (aarch_pac_ret_subtypes) },
-  { "bti", aarch_handle_bti_protection, NULL, 0 },
-  { NULL, NULL, NULL, 0 }
+  { "bti", false, aarch_handle_bti_protection, NULL, 0 },
+  { NULL, false, NULL, NULL, 0 }
 };
 
-/* Parses CONST_STR for branch protection features specified in
-   aarch64_branch_protect_types, and set any global variables required.  Returns
-   the parsing result and assigns LAST_STR to the last processed token from
-   CONST_STR so that it can be used for error reporting.  */
+/* In-place split *str at delim, return *str and set *str to the tail
+   of the string or NULL if the end is reached.  */
 
-enum aarch_parse_opt_result
-aarch_parse_branch_protection (const char *const_str, char** last_str)
+static char *
+next_tok (char **str, int delim)
 {
-  char *str_root = xstrdup (const_str);
-  char* token_save = NULL;
-  char *str = strtok_r (str_root, "+", &token_save);
-  enum aarch_parse_opt_result res = AARCH_PARSE_OK;
-  if (!str)
-    res = AARCH_PARSE_MISSING_ARG;
-  else
+  char *tok = *str;
+  for (char *p = tok; p && *p != '\0'; p++)
     {
-      char *next_str = strtok_r (NULL, "+", &token_save);
-      /* Reset the branch protection features to their defaults.  */
-      aarch_handle_no_branch_protection (NULL, NULL);
-
-      while (str && res == AARCH_PARSE_OK)
+      if (*p == delim)
 	{
-	  const aarch_branch_protect_type* type = aarch_branch_protect_types;
-	  bool found = false;
-	  /* Search for this type.  */
-	  while (type && type->name && !found && res == AARCH_PARSE_OK)
-	    {
-	      if (strcmp (str, type->name) == 0)
-		{
-		  found = true;
-		  res = type->handler (str, next_str);
-		  str = next_str;
-		  next_str = strtok_r (NULL, "+", &token_save);
-		}
-	      else
-		type++;
-	    }
-	  if (found && res == AARCH_PARSE_OK)
-	    {
-	      bool found_subtype = true;
-	      /* Loop through each token until we find one that isn't a
-		 subtype.  */
-	      while (found_subtype)
-		{
-		  found_subtype = false;
-		  const aarch_branch_protect_type *subtype = type->subtypes;
-		  /* Search for the subtype.  */
-		  while (str && subtype && subtype->name && !found_subtype
-			  && res == AARCH_PARSE_OK)
-		    {
-		      if (strcmp (str, subtype->name) == 0)
-			{
-			  found_subtype = true;
-			  res = subtype->handler (str, next_str);
-			  str = next_str;
-			  next_str = strtok_r (NULL, "+", &token_save);
-			}
-		      else
-			subtype++;
-		    }
-		}
-	    }
-	  else if (!found)
-	    res = AARCH_PARSE_INVALID_ARG;
+	  *p = '\0';
+	  *str = p + 1;
+	  return tok;
 	}
     }
-  /* Copy the last processed token into the argument to pass it back.
-    Used by option and attribute validation to print the offending token.  */
-  if (last_str)
-    {
-      if (str)
-	strcpy (*last_str, str);
-      else
-	*last_str = NULL;
-    }
-  return res;
+  *str = NULL;
+  return tok;
 }
 
+/* Parses CONST_STR for branch protection features specified in
+   aarch64_branch_protect_types, and set any global variables required.
+   Returns true on success.  */
+
 bool
-aarch_validate_mbranch_protection (const char *const_str)
+aarch_validate_mbranch_protection (const char *const_str, const char *opt)
 {
-  char *str = (char *) xmalloc (strlen (const_str));
-  enum aarch_parse_opt_result res =
-    aarch_parse_branch_protection (const_str, &str);
-  if (res == AARCH_PARSE_INVALID_ARG)
-    error ("invalid argument %<%s%> for %<-mbranch-protection=%>", str);
-  else if (res == AARCH_PARSE_MISSING_ARG)
-    error ("missing argument for %<-mbranch-protection=%>");
-  free (str);
-  return res == AARCH_PARSE_OK;
+  char *str_root = xstrdup (const_str);
+  char *next_str = str_root;
+  char *str = next_tok (&next_str, '+');
+  char *alone_str = NULL;
+  bool reject_alone = false;
+  bool res = true;
+
+  /* First entry is "none" and it is used to reset the state.  */
+  aarch_branch_protect_types[0].handler ();
+
+  while (str)
+    {
+      const aarch_branch_protect_type *type = aarch_branch_protect_types;
+      for (; type->name; type++)
+	if (strcmp (str, type->name) == 0)
+	  break;
+      if (type->name == NULL)
+	{
+	  res = false;
+	  if (strcmp (str, "") == 0)
+	    error ("missing feature or flag for %<%s%>", opt);
+	  else
+	    error ("invalid argument %<%s%> for %<%s%>", str, opt);
+	  break;
+	}
+
+      if (type->alone && alone_str == NULL)
+	alone_str = str;
+      else
+	reject_alone = true;
+      if (reject_alone && alone_str != NULL)
+	{
+	  res = false;
+	  error ("argument %<%s%> can only appear alone in %<%s%>",
+		 alone_str, opt);
+	  break;
+	}
+
+      type->handler ();
+      str = next_tok (&next_str, '+');
+      if (type->subtypes == NULL)
+	continue;
+
+      /* Loop through tokens until we find one that isn't a subtype.  */
+      while (str)
+	{
+	  const aarch_branch_protect_type *subtype = type->subtypes;
+	  for (; subtype->name; subtype++)
+	    if (strcmp (str, subtype->name) == 0)
+	      break;
+	  if (subtype->name == NULL)
+	    break;
+
+	  subtype->handler ();
+	  str = next_tok (&next_str, '+');
+	}
+    }
+
+  free (str_root);
+  return res;
 }
diff --git a/gcc/config/arm/aarch-common.h b/gcc/config/arm/aarch-common.h
index c6a67f0d05cc..f72e21127fc8 100644
--- a/gcc/config/arm/aarch-common.h
+++ b/gcc/config/arm/aarch-common.h
@@ -55,16 +55,10 @@ struct aarch_branch_protect_type
   /* The type's name that the user passes to the branch-protection option
      string.  */
   const char* name;
-  /* Function to handle the protection type and set global variables.
-     First argument is the string token corresponding with this type and the
-     second argument is the next token in the option string.
-     Return values:
-     * AARCH_PARSE_OK: Handling was sucessful.
-     * AARCH_INVALID_ARG: The type is invalid in this context and the caller
-     should print an error.
-     * AARCH_INVALID_FEATURE: The type is invalid and the handler prints its
-     own error.  */
-  enum aarch_parse_opt_result (*handler)(char*, char*);
+  /* The type can only appear alone, other types should be rejected.  */
+  int alone;
+  /* Function to handle the protection type and set global variables.  */
+  void (*handler)(void);
   /* A list of types that can follow this type in the option string.  */
   const struct aarch_branch_protect_type* subtypes;
   unsigned int num_subtypes;
diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc
index 5f45bd10f202..7f64e10255ff 100644
--- a/gcc/config/arm/arm.cc
+++ b/gcc/config/arm/arm.cc
@@ -3270,7 +3270,8 @@ arm_configure_build_target (struct arm_build_target *target,
 
   if (opts->x_arm_branch_protection_string)
     {
-      aarch_validate_mbranch_protection (opts->x_arm_branch_protection_string);
+      aarch_validate_mbranch_protection (opts->x_arm_branch_protection_string,
+					 "-mbranch-protection=");
 
       if (aarch_ra_sign_key != AARCH_KEY_A)
 	{
diff --git a/gcc/testsuite/gcc.target/aarch64/branch-protection-attr.c b/gcc/testsuite/gcc.target/aarch64/branch-protection-attr.c
index 272000c27474..cc6820a7b14d 100644
--- a/gcc/testsuite/gcc.target/aarch64/branch-protection-attr.c
+++ b/gcc/testsuite/gcc.target/aarch64/branch-protection-attr.c
@@ -4,19 +4,19 @@ void __attribute__ ((target("branch-protection=leaf")))
 foo1 ()
 {
 }
-/* { dg-error {invalid protection type 'leaf' in 'target\("branch-protection="\)' pragma or attribute} "" { target *-*-* } 5 } */
+/* { dg-error {invalid argument 'leaf' for 'target\("branch-protection="\)'} "" { target *-*-* } 5 } */
 /* { dg-error {pragma or attribute 'target\("branch-protection=leaf"\)' is not valid} "" { target *-*-* } 5 } */
 
 void __attribute__ ((target("branch-protection=none+pac-ret")))
 foo2 ()
 {
 }
-/* { dg-error "unexpected 'pac-ret' after 'none'" "" { target *-*-* } 12 } */
+/* { dg-error {argument 'none' can only appear alone in 'target\("branch-protection="\)'} "" { target *-*-* } 12 } */
 /* { dg-error {pragma or attribute 'target\("branch-protection=none\+pac-ret"\)' is not valid} "" { target *-*-* } 12 } */
 
 void __attribute__ ((target("branch-protection=")))
 foo3 ()
 {
 }
-/* { dg-error {missing argument to 'target\("branch-protection="\)' pragma or attribute} "" { target *-*-* } 19 } */
+/* { dg-error {missing feature or flag for 'target\("branch-protection="\)'} "" { target *-*-* } 19 } */
 /* { dg-error {pragma or attribute 'target\("branch-protection="\)' is not valid} "" { target *-*-* } 19 } */
diff --git a/gcc/testsuite/gcc.target/aarch64/branch-protection-option.c b/gcc/testsuite/gcc.target/aarch64/branch-protection-option.c
index 1b3bf4ee2b88..e2f847a31c44 100644
--- a/gcc/testsuite/gcc.target/aarch64/branch-protection-option.c
+++ b/gcc/testsuite/gcc.target/aarch64/branch-protection-option.c
@@ -1,4 +1,4 @@
 /* { dg-do "compile" } */
 /* { dg-options "-mbranch-protection=leaf -mbranch-protection=none+pac-ret" } */
 
-/* { dg-error "unexpected 'pac-ret' after 'none'"  "" { target *-*-* } 0 } */
+/* { dg-error "argument 'none' can only appear alone in '-mbranch-protection='" "" { target *-*-* } 0 } */