> -----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 <ni...@redhat.com> > > 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); > }