diff mbox series

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 | expand

Commit Message

Martin Liška May 20, 2021, 10:43 a.m. UTC 
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

Comments

Bernhard Reutner-Fischer May 20, 2021, 1:26 p.m. UTC | #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. UTC | #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. UTC | #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 } */
Jeff Law June 2, 2021, 6:50 p.m. UTC | #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
diff mbox series

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 } */