From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x62d.google.com (mail-pl1-x62d.google.com [IPv6:2607:f8b0:4864:20::62d]) by sourceware.org (Postfix) with ESMTPS id ABBA83858D39 for ; Fri, 12 May 2023 05:50:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org ABBA83858D39 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-pl1-x62d.google.com with SMTP id d9443c01a7336-1aaf2ede38fso92161385ad.2 for ; Thu, 11 May 2023 22:50:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1683870603; x=1686462603; 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=jpcOuqH7wyip/N4l4AMWS27+JDuzL1J8SOAsDedXizU=; b=UdP+ShLJIUM7aP52VMvF3CFkXiffwjj5lxkK3LImnwnS6Ex7aEBUhnYySjxSeKxyO/ VuizKcqCDCzg2WT68bByjN1/DvHQLkmhVBbmrFhxqd3a9nHSBwgN1uJAJ3e1LbVDfDtH dr5Rif+mQTGJqqgENTxV0fO0A/YelpMyKZqq/nUhgVHiCIS3/kWPDaH1gbv+vipS6EWN Gpq41ToVz6Hn9ic10pT+Np06YL1Zmn8W1vomNlIjzHnE9mpzO3DZ6lf2CjD/KA4e/Agh kn3EQeugXVP+kA0zGDbG3liPOb8Xs5BDL2g7hPMiE6jzGDlQJgrAfIG9Q4YzFBlxwY7F LQmQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683870603; x=1686462603; 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=jpcOuqH7wyip/N4l4AMWS27+JDuzL1J8SOAsDedXizU=; b=Ce/gOpqXRukI039aCR4EMsQnV+QSEmqMGctxt8C68HTjY9mojvZI5c/NwMj2Sv3ETV TkiQcFksjwGel8j7HwyE8Oc0L5KLdoT+zy/rJdPOVaC2C1heMXSB7JNGkXSHpNhPWOri DTBr9pmOGPVXSMs1QqZFa9KE4hOIPj5E1bpdcaQ0SQxqi+Fp+nhXNNwaQvV0wRDKqnBz n7dnpkrv5Y321b2T38XoKX3UPhNEuu57w9ax3aHbofjQjnuF728LQj47dHkDXXG5ozmL 7MAhAGUzLT+F7HpTQnGb7jOeVbsnkf7p6zS2dKAkTVXxBQtdqAxRfGd640Mj3sT0Oe8v Ss0Q== X-Gm-Message-State: AC+VfDyMhqWS3ch2+xOuJZof2WkOEl1J+2DYx1dpmOCQDWK68YpNWTXd kW8X00uCWvEB1/X6wtoAv8NJtUYXYbJ7NmQA7/o= X-Google-Smtp-Source: ACHHUZ6TcHIlNLHB2yQn+3e+lWMhgqD6s0ExQ2UZaWh10J0xzj1rHZn8HubIBnYO5Jxvm+7JB3snseICsPFP8b08veE= X-Received: by 2002:a17:902:ec83:b0:1ac:4d01:dff8 with SMTP id x3-20020a170902ec8300b001ac4d01dff8mr29452408plg.45.1683870602523; Thu, 11 May 2023 22:50:02 -0700 (PDT) MIME-Version: 1.0 References: <20230512054213.2211594-1-hongtao.liu@intel.com> In-Reply-To: <20230512054213.2211594-1-hongtao.liu@intel.com> From: Andrew Pinski Date: Thu, 11 May 2023 22:49:50 -0700 Message-ID: Subject: Re: [PATCH] Provide -fcf-protection=branch,return. To: liuhongt Cc: gcc-patches@gcc.gnu.org, crazylht@gmail.com, hjl.tools@gmail.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.4 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 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 combination > 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, 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_prote= ction) Init(CF_NONE) > +Common Joined > -fcf-protection=3D[full|branch|return|none|check] Instrument func= tions 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(un= known 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 &decoded= _options, > case OPT_fcf_protection_: > /* Default to link-time option, else append or check identical.= */ > 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->ar= g))) > { > 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_fragmen= t &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 combinati= on"); > + 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 opt_= 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/testsui= te/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 not= valid" "" { target *-*-* } 0 } */ > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-11.c b/gcc/testsui= te/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/testsui= te/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/testsuit= e/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 ign= ored" "" { target { *-*-* } } 0 } */ > diff --git a/gcc/testsuite/c-c++-common/fcf-protection-9.c b/gcc/testsuit= e/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 i= nstead of in a combination" "" { target { *-*-* } } 0 } */ > diff --git a/gcc/testsuite/gcc.target/i386/pr89701-1.c b/gcc/testsuite/gc= c.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/gc= c.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/gc= c.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 ign= ored" "" { 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/gc= c.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 ign= ored" "" { target { *-*-* } } 0 } */ > +/* { dg-final { scan-assembler-times ".note.gnu.property" 1 } } */ > +/* { dg-final { scan-assembler-times ".long 0x1" 1 } } */ > -- > 2.39.1.388.g2fc9e9ca3c >