Simplify option handling for -fsanitize-coverage

Message ID 7032aba0-c3ff-600f-f881-7f149728564f@suse.cz
State New
Headers show
Series
  • Simplify option handling for -fsanitize-coverage
Related show

Commit Message

Martin Liška May 20, 2021, 10:43 a.m.
The simplification patch improves option completion and
handling of the option.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

	* common.opt: Use proper Enum values.
	* opts.c (COVERAGE_SANITIZER_OPT): Remove.
	(parse_sanitizer_options): Handle only sanitizer_opts.
	(common_handle_option): Just assign value.

gcc/testsuite/ChangeLog:

	* gcc.dg/spellcheck-options-23.c: New test.
---
  gcc/common.opt                               | 11 +++++-
  gcc/opts.c                                   | 41 +++++---------------
  gcc/testsuite/gcc.dg/spellcheck-options-23.c |  5 +++
  3 files changed, 25 insertions(+), 32 deletions(-)
  create mode 100644 gcc/testsuite/gcc.dg/spellcheck-options-23.c

-- 
2.31.1

Comments

Ard Biesheuvel via Gcc-patches May 20, 2021, 1:26 p.m. | #1
On 20 May 2021 12:43:17 CEST, "Martin Liška" <mliska@suse.cz> wrote:

>  /* Given ARG, an unrecognized sanitizer option, return the best

>     matching sanitizer option, or NULL if there isn't one.

>     OPTS is array of candidate sanitizer options.

>-   CODE is OPT_fsanitize_, OPT_fsanitize_recover_ or

>-   OPT_fsanitize_coverage_.

>+   CODE is OPT_fsanitize_or OPT_fsanitize_recover_.


Shouldn't be there a space before "or" in OPT_fsanitize_or ?
thanks,
Martin Liška May 20, 2021, 1:29 p.m. | #2
On 5/20/21 3:26 PM, Bernhard Reutner-Fischer wrote:
> On 20 May 2021 12:43:17 CEST, "Martin Liška" <mliska@suse.cz> wrote:

> 

>>   /* Given ARG, an unrecognized sanitizer option, return the best

>>      matching sanitizer option, or NULL if there isn't one.

>>      OPTS is array of candidate sanitizer options.

>> -   CODE is OPT_fsanitize_, OPT_fsanitize_recover_ or

>> -   OPT_fsanitize_coverage_.

>> +   CODE is OPT_fsanitize_or OPT_fsanitize_recover_.

> 

> Shouldn't be there a space before "or" in OPT_fsanitize_or ?


Yes, please.

> thanks,

>

Thanks,
Martin
Martin Liška May 31, 2021, 10:46 a.m. | #3
PING^1

On 5/20/21 12:43 PM, Martin Liška wrote:
> The simplification patch improves option completion and

> handling of the option.

> 

> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

> 

> Ready to be installed?

> Thanks,

> Martin

> 

> gcc/ChangeLog:

> 

>      * common.opt: Use proper Enum values.

>      * opts.c (COVERAGE_SANITIZER_OPT): Remove.

>      (parse_sanitizer_options): Handle only sanitizer_opts.

>      (common_handle_option): Just assign value.

> 

> gcc/testsuite/ChangeLog:

> 

>      * gcc.dg/spellcheck-options-23.c: New test.

> ---

>   gcc/common.opt                               | 11 +++++-

>   gcc/opts.c                                   | 41 +++++---------------

>   gcc/testsuite/gcc.dg/spellcheck-options-23.c |  5 +++

>   3 files changed, 25 insertions(+), 32 deletions(-)

>   create mode 100644 gcc/testsuite/gcc.dg/spellcheck-options-23.c

> 

> diff --git a/gcc/common.opt b/gcc/common.opt

> index a75b44ee47e..de92e3e37aa 100644

> --- a/gcc/common.opt

> +++ b/gcc/common.opt

> @@ -1037,9 +1037,18 @@ Common Driver Joined

>   Select what to sanitize.

> 

>   fsanitize-coverage=

> -Common Joined

> +Common Joined RejectNegative Enum(sanitize_coverage)

>   Select type of coverage sanitization.

> 

> +Enum

> +Name(sanitize_coverage) Type(int)

> +

> +EnumValue

> +Enum(sanitize_coverage) String(trace-pc) Value(SANITIZE_COV_TRACE_PC)

> +

> +EnumValue

> +Enum(sanitize_coverage) String(trace-cmp) Value(SANITIZE_COV_TRACE_CMP)

> +

>   fasan-shadow-offset=

>   Common Joined RejectNegative Var(common_deferred_options) Defer

>   -fasan-shadow-offset=<number>    Use custom shadow memory offset.

> diff --git a/gcc/opts.c b/gcc/opts.c

> index fe6fddbf095..282da84f286 100644

> --- a/gcc/opts.c

> +++ b/gcc/opts.c

> @@ -1817,17 +1817,6 @@ const struct sanitizer_opts_s sanitizer_opts[] =

>     { NULL, 0U, 0UL, false }

>   };

> 

> -/* -f{,no-}sanitize-coverage= suboptions.  */

