> -----Original Message----- > From: Uros Bizjak [mailto:ubiz...@gmail.com] > Sent: Friday, October 13, 2017 10:02 AM > To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com> > Cc: gcc-patches@gcc.gnu.org > Subject: Re: 0006-Part-6.-Add-x86-tests-for-Intel-CET-implementation > > On Thu, Oct 12, 2017 at 8:54 PM, Tsimbalist, Igor V > <igor.v.tsimbal...@intel.com> wrote: > > Attached is an updated patch according to your comments. New tests are > > added to test ICF optimization in presence of nocf_check attribute. > --- a/gcc/testsuite/c-c++-common/fcf-protection-2.c > +++ b/gcc/testsuite/c-c++-common/fcf-protection-2.c > @@ -1,4 +1,4 @@ > /* { dg-do compile } */ > /* { dg-options "-fcf-protection=branch" } */ > -/* { dg-error "'-fcf-protection=branch' is not supported for this target" "" > { > target { "i?86-*-* x86_64-*-*" } } 0 } */ > +/* { dg-error "'-fcf-protection=branch' requires CET support on this > target. Use -mcet or one of -mibt, -mshstk options to enable CET" "" { target > { > "i?86-*-* x86_64-*-*" } } 0 } */ > > Checking for "-fcf-protection=branch' requires CET support on this target" > should be enough. No need to check the whole message here and in other > tests.
Fixed as you suggested. Also shortened the checking string for ignoring the attribute in attr-nocf-check-1.c and attr-nocf-check-3.c. > /* { dg-error "'-fcf-protection=branch' is not supported for this target" "" > { > target { ! "i?86-*-* x86_64-*-*" } } 0 } */ diff --git a/gcc/testsuite/c-c++- > common/fcf-protection-3.c > b/gcc/testsuite/c-c++-common/fcf-protection-3.c > > > --- a/gcc/testsuite/c-c++-common/fcf-protection-4.c > +++ b/gcc/testsuite/c-c++-common/fcf-protection-4.c > @@ -1,4 +1,4 @@ > /* { dg-do compile } */ > /* { dg-options "-fcf-protection=none" } */ > -/* { dg-bogus "'-fcf-protection=none' is not supported for this target" "" { > target { "i?86-*-* x86_64-*-*" } } 0 } */ > +/* { dg-bogus "'-fcf-protection=none' res CET support on this target. > Use -mcet or one of -mibt, -mshstk options to enable CET" "" { target { "i?86- > *-* x86_64-*-*" } } 0 } */ > /* { dg-bogus "'-fcf-protection=none' is not supported for this target" "" { > target { ! "i?86-*-* x86_64-*-*" } } 0 } */ diff --git a/gcc/testsuite/c-c++- > common/fcf-protection-5.c > b/gcc/testsuite/c-c++-common/fcf-protection-5.c > > The above test checks for bogus messages? -fcf-protection=none option > should not generate any messages. So, the test should check that -fcf- > protection=none doesn't generate any error. (And, there is a typo in the > message, /s/res/requires.) The gcc documentation says about dg-bogus This DejaGnu directive appears on a source line that should not get a message matching regexp... I decided to use dg-bogus to check the absence of the error. Now I removed both lines as any additional messages should be caught as an extra messages. Actually I will update the fcf-protection-4.c test in the generic patch. Updated patch is attached. Igor > Uros. > > > Igor > > > > > >> -----Original Message----- > >> From: Tsimbalist, Igor V > >> Sent: Tuesday, September 19, 2017 11:30 PM > >> To: Uros Bizjak <ubiz...@gmail.com> > >> Cc: gcc-patches@gcc.gnu.org; Tsimbalist, Igor V > >> <igor.v.tsimbal...@intel.com> > >> Subject: RE: 0006-Part-6.-Add-x86-tests-for-Intel-CET-implementation > >> > >> > -----Original Message----- > >> > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > >> > ow...@gcc.gnu.org] On Behalf Of Uros Bizjak > >> > Sent: Tuesday, September 19, 2017 6:13 PM > >> > To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com> > >> > Cc: gcc-patches@gcc.gnu.org > >> > Subject: Re: > >> > 0006-Part-6.-Add-x86-tests-for-Intel-CET-implementation > >> > > >> > On Tue, Sep 19, 2017 at 5:18 PM, Tsimbalist, Igor V > >> > <igor.v.tsimbal...@intel.com> wrote: > >> > >> -----Original Message----- > >> > >> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > >> > >> ow...@gcc.gnu.org] On Behalf Of Uros Bizjak > >> > >> Sent: Monday, September 18, 2017 12:17 PM > >> > >> To: gcc-patches@gcc.gnu.org > >> > >> Cc: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; > >> > >> Tsimbalist, Igor V <igor.v.tsimbal...@intel.com> > >> > >> Subject: Re: > >> > >> 0006-Part-6.-Add-x86-tests-for-Intel-CET-implementation > >> > >> > >> > >> Hello! > >> > >> > >> > >> > gcc/testsuite/ > >> > >> > > >> > >> > * g++.dg/cet-notrack-1.C: New test. > >> > >> > * gcc.target/i386/cet-intrin-1.c: Likewise. > >> > >> > * gcc.target/i386/cet-intrin-10.c: Likewise. > >> > >> > * gcc.target/i386/cet-intrin-2.c: Likewise. > >> > >> > * gcc.target/i386/cet-intrin-3.c: Likewise. > >> > >> > * gcc.target/i386/cet-intrin-4.c: Likewise. > >> > >> > * gcc.target/i386/cet-intrin-5.c: Likewise. > >> > >> > * gcc.target/i386/cet-intrin-6.c: Likewise. > >> > >> > * gcc.target/i386/cet-intrin-7.c: Likewise. > >> > >> > * gcc.target/i386/cet-intrin-8.c: Likewise. > >> > >> > * gcc.target/i386/cet-intrin-9.c: Likewise. > >> > >> > * gcc.target/i386/cet-label.c: Likewise. > >> > >> > * gcc.target/i386/cet-notrack-1a.c: Likewise. > >> > >> > * gcc.target/i386/cet-notrack-1b.c: Likewise. > >> > >> > * gcc.target/i386/cet-notrack-2a.c: Likewise. > >> > >> > * gcc.target/i386/cet-notrack-2b.c: Likewise. > >> > >> > * gcc.target/i386/cet-notrack-3.c: Likewise. > >> > >> > * gcc.target/i386/cet-notrack-4a.c: Likewise. > >> > >> > * gcc.target/i386/cet-notrack-4b.c: Likewise. > >> > >> > * gcc.target/i386/cet-notrack-5a.c: Likewise. > >> > >> > * gcc.target/i386/cet-notrack-5b.c: Likewise. > >> > >> > * gcc.target/i386/cet-notrack-6a.c: Likewise. > >> > >> > * gcc.target/i386/cet-notrack-6b.c: Likewise. > >> > >> > * gcc.target/i386/cet-notrack-7.c: Likewise. > >> > >> > * gcc.target/i386/cet-property-1.c: Likewise. > >> > >> > * gcc.target/i386/cet-property-2.c: Likewise. > >> > >> > * gcc.target/i386/cet-rdssp-1.c: Likewise. > >> > >> > * gcc.target/i386/cet-sjlj-1.c: Likewise. > >> > >> > * gcc.target/i386/cet-sjlj-2.c: Likewise. > >> > >> > * gcc.target/i386/cet-sjlj-3.c: Likewise. > >> > >> > * gcc.target/i386/cet-switch-1.c: Likewise. > >> > >> > * gcc.target/i386/cet-switch-2.c: Likewise. > >> > >> > * lib/target-supports.exp (check_effective_target_cet): New proc. > >> > >> > >> > >> A couple of questions: > >> > >> > >> > >> +/* { dg-do compile } */ > >> > >> +/* { dg-options "-O2 -mcet" } */ > >> > >> +/* { dg-final { scan-assembler-times "setssbsy" 2 } } */ > >> > >> + > >> > >> +#include <immintrin.h> > >> > >> + > >> > >> +void f1 (void) > >> > >> +{ > >> > >> + __builtin_ia32_setssbsy (); > >> > >> +} > >> > >> + > >> > >> +void f2 (void) > >> > >> +{ > >> > >> + _setssbsy (); > >> > >> +} > >> > >> > >> > >> Is there a reason that both, __builtin and intrinsic versions > >> > >> are tested in a couple of places? The intrinsic version is just > >> > >> a wrapper for __builtin, so IMO testing intrinsic version should > >> > >> be > >> enough. > >> > > No strong reason. Just to check that intrinsic names are > >> > > recognized and > >> > processed correctly. > >> > > The implementation could change and the test will catch > inconsistency. > >> > > I would also assume a user will use intrinsics that's why I add > >> > > intrinsic > >> check. > >> > Should I remove it? > >> > > >> > Actually, these __builtins are considered as implementation detail, > >> > and their use should be discouraged. They are deliberately not > >> > documented, and users should use intrinsic headers instead. That > >> > said, builtins won't change without a reason, since Ada needs them. > >> > > >> > It can happen that the test fails due to change of intrinsics, so > >> > I'd recommend to remove them. > >> Ok, I will remove intrinsic. > >> > >> > >> diff --git a/gcc/testsuite/gcc.target/i386/cet-rdssp-1.c > >> > >> b/gcc/testsuite/gcc.target/i386/cet-rdssp-1.c > >> > >> new file mode 100644 > >> > >> index 0000000..f9223a5 > >> > >> --- /dev/null > >> > >> +++ b/gcc/testsuite/gcc.target/i386/cet-rdssp-1.c > >> > >> @@ -0,0 +1,39 @@ > >> > >> +/* { dg-do run { target cet } } */ > >> > >> +/* { dg-options "-O2 -finstrument-control-flow -mcet" } */ > >> > >> > >> > >> The "target cet" directive just checks that CET instructions can > >> > >> be > >> > compiled. > >> > >> The test will (probably?) fail on targets with binutils that can > >> > >> compile CET instructions, but the target itself doesn't support CET. > >> > >> If this is the case, then check header has to be introduced, so > >> > >> the test can be bypassed on targets without runtime support. > >> > > The test will not fail even if a target doesn't support CET as 'rdssp' > >> > > instruction is a NOP on such target and further usage of CET > >> > > instruction is bypassed. In this case the code > >> > > > >> > > + ssp = rdssp (ssp); > >> > > > >> > > Will keep ssp as 0. > >> > > >> > I assume that this is true also for other runtime tests, and this > >> > clears my concern about runtime failures with updated binutils. > >> Yes, that's true for other runtime tests. > >> > >> Igor > >> > >> > > >> > Uros.
0006-Add-x86-tests-for-Intel-CET-implementation.patch
Description: 0006-Add-x86-tests-for-Intel-CET-implementation.patch