diff mbox

Do not suggest -fsanitize=all (PR driver/78863).

Message ID bd710cf5-d7f4-6df0-36e9-a849b9b191a6@suse.cz
State New
Headers show

Commit Message

Martin Liška Dec. 21, 2016, 10:20 a.m. UTC
On 12/21/2016 11:00 AM, Jakub Jelinek wrote:
> On Wed, Dec 21, 2016 at 10:34:13AM +0100, Martin Liška wrote:

>> As mentioned in the PR, we should not suggest option that is not allowed.

>> Fixed by explicit removal of suggestions that are not acceptable.

>>

>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

>>

>> Ready to be installed?

> 

> Wouldn't it be better not to register those? Like (untested):

> 

> --- gcc/gcc.c.jj	2016-11-14 19:57:10.000000000 +0100

> +++ gcc/gcc.c	2016-12-21 10:58:29.739873850 +0100

> @@ -7733,6 +7733,17 @@ driver::build_option_suggestions (void)

>  	  {

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

>  	      {

> +		struct cl_option optb;

> +		/* -fsanitize=all is not valid, only -fno-sanitize=all.

> +		   So don't register the positive misspelling candidates

> +		   for it.  */

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

> +		  {

> +		    optb = *option;

> +		    optb.opt_text = opt_text = "-fno-sanitize=";

> +		    optb.cl_reject_negative = true;

> +		    option = &optb;

> +		  }

>  		/* Get one arg at a time e.g. "-fsanitize=address".  */

>  		char *with_arg = concat (opt_text,

>  					 sanitizer_opts[j].name,

> 

> 

> 	Jakub

> 


I like your approach!
make check -k -j10 RUNTESTFLAGS="dg.exp=spellcheck-options-*" works fine.

Am I install the patch after it survives proper regression tests?
Thanks,
Martin

Comments

Jakub Jelinek Dec. 21, 2016, 10:28 a.m. UTC | #1
On Wed, Dec 21, 2016 at 11:20:33AM +0100, Martin Liška wrote:
> I like your approach!

> make check -k -j10 RUNTESTFLAGS="dg.exp=spellcheck-options-*" works fine.

> 

> Am I install the patch after it survives proper regression tests?


Ok.

Also, only related, seems we have misspelling candidates for cases like
-fsanitiz=ell
but not for -fsanitize=ell
(i.e. when the option is actually correct, just the argument to it (or part
of it) is misspelled).  It would need to be done probably in
parse_sanitizer_options when we diagnose it:
      if (! found && complain)
        error_at (loc, "unrecognized argument to -fsanitize%s= option: %q.*s",
                  code == OPT_fsanitize_ ? "" : "-recover", (int) len, p);
go through sanitizer_opts again in that case, add candidates (that are
valid for the particular option), and if there is a hint, add the hint to
this message.

	Jakub
Martin Liška Dec. 21, 2016, 3:03 p.m. UTC | #2
On 12/21/2016 11:28 AM, Jakub Jelinek wrote:
> On Wed, Dec 21, 2016 at 11:20:33AM +0100, Martin Liška wrote:

>> I like your approach!

>> make check -k -j10 RUNTESTFLAGS="dg.exp=spellcheck-options-*" works fine.

>>

>> Am I install the patch after it survives proper regression tests?

> 

> Ok.

> 

> Also, only related, seems we have misspelling candidates for cases like

> -fsanitiz=ell

> but not for -fsanitize=ell

> (i.e. when the option is actually correct, just the argument to it (or part

> of it) is misspelled).  It would need to be done probably in

> parse_sanitizer_options when we diagnose it:

>       if (! found && complain)

>         error_at (loc, "unrecognized argument to -fsanitize%s= option: %q.*s",

>                   code == OPT_fsanitize_ ? "" : "-recover", (int) len, p);

> go through sanitizer_opts again in that case, add candidates (that are

> valid for the particular option), and if there is a hint, add the hint to

> this message.

> 

> 	Jakub

> 


These look very similar to what I reported in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78877.
I've just added your case to the PR.

I'm going to install the patch.

M.
Martin Liška Jan. 2, 2017, 12:52 p.m. UTC | #3
On 12/21/2016 04:03 PM, Martin Liška wrote:
> On 12/21/2016 11:28 AM, Jakub Jelinek wrote:

>> On Wed, Dec 21, 2016 at 11:20:33AM +0100, Martin Liška wrote:

>>> I like your approach!

>>> make check -k -j10 RUNTESTFLAGS="dg.exp=spellcheck-options-*" works fine.

>>>

>>> Am I install the patch after it survives proper regression tests?

>>

>> Ok.

>>

>> Also, only related, seems we have misspelling candidates for cases like

>> -fsanitiz=ell

>> but not for -fsanitize=ell

>> (i.e. when the option is actually correct, just the argument to it (or part

>> of it) is misspelled).  It would need to be done probably in

>> parse_sanitizer_options when we diagnose it:

>>       if (! found && complain)

>>         error_at (loc, "unrecognized argument to -fsanitize%s= option: %q.*s",

>>                   code == OPT_fsanitize_ ? "" : "-recover", (int) len, p);

>> go through sanitizer_opts again in that case, add candidates (that are

>> valid for the particular option), and if there is a hint, add the hint to

>> this message.

>>

>> 	Jakub

>>

> 

> These look very similar to what I reported in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78877.

> I've just added your case to the PR.

> 

> I'm going to install the patch.

> 

> M.

> 


Is it fine to install the patch to gcc-6 branch?

Thanks,
Martin
Jakub Jelinek Jan. 2, 2017, 12:55 p.m. UTC | #4
On Mon, Jan 02, 2017 at 01:52:22PM +0100, Martin Liška wrote:
> Is it fine to install the patch to gcc-6 branch?


Ok.

	Jakub
diff mbox

Patch

From 2533c19b4bbd2d9900b043973b504be07343d05c Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 20 Dec 2016 12:16:02 +0100
Subject: [PATCH] Do not suggest -fsanitize=all (PR driver/78863).

gcc/ChangeLog:

2016-12-20  Jakub Jelinek  <jakub@redhat.com>
	    Martin Liska  <mliska@suse.cz>

	PR driver/78863
	* gcc.c (driver::build_option_suggestions): Do not add
	-fsanitize=all as a suggestion candidate.

gcc/testsuite/ChangeLog:

2016-12-20  Martin Liska  <mliska@suse.cz>

	PR driver/78863
	* gcc.dg/spellcheck-options-13.c: New test.
---
 gcc/gcc.c                                    | 11 +++++++++++
 gcc/testsuite/gcc.dg/spellcheck-options-13.c |  5 +++++
 2 files changed, 16 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-options-13.c

diff --git a/gcc/gcc.c b/gcc/gcc.c
index f78acd68606..69089484340 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -7733,6 +7733,17 @@  driver::build_option_suggestions (void)
 	  {
 	    for (int j = 0; sanitizer_opts[j].name != NULL; ++j)
 	      {
+		struct cl_option optb;
+		/* -fsanitize=all is not valid, only -fno-sanitize=all.
+		   So don't register the positive misspelling candidates
+		   for it.  */
+		if (sanitizer_opts[j].flag == ~0U && i == OPT_fsanitize_)
+		  {
+		    optb = *option;
+		    optb.opt_text = opt_text = "-fno-sanitize=";
+		    optb.cl_reject_negative = true;
+		    option = &optb;
+		  }
 		/* Get one arg at a time e.g. "-fsanitize=address".  */
 		char *with_arg = concat (opt_text,
 					 sanitizer_opts[j].name,
diff --git a/gcc/testsuite/gcc.dg/spellcheck-options-13.c b/gcc/testsuite/gcc.dg/spellcheck-options-13.c
new file mode 100644
index 00000000000..19b63af565b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-options-13.c
@@ -0,0 +1,5 @@ 
+/* PR driver/78863.  */
+
+/* { dg-do compile } */
+/* { dg-options "-fsanitize" } */
+/* { dg-error "unrecognized command line option .-fsanitize..$" "" { target *-*-* } 0 } */
-- 
2.11.0