> -const struct sanitizer_opts_s coverage_sanitizer_opts[] =

> -{

> -#define COVERAGE_SANITIZER_OPT(name, flags) \

> -    { #name, flags, sizeof #name - 1, true }

> -  COVERAGE_SANITIZER_OPT (trace-pc, SANITIZE_COV_TRACE_PC),

> -  COVERAGE_SANITIZER_OPT (trace-cmp, SANITIZE_COV_TRACE_CMP),

> -#undef COVERAGE_SANITIZER_OPT

> -  { NULL, 0U, 0UL, false }

> -};

> -

>   /* -fzero-call-used-regs= suboptions.  */

>   const struct zero_call_used_regs_opts_s zero_call_used_regs_opts[] =

>   {

> @@ -1878,8 +1867,7 @@ struct edit_distance_traits<const string_fragment &>

>   /* Given ARG, an unrecognized sanitizer option, return the best

>      matching sanitizer option, or NULL if there isn't one.

>      OPTS is array of candidate sanitizer options.

> -   CODE is OPT_fsanitize_, OPT_fsanitize_recover_ or

> -   OPT_fsanitize_coverage_.

> +   CODE is OPT_fsanitize_or OPT_fsanitize_recover_.

>      VALUE is non-zero for the regular form of the option, zero

>      for the "no-" form (e.g. "-fno-sanitize-recover=").  */

> 

> @@ -1919,12 +1907,6 @@ parse_sanitizer_options (const char *p, location_t loc, int scode,

>   {

>     enum opt_code code = (enum opt_code) scode;

> 

> -  const struct sanitizer_opts_s *opts;

> -  if (code == OPT_fsanitize_coverage_)

> -    opts = coverage_sanitizer_opts;

> -  else

> -    opts = sanitizer_opts;

> -

>     while (*p != 0)

>       {

>         size_t len, i;

> @@ -1942,11 +1924,12 @@ parse_sanitizer_options (const char *p, location_t loc, int scode,

>       }

> 

>         /* Check to see if the string matches an option class name.  */

> -      for (i = 0; opts[i].name != NULL; ++i)

> -    if (len == opts[i].len && memcmp (p, opts[i].name, len) == 0)

> +      for (i = 0; sanitizer_opts[i].name != NULL; ++i)

> +    if (len == sanitizer_opts[i].len

> +        && memcmp (p, sanitizer_opts[i].name, len) == 0)

>         {

>           /* Handle both -fsanitize and -fno-sanitize cases.  */

> -        if (value && opts[i].flag == ~0U)

> +        if (value && sanitizer_opts[i].flag == ~0U)

>             {

>           if (code == OPT_fsanitize_)

>             {

> @@ -1963,14 +1946,14 @@ parse_sanitizer_options (const char *p, location_t loc, int scode,

>              -fsanitize-recover=return if -fsanitize-recover=undefined

>              is selected.  */

>           if (code == OPT_fsanitize_recover_

> -            && opts[i].flag == SANITIZE_UNDEFINED)

> +            && sanitizer_opts[i].flag == SANITIZE_UNDEFINED)

>             flags |= (SANITIZE_UNDEFINED

>                   & ~(SANITIZE_UNREACHABLE | SANITIZE_RETURN));

>           else

> -          flags |= opts[i].flag;

> +          flags |= sanitizer_opts[i].flag;

>             }

>           else

> -          flags &= ~opts[i].flag;

> +          flags &= ~sanitizer_opts[i].flag;

>           found = true;

>           break;

>         }

> @@ -1979,13 +1962,11 @@ parse_sanitizer_options (const char *p, location_t loc, int scode,

>       {

>         const char *hint

>           = get_closest_sanitizer_option (string_fragment (p, len),

> -                        opts, code, value);

> +                        sanitizer_opts, code, value);

> 

>         const char *suffix;

>         if (code == OPT_fsanitize_recover_)

>           suffix = "-recover";

> -      else if (code == OPT_fsanitize_coverage_)

> -        suffix = "-coverage";

>         else

>           suffix = "";

> 

> @@ -2436,9 +2417,7 @@ common_handle_option (struct gcc_options *opts,

>         break;

> 

>       case OPT_fsanitize_coverage_:

> -      opts->x_flag_sanitize_coverage

> -    = parse_sanitizer_options (arg, loc, code,

> -                   opts->x_flag_sanitize_coverage, value, true);

> +      opts->x_flag_sanitize_coverage = value;

>         break;

> 

>       case OPT_O:

> diff --git a/gcc/testsuite/gcc.dg/spellcheck-options-23.c b/gcc/testsuite/gcc.dg/spellcheck-options-23.c

> new file mode 100644

> index 00000000000..575a28da504

> --- /dev/null

> +++ b/gcc/testsuite/gcc.dg/spellcheck-options-23.c

> @@ -0,0 +1,5 @@

> +/* { dg-do compile } */

> +/* { dg-options "-fsanitize-coverage=tracecmp" } */

> +

> +/* { dg-error "unrecognized argument in option '-fsanitize-coverage=tracecmp'" "" { target *-*-* } 0 } */

> +/* { dg-message "valid arguments to '-fsanitize-coverage=' are: trace-cmp trace-pc; did you mean 'trace-cmp'?" "" { target *-*-* } 0 } */
Ard Biesheuvel via Gcc-patches June 2, 2021, 6:50 p.m. | #4
On 5/31/2021 4:46 AM, Martin Liška wrote:
> PING^1

>

> On 5/20/21 12:43 PM, Martin Liška wrote:

>> The simplification patch improves option completion and

>> handling of the option.

>>

>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

>>

>> Ready to be installed?

>> Thanks,

>> Martin

>>

>> gcc/ChangeLog:

>>

>>      * common.opt: Use proper Enum values.

>>      * opts.c (COVERAGE_SANITIZER_OPT): Remove.

>>      (parse_sanitizer_options): Handle only sanitizer_opts.

>>      (common_handle_option): Just assign value.

>>

>> gcc/testsuite/ChangeLog:

>>

>>      * gcc.dg/spellcheck-options-23.c: New test.

OK
jeff

Patch

diff --git a/gcc/common.opt b/gcc/common.opt
index a75b44ee47e..de92e3e37aa 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1037,9 +1037,18 @@  Common Driver Joined
  Select what to sanitize.
  
  fsanitize-coverage=
-Common Joined
+Common Joined RejectNegative Enum(sanitize_coverage)
  Select type of coverage sanitization.
  
+Enum
+Name(sanitize_coverage) Type(int)
+
+EnumValue
+Enum(sanitize_coverage) String(trace-pc) Value(SANITIZE_COV_TRACE_PC)
+
+EnumValue
+Enum(sanitize_coverage) String(trace-cmp) Value(SANITIZE_COV_TRACE_CMP)
+
  fasan-shadow-offset=
  Common Joined RejectNegative Var(common_deferred_options) Defer
  -fasan-shadow-offset=<number>	Use custom shadow memory offset.
diff --git a/gcc/opts.c b/gcc/opts.c
index fe6fddbf095..282da84f286 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1817,17 +1817,6 @@  const struct sanitizer_opts_s sanitizer_opts[] =
    { NULL, 0U, 0UL, false }
  };
  
-/* -f{,no-}sanitize-coverage= suboptions.  */
-const struct sanitizer_opts_s coverage_sanitizer_opts[] =
-{
-#define COVERAGE_SANITIZER_OPT(name, flags) \
-    { #name, flags, sizeof #name - 1, true }
-  COVERAGE_SANITIZER_OPT (trace-pc, SANITIZE_COV_TRACE_PC),
-  COVERAGE_SANITIZER_OPT (trace-cmp, SANITIZE_COV_TRACE_CMP),
-#undef COVERAGE_SANITIZER_OPT
-  { NULL, 0U, 0UL, false }
-};
-
  /* -fzero-call-used-regs= suboptions.  */
  const struct zero_call_used_regs_opts_s zero_call_used_regs_opts[] =
  {
@@ -1878,8 +1867,7 @@  struct edit_distance_traits<const string_fragment &>
  /* Given ARG, an unrecognized sanitizer option, return the best
     matching sanitizer option, or NULL if there isn't one.
     OPTS is array of candidate sanitizer options.
-   CODE is OPT_fsanitize_, OPT_fsanitize_recover_ or
-   OPT_fsanitize_coverage_.
+   CODE is OPT_fsanitize_or OPT_fsanitize_recover_.
     VALUE is non-zero for the regular form of the option, zero
     for the "no-" form (e.g. "-fno-sanitize-recover=").  */
  
@@ -1919,12 +1907,6 @@  parse_sanitizer_options (const char *p, location_t loc, int scode,
  {
    enum opt_code code = (enum opt_code) scode;
  
-  const struct sanitizer_opts_s *opts;
-  if (code == OPT_fsanitize_coverage_)
-    opts = coverage_sanitizer_opts;
-  else
-    opts = sanitizer_opts;
-
    while (*p != 0)
      {
        size_t len, i;
@@ -1942,11 +1924,12 @@  parse_sanitizer_options (const char *p, location_t loc, int scode,
  	}
  
        /* Check to see if the string matches an option class name.  */
-      for (i = 0; opts[i].name != NULL; ++i)
-	if (len == opts[i].len && memcmp (p, opts[i].name, len) == 0)
+      for (i = 0; sanitizer_opts[i].name != NULL; ++i)
+	if (len == sanitizer_opts[i].len
+	    && memcmp (p, sanitizer_opts[i].name, len) == 0)
  	  {
  	    /* Handle both -fsanitize and -fno-sanitize cases.  */
-	    if (value && opts[i].flag == ~0U)
+	    if (value && sanitizer_opts[i].flag == ~0U)
  	      {
  		if (code == OPT_fsanitize_)
  		  {
@@ -1963,14 +1946,14 @@  parse_sanitizer_options (const char *p, location_t loc, int scode,
  		   -fsanitize-recover=return if -fsanitize-recover=undefined
  		   is selected.  */
  		if (code == OPT_fsanitize_recover_
-		    && opts[i].flag == SANITIZE_UNDEFINED)
+		    && sanitizer_opts[i].flag == SANITIZE_UNDEFINED)
  		  flags |= (SANITIZE_UNDEFINED
  			    & ~(SANITIZE_UNREACHABLE | SANITIZE_RETURN));
  		else
-		  flags |= opts[i].flag;
+		  flags |= sanitizer_opts[i].flag;
  	      }
  	    else
-	      flags &= ~opts[i].flag;
+	      flags &= ~sanitizer_opts[i].flag;
  	    found = true;
  	    break;
  	  }
@@ -1979,13 +1962,11 @@  parse_sanitizer_options (const char *p, location_t loc, int scode,
  	{
  	  const char *hint
  	    = get_closest_sanitizer_option (string_fragment (p, len),
-					    opts, code, value);
+					    sanitizer_opts, code, value);
  
  	  const char *suffix;
  	  if (code == OPT_fsanitize_recover_)
  	    suffix = "-recover";
-	  else if (code == OPT_fsanitize_coverage_)
-	    suffix = "-coverage";
  	  else
  	    suffix = "";
  
@@ -2436,9 +2417,7 @@  common_handle_option (struct gcc_options *opts,
        break;
  
      case OPT_fsanitize_coverage_:
-      opts->x_flag_sanitize_coverage
-	= parse_sanitizer_options (arg, loc, code,
-				   opts->x_flag_sanitize_coverage, value, true);
+      opts->x_flag_sanitize_coverage = value;
        break;
  
      case OPT_O:
diff --git a/gcc/testsuite/gcc.dg/spellcheck-options-23.c b/gcc/testsuite/gcc.dg/spellcheck-options-23.c
new file mode 100644
index 00000000000..575a28da504
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-options-23.c
@@ -0,0 +1,5 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fsanitize-coverage=tracecmp" } */
+
+/* { dg-error "unrecognized argument in option '-fsanitize-coverage=tracecmp'" "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments to '-fsanitize-coverage=' are: trace-cmp trace-pc; did you mean 'trace-cmp'?" "" { target *-*-* } 0 } */