diff mbox

[opt*.awk/middle-end] Detect wrong arguments to EnabledBy

Message ID CAESRpQDUmXdi_erzvJ5w4vRy_EaopA206Fvum6unY+jTz0Z+ag@mail.gmail.com
State New
Headers show

Commit Message

Manuel López-Ibáñez Aug. 8, 2015, 8:59 a.m. UTC
The argument to EnabledBy() must be one or more options with the
Common flag, otherwise the setting is silently ignored. This patch
detects this case when generating the options.c file and gives an
appropriate error. There were two options suffering from this: -Wchkp
is fixed by using LangEnabledBy instead, but -Wnull-dereference cannot
be enabled by -Wall or -Wextra because it triggers false positives
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=16351#c35)

Bootstrapped and regression tested on x86_64-linux-gnu.

OK?

gcc/ChangeLog:

2015-08-08  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    * doc/invoke.texi (Wnull-dereference): Move after Wnonnull.
    Not enabled by -Wall.
    * optc-gen.awk: Give nicer error messages. Detect if the argument
    of EnabledBy is not a Common option.
    * common.opt (Wnull-dereference): Not enabled by -Wall.
    * opt-functions.awk (lang_enabled_by): Nicer error messages.

gcc/c-family/ChangeLog:

2015-08-08  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    * c.opt (Wchkp): Use LangEnabledBy instead of EnabledBy.

Comments

Joseph Myers Aug. 9, 2015, 2:29 p.m. UTC | #1
On Sat, 8 Aug 2015, Manuel López-Ibáñez wrote:

> The argument to EnabledBy() must be one or more options with the
> Common flag, otherwise the setting is silently ignored. This patch
> detects this case when generating the options.c file and gives an
> appropriate error. There were two options suffering from this: -Wchkp
> is fixed by using LangEnabledBy instead, but -Wnull-dereference cannot
> be enabled by -Wall or -Wextra because it triggers false positives
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=16351#c35)
> 
> Bootstrapped and regression tested on x86_64-linux-gnu.
> 
> OK?

OK with the requirement that the options have the Common flag being added 
to the documentation of EnabledBy in options.texi.
Manuel López-Ibáñez Aug. 9, 2015, 7:31 p.m. UTC | #2
On 9 August 2015 at 16:29, Joseph Myers <joseph@codesourcery.com> wrote:
> On Sat, 8 Aug 2015, Manuel López-Ibáñez wrote:
>
>> The argument to EnabledBy() must be one or more options with the
>> Common flag, otherwise the setting is silently ignored. This patch
>> detects this case when generating the options.c file and gives an
>> appropriate error. There were two options suffering from this: -Wchkp
>> is fixed by using LangEnabledBy instead, but -Wnull-dereference cannot
>> be enabled by -Wall or -Wextra because it triggers false positives
>> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=16351#c35)
>>
>> Bootstrapped and regression tested on x86_64-linux-gnu.
>>
>> OK?
>
> OK with the requirement that the options have the Common flag being added
> to the documentation of EnabledBy in options.texi.

Done. Committed as https://gcc.gnu.org/r226751

Cheers,

Manuel.
diff mbox

Patch

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 226719)
+++ gcc/doc/invoke.texi	(working copy)
@@ -3720,10 +3720,19 @@  Warn about passing a null pointer for ar
 requiring a non-null value by the @code{nonnull} function attribute.
 
 @option{-Wnonnull} is included in @option{-Wall} and @option{-Wformat}.  It
 can be disabled with the @option{-Wno-nonnull} option.
 
+@item -Wnull-dereference
+@opindex Wnull-dereference
+@opindex Wno-null-dereference
+Warn if the compiler detects paths that trigger erroneous or
+undefined behavior due to dereferencing a null pointer.  This option
+is only active when @option{-fdelete-null-pointer-checks} is active,
+which is enabled by optimizations in most targets.  The precision of
+the warnings depends on the optimization options used.
+
 @item -Winit-self @r{(C, C++, Objective-C and Objective-C++ only)}
 @opindex Winit-self
 @opindex Wno-init-self
 Warn about uninitialized variables that are initialized with themselves.
 Note this option can only be used with the @option{-Wuninitialized} option.
@@ -4156,20 +4165,10 @@  All the above @option{-Wunused} options 
 
 In order to get a warning about an unused function parameter, you must
 either specify @option{-Wextra -Wunused} (note that @option{-Wall} implies
 @option{-Wunused}), or separately specify @option{-Wunused-parameter}.
 
-@item -Wnull-dereference
-@opindex Wnull-dereference
-@opindex Wno-null-dereference
-Warn if the compiler detects paths that trigger erroneous or
-undefined behavior due to dereferencing a null pointer.  This option
-is only active when @option{-fdelete-null-pointer-checks} is active,
-which is enabled by optimizations in most targets.  The precision of
-the warnings depends on the optimization options used.  This option is
-enabled by @option{-Wall}.
-
 @item -Wuninitialized
 @opindex Wuninitialized
 @opindex Wno-uninitialized
 Warn if an automatic variable is used without first being initialized
 or if a variable may be clobbered by a @code{setjmp} call. In C++,
Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 226719)
+++ gcc/c-family/c.opt	(working copy)
@@ -333,11 +333,11 @@  Warn about casts which discard qualifier
 Wchar-subscripts
 C ObjC C++ ObjC++ Var(warn_char_subscripts) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn about subscripts whose type is \"char\"
 
 Wchkp
