RE: [PATCH][GCC][AArch64] Validate and set default parameters for stack-clash. [Patch (3/3)]

[Tamar Christina <Tamar dot Christina at arm dot com>]

Hi All,

I am sending an updated patch which takes into account a
case where the set parameter value would not be safe to call.

No change in the cover letter.

Regards,
Tamar

> -----Original Message-----
> From: Tamar Christina <tamar.christina@arm.com>
> Sent: Wednesday, July 11, 2018 12:25
> To: gcc-patches@gcc.gnu.org
> Cc: nd <nd@arm.com>; James Greenhalgh <James.Greenhalgh@arm.com>;
> Richard Earnshaw <Richard.Earnshaw@arm.com>; Marcus Shawcroft
> <Marcus.Shawcroft@arm.com>
> Subject: [PATCH][GCC][AArch64] Validate and set default parameters for
> stack-clash. [Patch (3/3)]
> 
> Hi All,
> 
> This patch defines the default parameters and validation for the aarch64
> stack clash probing interval and guard sizes.  It cleans up the previous
> implementation and insures that at no point the invalidate arguments are
> present in the pipeline for AArch64.  Currently they are only corrected once
> cc1 initalizes the back-end.
> 
> The default for AArch64 is 64 KB for both of these and we only support 4 KB
> and 64 KB probes.  We also enforce that any value you set here for the
> parameters must be in sync.
> 
> If an invalid value is specified an error will be generated and compilation
> aborted.
> 
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> Target was tested with stack clash on and off by default.
> 
> Ok for trunk?
> 
> Thanks,
> Tamar
> 
> gcc/
> 2018-07-11  Tamar Christina  <tamar.christina@arm.com>
> 
> 	* common/config/aarch64/aarch64-common.c
> (TARGET_OPTION_DEFAULT_PARAM,
> 	aarch64_option_default_param):	New.
> 	(params.h): Include.
> 	(TARGET_OPTION_VALIDATE_PARAM,
> aarch64_option_validate_param): New.
> 	* config/aarch64/aarch64.c (aarch64_override_options_internal):
> Simplify
> 	stack-clash protection validation code.
> 
> --

diff --git a/gcc/common/config/aarch64/aarch64-common.c b/gcc/common/config/aarch64/aarch64-common.c
index 292fb818705d4650113da59a6d88cf2aa7c9e57d..30bbec1380d6db60475f0d770944af98f566773d 100644
--- a/gcc/common/config/aarch64/aarch64-common.c
+++ b/gcc/common/config/aarch64/aarch64-common.c
@@ -30,6 +30,7 @@
 #include "opts.h"
 #include "flags.h"
 #include "diagnostic.h"
+#include "params.h"
 
 #ifdef  TARGET_BIG_ENDIAN_DEFAULT
 #undef  TARGET_DEFAULT_TARGET_FLAGS
@@ -41,6 +42,10 @@
 
 #undef	TARGET_OPTION_OPTIMIZATION_TABLE
 #define TARGET_OPTION_OPTIMIZATION_TABLE aarch_option_optimization_table
+#undef TARGET_OPTION_DEFAULT_PARAMS
+#define TARGET_OPTION_DEFAULT_PARAMS aarch64_option_default_params
+#undef TARGET_OPTION_VALIDATE_PARAM
+#define TARGET_OPTION_VALIDATE_PARAM aarch64_option_validate_param
 
 /* Set default optimization options.  */
 static const struct default_options aarch_option_optimization_table[] =
@@ -60,6 +65,48 @@ static const struct default_options aarch_option_optimization_table[] =
     { OPT_LEVELS_NONE, 0, NULL, 0 }
   };
 
+/* Implement target validation TARGET_OPTION_DEFAULT_PARAM.  */
+
+static bool
+aarch64_option_validate_param (const int value, const int param)
+{
+  /* Check that both parameters are the same.  */
+  if (param == (int) PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE)
+    {
+      if (value != 12 && value != 16)
+	{
+	error ("only values 12 (4 KB) and 16 (64 KB) are supported for guard "
+		"size.  Given value %d (%llu KB) is out of range.\n",
+		value, (1ULL << value) / 1024ULL);
+	return false;
+	}
+    }
+
+  return true;
+}
+
+/* Implement TARGET_OPTION_DEFAULT_PARAMS.  */
+
+static void
+aarch64_option_default_params (void)
+{
+  /* We assume the guard page is 64k.  */
+  int index = (int) PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE;
+  if (!compiler_params[index].configure_value)
+     set_default_param_value (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE, 16);
+
+  int guard_size
+    = default_param_value (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE);
+
+  /* Set the interval parameter to be the same as the guard size.  This way the
+     mid-end code does the right thing for us.  */
+  set_default_param_value (PARAM_STACK_CLASH_PROTECTION_PROBE_INTERVAL,
+			   guard_size);
+
+  /* Validate the options.  */
+  aarch64_option_validate_param (guard_size, index);
+}
+
 /* Implement TARGET_HANDLE_OPTION.
    This function handles the target specific options for CPU/target selection.
 
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index f2a4e0b3db62d9da87458bff98c16e8fb876f431..3fe0e47c561dfb7d1abf06a3322b4a6df63c7a21 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10905,19 +10905,7 @@ aarch64_override_options_internal (struct gcc_options *opts)
 			 opts->x_param_values,
 			 global_options_set.x_param_values);
 
-  /* If the user hasn't change it via configure then set the default to 64 KB
-     for the backend.  */
-  if (DEFAULT_STK_CLASH_GUARD_SIZE == 0)
-      maybe_set_param_value (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE, 16,
-			    opts->x_param_values,
-			    global_options_set.x_param_values);
-
-  /* Validate the guard size.  */
   int guard_size = PARAM_VALUE (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE);
-  if (guard_size != 12 && guard_size != 16)
-      error ("only values 12 (4 KB) and 16 (64 KB) are supported for guard "
-	     "size.  Given value %d (%llu KB) is out of range.\n",
-	     guard_size, (1ULL << guard_size) / 1024ULL);
 
   /* Enforce that interval is the same size as size so the mid-end does the
      right thing.  */