Re: PR 84154: Fix checking -mibt and -mshstk options for control flow protection
On Tue, Feb 06, 2018 at 11:14:08AM +, Tsimbalist, Igor V wrote: > Coincidentally, I have worked on the same patch. Please look at the patch, I > uploaded it to the bug. The main differences are > > - updated the output messages to be more informative; > - updated the tests and add couple of new tests to check the messages; > - fixed a typo in the doc file related to fcf-protection; > > I am ok with the changes in i386.c but would like to update the messages. > Could you incorporate my changes and proceed? Or would you like me to finish > the fix? The r257414 change broke bootstrap on both x86_64-linux and i686-linux, in both the bootstrap ICEs during libitm build of aatree.cc: /.../gcc/obj70/./gcc/xg++ -B/.../gcc/obj70/./gcc/ -nostdinc++ -nostdinc++ -I/.../gcc/obj70/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu -I/.../gcc/obj70/x86_64-pc-linux-gnu/libstdc++-v3/include -I/.../gcc/libstdc++-v3/libsupc++ -I/.../gcc/libstdc++-v3/include/backward -I/.../gcc/libstdc++-v3/testsuite/util -L/.../gcc/obj70/x86_64-pc-linux-gnu/libstdc++-v3/src -L/.../gcc/obj70/x86_64-pc-linux-gnu/libstdc++-v3/src/.libs -L/.../gcc/obj70/x86_64-pc-linux-gnu/libstdc++-v3/libsupc++/.libs -B/.../gcc/obj70/x86_64-pc-linux-gnu/libstdc++-v3/src/.libs -B/.../gcc/obj70/x86_64-pc-linux-gnu/libstdc++-v3/libsupc++/.libs -B/usr/local/x86_64-pc-linux-gnu/bin/ -B/usr/local/x86_64-pc-linux-gnu/lib/ -isystem /usr/local/x86_64-pc-linux-gnu/include -isystem /usr/local/x86_64-pc-linux-gnu/sys-include -DHAVE_CONFIG_H -I. -I../../../libitm -I../../../libitm/config/linux/x86 -I../../../libitm/config/linux -I../../../libitm/config/x86 -I../../../libitm/config/posix -I../../../libitm/config/generic -I../../../libitm -mrtm -Wall -pthread -Werror -fcf-protection -mcet -std=gnu++0x -funwind-tables -fno-exceptions -fno-rtti -fabi-version=4 -g -O2 -D_GNU_SOURCE -MT aatree.lo -MD -MP -MF .deps/aatree.Tpo -c ../../../libitm/aatree.cc -fPIC -DPIC -o .libs/aatree.o /.../gcc/obj70/gcc/include/ia32intrin.h:56:28: internal compiler error: in ix86_option_override_internal, at config/i386/i386.c:4948 The problem is that the enum has {CF_NONE, CF_BRANCH, CF_RETURN, CF_FULL, CF_SET} values and the hook does: opts->x_flag_cf_protection = (cf_protection_level) (opts->x_flag_cf_protection | CF_SET); after the switch (note, bad formatting), and if called again, we end up with e.g. CF_FULL | CF_SET value which the switch doesn't handle. It isn't clear what you want to do, either ignore the CF_SET | ... values in the switch instead of gcc_unreachable on them, or switch (flag_cf_protection & ~CF_SET) or something similar. Jakub
Re: PR 84154: Fix checking -mibt and -mshstk options for control flow protection
Hi, On 07/02/2018 01:02, Tsimbalist, Igor V wrote: The issue is known and is covered by https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84248. The patch has been posted https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00276.html Ok, thanks, I missed the above. Paolo.
RE: PR 84154: Fix checking -mibt and -mshstk options for control flow protection
> -Original Message- > From: Paolo Carlini [mailto:paolo.carl...@oracle.com] > Sent: Wednesday, February 7, 2018 12:46 AM > To: Tsimbalist, Igor V ; gcc- > patc...@gcc.gnu.org > Cc: Nick Clifton ; hjl.to...@gmail.com; Uros Bizjak > > Subject: Re: PR 84154: Fix checking -mibt and -mshstk options for control > flow protection > > Hi, > > on a rather old x86_64-linux machine GCC doesn't build anymore with > r257414: > > libtool: compile: /xxx/Gcc/svn-dirs/trunk-build/./gcc/xg++ > -B/xxx/Gcc/svn-dirs/trunk-build/./gcc/ -nostdinc++ -nostdinc++ > -I/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++- > v3/include/x86_64-pc-linux-gnu > -I/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/include > -I/xxx/Gcc/svn-dirs/trunk/libstdc++-v3/libsupc++ > -I/xxx/Gcc/svn-dirs/trunk/libstdc++-v3/include/backward > -I/xxx/Gcc/svn-dirs/trunk/libstdc++-v3/testsuite/util > -L/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/src > -L/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/src/.libs > -L/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++- > v3/libsupc++/.libs > -B/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/src/.libs > -B/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++- > v3/libsupc++/.libs > -B/xxx/Gcc/svn-dirs/trunk-install/x86_64-pc-linux-gnu/bin/ > -B/xxx/Gcc/svn-dirs/trunk-install/x86_64-pc-linux-gnu/lib/ -isystem > /xxx/Gcc/svn-dirs/trunk-install/x86_64-pc-linux-gnu/include -isystem > /xxx/Gcc/svn-dirs/trunk-install/x86_64-pc-linux-gnu/sys-include > -DHAVE_CONFIG_H -I. -I../../../trunk/libitm > -I../../../trunk/libitm/config/linux/x86 > -I../../../trunk/libitm/config/linux -I../../../trunk/libitm/config/x86 > -I../../../trunk/libitm/config/posix > -I../../../trunk/libitm/config/generic -I../../../trunk/libitm -mrtm > -Wall -pthread -Werror -fcf-protection -mcet -std=gnu++0x > -funwind-tables -fno-exceptions -fno-rtti -fabi-version=4 -g -O2 > -D_GNU_SOURCE -MT beginend.lo -MD -MP -MF .deps/beginend.Tpo -c > ../../../trunk/libitm/beginend.cc -fPIC -DPIC -o .libs/beginend.o > In file included from > /xxx/Gcc/svn-dirs/trunk-build/gcc/include/x86intrin.h:27, > from ../../../trunk/libitm/config/x86/target.h:26, > from ../../../trunk/libitm/libitm_i.h:74, > from ../../../trunk/libitm/barrier.cc:25: > /xxx/Gcc/svn-dirs/trunk-build/gcc/include/ia32intrin.h:56:28: internal > compiler error: in ix86_option_override_internal, at config/i386/i386.c:4952 > #pragma GCC target("sse4.2") The issue is known and is covered by https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84248. The patch has been posted https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00276.html Igor > Paolo.
Re: PR 84154: Fix checking -mibt and -mshstk options for control flow protection
Hi, on a rather old x86_64-linux machine GCC doesn't build anymore with r257414: libtool: compile: /xxx/Gcc/svn-dirs/trunk-build/./gcc/xg++ -B/xxx/Gcc/svn-dirs/trunk-build/./gcc/ -nostdinc++ -nostdinc++ -I/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu -I/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/include -I/xxx/Gcc/svn-dirs/trunk/libstdc++-v3/libsupc++ -I/xxx/Gcc/svn-dirs/trunk/libstdc++-v3/include/backward -I/xxx/Gcc/svn-dirs/trunk/libstdc++-v3/testsuite/util -L/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/src -L/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/src/.libs -L/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/libsupc++/.libs -B/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/src/.libs -B/xxx/Gcc/svn-dirs/trunk-build/x86_64-pc-linux-gnu/libstdc++-v3/libsupc++/.libs -B/xxx/Gcc/svn-dirs/trunk-install/x86_64-pc-linux-gnu/bin/ -B/xxx/Gcc/svn-dirs/trunk-install/x86_64-pc-linux-gnu/lib/ -isystem /xxx/Gcc/svn-dirs/trunk-install/x86_64-pc-linux-gnu/include -isystem /xxx/Gcc/svn-dirs/trunk-install/x86_64-pc-linux-gnu/sys-include -DHAVE_CONFIG_H -I. -I../../../trunk/libitm -I../../../trunk/libitm/config/linux/x86 -I../../../trunk/libitm/config/linux -I../../../trunk/libitm/config/x86 -I../../../trunk/libitm/config/posix -I../../../trunk/libitm/config/generic -I../../../trunk/libitm -mrtm -Wall -pthread -Werror -fcf-protection -mcet -std=gnu++0x -funwind-tables -fno-exceptions -fno-rtti -fabi-version=4 -g -O2 -D_GNU_SOURCE -MT beginend.lo -MD -MP -MF .deps/beginend.Tpo -c ../../../trunk/libitm/beginend.cc -fPIC -DPIC -o .libs/beginend.o In file included from /xxx/Gcc/svn-dirs/trunk-build/gcc/include/x86intrin.h:27, from ../../../trunk/libitm/config/x86/target.h:26, from ../../../trunk/libitm/libitm_i.h:74, from ../../../trunk/libitm/barrier.cc:25: /xxx/Gcc/svn-dirs/trunk-build/gcc/include/ia32intrin.h:56:28: internal compiler error: in ix86_option_override_internal, at config/i386/i386.c:4952 #pragma GCC target("sse4.2") Paolo.
RE: PR 84154: Fix checking -mibt and -mshstk options for control flow protection
> -Original Message- > From: Rainer Orth [mailto:r...@cebitec.uni-bielefeld.de] > Sent: Tuesday, February 6, 2018 11:50 PM > To: Tsimbalist, Igor V > Cc: gcc-patches@gcc.gnu.org; Nick Clifton ; > hjl.to...@gmail.com; Uros Bizjak > Subject: Re: PR 84154: Fix checking -mibt and -mshstk options for control > flow protection > > Hi Igor, > > > Here is the updated patch. Please note the subject should say PR 84145. > > the two new testcases FAIL on all non-x86 targets (I've seen that on > sparc-sun-solaris2.11, there's a gcc-testresults posting for > powerpc64le-unknown-linux-gnu, and PR testsuite/84243 reports it for > aarch64-none-linux-gnu: > > +FAIL: c-c++-common/fcf-protection-6.c -std=gnu++11 (test for errors, > line ) > +FAIL: c-c++-common/fcf-protection-6.c -std=gnu++11 (test for excess > errors) > +FAIL: c-c++-common/fcf-protection-6.c -std=gnu++14 (test for errors, > line ) > +FAIL: c-c++-common/fcf-protection-6.c -std=gnu++14 (test for excess > errors) > +FAIL: c-c++-common/fcf-protection-6.c -std=gnu++98 (test for errors, > line ) > +FAIL: c-c++-common/fcf-protection-6.c -std=gnu++98 (test for excess > errors) > > Excess errors: > xg++: error: unrecognized command line option '-mshstk' > > +FAIL: c-c++-common/fcf-protection-7.c -std=gnu++11 (test for errors, > line ) > +FAIL: c-c++-common/fcf-protection-7.c -std=gnu++11 (test for excess > errors) > +FAIL: c-c++-common/fcf-protection-7.c -std=gnu++14 (test for errors, > line ) > +FAIL: c-c++-common/fcf-protection-7.c -std=gnu++14 (test for excess > errors) > +FAIL: c-c++-common/fcf-protection-7.c -std=gnu++98 (test for errors, > line ) > +FAIL: c-c++-common/fcf-protection-7.c -std=gnu++98 (test for excess > errors) > > Excess errors: > xg++: error: unrecognized command line option '-mibt' > > I think the right way to handle that is to pass -mshstk resp. -mibt on > x86 only. The following patch does this; tested with the appropriate > runtest invocation on i386-pc-solaris2.11 and sparc-sun-solaris2.11. > > Ok for mainline? Agree with the fix. Thanks for taking care of this issue. Igor > Rainer > > -- > - > Rainer Orth, Center for Biotechnology, Bielefeld University > > > 2018-02-06 Rainer Orth > > PR testsuite/84243 > * c-c++-common/fcf-protection-6.c: Only pass -mshstk on x86 > targets. > * c-c++-common/fcf-protection-7.c: Likewise for -mibt.
Re: PR 84154: Fix checking -mibt and -mshstk options for control flow protection
Hi Igor, > Here is the updated patch. Please note the subject should say PR 84145. the two new testcases FAIL on all non-x86 targets (I've seen that on sparc-sun-solaris2.11, there's a gcc-testresults posting for powerpc64le-unknown-linux-gnu, and PR testsuite/84243 reports it for aarch64-none-linux-gnu: +FAIL: c-c++-common/fcf-protection-6.c -std=gnu++11 (test for errors, line ) +FAIL: c-c++-common/fcf-protection-6.c -std=gnu++11 (test for excess errors) +FAIL: c-c++-common/fcf-protection-6.c -std=gnu++14 (test for errors, line ) +FAIL: c-c++-common/fcf-protection-6.c -std=gnu++14 (test for excess errors) +FAIL: c-c++-common/fcf-protection-6.c -std=gnu++98 (test for errors, line ) +FAIL: c-c++-common/fcf-protection-6.c -std=gnu++98 (test for excess errors) Excess errors: xg++: error: unrecognized command line option '-mshstk' +FAIL: c-c++-common/fcf-protection-7.c -std=gnu++11 (test for errors, line ) +FAIL: c-c++-common/fcf-protection-7.c -std=gnu++11 (test for excess errors) +FAIL: c-c++-common/fcf-protection-7.c -std=gnu++14 (test for errors, line ) +FAIL: c-c++-common/fcf-protection-7.c -std=gnu++14 (test for excess errors) +FAIL: c-c++-common/fcf-protection-7.c -std=gnu++98 (test for errors, line ) +FAIL: c-c++-common/fcf-protection-7.c -std=gnu++98 (test for excess errors) Excess errors: xg++: error: unrecognized command line option '-mibt' I think the right way to handle that is to pass -mshstk resp. -mibt on x86 only. The following patch does this; tested with the appropriate runtest invocation on i386-pc-solaris2.11 and sparc-sun-solaris2.11. Ok for mainline? Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University 2018-02-06 Rainer Orth PR testsuite/84243 * c-c++-common/fcf-protection-6.c: Only pass -mshstk on x86 targets. * c-c++-common/fcf-protection-7.c: Likewise for -mibt. # HG changeset patch # Parent bc1af87b4176f6f5ddc900980a18df826cbf4553 Don't pass x86-only options on non-x86 targets in c-c++-common/fcf-protection-[67].c diff --git a/gcc/testsuite/c-c++-common/fcf-protection-6.c b/gcc/testsuite/c-c++-common/fcf-protection-6.c --- a/gcc/testsuite/c-c++-common/fcf-protection-6.c +++ b/gcc/testsuite/c-c++-common/fcf-protection-6.c @@ -1,4 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-fcf-protection=branch -mshstk" } */ +/* { dg-options "-fcf-protection=branch" } */ +/* { dg-additional-options "-mshstk" { target { i?86-*-* x86_64-*-* } } } */ /* { dg-error "'-fcf-protection=branch' requires Intel CET.*-mcet or -mibt option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ /* { 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-7.c b/gcc/testsuite/c-c++-common/fcf-protection-7.c --- a/gcc/testsuite/c-c++-common/fcf-protection-7.c +++ b/gcc/testsuite/c-c++-common/fcf-protection-7.c @@ -1,4 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-fcf-protection=return -mibt" } */ +/* { dg-options "-fcf-protection=return" } */ +/* { dg-additional-options "-mibt" { target { i?86-*-* x86_64-*-* } } } */ /* { dg-error "'-fcf-protection=return' requires Intel CET.*-mcet or -mshstk option" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */ /* { dg-error "'-fcf-protection=return' is not supported for this target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */
Re: PR 84154: Fix checking -mibt and -mshstk options for control flow protection
On Tue, Feb 6, 2018 at 3:08 PM, Tsimbalist, Igor V wrote: >> -Original Message- >> From: Nick Clifton [mailto:ni...@redhat.com] >> Sent: Tuesday, February 6, 2018 1:16 PM >> To: Tsimbalist, Igor V ; hjl.to...@gmail.com >> Cc: gcc-patches@gcc.gnu.org >> Subject: Re: PR 84154: Fix checking -mibt and -mshstk options for control >> flow protection >> >> Hi Igor, >> >> >> Attached is a potential patch for PR 84145: >> >> >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84145 >> >> > Coincidentally, I have worked on the same patch. >> >> Great minds, etc :-) >> >> > Please look at the patch, I uploaded it to the bug. The main differences >> > are >> > >> > - updated the output messages to be more informative; >> > - updated the tests and add couple of new tests to check the messages; >> > - fixed a typo in the doc file related to fcf-protection; >> > >> > I am ok with the changes in i386.c but would like to update the messages. >> Could you incorporate my changes and proceed? Or would you like me to >> finish the fix? >> >> If you are happy to finish the fix then please do so. Your fix is >> more thorough than mine, so I am happy to see it go on. Although >> I should say that I am not an x86 maintainer, so I cannot approve >> it. > > Here is the updated patch. Please note the subject should say PR 84145. > > Ok for trunk? LGTM. Thanks, Uros.
RE: PR 84154: Fix checking -mibt and -mshstk options for control flow protection
> -Original Message- > From: Nick Clifton [mailto:ni...@redhat.com] > Sent: Tuesday, February 6, 2018 1:16 PM > To: Tsimbalist, Igor V ; hjl.to...@gmail.com > Cc: gcc-patches@gcc.gnu.org > Subject: Re: PR 84154: Fix checking -mibt and -mshstk options for control > flow protection > > Hi Igor, > > >> Attached is a potential patch for PR 84145: > >> > >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84145 > > > Coincidentally, I have worked on the same patch. > > Great minds, etc :-) > > > Please look at the patch, I uploaded it to the bug. The main differences are > > > > - updated the output messages to be more informative; > > - updated the tests and add couple of new tests to check the messages; > > - fixed a typo in the doc file related to fcf-protection; > > > > I am ok with the changes in i386.c but would like to update the messages. > Could you incorporate my changes and proceed? Or would you like me to > finish the fix? > > If you are happy to finish the fix then please do so. Your fix is > more thorough than mine, so I am happy to see it go on. Although > I should say that I am not an x86 maintainer, so I cannot approve > it. Here is the updated patch. Please note the subject should say PR 84145. Ok for trunk? > Cheers > Nick > 0001-Fix-checking-mibt-and-mshstk-options-for-control-flo.patch Description: 0001-Fix-checking-mibt-and-mshstk-options-for-control-flo.patch
Re: PR 84154: Fix checking -mibt and -mshstk options for control flow protection
Hi Igor, >> Attached is a potential patch for PR 84145: >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84145 > Coincidentally, I have worked on the same patch. Great minds, etc :-) > Please look at the patch, I uploaded it to the bug. The main differences are > > - updated the output messages to be more informative; > - updated the tests and add couple of new tests to check the messages; > - fixed a typo in the doc file related to fcf-protection; > > I am ok with the changes in i386.c but would like to update the messages. > Could you incorporate my changes and proceed? Or would you like me to finish > the fix? If you are happy to finish the fix then please do so. Your fix is more thorough than mine, so I am happy to see it go on. Although I should say that I am not an x86 maintainer, so I cannot approve it. Cheers Nick
RE: PR 84154: Fix checking -mibt and -mshstk options for control flow protection
> -Original Message- > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > ow...@gcc.gnu.org] On Behalf Of Nick Clifton > Sent: Monday, February 5, 2018 4:15 PM > To: hjl.to...@gmail.com > Cc: gcc-patches@gcc.gnu.org > Subject: RFA: PR 84154: Fix checking -mibt and -mshstk options for control > flow protection > > Hi H.J. > > Attached is a potential patch for PR 84145: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84145 > > The problem was that the code to check that the --mibt and/or -mshstk > options have been correctly enabled for control flow protection did > not take into account that the wrong option might have been enabled. > > So the patch inverts the test, looking at the value of > flag_cf_protection first and then checking to see if the needed x86 > specific options have been enabled. This gives results like this: > > >% gcc -c main.c >% gcc -c main.c -fcf-protection=full > cc1: error: '-fcf-protection=full' requires CET support on this target. Use - > mcet or both of -mibt and -mshstk options to enable CET >% gcc -c main.c -fcf-protection=full -mcet >% gcc -c main.c -fcf-protection=full -mibt > cc1: error: '-fcf-protection=full' requires CET support on this target. Use - > mcet or both of -mibt and -mshstk options to enable CET >% gcc -c main.c -fcf-protection=full -mibt -mshstk >% gcc -c main.c -fcf-protection=branch > cc1: error: '-fcf-protection=branch' requires CET support on this target. Use > - > mcet or -mibt to enable CET >% gcc -c main.c -fcf-protection=branch -mcet >% gcc -c main.c -fcf-protection=branch -mibt >% gcc -c main.c -fcf-protection=branch -mshstk > cc1: error: '-fcf-protection=branch' requires CET support on this target. Use > - > mcet or -mibt to enable CET >% gcc -c main.c -fcf-protection=return > cc1: error: '-fcf-protection=return' requires CET support on this target. Use > - > mcet or -mshstk to enable CET >% gcc -c main.c -fcf-protection=return -mcet >% gcc -c main.c -fcf-protection=return -mibt > cc1: error: '-fcf-protection=return' requires CET support on this target. Use > - > mcet or -mshstk to enable CET >% gcc -c main.c -fcf-protection=return -mshstk >% > > What do you think ? Is the patch OK for the mainline ? Coincidentally, I have worked on the same patch. Please look at the patch, I uploaded it to the bug. The main differences are - updated the output messages to be more informative; - updated the tests and add couple of new tests to check the messages; - fixed a typo in the doc file related to fcf-protection; I am ok with the changes in i386.c but would like to update the messages. Could you incorporate my changes and proceed? Or would you like me to finish the fix? Thanks, Igor > Cheers > Nick > > gcc/ChangeLog > 2018-02-05 Nick Clifton > > PR 84145 > * config/i386/i386.c (ix86_option_override_internal): Rework > checks for -fcf-protection and -mibt/-mshstk. > > Index: gcc/config/i386/i386.c > === > > --- gcc/config/i386/i386.c(revision 257389) > +++ gcc/config/i386/i386.c(working copy) > @@ -4915,30 +4915,43 @@ >/* Do not support control flow instrumentation if CET is not enabled. */ >if (opts->x_flag_cf_protection != CF_NONE) > { > - if (!(TARGET_IBT_P (opts->x_ix86_isa_flags2) > - || TARGET_SHSTK_P (opts->x_ix86_isa_flags))) > + switch (flag_cf_protection) > { > - if (flag_cf_protection == CF_FULL) > + case CF_NONE: > + break; > + case CF_BRANCH: > + if (! TARGET_IBT_P (opts->x_ix86_isa_flags2)) > { > - error ("%<-fcf-protection=full%> requires CET support " > - "on this target. Use -mcet or one of -mibt, " > - "-mshstk options to enable CET"); > + error ("%<-fcf-protection=branch%> requires CET support " > + "on this target. Use -mcet or -mibt to enable CET"); > + flag_cf_protection = CF_NONE; > + return false; > } > - else if (flag_cf_protection == CF_BRANCH) > + break; > + case CF_RETURN: > + if (! TARGET_SHSTK_P (opts->x_ix86_isa_flags)) > { > - error ("%<-fcf-protection=branch%> requires CET support " > - "on this target. Use -mcet or one of -mibt, " > - "-mshstk options to enable CET"); > + error ("%<-fcf-p
RFA: PR 84154: Fix checking -mibt and -mshstk options for control flow protection
Hi H.J. Attached is a potential patch for PR 84145: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84145 The problem was that the code to check that the --mibt and/or -mshstk options have been correctly enabled for control flow protection did not take into account that the wrong option might have been enabled. So the patch inverts the test, looking at the value of flag_cf_protection first and then checking to see if the needed x86 specific options have been enabled. This gives results like this: % gcc -c main.c % gcc -c main.c -fcf-protection=full cc1: error: '-fcf-protection=full' requires CET support on this target. Use -mcet or both of -mibt and -mshstk options to enable CET % gcc -c main.c -fcf-protection=full -mcet % gcc -c main.c -fcf-protection=full -mibt cc1: error: '-fcf-protection=full' requires CET support on this target. Use -mcet or both of -mibt and -mshstk options to enable CET % gcc -c main.c -fcf-protection=full -mibt -mshstk % gcc -c main.c -fcf-protection=branch cc1: error: '-fcf-protection=branch' requires CET support on this target. Use -mcet or -mibt to enable CET % gcc -c main.c -fcf-protection=branch -mcet % gcc -c main.c -fcf-protection=branch -mibt % gcc -c main.c -fcf-protection=branch -mshstk cc1: error: '-fcf-protection=branch' requires CET support on this target. Use -mcet or -mibt to enable CET % gcc -c main.c -fcf-protection=return cc1: error: '-fcf-protection=return' requires CET support on this target. Use -mcet or -mshstk to enable CET % gcc -c main.c -fcf-protection=return -mcet % gcc -c main.c -fcf-protection=return -mibt cc1: error: '-fcf-protection=return' requires CET support on this target. Use -mcet or -mshstk to enable CET % gcc -c main.c -fcf-protection=return -mshstk % What do you think ? Is the patch OK for the mainline ? Cheers Nick gcc/ChangeLog 2018-02-05 Nick Clifton PR 84145 * config/i386/i386.c (ix86_option_override_internal): Rework checks for -fcf-protection and -mibt/-mshstk. Index: gcc/config/i386/i386.c === --- gcc/config/i386/i386.c (revision 257389) +++ gcc/config/i386/i386.c (working copy) @@ -4915,30 +4915,43 @@ /* Do not support control flow instrumentation if CET is not enabled. */ if (opts->x_flag_cf_protection != CF_NONE) { - if (!(TARGET_IBT_P (opts->x_ix86_isa_flags2) - || TARGET_SHSTK_P (opts->x_ix86_isa_flags))) + switch (flag_cf_protection) { - if (flag_cf_protection == CF_FULL) + case CF_NONE: + break; + case CF_BRANCH: + if (! TARGET_IBT_P (opts->x_ix86_isa_flags2)) { - error ("%<-fcf-protection=full%> requires CET support " -"on this target. Use -mcet or one of -mibt, " -"-mshstk options to enable CET"); + error ("%<-fcf-protection=branch%> requires CET support " +"on this target. Use -mcet or -mibt to enable CET"); + flag_cf_protection = CF_NONE; + return false; } - else if (flag_cf_protection == CF_BRANCH) + break; + case CF_RETURN: + if (! TARGET_SHSTK_P (opts->x_ix86_isa_flags)) { - error ("%<-fcf-protection=branch%> requires CET support " -"on this target. Use -mcet or one of -mibt, " -"-mshstk options to enable CET"); + error ("%<-fcf-protection=return%> requires CET support " +"on this target. Use -mcet or -mshstk to enable CET"); + flag_cf_protection = CF_NONE; + return false; } - else if (flag_cf_protection == CF_RETURN) + break; + case CF_FULL: + if ( ! TARGET_IBT_P (opts->x_ix86_isa_flags2) +|| ! TARGET_SHSTK_P (opts->x_ix86_isa_flags)) { - error ("%<-fcf-protection=return%> requires CET support " -"on this target. Use -mcet or one of -mibt, " + error ("%<-fcf-protection=full%> requires CET support " +"on this target. Use -mcet or both of -mibt and " "-mshstk options to enable CET"); + flag_cf_protection = CF_NONE; + return false; } - flag_cf_protection = CF_NONE; - return false; + break; + default: + gcc_unreachable (); } + opts->x_flag_cf_protection = (cf_protection_level) (opts->x_flag_cf_protection | CF_SET); }