-C ObjC C++ ObjC++ Var(warn_chkp) Warning EnabledBy(Wall)
+C ObjC C++ ObjC++ Var(warn_chkp) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn about memory access errors found by Pointer Bounds Checker
 
 Wclobbered
 C ObjC C++ ObjC++ Var(warn_clobbered) Warning EnabledBy(Wextra)
 Warn about variables that might be changed by \"longjmp\" or \"vfork\"
Index: gcc/optc-gen.awk
===================================================================
--- gcc/optc-gen.awk	(revision 226719)
+++ gcc/optc-gen.awk	(working copy)
@@ -28,11 +28,25 @@ 
 #            [-v header_name=header.h] < inputfile > options.c
 
 # Dump that array of options into a C file.
 END {
 
-# Record first EnabledBy and LangEnabledBy uses.
+
+# Combine the flags of identical switches.  Switches
+# appear many times if they are handled by many front
+# ends, for example.
+for (i = 0; i < n_opts; i++) {
+    merged_flags[i] = flags[i]
+}
+for (i = 0; i < n_opts; i++) {
+    while(i + 1 != n_opts && opts[i] == opts[i + 1] ) {
+	merged_flags[i + 1] = merged_flags[i] " " merged_flags[i + 1];
+	i++;
+    }
+}
+
+# Record EnabledBy and LangEnabledBy uses.
 n_enabledby = 0;
 for (i = 0; i < n_langs; i++) {
     n_enabledby_lang[i] = 0;
 }
 for (i = 0; i < n_opts; i++) {
@@ -46,19 +60,23 @@  for (i = 0; i < n_opts; i++) {
             # EnabledBy(arg) or EnabledBy(arg1 || arg2 || arg3)
             split_sep = " \\|\\| ";
         }
         n_enabledby_names = split(enabledby_arg, enabledby_names, split_sep);
         if (logical_and != 0 && n_enabledby_names > 2) {
-            print "#error EnabledBy (Wfoo && Wbar && Wbaz) not currently supported"
+            print "#error " opts[i] " EnabledBy(Wfoo && Wbar && Wbaz) currently not supported"
         }
         for (j = 1; j <= n_enabledby_names; j++) {
             enabledby_name = enabledby_names[j];
             enabledby_index = opt_numbers[enabledby_name];
             if (enabledby_index == "") {
-                print "#error Enabledby: " enabledby_name 
-            } else {
-                condition = "";
+                print "#error " opts[i] " Enabledby(" enabledby_name "), unknown option '" enabledby_name "'"
+            } else if (!flag_set_p("Common", merged_flags[enabledby_index])) {
+		print "#error " opts[i] " Enabledby(" enabledby_name "), '" \
+		    enabledby_name "' must have flag 'Common'"		\
+		    " to use Enabledby(), otherwise use LangEnabledBy()"
+	    } else {
+		condition = "";
                 if (logical_and != 0) {
                     opt_var_name_1 = search_var_name(enabledby_names[1], opt_numbers, opts, flags, n_opts);
                     opt_var_name_2 = search_var_name(enabledby_names[2], opt_numbers, opts, flags, n_opts);
                     if (opt_var_name_1 == "") {
                         print "#error " enabledby_names[1] " does not have a Var() flag"
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 226719)
+++ gcc/common.opt	(working copy)
@@ -591,11 +591,11 @@  Common RejectNegative Joined Warning Und
 Wlarger-than=
 Common RejectNegative Joined UInteger Warning
 -Wlarger-than=<number>	Warn if an object is larger than <number> bytes
 
 Wnull-dereference
-Common Var(warn_null_dereference) Warning EnabledBy(Wall)
+Common Var(warn_null_dereference) Warning
 Warn if dereferencing a NULL pointer may lead to erroneous or undefined behavior
 
 Wunsafe-loop-optimizations
 Common Var(warn_unsafe_loop_optimizations) Warning
 Warn if the loop cannot be optimized due to nontrivial assumptions.
Index: gcc/opt-functions.awk
===================================================================
--- gcc/opt-functions.awk	(revision 226719)
+++ gcc/opt-functions.awk	(working copy)
@@ -322,21 +322,21 @@  function lang_enabled_by(enabledby_langs
     if (enabledby_posarg != "" && enabledby_negarg != "") {
         with_args = "," enabledby_posarg "," enabledby_negarg
     } else if (enabledby_posarg == "" && enabledby_negarg == "") {
         with_args = ""
     } else {
-        print "#error LangEnabledBy("enabledby_langs","enabledby_name", " \
+        print "#error " opts[i] " LangEnabledBy("enabledby_langs","enabledby_name", " \
             enabledby_posarg", " enabledby_negargs                  \
             ") with three arguments, it should have either 2 or 4"
     }
 
     n_enabledby_array = split(enabledby_name, enabledby_array, " \\|\\| ");
     for (k = 1; k <= n_enabledby_array; k++) {
         enabledby_index = opt_numbers[enabledby_array[k]];
         if (enabledby_index == "") {
-             print "#error LangEnabledBy("enabledby_langs","enabledby_name", " \
-                 enabledby_posarg", " enabledby_negargs") has invalid ENABLEDBY_NAME"
+             print "#error " opts[i] " LangEnabledBy("enabledby_langs","enabledby_name", " \
+                 enabledby_posarg", " enabledby_negargs"), unknown option '" enabledby_name "'"
         } else {
             for (j = 1; j <= n_enabledby_arg_langs; j++) {
                  lang_name = lang_sanitized_name(enabledby_arg_langs[j]);
                  lang_index = lang_numbers[enabledby_arg_langs[j]];
                  if (enables[lang_name,enabledby_array[k]] == "") {