diff mbox series

x86: Error on -fcf-protection with incompatible target

Message ID 20210114140514.2760199-1-hjl.tools@gmail.com
State New
Headers show
Series x86: Error on -fcf-protection with incompatible target | expand

Commit Message

H.J. Lu Jan. 14, 2021, 2:05 p.m. UTC 
-fcf-protection with CF_BRANCH inserts ENDBR32 at function entries.
ENDBR32 is NOP only on 64-bit processors and 32-bit TARGET_CMOVE
processors.  Issue an error for -fcf-protection with CF_BRANCH when
compiling for 32-bit non-TARGET_CMOVE targets.

gcc/

	PR target/98667
	* config/i386/i386-options.c (ix86_option_override_internal):
	Issue an error for -fcf-protection with CF_BRANCH when compiling
	for 32-bit non-TARGET_CMOVE targets.

gcc/testsuite/

	PR target/98667
	* gcc.target/i386/pr98667-1.c: New file.
	* gcc.target/i386/pr98667-2.c: Likewise.
	* gcc.target/i386/pr98667-3.c: Likewise.
---
 gcc/config/i386/i386-options.c            | 9 ++++++++-
 gcc/testsuite/gcc.target/i386/pr98667-1.c | 9 +++++++++
 gcc/testsuite/gcc.target/i386/pr98667-2.c | 9 +++++++++
 gcc/testsuite/gcc.target/i386/pr98667-3.c | 7 +++++++
 4 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr98667-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr98667-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr98667-3.c

Comments

