public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Provide -fcf-protection=branch,return.
@ 2023-05-12  5:42 liuhongt
  2023-05-12  5:49 ` Andrew Pinski
  0 siblings, 1 reply; 7+ messages in thread
From: liuhongt @ 2023-05-12  5:42 UTC (permalink / raw)
  To: gcc-patches; +Cc: crazylht, hjl.tools

Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
Ok for trunk?

gcc/ChangeLog:

	PR target/89701
	* common.opt: Refactor -fcf-protection= to support combination
	of param.
	* lto-wrapper.c (merge_and_complain): Adjusted.
	* opts.c (parse_cf_protection_options): New.
	(common_handle_option): Decode argument for -fcf-protection=.
	* opts.h (parse_cf_protection_options): Declare.

gcc/testsuite/ChangeLog:

	PR target/89701
	* c-c++-common/fcf-protection-8.c: New test.
	* c-c++-common/fcf-protection-9.c: New test.
	* c-c++-common/fcf-protection-10.c: New test.
	* gcc.target/i386/pr89701-1.c: New test.
	* gcc.target/i386/pr89701-2.c: New test.
	* gcc.target/i386/pr89701-3.c: New test.
	* gcc.target/i386/pr89701-4.c: New test.
---
 gcc/common.opt                                | 24 ++----
 gcc/lto-wrapper.cc                            | 21 +++--
 gcc/opts.cc                                   | 79 +++++++++++++++++++
 gcc/opts.h                                    |  1 +
 .../c-c++-common/fcf-protection-10.c          |  3 +
 .../c-c++-common/fcf-protection-11.c          |  2 +
 .../c-c++-common/fcf-protection-12.c          |  2 +
 gcc/testsuite/c-c++-common/fcf-protection-8.c |  3 +
 gcc/testsuite/c-c++-common/fcf-protection-9.c |  3 +
 gcc/testsuite/gcc.target/i386/pr89701-1.c     |  4 +
 gcc/testsuite/gcc.target/i386/pr89701-2.c     |  4 +
 gcc/testsuite/gcc.target/i386/pr89701-3.c     |  5 ++
 gcc/testsuite/gcc.target/i386/pr89701-4.c     |  5 ++
 13 files changed, 130 insertions(+), 26 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-10.c
 create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-11.c
 create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-12.c
 create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-8.c
 create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-9.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-4.c

diff --git a/gcc/common.opt b/gcc/common.opt
index a28ca13385a..ac12da52733 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -229,6 +229,10 @@ bool dump_base_name_prefixed = false
 Variable
 unsigned int flag_zero_call_used_regs
 
+;; What the CF check should instrument
+Variable
+unsigned int flag_cf_protection = 0
+
 ###
 Driver
 
@@ -1886,28 +1890,10 @@ fcf-protection
 Common RejectNegative Alias(fcf-protection=,full)
 
 fcf-protection=
-Common Joined RejectNegative Enum(cf_protection_level) Var(flag_cf_protection) Init(CF_NONE)
+Common Joined
 -fcf-protection=[full|branch|return|none|check]	Instrument functions with checks to verify jump/call/return control-flow transfer
 instructions have valid targets.
 
-Enum
-Name(cf_protection_level) Type(enum cf_protection_level) UnknownError(unknown Control-Flow Protection Level %qs)
-
-EnumValue
-Enum(cf_protection_level) String(full) Value(CF_FULL)
-
-EnumValue
-Enum(cf_protection_level) String(branch) Value(CF_BRANCH)
-
-EnumValue
-Enum(cf_protection_level) String(return) Value(CF_RETURN)
-
-EnumValue
-Enum(cf_protection_level) String(check) Value(CF_CHECK)
-
-EnumValue
-Enum(cf_protection_level) String(none) Value(CF_NONE)
-
 finstrument-functions
 Common Var(flag_instrument_function_entry_exit,1)
 Instrument function entry and exit with profiling calls.
diff --git a/gcc/lto-wrapper.cc b/gcc/lto-wrapper.cc
index 5186d040ce0..568c8af659d 100644
--- a/gcc/lto-wrapper.cc
+++ b/gcc/lto-wrapper.cc
@@ -359,26 +359,33 @@ merge_and_complain (vec<cl_decoded_option> &decoded_options,
 	case OPT_fcf_protection_:
 	  /* Default to link-time option, else append or check identical.  */
 	  if (!cf_protection_option
-	      || cf_protection_option->value == CF_CHECK)
+	      || !memcmp (cf_protection_option->arg, "check", 5))
 	    {
+	      const char* parg = decoded_options[existing_opt].arg;
 	      if (existing_opt == -1)
 		decoded_options.safe_push (*foption);
-	      else if (decoded_options[existing_opt].value != foption->value)
+	      else if ((strlen (parg) != strlen (foption->arg))
+		       || memcmp (parg, foption->arg, strlen (foption->arg)))
 		{
 		  if (cf_protection_option
-		      && cf_protection_option->value == CF_CHECK)
+		      && !memcmp (cf_protection_option->arg, "check", 5))
 		    fatal_error (input_location,
 				 "option %qs with mismatching values"
 				 " (%s, %s)",
 				 "-fcf-protection",
-				 decoded_options[existing_opt].arg,
+				 parg,
 				 foption->arg);
 		  else
 		    {
 		      /* Merge and update the -fcf-protection option.  */
-		      decoded_options[existing_opt].value
-			&= (foption->value & CF_FULL);
-		      switch (decoded_options[existing_opt].value)
+		      HOST_WIDE_INT flags1
+			= parse_cf_protection_options (foption->arg,
+						       input_location);
+		      HOST_WIDE_INT flags2
+			= parse_cf_protection_options (parg,
+						       input_location);
+		      flags2 &= (flags1 & CF_FULL);
+		      switch (flags2)
 			{
 			case CF_NONE:
 			  decoded_options[existing_opt].arg = "none";
diff --git a/gcc/opts.cc b/gcc/opts.cc
index 86b94d62b58..6389383bc92 100644
--- a/gcc/opts.cc
+++ b/gcc/opts.cc
@@ -2187,6 +2187,81 @@ get_closest_sanitizer_option (const string_fragment &arg,
   return bm.get_best_meaningful_candidate ();
 }
 
+unsigned int
+parse_cf_protection_options (const char *p, location_t loc)
+{
+  unsigned int flags = 0;
+  bool combined = false;
+  while (*p != 0)
+    {
+      size_t len;
+      const char *comma = strchr (p, ',');
+      if (comma == NULL)
+	len = strlen (p);
+      else
+	{
+	  combined = true;
+	  len = comma - p;
+	}
+
+      if (len == 0)
+	{
+	  p = comma + 1;
+	  continue;
+	}
+
+      switch (len)
+	{
+	case 4:
+	  if (!memcmp (p, "full", 4))
+	    {
+	      if (combined && flags != CF_NONE)
+		warning_at (loc, 0, "better to use %<-fcf-protection=%> "
+			    "option: full alone instead of in a combination");
+	      flags |= CF_FULL;
+	    }
+	  else if (!memcmp (p, "none", 4))
+	    {
+	      if (combined && flags != CF_NONE)
+		warning_at (loc, 0, "combination of %<-fcf-protection=%> "
+			    "option: none is ignored");
+	    }
+	  else
+	    error_at (loc, "unrecognized argument to %<-fcf-protection=%> "
+		      "option: %.4s", p);
+	  break;
+	case 5:
+	  if (!memcmp (p, "check", 5))
+	    {
+	      if (combined && flags != CF_CHECK)
+		error_at (loc, "Combination for %<-fcf-protection=%> "
+			  "option: check is not valid");
+	      flags |= CF_CHECK;
+	    }
+	  else
+	    error_at (loc, "unrecognized argument to %<-fcf-protection=%> "
+		      "option: %.5s", p);
+	  break;
+	case 6:
+	  if (!memcmp (p, "branch", 6))
+	    flags |= CF_BRANCH;
+	  else if (!memcmp (p, "return", 6))
+	    flags |= CF_RETURN;
+	  else
+	    error_at (loc, "unrecognized argument to %<-fcf-protection=%> "
+		      "option: %.6s", p);
+	  break;
+	default:
+	  error_at (loc, "unrecognized argument to %<-fcf-protection=%> "
+		    "option: %.*s", (int) len, p);
+	}
+
+      if (comma == NULL)
+	break;
+      p = comma + 1;
+    }
+  return flags;
+}
 /* Parse comma separated sanitizer suboptions from P for option SCODE,
    adjust previous FLAGS and return new ones.  If COMPLAIN is false,
    don't issue diagnostics.  */
@@ -2671,6 +2746,10 @@ common_handle_option (struct gcc_options *opts,
     case OPT__completion_:
       break;
 
+    case OPT_fcf_protection_:
+      opts->x_flag_cf_protection
+	= parse_cf_protection_options (arg, loc);
+      break;
     case OPT_fsanitize_:
       opts_set->x_flag_sanitize = true;
       opts->x_flag_sanitize
diff --git a/gcc/opts.h b/gcc/opts.h
index 9959a440ca1..00d396d95f8 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -425,6 +425,7 @@ extern void control_warning_option (unsigned int opt_index, int kind,
 extern char *write_langs (unsigned int mask);
 extern void print_ignored_options (void);
 extern void handle_common_deferred_options (void);
+extern unsigned int parse_cf_protection_options (const char*, location_t);
 unsigned int parse_sanitizer_options (const char *, location_t, int,
 				      unsigned int, int, bool);
 
diff --git a/gcc/testsuite/c-c++-common/fcf-protection-10.c b/gcc/testsuite/c-c++-common/fcf-protection-10.c
new file mode 100644
index 00000000000..8692a08374b
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fcf-protection-10.c
@@ -0,0 +1,3 @@
+/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
+/* { dg-options "-fcf-protection=branch,check" } */
+/* { dg-error "Combination for '-fcf-protection=' option: check is not valid" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/c-c++-common/fcf-protection-11.c b/gcc/testsuite/c-c++-common/fcf-protection-11.c
new file mode 100644
index 00000000000..2e566350ccd
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fcf-protection-11.c
@@ -0,0 +1,2 @@
+/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
+/* { dg-options "-fcf-protection=branch,return" } */
diff --git a/gcc/testsuite/c-c++-common/fcf-protection-12.c b/gcc/testsuite/c-c++-common/fcf-protection-12.c
new file mode 100644
index 00000000000..b39c2f8e25d
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fcf-protection-12.c
@@ -0,0 +1,2 @@
+/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
+/* { dg-options "-fcf-protection=return,branch" } */
diff --git a/gcc/testsuite/c-c++-common/fcf-protection-8.c b/gcc/testsuite/c-c++-common/fcf-protection-8.c
new file mode 100644
index 00000000000..33e46223b6b
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fcf-protection-8.c
@@ -0,0 +1,3 @@
+/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
+/* { dg-options "-fcf-protection=branch,none" } */
+/* { dg-warning "combination of '-fcf-protection=' option: none is ignored" "" { target { *-*-* } } 0 } */
diff --git a/gcc/testsuite/c-c++-common/fcf-protection-9.c b/gcc/testsuite/c-c++-common/fcf-protection-9.c
new file mode 100644
index 00000000000..7848fe4b959
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fcf-protection-9.c
@@ -0,0 +1,3 @@
+/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
+/* { dg-options "-fcf-protection=branch,full" } */
+/* { dg-warning "better to use '-fcf-protection=' option: full alone instead of in a combination" "" { target { *-*-* } } 0 } */
diff --git a/gcc/testsuite/gcc.target/i386/pr89701-1.c b/gcc/testsuite/gcc.target/i386/pr89701-1.c
new file mode 100644
index 00000000000..1879c9ab4d8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr89701-1.c
@@ -0,0 +1,4 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-fcf-protection=branch,return" } */
+/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
+/* { dg-final { scan-assembler-times ".long	0x3" 1 } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr89701-2.c b/gcc/testsuite/gcc.target/i386/pr89701-2.c
new file mode 100644
index 00000000000..d5100575028
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr89701-2.c
@@ -0,0 +1,4 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-fcf-protection=return,branch" } */
+/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
+/* { dg-final { scan-assembler-times ".long	0x3" 1 } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr89701-3.c b/gcc/testsuite/gcc.target/i386/pr89701-3.c
new file mode 100644
index 00000000000..1505051a2bb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr89701-3.c
@@ -0,0 +1,5 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-fcf-protection=return,none" } */
+/* { dg-warning "combination of '-fcf-protection=' option: none is ignored" "" { target { *-*-linux* } } 0 } */
+/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
+/* { dg-final { scan-assembler-times ".long	0x2" 1 } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr89701-4.c b/gcc/testsuite/gcc.target/i386/pr89701-4.c
new file mode 100644
index 00000000000..242b8810abb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr89701-4.c
@@ -0,0 +1,5 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-fcf-protection=branch,none" } */
+/* { dg-warning "combination of '-fcf-protection=' option: none is ignored" "" { target { *-*-* } } 0 } */
+/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
+/* { dg-final { scan-assembler-times ".long	0x1" 1 } } */
-- 
2.39.1.388.g2fc9e9ca3c


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Provide -fcf-protection=branch,return.
  2023-05-12  5:42 [PATCH] Provide -fcf-protection=branch,return liuhongt
@ 2023-05-12  5:49 ` Andrew Pinski
  2023-05-12  6:21   ` Hongtao Liu
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Pinski @ 2023-05-12  5:49 UTC (permalink / raw)
  To: liuhongt; +Cc: gcc-patches, crazylht, hjl.tools

On Thu, May 11, 2023 at 10:45 PM liuhongt via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
> Ok for trunk?
>
> gcc/ChangeLog:
>
>         PR target/89701
>         * common.opt: Refactor -fcf-protection= to support combination
>         of param.
>         * lto-wrapper.c (merge_and_complain): Adjusted.
>         * opts.c (parse_cf_protection_options): New.
>         (common_handle_option): Decode argument for -fcf-protection=.
>         * opts.h (parse_cf_protection_options): Declare.

I think this could be simplified if you use either EnumSet or
EnumBitSet instead in common.opt for `-fcf-protection=`.

Thanks,
Andrew

>
> gcc/testsuite/ChangeLog:
>
>         PR target/89701
>         * c-c++-common/fcf-protection-8.c: New test.
>         * c-c++-common/fcf-protection-9.c: New test.
>         * c-c++-common/fcf-protection-10.c: New test.
>         * gcc.target/i386/pr89701-1.c: New test.
>         * gcc.target/i386/pr89701-2.c: New test.
>         * gcc.target/i386/pr89701-3.c: New test.
>         * gcc.target/i386/pr89701-4.c: New test.
> ---
>  gcc/common.opt                                | 24 ++----
>  gcc/lto-wrapper.cc                            | 21 +++--
>  gcc/opts.cc                                   | 79 +++++++++++++++++++
>  gcc/opts.h                                    |  1 +
>  .../c-c++-common/fcf-protection-10.c          |  3 +
>  .../c-c++-common/fcf-protection-11.c          |  2 +
>  .../c-c++-common/fcf-protection-12.c          |  2 +
>  gcc/testsuite/c-c++-common/fcf-protection-8.c |  3 +
>  gcc/testsuite/c-c++-common/fcf-protection-9.c |  3 +
>  gcc/testsuite/gcc.target/i386/pr89701-1.c     |  4 +
>  gcc/testsuite/gcc.target/i386/pr89701-2.c     |  4 +
>  gcc/testsuite/gcc.target/i386/pr89701-3.c     |  5 ++
>  gcc/testsuite/gcc.target/i386/pr89701-4.c     |  5 ++
>  13 files changed, 130 insertions(+), 26 deletions(-)
>  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-10.c
>  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-11.c
>  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-12.c
>  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-8.c
>  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-9.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-1.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-2.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-3.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-4.c
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index a28ca13385a..ac12da52733 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -229,6 +229,10 @@ bool dump_base_name_prefixed = false
>  Variable
>  unsigned int flag_zero_call_used_regs
>
> +;; What the CF check should instrument
> +Variable
> +unsigned int flag_cf_protection = 0
> +
>  ###
>  Driver
>
> @@ -1886,28 +1890,10 @@ fcf-protection
>  Common RejectNegative Alias(fcf-protection=,full)
>
>  fcf-protection=
> -Common Joined RejectNegative Enum(cf_protection_level) Var(flag_cf_protection) Init(CF_NONE)
> +Common Joined
>  -fcf-protection=[full|branch|return|none|check]        Instrument functions with checks to verify jump/call/return control-flow transfer
>  instructions have valid targets.
>
> -Enum
> -Name(cf_protection_level) Type(enum cf_protection_level) UnknownError(unknown Control-Flow Protection Level %qs)
> -
> -EnumValue
> -Enum(cf_protection_level) String(full) Value(CF_FULL)
> -
> -EnumValue
> -Enum(cf_protection_level) String(branch) Value(CF_BRANCH)
> -
> -EnumValue
> -Enum(cf_protection_level) String(return) Value(CF_RETURN)
> -
> -EnumValue
> -Enum(cf_protection_level) String(check) Value(CF_CHECK)
> -
> -EnumValue
> -Enum(cf_protection_level) String(none) Value(CF_NONE)
> -
>  finstrument-functions
>  Common Var(flag_instrument_function_entry_exit,1)
>  Instrument function entry and exit with profiling calls.
> diff --git a/gcc/lto-wrapper.cc b/gcc/lto-wrapper.cc
> index 5186d040ce0..568c8af659d 100644
> --- a/gcc/lto-wrapper.cc
> +++ b/gcc/lto-wrapper.cc
> @@ -359,26 +359,33 @@ merge_and_complain (vec<cl_decoded_option> &decoded_options,
>         case OPT_fcf_protection_:
>           /* Default to link-time option, else append or check identical.  */
>           if (!cf_protection_option
> -             || cf_protection_option->value == CF_CHECK)
> +             || !memcmp (cf_protection_option->arg, "check", 5))
>             {
> +             const char* parg = decoded_options[existing_opt].arg;
>               if (existing_opt == -1)
>                 decoded_options.safe_push (*foption);
> -             else if (decoded_options[existing_opt].value != foption->value)
> +             else if ((strlen (parg) != strlen (foption->arg))
> +                      || memcmp (parg, foption->arg, strlen (foption->arg)))
>                 {
>                   if (cf_protection_option
> -                     && cf_protection_option->value == CF_CHECK)
> +                     && !memcmp (cf_protection_option->arg, "check", 5))
>                     fatal_error (input_location,
>                                  "option %qs with mismatching values"
>                                  " (%s, %s)",
>                                  "-fcf-protection",
> -                                decoded_options[existing_opt].arg,
> +                                parg,
>                                  foption->arg);
>                   else
>                     {
>                       /* Merge and update the -fcf-protection option.  */
> -                     decoded_options[existing_opt].value
> -                       &= (foption->value & CF_FULL);
> -                     switch (decoded_options[existing_opt].value)
> +                     HOST_WIDE_INT flags1
> +                       = parse_cf_protection_options (foption->arg,
> +                                                      input_location);
> +                     HOST_WIDE_INT flags2
> +                       = parse_cf_protection_options (parg,
> +                                                      input_location);
> +                     flags2 &= (flags1 & CF_FULL);
> +                     switch (flags2)
>                         {
>                         case CF_NONE:
>                           decoded_options[existing_opt].arg = "none";
> diff --git a/gcc/opts.cc b/gcc/opts.cc
> index 86b94d62b58..6389383bc92 100644
> --- a/gcc/opts.cc
> +++ b/gcc/opts.cc
> @@ -2187,6 +2187,81 @@ get_closest_sanitizer_option (const string_fragment &arg,
>    return bm.get_best_meaningful_candidate ();
>  }
>
> +unsigned int
> +parse_cf_protection_options (const char *p, location_t loc)
> +{
> +  unsigned int flags = 0;
> +  bool combined = false;
> +  while (*p != 0)
> +    {
> +      size_t len;
> +      const char *comma = strchr (p, ',');
> +      if (comma == NULL)
> +       len = strlen (p);
> +      else
> +       {
> +         combined = true;
> +         len = comma - p;
> +       }
> +
> +      if (len == 0)
> +       {
> +         p = comma + 1;
> +         continue;
> +       }
> +
> +      switch (len)
> +       {
> +       case 4:
> +         if (!memcmp (p, "full", 4))
> +           {
> +             if (combined && flags != CF_NONE)
> +               warning_at (loc, 0, "better to use %<-fcf-protection=%> "
> +                           "option: full alone instead of in a combination");
> +             flags |= CF_FULL;
> +           }
> +         else if (!memcmp (p, "none", 4))
> +           {
> +             if (combined && flags != CF_NONE)
> +               warning_at (loc, 0, "combination of %<-fcf-protection=%> "
> +                           "option: none is ignored");
> +           }
> +         else
> +           error_at (loc, "unrecognized argument to %<-fcf-protection=%> "
> +                     "option: %.4s", p);
> +         break;
> +       case 5:
> +         if (!memcmp (p, "check", 5))
> +           {
> +             if (combined && flags != CF_CHECK)
> +               error_at (loc, "Combination for %<-fcf-protection=%> "
> +                         "option: check is not valid");
> +             flags |= CF_CHECK;
> +           }
> +         else
> +           error_at (loc, "unrecognized argument to %<-fcf-protection=%> "
> +                     "option: %.5s", p);
> +         break;
> +       case 6:
> +         if (!memcmp (p, "branch", 6))
> +           flags |= CF_BRANCH;
> +         else if (!memcmp (p, "return", 6))
> +           flags |= CF_RETURN;
> +         else
> +           error_at (loc, "unrecognized argument to %<-fcf-protection=%> "
> +                     "option: %.6s", p);
> +         break;
> +       default:
> +         error_at (loc, "unrecognized argument to %<-fcf-protection=%> "
> +                   "option: %.*s", (int) len, p);
> +       }
> +
> +      if (comma == NULL)
> +       break;
> +      p = comma + 1;
> +    }
> +  return flags;
> +}
>  /* Parse comma separated sanitizer suboptions from P for option SCODE,
>     adjust previous FLAGS and return new ones.  If COMPLAIN is false,
>     don't issue diagnostics.  */
> @@ -2671,6 +2746,10 @@ common_handle_option (struct gcc_options *opts,
>      case OPT__completion_:
>        break;
>
> +    case OPT_fcf_protection_:
> +      opts->x_flag_cf_protection
> +       = parse_cf_protection_options (arg, loc);
> +      break;
>      case OPT_fsanitize_:
>        opts_set->x_flag_sanitize = true;
>        opts->x_flag_sanitize
> diff --git a/gcc/opts.h b/gcc/opts.h
> index 9959a440ca1..00d396d95f8 100644
> --- a/gcc/opts.h
> +++ b/gcc/opts.h
> @@ -425,6 +425,7 @@ extern void control_warning_option (unsigned int opt_index, int kind,
>  extern char *write_langs (unsigned int mask);
>  extern void print_ignored_options (void);
>  extern void handle_common_deferred_options (void);
> +extern unsigned int parse_cf_protection_options (const char*, location_t);
>  unsigned int parse_sanitizer_options (const char *, location_t, int,
>                                       unsigned int, int, bool);
>
> diff --git a/gcc/testsuite/c-c++-common/fcf-protection-10.c b/gcc/testsuite/c-c++-common/fcf-protection-10.c
> new file mode 100644
> index 00000000000..8692a08374b
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fcf-protection-10.c
> @@ -0,0 +1,3 @@
> +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> +/* { dg-options "-fcf-protection=branch,check" } */
> +/* { dg-error "Combination for '-fcf-protection=' option: check is not valid" "" { target *-*-* } 0 } */
> diff --git a/gcc/testsuite/c-c++-common/fcf-protection-11.c b/gcc/testsuite/c-c++-common/fcf-protection-11.c
> new file mode 100644
> index 00000000000..2e566350ccd
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fcf-protection-11.c
> @@ -0,0 +1,2 @@
> +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> +/* { dg-options "-fcf-protection=branch,return" } */
> diff --git a/gcc/testsuite/c-c++-common/fcf-protection-12.c b/gcc/testsuite/c-c++-common/fcf-protection-12.c
> new file mode 100644
> index 00000000000..b39c2f8e25d
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fcf-protection-12.c
> @@ -0,0 +1,2 @@
> +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> +/* { dg-options "-fcf-protection=return,branch" } */
> diff --git a/gcc/testsuite/c-c++-common/fcf-protection-8.c b/gcc/testsuite/c-c++-common/fcf-protection-8.c
> new file mode 100644
> index 00000000000..33e46223b6b
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fcf-protection-8.c
> @@ -0,0 +1,3 @@
> +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> +/* { dg-options "-fcf-protection=branch,none" } */
> +/* { dg-warning "combination of '-fcf-protection=' option: none is ignored" "" { target { *-*-* } } 0 } */
> diff --git a/gcc/testsuite/c-c++-common/fcf-protection-9.c b/gcc/testsuite/c-c++-common/fcf-protection-9.c
> new file mode 100644
> index 00000000000..7848fe4b959
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fcf-protection-9.c
> @@ -0,0 +1,3 @@
> +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> +/* { dg-options "-fcf-protection=branch,full" } */
> +/* { dg-warning "better to use '-fcf-protection=' option: full alone instead of in a combination" "" { target { *-*-* } } 0 } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr89701-1.c b/gcc/testsuite/gcc.target/i386/pr89701-1.c
> new file mode 100644
> index 00000000000..1879c9ab4d8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr89701-1.c
> @@ -0,0 +1,4 @@
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-fcf-protection=branch,return" } */
> +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
> +/* { dg-final { scan-assembler-times ".long    0x3" 1 } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr89701-2.c b/gcc/testsuite/gcc.target/i386/pr89701-2.c
> new file mode 100644
> index 00000000000..d5100575028
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr89701-2.c
> @@ -0,0 +1,4 @@
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-fcf-protection=return,branch" } */
> +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
> +/* { dg-final { scan-assembler-times ".long    0x3" 1 } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr89701-3.c b/gcc/testsuite/gcc.target/i386/pr89701-3.c
> new file mode 100644
> index 00000000000..1505051a2bb
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr89701-3.c
> @@ -0,0 +1,5 @@
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-fcf-protection=return,none" } */
> +/* { dg-warning "combination of '-fcf-protection=' option: none is ignored" "" { target { *-*-linux* } } 0 } */
> +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
> +/* { dg-final { scan-assembler-times ".long    0x2" 1 } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr89701-4.c b/gcc/testsuite/gcc.target/i386/pr89701-4.c
> new file mode 100644
> index 00000000000..242b8810abb
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr89701-4.c
> @@ -0,0 +1,5 @@
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-fcf-protection=branch,none" } */
> +/* { dg-warning "combination of '-fcf-protection=' option: none is ignored" "" { target { *-*-* } } 0 } */
> +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
> +/* { dg-final { scan-assembler-times ".long    0x1" 1 } } */
> --
> 2.39.1.388.g2fc9e9ca3c
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Provide -fcf-protection=branch,return.
  2023-05-12  5:49 ` Andrew Pinski
