From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb1-xb2a.google.com (mail-yb1-xb2a.google.com [IPv6:2607:f8b0:4864:20::b2a]) by sourceware.org (Postfix) with ESMTPS id 6B9BE3858D39 for ; Fri, 12 May 2023 06:21:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6B9BE3858D39 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-yb1-xb2a.google.com with SMTP id 3f1490d57ef6-ba6954e8160so2606535276.2 for ; Thu, 11 May 2023 23:21:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1683872517; x=1686464517; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=/UyIx+qcBj3cey2lxWF4d3B6gP6Qgufgzv+sshhu99s=; b=Jw0SW9UQOrx9KX6uSJUph4kkXNGrejV67rhTeQZI3biuerHZrTYycULcTiNnpv3Olt k+ZKK4yKrcfD3Csp5Scv5TMKxCPvSFfjcZtkBT5jXicvRYuI6T90KKmzFci57cAMeE5V a1DsQCkw6yw1JaCRnDrwomEtEKbzTAojXo9o8ScJDXZr+K0c3g4UeLdazXfQK4/4wwwS laqgOoFZRpliJct5T6up7OZUOsjyuwNUGJXrt71+PXSSBqCQipLE+Jso3/XF6MXdm47N 40YV01uab6f+d/smprFdQnAknxC/nVCIUj9xiJzu6wo+h06rza3efFo5/xm5bwO41Fhh ZBgg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683872517; x=1686464517; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=/UyIx+qcBj3cey2lxWF4d3B6gP6Qgufgzv+sshhu99s=; b=jo84tLKqJ8ZuPZnMDF27/am+/q4CkuMwJ7Es8t9WlHVynpbIF2dOwQt+lfCunZG0Cb Smmj9PdgozLzLVYoOHVfGOVlzwnZlNtNXDk/gPN3ACsY3dy+MLzB643RN0cD1Fv0crDo m0nhhD3FWEuwgmb9wJbK9ei7CdQ81rd+bYpIaHNMPfRC4nMhQ4IL1UwQFlHT0E4EWcC6 EAnfv2gr3jYuS14JIraKE/QgKYCXa6K/hw44OaXGuaQ9yI57iqvzHMkJzOuP8T78qTRg p3GclWZPVPJh5Gexm3whjQ6a+4oHmzt4IGRXJDH9k5S8FR5UvPnOFYFvDG5d7YfWgBWX OMFQ== X-Gm-Message-State: AC+VfDxJSA0y/byaSKLnVnHgeImH+g0Y3SD6yW/UBDVFhjhI/AymmwSS vIkrawl16b+mVRQ6OtOuZWOtS8ePgfpck5W62y0= X-Google-Smtp-Source: ACHHUZ6mHFTZN7ISlg/VUr/crXRGoIPv2KUt5DAL0YIMAlr4lklpXDV/Kw1dMadVPnyL0rcGnD48kggCshC/UWMl3/s= X-Received: by 2002:a25:20d5:0:b0:b9d:b255:d726 with SMTP id g204-20020a2520d5000000b00b9db255d726mr20565940ybg.42.1683872516677; Thu, 11 May 2023 23:21:56 -0700 (PDT) MIME-Version: 1.0 References: <20230512054213.2211594-1-hongtao.liu@intel.com> In-Reply-To: From: Hongtao Liu Date: Fri, 12 May 2023 14:21:45 +0800 Message-ID: Subject: Re: [PATCH] Provide -fcf-protection=branch,return. To: Andrew Pinski Cc: liuhongt , gcc-patches@gcc.gnu.org, hjl.tools@gmail.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Fri, May 12, 2023 at 1:50=E2=80=AFPM Andrew Pinski w= rote: > > On Thu, May 11, 2023 at 10:45=E2=80=AFPM liuhongt via Gcc-patches > wrote: > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}. > > Ok for trunk? > > > > gcc/ChangeLog: > > > > PR target/89701 > > * common.opt: Refactor -fcf-protection=3D to support combinatio= n > > of param. > > * lto-wrapper.c (merge_and_complain): Adjusted. > > * opts.c (parse_cf_protection_options): New. > > (common_handle_option): Decode argument for -fcf-protection=3D. > > * 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=3D`. 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 =3D false > > Variable > > unsigned int flag_zero_call_used_regs > > > > +;; What the CF check should instrument > > +Variable > > +unsigned int flag_cf_protection =3D 0 > > + > > ### > > Driver > > > > @@ -1886,28 +1890,10 @@ fcf-protection > > Common RejectNegative Alias(fcf-protection=3D,full) > > > > fcf-protection=3D > > -Common Joined RejectNegative Enum(cf_protection_level) Var(flag_cf_pro= tection) Init(CF_NONE) > > +Common Joined > > -fcf-protection=3D[full|branch|return|none|check] Instrument fu= nctions 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 &decod= ed_options, > > case OPT_fcf_protection_: > > /* Default to link-time option, else append or check identica= l. */ > > if (!cf_protection_option > > - || cf_protection_option->value =3D=3D CF_CHECK) > > + || !memcmp (cf_protection_option->arg, "check", 5)) > > { > > + const char* parg =3D decoded_options[existing_opt].arg; > > if (existing_opt =3D=3D -1) > > decoded_options.safe_push (*foption); > > - else if (decoded_options[existing_opt].value !=3D foption= ->value) > > + else if ((strlen (parg) !=3D strlen (foption->arg)) > > + || memcmp (parg, foption->arg, strlen (foption->= arg))) > > { > > if (cf_protection_option > > - && cf_protection_option->value =3D=3D 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 > > - &=3D (foption->value & CF_FULL); > > - switch (decoded_options[existing_opt].value) > > + HOST_WIDE_INT flags1 > > + =3D parse_cf_protection_options (foption->arg, > > + input_location); > > + HOST_WIDE_INT flags2 > > + =3D parse_cf_protection_options (parg, > > + input_location); > > + flags2 &=3D (flags1 & CF_FULL); > > + switch (flags2) > > { > > case CF_NONE: > > decoded_options[existing_opt].arg =3D "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_fragm= ent &arg, > > return bm.get_best_meaningful_candidate (); > > } > > > > +unsigned int > > +parse_cf_protection_options (const char *p, location_t loc) > > +{ > > + unsigned int flags =3D 0; > > + bool combined =3D false; > > + while (*p !=3D 0) > > + { > > + size_t len; > > + const char *comma =3D strchr (p, ','); > > + if (comma =3D=3D NULL) > > + len =3D strlen (p); > > + else > > + { > > + combined =3D true; > > + len =3D comma - p; > > + } > > + > > + if (len =3D=3D 0) > > + { > > + p =3D comma + 1; > > + continue; > > + } > > + > > + switch (len) > > + { > > + case 4: > > + if (!memcmp (p, "full", 4)) > > + { > > + if (combined && flags !=3D CF_NONE) > > + warning_at (loc, 0, "better to use %<-fcf-protection=3D= %> " > > + "option: full alone instead of in a combina= tion"); > > + flags |=3D CF_FULL; > > + } > > + else if (!memcmp (p, "none", 4)) > > + { > > + if (combined && flags !=3D CF_NONE) > > + warning_at (loc, 0, "combination of %<-fcf-protection= =3D%> " > > + "option: none is ignored"); > > + } > > + else > > + error_at (loc, "unrecognized argument to %<-fcf-protection= =3D%> " > > + "option: %.4s", p); > > + break; > > + case 5: > > + if (!memcmp (p, "check", 5)) > > + { > > + if (combined && flags !=3D CF_CHECK) > > + error_at (loc, "Combination for %<-fcf-protection=3D%> = " > > + "option: check is not valid"); > > + flags |=3D CF_CHECK; > > + } > > + else > > + error_at (loc, "unrecognized argument to %<-fcf-protection= =3D%> " > > + "option: %.5s", p); > > + break; > > + case 6: > > + if (!memcmp (p, "branch", 6)) > > + flags |=3D CF_BRANCH; > > + else if (!memcmp (p, "return", 6)) > > + flags |=3D CF_RETURN; > > + else > > + error_at (loc, "unrecognized argument to %<-fcf-protection= =3D%> " > > + "option: %.6s", p); > > + break; > > + default: > > + error_at (loc, "unrecognized argument to %<-fcf-protection=3D= %> " > > + "option: %.*s", (int) len, p); > > + } > > + > > + if (comma =3D=3D NULL) > > + break; > > + p =3D 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 > > + =3D parse_cf_protection_options (arg, loc); > > + break; > > case OPT_fsanitize_: > > opts_set->x_flag_sanitize =3D 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 op= t_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/tests= uite/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=3Dbranch,check" } */ > > +/* { dg-error "Combination for '-fcf-protection=3D' option: check is n= ot valid" "" { target *-*-* } 0 } */ > > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-11.c b/gcc/tests= uite/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=3Dbranch,return" } */ > > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-12.c b/gcc/tests= uite/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=3Dreturn,branch" } */ > > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-8.c b/gcc/testsu= ite/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=3Dbranch,none" } */ > > +/* { dg-warning "combination of '-fcf-protection=3D' option: none is i= gnored" "" { target { *-*-* } } 0 } */ > > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-9.c b/gcc/testsu= ite/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=3Dbranch,full" } */ > > +/* { dg-warning "better to use '-fcf-protection=3D' 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=3Dbranch,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=3Dreturn,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=3Dreturn,none" } */ > > +/* { dg-warning "combination of '-fcf-protection=3D' option: none is i= gnored" "" { 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=3Dbranch,none" } */ > > +/* { dg-warning "combination of '-fcf-protection=3D' option: none is i= gnored" "" { target { *-*-* } } 0 } */ > > +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */ > > +/* { dg-final { scan-assembler-times ".long 0x1" 1 } } */ > > -- > > 2.39.1.388.g2fc9e9ca3c > > --=20 BR, Hongtao