Uros Bizjak Jan. 14, 2021, 2:51 p.m. UTC | #1 
On Thu, Jan 14, 2021 at 3:05 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> -fcf-protection with CF_BRANCH inserts ENDBR32 at function entries.
> ENDBR32 is NOP only on 64-bit processors and 32-bit TARGET_CMOVE
> processors.  Issue an error for -fcf-protection with CF_BRANCH when
> compiling for 32-bit non-TARGET_CMOVE targets.
>
> gcc/
>
>         PR target/98667
>         * config/i386/i386-options.c (ix86_option_override_internal):
>         Issue an error for -fcf-protection with CF_BRANCH when compiling
>         for 32-bit non-TARGET_CMOVE targets.
>
> gcc/testsuite/
>
>         PR target/98667
>         * gcc.target/i386/pr98667-1.c: New file.
>         * gcc.target/i386/pr98667-2.c: Likewise.
>         * gcc.target/i386/pr98667-3.c: Likewise.
> ---
>  gcc/config/i386/i386-options.c            | 9 ++++++++-
>  gcc/testsuite/gcc.target/i386/pr98667-1.c | 9 +++++++++
>  gcc/testsuite/gcc.target/i386/pr98667-2.c | 9 +++++++++
>  gcc/testsuite/gcc.target/i386/pr98667-3.c | 7 +++++++
>  4 files changed, 33 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr98667-1.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr98667-2.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr98667-3.c
>
> diff --git a/gcc/config/i386/i386-options.c b/gcc/config/i386/i386-options.c
> index 4e0165ff32c..1489871b36f 100644
> --- a/gcc/config/i386/i386-options.c
> +++ b/gcc/config/i386/i386-options.c
> @@ -3016,8 +3016,15 @@ ix86_option_override_internal (bool main_args_p,
>      }
>
>    if (opts->x_flag_cf_protection != CF_NONE)
> -    opts->x_flag_cf_protection
> +    {
> +      if ((opts->x_flag_cf_protection & CF_BRANCH) == CF_BRANCH
> +         && !TARGET_64BIT
> +         && !TARGET_CMOVE)

You need TARGET_CMOV (note, no E) here. Also, please put both tests on one line.

LGTM with the above change.

Thanks,
Uros.

> +       error ("%<-fcf-protection%> is not compatible with this target");
> +
> +      opts->x_flag_cf_protection
>        = (cf_protection_level) (opts->x_flag_cf_protection | CF_SET);
> +    }
>
>    if (ix86_tune_features [X86_TUNE_AVOID_256FMA_CHAINS])
>      SET_OPTION_IF_UNSET (opts, opts_set, param_avoid_fma_max_bits, 256);
> diff --git a/gcc/testsuite/gcc.target/i386/pr98667-1.c b/gcc/testsuite/gcc.target/i386/pr98667-1.c
> new file mode 100644
> index 00000000000..5bf0c9285a8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr98667-1.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile { target ia32 } } */
> +/* { dg-options "-O2 -fcf-protection -march=i486" } */
> +
> +void
> +test (void)
> +{
> +}
> +
> +/* { dg-error "'-fcf-protection' is not compatible with this target" "" { target *-*-* } 0 } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr98667-2.c b/gcc/testsuite/gcc.target/i386/pr98667-2.c
> new file mode 100644
> index 00000000000..bc3a78c9641
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr98667-2.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile { target ia32 } } */
> +/* { dg-options "-O2 -fcf-protection=branch -march=i486" } */
> +
> +void
> +test (void)
> +{
> +}
> +
> +/* { dg-error "'-fcf-protection' is not compatible with this target" "" { target *-*-* } 0 } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr98667-3.c b/gcc/testsuite/gcc.target/i386/pr98667-3.c
> new file mode 100644
> index 00000000000..a6ea6d04331
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr98667-3.c
> @@ -0,0 +1,7 @@
> +/* { dg-do compile { target ia32 } } */
> +/* { dg-options "-O2 -fcf-protection=return -march=i486" } */
> +
> +void
> +test (void)
> +{
> +}
> --
> 2.29.2
>
H.J. Lu Jan. 14, 2021, 3:18 p.m. UTC | #2 
On Thu, Jan 14, 2021 at 6:51 AM Uros Bizjak <ubizjak@gmail.com> wrote:
>
> On Thu, Jan 14, 2021 at 3:05 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > -fcf-protection with CF_BRANCH inserts ENDBR32 at function entries.
> > ENDBR32 is NOP only on 64-bit processors and 32-bit TARGET_CMOVE
> > processors.  Issue an error for -fcf-protection with CF_BRANCH when
> > compiling for 32-bit non-TARGET_CMOVE targets.
> >
> > gcc/
> >
> >         PR target/98667
> >         * config/i386/i386-options.c (ix86_option_override_internal):
> >         Issue an error for -fcf-protection with CF_BRANCH when compiling
> >         for 32-bit non-TARGET_CMOVE targets.
> >
> > gcc/testsuite/
> >
> >         PR target/98667
> >         * gcc.target/i386/pr98667-1.c: New file.
> >         * gcc.target/i386/pr98667-2.c: Likewise.
> >         * gcc.target/i386/pr98667-3.c: Likewise.
> > ---
> >  gcc/config/i386/i386-options.c            | 9 ++++++++-
> >  gcc/testsuite/gcc.target/i386/pr98667-1.c | 9 +++++++++
> >  gcc/testsuite/gcc.target/i386/pr98667-2.c | 9 +++++++++
> >  gcc/testsuite/gcc.target/i386/pr98667-3.c | 7 +++++++
> >  4 files changed, 33 insertions(+), 1 deletion(-)
> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr98667-1.c
> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr98667-2.c
> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr98667-3.c
> >
> > diff --git a/gcc/config/i386/i386-options.c b/gcc/config/i386/i386-options.c
> > index 4e0165ff32c..1489871b36f 100644
> > --- a/gcc/config/i386/i386-options.c
> > +++ b/gcc/config/i386/i386-options.c
> > @@ -3016,8 +3016,15 @@ ix86_option_override_internal (bool main_args_p,
> >      }
> >
> >    if (opts->x_flag_cf_protection != CF_NONE)
> > -    opts->x_flag_cf_protection
> > +    {
> > +      if ((opts->x_flag_cf_protection & CF_BRANCH) == CF_BRANCH
> > +         && !TARGET_64BIT
> > +         && !TARGET_CMOVE)
>
> You need TARGET_CMOV (note, no E) here. Also, please put both tests on one line.
>
> LGTM with the above change.

This is the patch I am checking in.

Thanks.
Matthias Klose Jan. 15, 2021, 11:39 a.m. UTC | #3 
On 1/14/21 4:18 PM, H.J. Lu via Gcc-patches wrote:
> On Thu, Jan 14, 2021 at 6:51 AM Uros Bizjak <ubizjak@gmail.com> wrote:
>>
>> On Thu, Jan 14, 2021 at 3:05 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>
>>> -fcf-protection with CF_BRANCH inserts ENDBR32 at function entries.
>>> ENDBR32 is NOP only on 64-bit processors and 32-bit TARGET_CMOVE
>>> processors.  Issue an error for -fcf-protection with CF_BRANCH when
>>> compiling for 32-bit non-TARGET_CMOVE targets.
>>>
>>> gcc/
>>>
>>>         PR target/98667
>>>         * config/i386/i386-options.c (ix86_option_override_internal):
>>>         Issue an error for -fcf-protection with CF_BRANCH when compiling
>>>         for 32-bit non-TARGET_CMOVE targets.
>>>
>>> gcc/testsuite/
>>>
>>>         PR target/98667
>>>         * gcc.target/i386/pr98667-1.c: New file.
>>>         * gcc.target/i386/pr98667-2.c: Likewise.
>>>         * gcc.target/i386/pr98667-3.c: Likewise.
>>> ---
>>>  gcc/config/i386/i386-options.c            | 9 ++++++++-
>>>  gcc/testsuite/gcc.target/i386/pr98667-1.c | 9 +++++++++
>>>  gcc/testsuite/gcc.target/i386/pr98667-2.c | 9 +++++++++
>>>  gcc/testsuite/gcc.target/i386/pr98667-3.c | 7 +++++++
>>>  4 files changed, 33 insertions(+), 1 deletion(-)
>>>  create mode 100644 gcc/testsuite/gcc.target/i386/pr98667-1.c
>>>  create mode 100644 gcc/testsuite/gcc.target/i386/pr98667-2.c
>>>  create mode 100644 gcc/testsuite/gcc.target/i386/pr98667-3.c
>>>
>>> diff --git a/gcc/config/i386/i386-options.c b/gcc/config/i386/i386-options.c
>>> index 4e0165ff32c..1489871b36f 100644
>>> --- a/gcc/config/i386/i386-options.c
>>> +++ b/gcc/config/i386/i386-options.c
>>> @@ -3016,8 +3016,15 @@ ix86_option_override_internal (bool main_args_p,
>>>      }
>>>
>>>    if (opts->x_flag_cf_protection != CF_NONE)
>>> -    opts->x_flag_cf_protection
>>> +    {
>>> +      if ((opts->x_flag_cf_protection & CF_BRANCH) == CF_BRANCH
>>> +         && !TARGET_64BIT
>>> +         && !TARGET_CMOVE)
>>
>> You need TARGET_CMOV (note, no E) here. Also, please put both tests on one line.
>>
>> LGTM with the above change.
> 
> This is the patch I am checking in.

I might be doing something wrong, but this breaks the -m32 multilib build for
me. looking at a trunk 20210110 build, -fcf-protection -mshstk are passed to the
-m32 build as well, and now errors out.

Matthias
diff mbox series

Patch

diff --git a/gcc/config/i386/i386-options.c b/gcc/config/i386/i386-options.c
index 4e0165ff32c..1489871b36f 100644
--- a/gcc/config/i386/i386-options.c
+++ b/gcc/config/i386/i386-options.c
@@ -3016,8 +3016,15 @@  ix86_option_override_internal (bool main_args_p,
     }
 
   if (opts->x_flag_cf_protection != CF_NONE)
-    opts->x_flag_cf_protection
+    {
+      if ((opts->x_flag_cf_protection & CF_BRANCH) == CF_BRANCH
+	  && !TARGET_64BIT
+	  && !TARGET_CMOVE)
+	error ("%<-fcf-protection%> is not compatible with this target");
+
+      opts->x_flag_cf_protection
       = (cf_protection_level) (opts->x_flag_cf_protection | CF_SET);
+    }
 
   if (ix86_tune_features [X86_TUNE_AVOID_256FMA_CHAINS])
     SET_OPTION_IF_UNSET (opts, opts_set, param_avoid_fma_max_bits, 256);
diff --git a/gcc/testsuite/gcc.target/i386/pr98667-1.c b/gcc/testsuite/gcc.target/i386/pr98667-1.c
new file mode 100644
index 00000000000..5bf0c9285a8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr98667-1.c
@@ -0,0 +1,9 @@ 
+/* { dg-do compile { target ia32 } } */
+/* { dg-options "-O2 -fcf-protection -march=i486" } */
+
+void
+test (void)
+{
+}
+
+/* { dg-error "'-fcf-protection' is not compatible with this target" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/i386/pr98667-2.c b/gcc/testsuite/gcc.target/i386/pr98667-2.c
new file mode 100644
index 00000000000..bc3a78c9641
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr98667-2.c
@@ -0,0 +1,9 @@ 
+/* { dg-do compile { target ia32 } } */
+/* { dg-options "-O2 -fcf-protection=branch -march=i486" } */
+
+void
+test (void)
+{
+}
+
+/* { dg-error "'-fcf-protection' is not compatible with this target" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/i386/pr98667-3.c b/gcc/testsuite/gcc.target/i386/pr98667-3.c
new file mode 100644
index 00000000000..a6ea6d04331
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr98667-3.c
@@ -0,0 +1,7 @@ 
+/* { dg-do compile { target ia32 } } */
+/* { dg-options "-O2 -fcf-protection=return -march=i486" } */
+
+void
+test (void)
+{
+}