@ 2023-05-12  6:21   ` Hongtao Liu
  2023-05-13  9:20     ` [PATCH V2] " liuhongt
  0 siblings, 1 reply; 7+ messages in thread
From: Hongtao Liu @ 2023-05-12  6:21 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: liuhongt, gcc-patches, hjl.tools

On Fri, May 12, 2023 at 1:50 PM Andrew Pinski <pinskia@gmail.com> wrote:
>
> On Thu, May 11, 2023 at 10:45 PM liuhongt via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
> > Ok for trunk?
> >
> > gcc/ChangeLog:
> >
> >         PR target/89701
> >         * common.opt: Refactor -fcf-protection= to support combination
> >         of param.
> >         * lto-wrapper.c (merge_and_complain): Adjusted.
> >         * opts.c (parse_cf_protection_options): New.
> >         (common_handle_option): Decode argument for -fcf-protection=.
> >         * opts.h (parse_cf_protection_options): Declare.
>
> I think this could be simplified if you use either EnumSet or
> EnumBitSet instead in common.opt for `-fcf-protection=`.
Thanks, I didn't know that, i'll try to refactor the patch to EnumSet
or EnumBitSet
>
> Thanks,
> Andrew
>
> >
> > gcc/testsuite/ChangeLog:
> >
> >         PR target/89701
> >         * c-c++-common/fcf-protection-8.c: New test.
> >         * c-c++-common/fcf-protection-9.c: New test.
> >         * c-c++-common/fcf-protection-10.c: New test.
> >         * gcc.target/i386/pr89701-1.c: New test.
> >         * gcc.target/i386/pr89701-2.c: New test.
> >         * gcc.target/i386/pr89701-3.c: New test.
> >         * gcc.target/i386/pr89701-4.c: New test.
> > ---
> >  gcc/common.opt                                | 24 ++----
> >  gcc/lto-wrapper.cc                            | 21 +++--
> >  gcc/opts.cc                                   | 79 +++++++++++++++++++
> >  gcc/opts.h                                    |  1 +
> >  .../c-c++-common/fcf-protection-10.c          |  3 +
> >  .../c-c++-common/fcf-protection-11.c          |  2 +
> >  .../c-c++-common/fcf-protection-12.c          |  2 +
> >  gcc/testsuite/c-c++-common/fcf-protection-8.c |  3 +
> >  gcc/testsuite/c-c++-common/fcf-protection-9.c |  3 +
> >  gcc/testsuite/gcc.target/i386/pr89701-1.c     |  4 +
> >  gcc/testsuite/gcc.target/i386/pr89701-2.c     |  4 +
> >  gcc/testsuite/gcc.target/i386/pr89701-3.c     |  5 ++
> >  gcc/testsuite/gcc.target/i386/pr89701-4.c     |  5 ++
> >  13 files changed, 130 insertions(+), 26 deletions(-)
> >  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-10.c
> >  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-11.c
> >  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-12.c
> >  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-8.c
> >  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-9.c
> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-1.c
> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-2.c
> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-3.c
> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-4.c
> >
> > diff --git a/gcc/common.opt b/gcc/common.opt
> > index a28ca13385a..ac12da52733 100644
> > --- a/gcc/common.opt
> > +++ b/gcc/common.opt
> > @@ -229,6 +229,10 @@ bool dump_base_name_prefixed = false
> >  Variable
> >  unsigned int flag_zero_call_used_regs
> >
> > +;; What the CF check should instrument
> > +Variable
> > +unsigned int flag_cf_protection = 0
> > +
> >  ###
> >  Driver
> >
> > @@ -1886,28 +1890,10 @@ fcf-protection
> >  Common RejectNegative Alias(fcf-protection=,full)
> >
> >  fcf-protection=
> > -Common Joined RejectNegative Enum(cf_protection_level) Var(flag_cf_protection) Init(CF_NONE)
> > +Common Joined
> >  -fcf-protection=[full|branch|return|none|check]        Instrument functions with checks to verify jump/call/return control-flow transfer
> >  instructions have valid targets.
> >
> > -Enum
> > -Name(cf_protection_level) Type(enum cf_protection_level) UnknownError(unknown Control-Flow Protection Level %qs)
> > -
> > -EnumValue
> > -Enum(cf_protection_level) String(full) Value(CF_FULL)
> > -
> > -EnumValue
> > -Enum(cf_protection_level) String(branch) Value(CF_BRANCH)
> > -
> > -EnumValue
> > -Enum(cf_protection_level) String(return) Value(CF_RETURN)
> > -
> > -EnumValue
> > -Enum(cf_protection_level) String(check) Value(CF_CHECK)
> > -
> > -EnumValue
> > -Enum(cf_protection_level) String(none) Value(CF_NONE)
> > -
> >  finstrument-functions
> >  Common Var(flag_instrument_function_entry_exit,1)
> >  Instrument function entry and exit with profiling calls.
> > diff --git a/gcc/lto-wrapper.cc b/gcc/lto-wrapper.cc
> > index 5186d040ce0..568c8af659d 100644
> > --- a/gcc/lto-wrapper.cc
> > +++ b/gcc/lto-wrapper.cc
> > @@ -359,26 +359,33 @@ merge_and_complain (vec<cl_decoded_option> &decoded_options,
> >         case OPT_fcf_protection_:
> >           /* Default to link-time option, else append or check identical.  */
> >           if (!cf_protection_option
> > -             || cf_protection_option->value == CF_CHECK)
> > +             || !memcmp (cf_protection_option->arg, "check", 5))
> >             {
> > +             const char* parg = decoded_options[existing_opt].arg;
> >               if (existing_opt == -1)
> >                 decoded_options.safe_push (*foption);
> > -             else if (decoded_options[existing_opt].value != foption->value)
> > +             else if ((strlen (parg) != strlen (foption->arg))
> > +                      || memcmp (parg, foption->arg, strlen (foption->arg)))
> >                 {
> >                   if (cf_protection_option
> > -                     && cf_protection_option->value == CF_CHECK)
> > +                     && !memcmp (cf_protection_option->arg, "check", 5))
> >                     fatal_error (input_location,
> >                                  "option %qs with mismatching values"
> >                                  " (%s, %s)",
> >                                  "-fcf-protection",
> > -                                decoded_options[existing_opt].arg,
> > +                                parg,
> >                                  foption->arg);
> >                   else
> >                     {
> >                       /* Merge and update the -fcf-protection option.  */
> > -                     decoded_options[existing_opt].value
> > -                       &= (foption->value & CF_FULL);
> > -                     switch (decoded_options[existing_opt].value)
> > +                     HOST_WIDE_INT flags1
> > +                       = parse_cf_protection_options (foption->arg,
> > +                                                      input_location);
> > +                     HOST_WIDE_INT flags2
> > +                       = parse_cf_protection_options (parg,
> > +                                                      input_location);
> > +                     flags2 &= (flags1 & CF_FULL);
> > +                     switch (flags2)
> >                         {
> >                         case CF_NONE:
> >                           decoded_options[existing_opt].arg = "none";
> > diff --git a/gcc/opts.cc b/gcc/opts.cc
> > index 86b94d62b58..6389383bc92 100644
> > --- a/gcc/opts.cc
> > +++ b/gcc/opts.cc
> > @@ -2187,6 +2187,81 @@ get_closest_sanitizer_option (const string_fragment &arg,
> >    return bm.get_best_meaningful_candidate ();
> >  }
> >
> > +unsigned int
> > +parse_cf_protection_options (const char *p, location_t loc)
> > +{
> > +  unsigned int flags = 0;
> > +  bool combined = false;
> > +  while (*p != 0)
> > +    {
> > +      size_t len;
> > +      const char *comma = strchr (p, ',');
> > +      if (comma == NULL)
> > +       len = strlen (p);
> > +      else
> > +       {
> > +         combined = true;
> > +         len = comma - p;
> > +       }
> > +
> > +      if (len == 0)
> > +       {
> > +         p = comma + 1;
> > +         continue;
> > +       }
> > +
> > +      switch (len)
> > +       {
> > +       case 4:
> > +         if (!memcmp (p, "full", 4))
> > +           {
> > +             if (combined && flags != CF_NONE)
> > +               warning_at (loc, 0, "better to use %<-fcf-protection=%> "
> > +                           "option: full alone instead of in a combination");
> > +             flags |= CF_FULL;
> > +           }
> > +         else if (!memcmp (p, "none", 4))
> > +           {
> > +             if (combined && flags != CF_NONE)
> > +               warning_at (loc, 0, "combination of %<-fcf-protection=%> "
> > +                           "option: none is ignored");
> > +           }
> > +         else
> > +           error_at (loc, "unrecognized argument to %<-fcf-protection=%> "
> > +                     "option: %.4s", p);
> > +         break;
> > +       case 5:
> > +         if (!memcmp (p, "check", 5))
> > +           {
> > +             if (combined && flags != CF_CHECK)
> > +               error_at (loc, "Combination for %<-fcf-protection=%> "
> > +                         "option: check is not valid");
> > +             flags |= CF_CHECK;
> > +           }
> > +         else
> > +           error_at (loc, "unrecognized argument to %<-fcf-protection=%> "
> > +                     "option: %.5s", p);
> > +         break;
> > +       case 6:
> > +         if (!memcmp (p, "branch", 6))
> > +           flags |= CF_BRANCH;
> > +         else if (!memcmp (p, "return", 6))
> > +           flags |= CF_RETURN;
> > +         else
> > +           error_at (loc, "unrecognized argument to %<-fcf-protection=%> "
> > +                     "option: %.6s", p);
> > +         break;
> > +       default:
> > +         error_at (loc, "unrecognized argument to %<-fcf-protection=%> "
> > +                   "option: %.*s", (int) len, p);
> > +       }
> > +
> > +      if (comma == NULL)
> > +       break;
> > +      p = comma + 1;
> > +    }
> > +  return flags;
> > +}
> >  /* Parse comma separated sanitizer suboptions from P for option SCODE,
> >     adjust previous FLAGS and return new ones.  If COMPLAIN is false,
> >     don't issue diagnostics.  */
> > @@ -2671,6 +2746,10 @@ common_handle_option (struct gcc_options *opts,
> >      case OPT__completion_:
> >        break;
> >
> > +    case OPT_fcf_protection_:
> > +      opts->x_flag_cf_protection
> > +       = parse_cf_protection_options (arg, loc);
> > +      break;
> >      case OPT_fsanitize_:
> >        opts_set->x_flag_sanitize = true;
> >        opts->x_flag_sanitize
> > diff --git a/gcc/opts.h b/gcc/opts.h
> > index 9959a440ca1..00d396d95f8 100644
> > --- a/gcc/opts.h
> > +++ b/gcc/opts.h
> > @@ -425,6 +425,7 @@ extern void control_warning_option (unsigned int opt_index, int kind,
> >  extern char *write_langs (unsigned int mask);
> >  extern void print_ignored_options (void);
> >  extern void handle_common_deferred_options (void);
> > +extern unsigned int parse_cf_protection_options (const char*, location_t);
> >  unsigned int parse_sanitizer_options (const char *, location_t, int,
> >                                       unsigned int, int, bool);
> >
> > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-10.c b/gcc/testsuite/c-c++-common/fcf-protection-10.c
> > new file mode 100644
> > index 00000000000..8692a08374b
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/fcf-protection-10.c
> > @@ -0,0 +1,3 @@
> > +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> > +/* { dg-options "-fcf-protection=branch,check" } */
> > +/* { dg-error "Combination for '-fcf-protection=' option: check is not valid" "" { target *-*-* } 0 } */
> > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-11.c b/gcc/testsuite/c-c++-common/fcf-protection-11.c
> > new file mode 100644
> > index 00000000000..2e566350ccd
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/fcf-protection-11.c
> > @@ -0,0 +1,2 @@
> > +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> > +/* { dg-options "-fcf-protection=branch,return" } */
> > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-12.c b/gcc/testsuite/c-c++-common/fcf-protection-12.c
> > new file mode 100644
> > index 00000000000..b39c2f8e25d
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/fcf-protection-12.c
> > @@ -0,0 +1,2 @@
> > +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> > +/* { dg-options "-fcf-protection=return,branch" } */
> > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-8.c b/gcc/testsuite/c-c++-common/fcf-protection-8.c
> > new file mode 100644
> > index 00000000000..33e46223b6b
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/fcf-protection-8.c
> > @@ -0,0 +1,3 @@
> > +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> > +/* { dg-options "-fcf-protection=branch,none" } */
> > +/* { dg-warning "combination of '-fcf-protection=' option: none is ignored" "" { target { *-*-* } } 0 } */
> > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-9.c b/gcc/testsuite/c-c++-common/fcf-protection-9.c
> > new file mode 100644
> > index 00000000000..7848fe4b959
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/fcf-protection-9.c
> > @@ -0,0 +1,3 @@
> > +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> > +/* { dg-options "-fcf-protection=branch,full" } */
> > +/* { dg-warning "better to use '-fcf-protection=' option: full alone instead of in a combination" "" { target { *-*-* } } 0 } */
> > diff --git a/gcc/testsuite/gcc.target/i386/pr89701-1.c b/gcc/testsuite/gcc.target/i386/pr89701-1.c
> > new file mode 100644
> > index 00000000000..1879c9ab4d8
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr89701-1.c
> > @@ -0,0 +1,4 @@
> > +/* { dg-do compile { target *-*-linux* } } */
> > +/* { dg-options "-fcf-protection=branch,return" } */
> > +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
> > +/* { dg-final { scan-assembler-times ".long    0x3" 1 } } */
> > diff --git a/gcc/testsuite/gcc.target/i386/pr89701-2.c b/gcc/testsuite/gcc.target/i386/pr89701-2.c
> > new file mode 100644
> > index 00000000000..d5100575028
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr89701-2.c
> > @@ -0,0 +1,4 @@
> > +/* { dg-do compile { target *-*-linux* } } */
> > +/* { dg-options "-fcf-protection=return,branch" } */
> > +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
> > +/* { dg-final { scan-assembler-times ".long    0x3" 1 } } */
> > diff --git a/gcc/testsuite/gcc.target/i386/pr89701-3.c b/gcc/testsuite/gcc.target/i386/pr89701-3.c
> > new file mode 100644
> > index 00000000000..1505051a2bb
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr89701-3.c
> > @@ -0,0 +1,5 @@
> > +/* { dg-do compile { target *-*-linux* } } */
> > +/* { dg-options "-fcf-protection=return,none" } */
> > +/* { dg-warning "combination of '-fcf-protection=' option: none is ignored" "" { target { *-*-linux* } } 0 } */
> > +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
> > +/* { dg-final { scan-assembler-times ".long    0x2" 1 } } */
> > diff --git a/gcc/testsuite/gcc.target/i386/pr89701-4.c b/gcc/testsuite/gcc.target/i386/pr89701-4.c
> > new file mode 100644
> > index 00000000000..242b8810abb
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr89701-4.c
> > @@ -0,0 +1,5 @@
> > +/* { dg-do compile { target *-*-linux* } } */
> > +/* { dg-options "-fcf-protection=branch,none" } */
> > +/* { dg-warning "combination of '-fcf-protection=' option: none is ignored" "" { target { *-*-* } } 0 } */
> > +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
> > +/* { dg-final { scan-assembler-times ".long    0x1" 1 } } */
> > --
> > 2.39.1.388.g2fc9e9ca3c
> >



-- 
BR,
Hongtao

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH V2] Provide -fcf-protection=branch,return.
  2023-05-12  6:21   ` Hongtao Liu
@ 2023-05-13  9:20     ` liuhongt
  2023-05-22  8:08       ` Hongtao Liu
  0 siblings, 1 reply; 7+ messages in thread
From: liuhongt @ 2023-05-13  9:20 UTC (permalink / raw)
  To: gcc-patches; +Cc: crazylht, hjl.tools

> I think this could be simplified if you use either EnumSet or
> EnumBitSet instead in common.opt for `-fcf-protection=`.

Use EnumSet instead of EnumBitSet since CF_FULL is not power of 2.
It is a bit tricky for sets classification, cf_branch and cf_return
should be in different sets, but they both "conflicts" cf_full,
cf_none. And current EnumSet don't handle this well.

So in the current implementation, only cf_full,cf_none are exclusive
to each other, but they can be combined with any cf_branch, cf_return,
cf_check. It's not perfect, but still an improvement than original
one.

gcc/ChangeLog:

	* common.opt: (fcf-protection=): Add EnumSet attribute to
	support combination of params.

gcc/testsuite/ChangeLog:

	* c-c++-common/fcf-protection-10.c: New test.
	* c-c++-common/fcf-protection-11.c: New test.
	* c-c++-common/fcf-protection-12.c: New test.
	* c-c++-common/fcf-protection-8.c: New test.
	* c-c++-common/fcf-protection-9.c: New test.
	* gcc.target/i386/pr89701-1.c: New test.
	* gcc.target/i386/pr89701-2.c: New test.
	* gcc.target/i386/pr89701-3.c: New test.
---
 gcc/common.opt                                 | 12 ++++++------
 gcc/testsuite/c-c++-common/fcf-protection-10.c |  2 ++
 gcc/testsuite/c-c++-common/fcf-protection-11.c |  2 ++
 gcc/testsuite/c-c++-common/fcf-protection-12.c |  2 ++
 gcc/testsuite/c-c++-common/fcf-protection-8.c  |  2 ++
 gcc/testsuite/c-c++-common/fcf-protection-9.c  |  2 ++
 gcc/testsuite/gcc.target/i386/pr89701-1.c      |  4 ++++
 gcc/testsuite/gcc.target/i386/pr89701-2.c      |  4 ++++
 gcc/testsuite/gcc.target/i386/pr89701-3.c      |  4 ++++
 9 files changed, 28 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-10.c
 create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-11.c
 create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-12.c
 create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-8.c
 create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-9.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-3.c

diff --git a/gcc/common.opt b/gcc/common.opt
index a28ca13385a..02f2472959a 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1886,7 +1886,7 @@ fcf-protection
 Common RejectNegative Alias(fcf-protection=,full)
 
 fcf-protection=
-Common Joined RejectNegative Enum(cf_protection_level) Var(flag_cf_protection) Init(CF_NONE)
+Common Joined RejectNegative Enum(cf_protection_level) EnumSet Var(flag_cf_protection) Init(CF_NONE)
 -fcf-protection=[full|branch|return|none|check]	Instrument functions with checks to verify jump/call/return control-flow transfer
 instructions have valid targets.
 
@@ -1894,19 +1894,19 @@ Enum
 Name(cf_protection_level) Type(enum cf_protection_level) UnknownError(unknown Control-Flow Protection Level %qs)
 
 EnumValue
-Enum(cf_protection_level) String(full) Value(CF_FULL)
+Enum(cf_protection_level) String(full) Value(CF_FULL) Set(1)
 
 EnumValue
-Enum(cf_protection_level) String(branch) Value(CF_BRANCH)
+Enum(cf_protection_level) String(branch) Value(CF_BRANCH) Set(2)
 
 EnumValue
-Enum(cf_protection_level) String(return) Value(CF_RETURN)
+Enum(cf_protection_level) String(return) Value(CF_RETURN) Set(3)
 
 EnumValue
-Enum(cf_protection_level) String(check) Value(CF_CHECK)
+Enum(cf_protection_level) String(check) Value(CF_CHECK) Set(4)
 
 EnumValue
-Enum(cf_protection_level) String(none) Value(CF_NONE)
+Enum(cf_protection_level) String(none) Value(CF_NONE) Set(1)
 
 finstrument-functions
 Common Var(flag_instrument_function_entry_exit,1)
diff --git a/gcc/testsuite/c-c++-common/fcf-protection-10.c b/gcc/testsuite/c-c++-common/fcf-protection-10.c
new file mode 100644
index 00000000000..b271d134e52
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fcf-protection-10.c
@@ -0,0 +1,2 @@
+/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
+/* { dg-options "-fcf-protection=branch,check" } */
diff --git a/gcc/testsuite/c-c++-common/fcf-protection-11.c b/gcc/testsuite/c-c++-common/fcf-protection-11.c
new file mode 100644
index 00000000000..2e566350ccd
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fcf-protection-11.c
@@ -0,0 +1,2 @@
+/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
+/* { dg-options "-fcf-protection=branch,return" } */
diff --git a/gcc/testsuite/c-c++-common/fcf-protection-12.c b/gcc/testsuite/c-c++-common/fcf-protection-12.c
new file mode 100644
index 00000000000..b39c2f8e25d
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fcf-protection-12.c
@@ -0,0 +1,2 @@
+/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
+/* { dg-options "-fcf-protection=return,branch" } */
diff --git a/gcc/testsuite/c-c++-common/fcf-protection-8.c b/gcc/testsuite/c-c++-common/fcf-protection-8.c
new file mode 100644
index 00000000000..3b97095a92c
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fcf-protection-8.c
@@ -0,0 +1,2 @@
+/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
+/* { dg-options "-fcf-protection=branch,none" } */
diff --git a/gcc/testsuite/c-c++-common/fcf-protection-9.c b/gcc/testsuite/c-c++-common/fcf-protection-9.c
new file mode 100644
index 00000000000..6a37e749fcb
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fcf-protection-9.c
@@ -0,0 +1,2 @@
+/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
+/* { dg-options "-fcf-protection=branch,full" } */
diff --git a/gcc/testsuite/gcc.target/i386/pr89701-1.c b/gcc/testsuite/gcc.target/i386/pr89701-1.c
new file mode 100644
index 00000000000..1879c9ab4d8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr89701-1.c
@@ -0,0 +1,4 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-fcf-protection=branch,return" } */
+/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
+/* { dg-final { scan-assembler-times ".long	0x3" 1 } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr89701-2.c b/gcc/testsuite/gcc.target/i386/pr89701-2.c
new file mode 100644
index 00000000000..d5100575028
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr89701-2.c
@@ -0,0 +1,4 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-fcf-protection=return,branch" } */
+/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
+/* { dg-final { scan-assembler-times ".long	0x3" 1 } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr89701-3.c b/gcc/testsuite/gcc.target/i386/pr89701-3.c
new file mode 100644
index 00000000000..88afb546fbf
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr89701-3.c
@@ -0,0 +1,4 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-fcf-protection=return,none" } */
+/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
+/* { dg-final { scan-assembler-times ".long	0x2" 1 } } */
-- 
2.39.1.388.g2fc9e9ca3c


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] Provide -fcf-protection=branch,return.
  2023-05-13  9:20     ` [PATCH V2] " liuhongt
@ 2023-05-22  8:08       ` Hongtao Liu
  2023-07-12  7:27         ` Hongtao Liu
  0 siblings, 1 reply; 7+ messages in thread
From: Hongtao Liu @ 2023-05-22  8:08 UTC (permalink / raw)
  To: liuhongt; +Cc: gcc-patches, hjl.tools

ping.

On Sat, May 13, 2023 at 5:20 PM liuhongt <hongtao.liu@intel.com> wrote:
>
> > I think this could be simplified if you use either EnumSet or
> > EnumBitSet instead in common.opt for `-fcf-protection=`.
>
> Use EnumSet instead of EnumBitSet since CF_FULL is not power of 2.
> It is a bit tricky for sets classification, cf_branch and cf_return
> should be in different sets, but they both "conflicts" cf_full,
> cf_none. And current EnumSet don't handle this well.
>
> So in the current implementation, only cf_full,cf_none are exclusive
> to each other, but they can be combined with any cf_branch, cf_return,
> cf_check. It's not perfect, but still an improvement than original
> one.
>
> gcc/ChangeLog:
>
>         * common.opt: (fcf-protection=): Add EnumSet attribute to
>         support combination of params.
>
> gcc/testsuite/ChangeLog:
>
>         * c-c++-common/fcf-protection-10.c: New test.
>         * c-c++-common/fcf-protection-11.c: New test.
>         * c-c++-common/fcf-protection-12.c: New test.
>         * c-c++-common/fcf-protection-8.c: New test.
>         * c-c++-common/fcf-protection-9.c: New test.
>         * gcc.target/i386/pr89701-1.c: New test.
>         * gcc.target/i386/pr89701-2.c: New test.
>         * gcc.target/i386/pr89701-3.c: New test.
> ---
>  gcc/common.opt                                 | 12 ++++++------
>  gcc/testsuite/c-c++-common/fcf-protection-10.c |  2 ++
>  gcc/testsuite/c-c++-common/fcf-protection-11.c |  2 ++
>  gcc/testsuite/c-c++-common/fcf-protection-12.c |  2 ++
>  gcc/testsuite/c-c++-common/fcf-protection-8.c  |  2 ++
>  gcc/testsuite/c-c++-common/fcf-protection-9.c  |  2 ++
>  gcc/testsuite/gcc.target/i386/pr89701-1.c      |  4 ++++
>  gcc/testsuite/gcc.target/i386/pr89701-2.c      |  4 ++++
>  gcc/testsuite/gcc.target/i386/pr89701-3.c      |  4 ++++
>  9 files changed, 28 insertions(+), 6 deletions(-)
>  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-10.c
>  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-11.c
>  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-12.c
>  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-8.c
>  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-9.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-1.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-2.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-3.c
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index a28ca13385a..02f2472959a 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -1886,7 +1886,7 @@ fcf-protection
>  Common RejectNegative Alias(fcf-protection=,full)
>
>  fcf-protection=
> -Common Joined RejectNegative Enum(cf_protection_level) Var(flag_cf_protection) Init(CF_NONE)
> +Common Joined RejectNegative Enum(cf_protection_level) EnumSet Var(flag_cf_protection) Init(CF_NONE)
>  -fcf-protection=[full|branch|return|none|check]        Instrument functions with checks to verify jump/call/return control-flow transfer
>  instructions have valid targets.
>
> @@ -1894,19 +1894,19 @@ Enum
>  Name(cf_protection_level) Type(enum cf_protection_level) UnknownError(unknown Control-Flow Protection Level %qs)
>
>  EnumValue
> -Enum(cf_protection_level) String(full) Value(CF_FULL)
> +Enum(cf_protection_level) String(full) Value(CF_FULL) Set(1)
>
>  EnumValue
> -Enum(cf_protection_level) String(branch) Value(CF_BRANCH)
> +Enum(cf_protection_level) String(branch) Value(CF_BRANCH) Set(2)
>
>  EnumValue
> -Enum(cf_protection_level) String(return) Value(CF_RETURN)
> +Enum(cf_protection_level) String(return) Value(CF_RETURN) Set(3)
>
>  EnumValue
> -Enum(cf_protection_level) String(check) Value(CF_CHECK)
> +Enum(cf_protection_level) String(check) Value(CF_CHECK) Set(4)
>
>  EnumValue
> -Enum(cf_protection_level) String(none) Value(CF_NONE)
> +Enum(cf_protection_level) String(none) Value(CF_NONE) Set(1)
>
>  finstrument-functions
>  Common Var(flag_instrument_function_entry_exit,1)
> diff --git a/gcc/testsuite/c-c++-common/fcf-protection-10.c b/gcc/testsuite/c-c++-common/fcf-protection-10.c
> new file mode 100644
> index 00000000000..b271d134e52
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fcf-protection-10.c
> @@ -0,0 +1,2 @@
> +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> +/* { dg-options "-fcf-protection=branch,check" } */
> diff --git a/gcc/testsuite/c-c++-common/fcf-protection-11.c b/gcc/testsuite/c-c++-common/fcf-protection-11.c
> new file mode 100644
> index 00000000000..2e566350ccd
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fcf-protection-11.c
> @@ -0,0 +1,2 @@
> +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> +/* { dg-options "-fcf-protection=branch,return" } */
> diff --git a/gcc/testsuite/c-c++-common/fcf-protection-12.c b/gcc/testsuite/c-c++-common/fcf-protection-12.c
> new file mode 100644
> index 00000000000..b39c2f8e25d
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fcf-protection-12.c
> @@ -0,0 +1,2 @@
> +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> +/* { dg-options "-fcf-protection=return,branch" } */
> diff --git a/gcc/testsuite/c-c++-common/fcf-protection-8.c b/gcc/testsuite/c-c++-common/fcf-protection-8.c
> new file mode 100644
> index 00000000000..3b97095a92c
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fcf-protection-8.c
> @@ -0,0 +1,2 @@
> +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> +/* { dg-options "-fcf-protection=branch,none" } */
> diff --git a/gcc/testsuite/c-c++-common/fcf-protection-9.c b/gcc/testsuite/c-c++-common/fcf-protection-9.c
> new file mode 100644
> index 00000000000..6a37e749fcb
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fcf-protection-9.c
> @@ -0,0 +1,2 @@
> +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> +/* { dg-options "-fcf-protection=branch,full" } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr89701-1.c b/gcc/testsuite/gcc.target/i386/pr89701-1.c
> new file mode 100644
> index 00000000000..1879c9ab4d8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr89701-1.c
> @@ -0,0 +1,4 @@
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-fcf-protection=branch,return" } */
> +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
> +/* { dg-final { scan-assembler-times ".long    0x3" 1 } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr89701-2.c b/gcc/testsuite/gcc.target/i386/pr89701-2.c
> new file mode 100644
> index 00000000000..d5100575028
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr89701-2.c
> @@ -0,0 +1,4 @@
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-fcf-protection=return,branch" } */
> +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
> +/* { dg-final { scan-assembler-times ".long    0x3" 1 } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr89701-3.c b/gcc/testsuite/gcc.target/i386/pr89701-3.c
> new file mode 100644
> index 00000000000..88afb546fbf
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr89701-3.c
> @@ -0,0 +1,4 @@
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-fcf-protection=return,none" } */
> +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
> +/* { dg-final { scan-assembler-times ".long    0x2" 1 } } */
> --
> 2.39.1.388.g2fc9e9ca3c
>


-- 
BR,
Hongtao

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] Provide -fcf-protection=branch,return.
  2023-05-22  8:08       ` Hongtao Liu
@ 2023-07-12  7:27         ` Hongtao Liu
  2023-07-19  8:37           ` Hongtao Liu
  0 siblings, 1 reply; 7+ messages in thread
From: Hongtao Liu @ 2023-07-12  7:27 UTC (permalink / raw)
  To: liuhongt; +Cc: gcc-patches, hjl.tools

ping.

On Mon, May 22, 2023 at 4:08 PM Hongtao Liu <crazylht@gmail.com> wrote:
>
> ping.
>
> On Sat, May 13, 2023 at 5:20 PM liuhongt <hongtao.liu@intel.com> wrote:
> >
> > > I think this could be simplified if you use either EnumSet or
> > > EnumBitSet instead in common.opt for `-fcf-protection=`.
> >
> > Use EnumSet instead of EnumBitSet since CF_FULL is not power of 2.
> > It is a bit tricky for sets classification, cf_branch and cf_return
> > should be in different sets, but they both "conflicts" cf_full,
> > cf_none. And current EnumSet don't handle this well.
> >
> > So in the current implementation, only cf_full,cf_none are exclusive
> > to each other, but they can be combined with any cf_branch, cf_return,
> > cf_check. It's not perfect, but still an improvement than original
> > one.
> >
> > gcc/ChangeLog:
> >
> >         * common.opt: (fcf-protection=): Add EnumSet attribute to
> >         support combination of params.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * c-c++-common/fcf-protection-10.c: New test.
> >         * c-c++-common/fcf-protection-11.c: New test.
> >         * c-c++-common/fcf-protection-12.c: New test.
> >         * c-c++-common/fcf-protection-8.c: New test.
> >         * c-c++-common/fcf-protection-9.c: New test.
> >         * gcc.target/i386/pr89701-1.c: New test.
> >         * gcc.target/i386/pr89701-2.c: New test.
> >         * gcc.target/i386/pr89701-3.c: New test.
> > ---
> >  gcc/common.opt                                 | 12 ++++++------
> >  gcc/testsuite/c-c++-common/fcf-protection-10.c |  2 ++
> >  gcc/testsuite/c-c++-common/fcf-protection-11.c |  2 ++
> >  gcc/testsuite/c-c++-common/fcf-protection-12.c |  2 ++
> >  gcc/testsuite/c-c++-common/fcf-protection-8.c  |  2 ++
> >  gcc/testsuite/c-c++-common/fcf-protection-9.c  |  2 ++
> >  gcc/testsuite/gcc.target/i386/pr89701-1.c      |  4 ++++
> >  gcc/testsuite/gcc.target/i386/pr89701-2.c      |  4 ++++
> >  gcc/testsuite/gcc.target/i386/pr89701-3.c      |  4 ++++
> >  9 files changed, 28 insertions(+), 6 deletions(-)
> >  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-10.c
> >  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-11.c
> >  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-12.c
> >  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-8.c
> >  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-9.c
> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-1.c
> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-2.c
> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-3.c
> >
> > diff --git a/gcc/common.opt b/gcc/common.opt
> > index a28ca13385a..02f2472959a 100644
> > --- a/gcc/common.opt
> > +++ b/gcc/common.opt
> > @@ -1886,7 +1886,7 @@ fcf-protection
> >  Common RejectNegative Alias(fcf-protection=,full)
> >
> >  fcf-protection=
> > -Common Joined RejectNegative Enum(cf_protection_level) Var(flag_cf_protection) Init(CF_NONE)
> > +Common Joined RejectNegative Enum(cf_protection_level) EnumSet Var(flag_cf_protection) Init(CF_NONE)
> >  -fcf-protection=[full|branch|return|none|check]        Instrument functions with checks to verify jump/call/return control-flow transfer
> >  instructions have valid targets.
> >
> > @@ -1894,19 +1894,19 @@ Enum
> >  Name(cf_protection_level) Type(enum cf_protection_level) UnknownError(unknown Control-Flow Protection Level %qs)
> >
> >  EnumValue
> > -Enum(cf_protection_level) String(full) Value(CF_FULL)
> > +Enum(cf_protection_level) String(full) Value(CF_FULL) Set(1)
> >
> >  EnumValue
> > -Enum(cf_protection_level) String(branch) Value(CF_BRANCH)
> > +Enum(cf_protection_level) String(branch) Value(CF_BRANCH) Set(2)
> >
> >  EnumValue
> > -Enum(cf_protection_level) String(return) Value(CF_RETURN)
> > +Enum(cf_protection_level) String(return) Value(CF_RETURN) Set(3)
> >
> >  EnumValue
> > -Enum(cf_protection_level) String(check) Value(CF_CHECK)
> > +Enum(cf_protection_level) String(check) Value(CF_CHECK) Set(4)
> >
> >  EnumValue
> > -Enum(cf_protection_level) String(none) Value(CF_NONE)
> > +Enum(cf_protection_level) String(none) Value(CF_NONE) Set(1)
> >
> >  finstrument-functions
> >  Common Var(flag_instrument_function_entry_exit,1)
> > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-10.c b/gcc/testsuite/c-c++-common/fcf-protection-10.c
> > new file mode 100644
> > index 00000000000..b271d134e52
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/fcf-protection-10.c
> > @@ -0,0 +1,2 @@
> > +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> > +/* { dg-options "-fcf-protection=branch,check" } */
> > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-11.c b/gcc/testsuite/c-c++-common/fcf-protection-11.c
> > new file mode 100644
> > index 00000000000..2e566350ccd
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/fcf-protection-11.c
> > @@ -0,0 +1,2 @@
> > +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> > +/* { dg-options "-fcf-protection=branch,return" } */
> > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-12.c b/gcc/testsuite/c-c++-common/fcf-protection-12.c
> > new file mode 100644
> > index 00000000000..b39c2f8e25d
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/fcf-protection-12.c
> > @@ -0,0 +1,2 @@
> > +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> > +/* { dg-options "-fcf-protection=return,branch" } */
> > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-8.c b/gcc/testsuite/c-c++-common/fcf-protection-8.c
> > new file mode 100644
> > index 00000000000..3b97095a92c
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/fcf-protection-8.c
> > @@ -0,0 +1,2 @@
> > +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> > +/* { dg-options "-fcf-protection=branch,none" } */
> > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-9.c b/gcc/testsuite/c-c++-common/fcf-protection-9.c
> > new file mode 100644
> > index 00000000000..6a37e749fcb
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/fcf-protection-9.c
> > @@ -0,0 +1,2 @@
> > +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> > +/* { dg-options "-fcf-protection=branch,full" } */
> > diff --git a/gcc/testsuite/gcc.target/i386/pr89701-1.c b/gcc/testsuite/gcc.target/i386/pr89701-1.c
> > new file mode 100644
> > index 00000000000..1879c9ab4d8
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr89701-1.c
> > @@ -0,0 +1,4 @@
> > +/* { dg-do compile { target *-*-linux* } } */
> > +/* { dg-options "-fcf-protection=branch,return" } */
> > +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
> > +/* { dg-final { scan-assembler-times ".long    0x3" 1 } } */
> > diff --git a/gcc/testsuite/gcc.target/i386/pr89701-2.c b/gcc/testsuite/gcc.target/i386/pr89701-2.c
> > new file mode 100644
> > index 00000000000..d5100575028
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr89701-2.c
> > @@ -0,0 +1,4 @@
> > +/* { dg-do compile { target *-*-linux* } } */
> > +/* { dg-options "-fcf-protection=return,branch" } */
> > +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
> > +/* { dg-final { scan-assembler-times ".long    0x3" 1 } } */
> > diff --git a/gcc/testsuite/gcc.target/i386/pr89701-3.c b/gcc/testsuite/gcc.target/i386/pr89701-3.c
> > new file mode 100644
> > index 00000000000..88afb546fbf
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr89701-3.c
> > @@ -0,0 +1,4 @@
> > +/* { dg-do compile { target *-*-linux* } } */
> > +/* { dg-options "-fcf-protection=return,none" } */
> > +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
> > +/* { dg-final { scan-assembler-times ".long    0x2" 1 } } */
> > --
> > 2.39.1.388.g2fc9e9ca3c
> >
>
>
> --
> BR,
> Hongtao



-- 
BR,
Hongtao

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] Provide -fcf-protection=branch,return.
  2023-07-12  7:27         ` Hongtao Liu
@ 2023-07-19  8:37           ` Hongtao Liu
  0 siblings, 0 replies; 7+ messages in thread
From: Hongtao Liu @ 2023-07-19  8:37 UTC (permalink / raw)
  To: liuhongt; +Cc: gcc-patches, hjl.tools

On Wed, Jul 12, 2023 at 3:27 PM Hongtao Liu <crazylht@gmail.com> wrote:
>
> ping.
>
> On Mon, May 22, 2023 at 4:08 PM Hongtao Liu <crazylht@gmail.com> wrote:
> >
> > ping.
> >
> > On Sat, May 13, 2023 at 5:20 PM liuhongt <hongtao.liu@intel.com> wrote:
> > >
> > > > I think this could be simplified if you use either EnumSet or
> > > > EnumBitSet instead in common.opt for `-fcf-protection=`.
> > >
> > > Use EnumSet instead of EnumBitSet since CF_FULL is not power of 2.
> > > It is a bit tricky for sets classification, cf_branch and cf_return
> > > should be in different sets, but they both "conflicts" cf_full,
> > > cf_none. And current EnumSet don't handle this well.
> > >
> > > So in the current implementation, only cf_full,cf_none are exclusive
> > > to each other, but they can be combined with any cf_branch, cf_return,
> > > cf_check. It's not perfect, but still an improvement than original
> > > one.
> > >
I'm going to commit this patch if there's no objection, it's just a
refactor of option -fcf-protection=.
If there's any regression observed, I will fix(or revert the patch).
> > > gcc/ChangeLog:
> > >
> > >         * common.opt: (fcf-protection=): Add EnumSet attribute to
> > >         support combination of params.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > >         * c-c++-common/fcf-protection-10.c: New test.
> > >         * c-c++-common/fcf-protection-11.c: New test.
> > >         * c-c++-common/fcf-protection-12.c: New test.
> > >         * c-c++-common/fcf-protection-8.c: New test.
> > >         * c-c++-common/fcf-protection-9.c: New test.
> > >         * gcc.target/i386/pr89701-1.c: New test.
> > >         * gcc.target/i386/pr89701-2.c: New test.
> > >         * gcc.target/i386/pr89701-3.c: New test.
> > > ---
> > >  gcc/common.opt                                 | 12 ++++++------
> > >  gcc/testsuite/c-c++-common/fcf-protection-10.c |  2 ++
> > >  gcc/testsuite/c-c++-common/fcf-protection-11.c |  2 ++
> > >  gcc/testsuite/c-c++-common/fcf-protection-12.c |  2 ++
> > >  gcc/testsuite/c-c++-common/fcf-protection-8.c  |  2 ++
> > >  gcc/testsuite/c-c++-common/fcf-protection-9.c  |  2 ++
> > >  gcc/testsuite/gcc.target/i386/pr89701-1.c      |  4 ++++
> > >  gcc/testsuite/gcc.target/i386/pr89701-2.c      |  4 ++++
> > >  gcc/testsuite/gcc.target/i386/pr89701-3.c      |  4 ++++
> > >  9 files changed, 28 insertions(+), 6 deletions(-)
> > >  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-10.c
> > >  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-11.c
> > >  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-12.c
> > >  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-8.c
> > >  create mode 100644 gcc/testsuite/c-c++-common/fcf-protection-9.c
> > >  create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-1.c
> > >  create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-2.c
> > >  create mode 100644 gcc/testsuite/gcc.target/i386/pr89701-3.c
> > >
> > > diff --git a/gcc/common.opt b/gcc/common.opt
> > > index a28ca13385a..02f2472959a 100644
> > > --- a/gcc/common.opt
> > > +++ b/gcc/common.opt
> > > @@ -1886,7 +1886,7 @@ fcf-protection
> > >  Common RejectNegative Alias(fcf-protection=,full)
> > >
> > >  fcf-protection=
> > > -Common Joined RejectNegative Enum(cf_protection_level) Var(flag_cf_protection) Init(CF_NONE)
> > > +Common Joined RejectNegative Enum(cf_protection_level) EnumSet Var(flag_cf_protection) Init(CF_NONE)
> > >  -fcf-protection=[full|branch|return|none|check]        Instrument functions with checks to verify jump/call/return control-flow transfer
> > >  instructions have valid targets.
> > >
> > > @@ -1894,19 +1894,19 @@ Enum
> > >  Name(cf_protection_level) Type(enum cf_protection_level) UnknownError(unknown Control-Flow Protection Level %qs)
> > >
> > >  EnumValue
> > > -Enum(cf_protection_level) String(full) Value(CF_FULL)
> > > +Enum(cf_protection_level) String(full) Value(CF_FULL) Set(1)
> > >
> > >  EnumValue
> > > -Enum(cf_protection_level) String(branch) Value(CF_BRANCH)
> > > +Enum(cf_protection_level) String(branch) Value(CF_BRANCH) Set(2)
> > >
> > >  EnumValue
> > > -Enum(cf_protection_level) String(return) Value(CF_RETURN)
> > > +Enum(cf_protection_level) String(return) Value(CF_RETURN) Set(3)
> > >
> > >  EnumValue
> > > -Enum(cf_protection_level) String(check) Value(CF_CHECK)
> > > +Enum(cf_protection_level) String(check) Value(CF_CHECK) Set(4)
> > >
> > >  EnumValue
> > > -Enum(cf_protection_level) String(none) Value(CF_NONE)
> > > +Enum(cf_protection_level) String(none) Value(CF_NONE) Set(1)
> > >
> > >  finstrument-functions
> > >  Common Var(flag_instrument_function_entry_exit,1)
> > > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-10.c b/gcc/testsuite/c-c++-common/fcf-protection-10.c
> > > new file mode 100644
> > > index 00000000000..b271d134e52
> > > --- /dev/null
> > > +++ b/gcc/testsuite/c-c++-common/fcf-protection-10.c
> > > @@ -0,0 +1,2 @@
> > > +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> > > +/* { dg-options "-fcf-protection=branch,check" } */
> > > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-11.c b/gcc/testsuite/c-c++-common/fcf-protection-11.c
> > > new file mode 100644
> > > index 00000000000..2e566350ccd
> > > --- /dev/null
> > > +++ b/gcc/testsuite/c-c++-common/fcf-protection-11.c
> > > @@ -0,0 +1,2 @@
> > > +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> > > +/* { dg-options "-fcf-protection=branch,return" } */
> > > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-12.c b/gcc/testsuite/c-c++-common/fcf-protection-12.c
> > > new file mode 100644
> > > index 00000000000..b39c2f8e25d
> > > --- /dev/null
> > > +++ b/gcc/testsuite/c-c++-common/fcf-protection-12.c
> > > @@ -0,0 +1,2 @@
> > > +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> > > +/* { dg-options "-fcf-protection=return,branch" } */
> > > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-8.c b/gcc/testsuite/c-c++-common/fcf-protection-8.c
> > > new file mode 100644
> > > index 00000000000..3b97095a92c
> > > --- /dev/null
> > > +++ b/gcc/testsuite/c-c++-common/fcf-protection-8.c
> > > @@ -0,0 +1,2 @@
> > > +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> > > +/* { dg-options "-fcf-protection=branch,none" } */
> > > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-9.c b/gcc/testsuite/c-c++-common/fcf-protection-9.c
> > > new file mode 100644
> > > index 00000000000..6a37e749fcb
> > > --- /dev/null
> > > +++ b/gcc/testsuite/c-c++-common/fcf-protection-9.c
> > > @@ -0,0 +1,2 @@
> > > +/* { dg-do compile { target { "i?86-*-* x86_64-*-*" } } } */
> > > +/* { dg-options "-fcf-protection=branch,full" } */
> > > diff --git a/gcc/testsuite/gcc.target/i386/pr89701-1.c b/gcc/testsuite/gcc.target/i386/pr89701-1.c
> > > new file mode 100644
> > > index 00000000000..1879c9ab4d8
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/i386/pr89701-1.c
> > > @@ -0,0 +1,4 @@
> > > +/* { dg-do compile { target *-*-linux* } } */
> > > +/* { dg-options "-fcf-protection=branch,return" } */
> > > +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
> > > +/* { dg-final { scan-assembler-times ".long    0x3" 1 } } */
> > > diff --git a/gcc/testsuite/gcc.target/i386/pr89701-2.c b/gcc/testsuite/gcc.target/i386/pr89701-2.c
> > > new file mode 100644
> > > index 00000000000..d5100575028
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/i386/pr89701-2.c
> > > @@ -0,0 +1,4 @@
> > > +/* { dg-do compile { target *-*-linux* } } */
> > > +/* { dg-options "-fcf-protection=return,branch" } */
> > > +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
> > > +/* { dg-final { scan-assembler-times ".long    0x3" 1 } } */
> > > diff --git a/gcc/testsuite/gcc.target/i386/pr89701-3.c b/gcc/testsuite/gcc.target/i386/pr89701-3.c
> > > new file mode 100644
> > > index 00000000000..88afb546fbf
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/i386/pr89701-3.c
> > > @@ -0,0 +1,4 @@
> > > +/* { dg-do compile { target *-*-linux* } } */
> > > +/* { dg-options "-fcf-protection=return,none" } */
> > > +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */
> > > +/* { dg-final { scan-assembler-times ".long    0x2" 1 } } */
> > > --
> > > 2.39.1.388.g2fc9e9ca3c
> > >
> >
> >
> > --
> > BR,
> > Hongtao
>
>
>
> --
> BR,
> Hongtao



-- 
BR,
Hongtao

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-07-19  8:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-12  5:42 [PATCH] Provide -fcf-protection=branch,return liuhongt
2023-05-12  5:49 ` Andrew Pinski
2023-05-12  6:21   ` Hongtao Liu
2023-05-13  9:20     ` [PATCH V2] " liuhongt
2023-05-22  8:08       ` Hongtao Liu
2023-07-12  7:27         ` Hongtao Liu
2023-07-19  8:37           ` Hongtao Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).