diff mbox

[v2] Offer suggestions for unrecognized sanitizer options (PR driver/78877)

Message ID 1484099246-25276-1-git-send-email-dmalcolm@redhat.com
State New
Headers show

Commit Message

David Malcolm Jan. 11, 2017, 1:47 a.m. UTC
On Tue, 2017-01-10 at 20:49 +0100, Jakub Jelinek wrote:
> On Tue, Jan 10, 2017 at 03:02:49PM -0500, David Malcolm wrote:
> > +/* Given ARG, an unrecognized sanitizer option, return the best
> > +   matching sanitizer option, or NULL if there isn't one.  */
> > +
> > +static const char *
> > +get_closest_sanitizer_option (const string_fragment &arg)
> > +{
> > +  best_match <const string_fragment &, const char*> bm (arg);
> > +  for (int i = 0; sanitizer_opts[i].name != NULL; ++i)
> > +    bm.consider (sanitizer_opts[i].name);
> > +  return bm.get_best_meaningful_candidate ();
> 
> I think we shouldn't suggest invalid option combinations, so that
> means
> passing code to get_closest_sanitizer_option (so that we know if it
> is
> OPT_fsanitize_ or OPT_fsanitize_recover_) and a bool whether it is
> the -fno-sanitize* or -fsanitize* (value).  Then we shouldn't add to
> bm.consider:
> 1) for -fsanitize= (and not -fno-sanitize=) the "all" option
>    (sanitizer_opts[i].flag == ~0U && code == OPT_fsanitize_ && value
>    is a cheap test for that case)
> 2) for -fsanitize-recover= (and not -fno-sanitize-recover=) the
>    non-recoverable options
>    (!sanitizer_opts[i].can_recover && code == OPT_fsanitize_recover_
> && value)
> Otherwise it looks good to me.
> 
> 	Jakub

Thanks.

Here's a v2 of the patch which I believe adds the conditions you
suggest (and new test cases for them).

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.

OK for trunk?

gcc/ChangeLog:
	PR driver/78877
	* opts.c: Include "spellcheck.h"
	(struct string_fragment): New struct.
	(struct edit_distance_traits<const string_fragment &>): New
	struct.
	(get_closest_sanitizer_option): New function.
	(parse_sanitizer_options): Offer suggestions for unrecognized arguments.

gcc/testsuite/ChangeLog:
	PR driver/78877
	* gcc.dg/spellcheck-options-14.c: New test case.
	* gcc.dg/spellcheck-options-15.c: New test case.
	* gcc.dg/spellcheck-options-16.c: New test case.
---
 gcc/opts.c                                   | 74 +++++++++++++++++++++++++++-
 gcc/testsuite/gcc.dg/spellcheck-options-14.c |  8 +++
 gcc/testsuite/gcc.dg/spellcheck-options-15.c |  7 +++
 gcc/testsuite/gcc.dg/spellcheck-options-16.c |  9 ++++
 4 files changed, 96 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-options-14.c
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-options-15.c
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-options-16.c

Comments

Jakub Jelinek Jan. 11, 2017, 8:30 a.m. UTC | #1
On Tue, Jan 10, 2017 at 08:47:26PM -0500, David Malcolm wrote:
> +      /* For -fsanitize-recover= (and not -fno-sanitize-recover=),
> +	 don't offer the non-recoverable options.  */
> +      if (!sanitizer_opts[i].can_recover && code == OPT_fsanitize_recover_
> +	  && value)

If a condition doesn't fit on a single line, usually we want to put every
subcondition on separate line, so:
      if (!sanitizer_opts[i].can_recover
	  && code == OPT_fsanitize_recover_
	  && value)

> +	continue;
> +
> +      bm.consider (sanitizer_opts[i].name);
> +    }
> +  return bm.get_best_meaningful_candidate ();
> +}
> +
>  /* 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.  */
> @@ -1572,8 +1629,21 @@ parse_sanitizer_options (const char *p, location_t loc, int scode,
>  	  }
>  
>        if (! found && complain)
> -	error_at (loc, "unrecognized argument to -fsanitize%s= option: %q.*s",
> -		  code == OPT_fsanitize_ ? "" : "-recover", (int) len, p);
> +	{
> +	  const char *optname
> +	    = code == OPT_fsanitize_ ? "-fsanitize" : "-fsanitize-recover";

Because value matters too, I think it would be nice to also differentiate
between -fsanitize and -fno-sanitize in the diagnostics (I know the old
code didn't do that either).
So use -f%ssanitize%s= and corresponding
value ? "" : "no-", code == OPT_fsanitize_ ? "" : "-recover"
This is normally against translation rules, but I think translators should
not translate the option name and thus -f%ssanitize%s= should be kept as is
somewhere in the translation string.

> +	  const char *hint
> +	    = get_closest_sanitizer_option (string_fragment (p, len),
> +					    code, value);
> +
> +	  if (hint)
> +	    error_at (loc, "unrecognized argument to %s= option: %q.*s;"
> +		      " did you mean %qs",
> +		      optname, (int) len, p, hint);
> +	  else
> +	    error_at (loc, "unrecognized argument to %s= option: %q.*s",
> +		      optname, (int) len, p);
> +	}

Ok for trunk with those changes.

	Jakub
diff mbox

Patch

diff --git a/gcc/opts.c b/gcc/opts.c
index 059a61a..c4e3bbd 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -30,6 +30,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "opts-diagnostic.h"
 #include "insn-attr-common.h"
 #include "common/common-target.h"
+#include "spellcheck.h"
 
 static void set_Wstrict_aliasing (struct gcc_options *opts, int onoff);
 
@@ -1511,6 +1512,62 @@  const struct sanitizer_opts_s sanitizer_opts[] =
   { NULL, 0U, 0UL, false }
 };
 
+/* A struct for describing a run of chars within a string.  */
+
+struct string_fragment
+{
+  string_fragment (const char *start, size_t len)
+  : m_start (start), m_len (len) {}
+
+  const char *m_start;
+  size_t m_len;
+};
+
+/* Specialization of edit_distance_traits for string_fragment,
+   for use by get_closest_sanitizer_option.  */
+
+template <>
+struct edit_distance_traits<const string_fragment &>
+{
+  static size_t get_length (const string_fragment &fragment)
+  {
+    return fragment.m_len;
+  }
+
+  static const char *get_string (const string_fragment &fragment)
+  {
+    return fragment.m_start;
+  }
+};
+
+/* Given ARG, an unrecognized sanitizer option, return the best
+   matching sanitizer option, or NULL if there isn't one.
+   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=").  */
+
+static const char *
+get_closest_sanitizer_option (const string_fragment &arg,
+			      enum opt_code code, int value)
+{
+  best_match <const string_fragment &, const char*> bm (arg);
+  for (int i = 0; sanitizer_opts[i].name != NULL; ++i)
+    {
+      /* -fsanitize=all is not valid, so don't offer it.  */
+      if (sanitizer_opts[i].flag == ~0U && code == OPT_fsanitize_ && value)
+	continue;
+
+      /* For -fsanitize-recover= (and not -fno-sanitize-recover=),
+	 don't offer the non-recoverable options.  */
+      if (!sanitizer_opts[i].can_recover && code == OPT_fsanitize_recover_
+	  && value)
+	continue;
+
+      bm.consider (sanitizer_opts[i].name);
+    }
+  return bm.get_best_meaningful_candidate ();
+}
+
 /* 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.  */
@@ -1572,8 +1629,21 @@  parse_sanitizer_options (const char *p, location_t loc, int scode,
 	  }
 
       if (! found && complain)
-	error_at (loc, "unrecognized argument to -fsanitize%s= option: %q.*s",
-		  code == OPT_fsanitize_ ? "" : "-recover", (int) len, p);
+	{
+	  const char *optname
+	    = code == OPT_fsanitize_ ? "-fsanitize" : "-fsanitize-recover";
+	  const char *hint
+	    = get_closest_sanitizer_option (string_fragment (p, len),
+					    code, value);
+
+	  if (hint)
+	    error_at (loc, "unrecognized argument to %s= option: %q.*s;"
+		      " did you mean %qs",
+		      optname, (int) len, p, hint);
+	  else
+	    error_at (loc, "unrecognized argument to %s= option: %q.*s",
+		      optname, (int) len, p);
+	}
 
       if (comma == NULL)
 	break;
diff --git a/gcc/testsuite/gcc.dg/spellcheck-options-14.c b/gcc/testsuite/gcc.dg/spellcheck-options-14.c
new file mode 100644
index 0000000..5582460
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-options-14.c
@@ -0,0 +1,8 @@ 
+/* Verify that we offer suggestions for misspelled sanitizer options
+   (PR driver/78877).  */
+
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=addres,nul,this-is-not-a-sanitizer-option" } */
+/* { dg-error "unrecognized argument to -fsanitize= option: .addres.; did you mean .address." "" { target *-*-* } 0 } */
+/* { dg-error "unrecognized argument to -fsanitize= option: .nul.; did you mean .null." "" { target *-*-* } 0 } */
+/* { dg-error "unrecognized argument to -fsanitize= option: .this-is-not-a-sanitizer-option." "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.dg/spellcheck-options-15.c b/gcc/testsuite/gcc.dg/spellcheck-options-15.c
new file mode 100644
index 0000000..089e698
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-options-15.c
@@ -0,0 +1,7 @@ 
+/* Verify that we don't offer -fsanitize=all as a suggestion for misspelled
+   sanitizer options (PR driver/78877).  */
+
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=al" } */
+/* { dg-bogus "did you mean" "" { target *-*-* } 0 } */
+/* { dg-error "unrecognized argument to -fsanitize= option: .al." "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.dg/spellcheck-options-16.c b/gcc/testsuite/gcc.dg/spellcheck-options-16.c
new file mode 100644
index 0000000..7019576
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-options-16.c
@@ -0,0 +1,9 @@ 
+/* Verify that we don't offer non-recoverable options as suggestions
+   for misspelled -fsanitize-recover= arguments (PR driver/78877).  */
+
+/* { dg-do compile } */
+/* { dg-options "-fsanitize-recover=threed" } */
+/* Ensure we don't offer non-recoverable "-fsanitize-recover=thread"
+   as a suggestion.  */
+/* { dg-bogus "did you mean" "" { target *-*-* } 0 } */
+/* { dg-error "unrecognized argument to -fsanitize-recover= option: .threed." "" { target *-*-* } 0 } */