diff mbox

[3/3] Introduce IntegerRange for options (PR driver/79659).

Message ID 4c2a43af-79e3-a3c6-69d6-8ebc00a3a150@suse.cz
State New
Headers show

Commit Message

Martin Liška March 15, 2017, 9:58 a.m. UTC
Huh, I forgot to attach the patch.

Martin

Comments

Jeff Law June 28, 2017, 4:52 a.m. UTC | #1
On 03/15/2017 03:58 AM, Martin Liška wrote:
> Huh, I forgot to attach the patch.
> 
> Martin
> 
> 0001-Introduce-IntegerRange-for-options-PR-driver-79659.patch
> 
> 
> From bb89456e6cecfa9497cf8e265d2083e762d5bc3e Mon Sep 17 00:00:00 2001
> From: marxin <mliska@suse.cz>
> Date: Mon, 27 Feb 2017 14:07:03 +0100
> Subject: [PATCH] Introduce IntegerRange for options (PR driver/79659).
> 
> gcc/ChangeLog:
> 
> 2017-02-28  Martin Liska  <mliska@suse.cz>
> 
> 	PR driver/79659
> 	* common.opt: Add IntegerRange to various options.
> 	* opt-functions.awk (integer_range_info): New function.
> 	* optc-gen.awk: Add integer_range_info to cl_options struct.
> 	* opts-common.c (decode_cmdline_option): Handle
> 	CL_ERR_INT_RANGE_ARG.
> 	(cmdline_handle_error): Likewise.
> 	* opts.c (print_filtered_help): Show valid interval in
> 	when --help is provided.
> 	* opts.h (struct cl_option): Add range_min and range_max fields.
> 	* config/i386/i386.opt: Add IntegerRange for -mbranch-cost.
> 
> gcc/c-family/ChangeLog:
> 
> 2017-02-28  Martin Liska  <mliska@suse.cz>
> 
> 	PR driver/79659
> 	* c.opt: Add IntegerRange to various options.
> 
> gcc/testsuite/ChangeLog:
> 
> 2017-02-28  Martin Liska  <mliska@suse.cz>
> 
> 	PR driver/79659
> 	* g++.dg/opt/pr79659.C: New test.
Presumably this never fully moved forward because it wasn't a regression?

This looks quite reasonable to me.  I'm not sure of the state of the
prereqs and you may want/need to add IntegerRange checks on newly added
options since this was first submitted.

If the prereqs are ack'd, then as far as I'm concerned this is good to
go and you're free to add any new IntegerRange checks you deem
necessary/desirable.

jeff
Martin Liška June 28, 2017, 12:43 p.m. UTC | #2
On 06/28/2017 06:52 AM, Jeff Law wrote:
> On 03/15/2017 03:58 AM, Martin Liška wrote:
>> Huh, I forgot to attach the patch.
>>
>> Martin
>>
>> 0001-Introduce-IntegerRange-for-options-PR-driver-79659.patch
>>
>>
>> From bb89456e6cecfa9497cf8e265d2083e762d5bc3e Mon Sep 17 00:00:00 2001
>> From: marxin <mliska@suse.cz>
>> Date: Mon, 27 Feb 2017 14:07:03 +0100
>> Subject: [PATCH] Introduce IntegerRange for options (PR driver/79659).
>>
>> gcc/ChangeLog:
>>
>> 2017-02-28  Martin Liska  <mliska@suse.cz>
>>
>> 	PR driver/79659
>> 	* common.opt: Add IntegerRange to various options.
>> 	* opt-functions.awk (integer_range_info): New function.
>> 	* optc-gen.awk: Add integer_range_info to cl_options struct.
>> 	* opts-common.c (decode_cmdline_option): Handle
>> 	CL_ERR_INT_RANGE_ARG.
>> 	(cmdline_handle_error): Likewise.
>> 	* opts.c (print_filtered_help): Show valid interval in
>> 	when --help is provided.
>> 	* opts.h (struct cl_option): Add range_min and range_max fields.
>> 	* config/i386/i386.opt: Add IntegerRange for -mbranch-cost.
>>
>> gcc/c-family/ChangeLog:
>>
>> 2017-02-28  Martin Liska  <mliska@suse.cz>
>>
>> 	PR driver/79659
>> 	* c.opt: Add IntegerRange to various options.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2017-02-28  Martin Liska  <mliska@suse.cz>
>>
>> 	PR driver/79659
>> 	* g++.dg/opt/pr79659.C: New test.
> Presumably this never fully moved forward because it wasn't a regression?
> 
> This looks quite reasonable to me.  I'm not sure of the state of the
> prereqs and you may want/need to add IntegerRange checks on newly added
> options since this was first submitted.
> 
> If the prereqs are ack'd, then as far as I'm concerned this is good to
> go and you're free to add any new IntegerRange checks you deem
> necessary/desirable.
> 
> jeff
> 

Thank you Jeff for looking at the patch. I've just re-tested the patch and
I'm going to install it.

Martin
Rainer Orth June 28, 2017, 8:14 p.m. UTC | #3
Hi Martin,

> On 06/28/2017 06:52 AM, Jeff Law wrote:
>> On 03/15/2017 03:58 AM, Martin Liška wrote:
>>> Huh, I forgot to attach the patch.
>>>
>>> Martin
>>>
>>> 0001-Introduce-IntegerRange-for-options-PR-driver-79659.patch
>>>
>>>
>>> From bb89456e6cecfa9497cf8e265d2083e762d5bc3e Mon Sep 17 00:00:00 2001
>>> From: marxin <mliska@suse.cz>
>>> Date: Mon, 27 Feb 2017 14:07:03 +0100
>>> Subject: [PATCH] Introduce IntegerRange for options (PR driver/79659).
>>>
>>> gcc/ChangeLog:
>>>
>>> 2017-02-28  Martin Liska  <mliska@suse.cz>
>>>
>>> 	PR driver/79659
>>> 	* common.opt: Add IntegerRange to various options.
>>> 	* opt-functions.awk (integer_range_info): New function.
>>> 	* optc-gen.awk: Add integer_range_info to cl_options struct.
>>> 	* opts-common.c (decode_cmdline_option): Handle
>>> 	CL_ERR_INT_RANGE_ARG.
>>> 	(cmdline_handle_error): Likewise.
>>> 	* opts.c (print_filtered_help): Show valid interval in
>>> 	when --help is provided.
>>> 	* opts.h (struct cl_option): Add range_min and range_max fields.
>>> 	* config/i386/i386.opt: Add IntegerRange for -mbranch-cost.
>>>
>>> gcc/c-family/ChangeLog:
>>>
>>> 2017-02-28  Martin Liska  <mliska@suse.cz>
>>>
>>> 	PR driver/79659
>>> 	* c.opt: Add IntegerRange to various options.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2017-02-28  Martin Liska  <mliska@suse.cz>
>>>
>>> 	PR driver/79659
>>> 	* g++.dg/opt/pr79659.C: New test.
>> Presumably this never fully moved forward because it wasn't a regression?
>> 
>> This looks quite reasonable to me.  I'm not sure of the state of the
>> prereqs and you may want/need to add IntegerRange checks on newly added
>> options since this was first submitted.
>> 
>> If the prereqs are ack'd, then as far as I'm concerned this is good to
>> go and you're free to add any new IntegerRange checks you deem
>> necessary/desirable.
>> 
>> jeff
>> 
>
> Thank you Jeff for looking at the patch. I've just re-tested the patch and
> I'm going to install it.

seems you didn't test thoroughly enough: your patch introduced a couple
of testsuite regressions on i386-pc-solaris2.12 and x86_64-pc-linux-gnu
(any x86 target, in fact):

+FAIL: gcc.dg/uninit-pred-7_d.c (test for excess errors)
+FAIL: gcc.dg/uninit-pred-7_d.c warning (test for warnings, line 48)
+FAIL: gcc.dg/uninit-pred-8_d.c (test for excess errors)
+FAIL: gcc.dg/uninit-pred-8_d.c warning (test for warnings, line 42)

+FAIL: gcc.target/i386/branch-cost1.c (test for excess errors)
+UNRESOLVED: gcc.target/i386/branch-cost1.c scan-tree-dump-not gimple " & "
+UNRESOLVED: gcc.target/i386/branch-cost1.c scan-tree-dump-times gimple "if " 2
+FAIL: gcc.target/i386/branch-cost4.c (test for excess errors)
+UNRESOLVED: gcc.target/i386/branch-cost4.c scan-tree-dump-not gimple " & "
+UNRESOLVED: gcc.target/i386/branch-cost4.c scan-tree-dump-times gimple "if " 2

In all cases, you get

Excess errors:
xgcc: error: argument to '-mbranch-cost=' is not between 1 and 5

since the tests are compiled with -mbranch-cost=0.

Please fix.

	Rainer
diff mbox

Patch

From bb89456e6cecfa9497cf8e265d2083e762d5bc3e Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Mon, 27 Feb 2017 14:07:03 +0100
Subject: [PATCH] Introduce IntegerRange for options (PR driver/79659).

gcc/ChangeLog:

2017-02-28  Martin Liska  <mliska@suse.cz>

	PR driver/79659
	* common.opt: Add IntegerRange to various options.
	* opt-functions.awk (integer_range_info): New function.
	* optc-gen.awk: Add integer_range_info to cl_options struct.
	* opts-common.c (decode_cmdline_option): Handle
	CL_ERR_INT_RANGE_ARG.
	(cmdline_handle_error): Likewise.
	* opts.c (print_filtered_help): Show valid interval in
	when --help is provided.
	* opts.h (struct cl_option): Add range_min and range_max fields.
	* config/i386/i386.opt: Add IntegerRange for -mbranch-cost.

gcc/c-family/ChangeLog:

2017-02-28  Martin Liska  <mliska@suse.cz>

	PR driver/79659
	* c.opt: Add IntegerRange to various options.

gcc/testsuite/ChangeLog:

2017-02-28  Martin Liska  <mliska@suse.cz>

	PR driver/79659
	* g++.dg/opt/pr79659.C: New test.
---
 gcc/c-family/c.opt                 | 20 ++++++++++----------
 gcc/common.opt                     |  8 ++++----
 gcc/config/i386/i386.opt           |  4 ++--
 gcc/opt-functions.awk              | 11 +++++++++++
 gcc/optc-gen.awk                   |  4 ++--
 gcc/opts-common.c                  | 12 ++++++++++++
 gcc/opts.c                         |  9 +++++++++
 gcc/opts.h                         |  9 +++++++--
 gcc/testsuite/g++.dg/opt/pr79659.C |  5 +++++
 9 files changed, 62 insertions(+), 20 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/opt/pr79659.C

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index cf459ab4427..d071c180bd1 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -529,7 +529,7 @@  C ObjC C++ ObjC++ Var(warn_format_nonliteral) Warning LangEnabledBy(C ObjC C++ O
 Warn about format strings that are not literals.
 
 Wformat-overflow
-C ObjC C++ LTO ObjC++ Warning Alias(Wformat-overflow=, 1, 0)
+C ObjC C++ LTO ObjC++ Warning Alias(Wformat-overflow=, 1, 0) IntegerRange(0, 2)
 Warn about function calls with format strings that write past the end
 of the destination region.  Same as -Wformat-overflow=1.
 
@@ -555,16 +555,16 @@  C ObjC C++ ObjC++ Var(warn_format_zero_length) Warning LangEnabledBy(C ObjC C++
 Warn about zero-length formats.
 
 Wformat=
-C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_format) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall, 1, 0)
+C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_format) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall, 1, 0) IntegerRange(0, 2)
 Warn about printf/scanf/strftime/strfmon format string anomalies.
 
 Wformat-overflow=
-C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_overflow) Warning LangEnabledBy(C ObjC C++ ObjC++,Wformat=, warn_format >= 1, 0)
+C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_overflow) Warning LangEnabledBy(C ObjC C++ ObjC++,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
 Warn about function calls with format strings that write past the end
 of the destination region.
 
 Wformat-truncation=
-C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) Warning LangEnabledBy(C ObjC C++ ObjC++,Wformat=, warn_format >= 1, 0)
+C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) Warning LangEnabledBy(C ObjC C++ ObjC++,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
 Warn about calls to snprintf and similar functions that truncate output.
 
 Wignored-qualifiers
@@ -712,7 +712,7 @@  Warn about buffer overflow in string manipulation functions like memcpy
 and strcpy.
 
 Wstringop-overflow=
-C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_stringop_overflow) Init(2) Warning
+C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_stringop_overflow) Init(2) Warning IntegerRange(0, 4)
 Under the control of Object Size type, warn about buffer overflow in string
 manipulation functions like memcpy and strcpy.
 
@@ -908,7 +908,7 @@  C++ Warning Alias(Wplacement-new=, 1, 0)
 Warn for placement new expressions with undefined behavior.
 
 Wplacement-new=
-C++ Joined RejectNegative UInteger Var(warn_placement_new) Init(-1) Warning
+C++ Joined RejectNegative UInteger Var(warn_placement_new) Init(-1) Warning IntegerRange(0, 2)
 Warn for placement new expressions with undefined behavior.
 
 Wredundant-decls
@@ -948,7 +948,7 @@  C ObjC C++ ObjC++ Warning Alias(Wshift-overflow=, 1, 0)
 Warn if left shift of a signed value overflows.
 
 Wshift-overflow=
-C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_shift_overflow) Init(-1) Warning
+C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_shift_overflow) Init(-1) Warning IntegerRange(0, 2)
 Warn if left shift of a signed value overflows.
 
 Wshift-count-negative
@@ -988,11 +988,11 @@  C ObjC Var(warn_strict_prototypes) Warning
 Warn about unprototyped function declarations.
 
 Wstrict-aliasing=
-C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall, 3, 0)
+C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall, 3, 0) IntegerRange(0, 3)
 ;
 
 Wstrict-overflow=
-C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall, 1, 0)
+C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall, 1, 0) IntegerRange(0, 5)
 ;
 
 Wstrict-selector-match
@@ -1080,7 +1080,7 @@  C ObjC C++ ObjC++ Warning Alias(Wunused-const-variable=, 2, 0)
 Warn when a const variable is unused.
 
 Wunused-const-variable=
-C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_unused_const_variable) Warning LangEnabledBy(C ObjC,Wunused-variable, 1, 0)
+C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_unused_const_variable) Warning LangEnabledBy(C ObjC,Wunused-variable, 1, 0) IntegerRange(0, 2)
 Warn when a const variable is unused.
 
 Wvariadic-macros
diff --git a/gcc/common.opt b/gcc/common.opt
index 4021622cf5c..6c7be565e8c 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -546,7 +546,7 @@  Common Var(warn_array_bounds) Warning
 Warn if an array is accessed out of bounds.
 
 Warray-bounds=
-Common Joined RejectNegative UInteger Var(warn_array_bounds) Warning
+Common Joined RejectNegative UInteger Var(warn_array_bounds) Warning IntegerRange(0, 2)
 Warn if an array is accessed out of bounds.
 
 Wattributes
@@ -601,7 +601,7 @@  Wimplicit-fallthrough
 Common Alias(Wimplicit-fallthrough=,3,0) Warning
 
 Wimplicit-fallthrough=
-Common Var(warn_implicit_fallthrough) RejectNegative Joined UInteger Warning
+Common Var(warn_implicit_fallthrough) RejectNegative Joined UInteger Warning IntegerRange(0, 5)
 Warn when a switch case falls through.
 
 Winline
@@ -1774,7 +1774,7 @@  Specify the algorithm to partition symbols and vars at linktime.
 
 ; The initial value of -1 comes from Z_DEFAULT_COMPRESSION in zlib.h.
 flto-compression-level=
-Common Joined RejectNegative UInteger Var(flag_lto_compression_level) Init(-1)
+Common Joined RejectNegative UInteger Var(flag_lto_compression_level) Init(-1) IntegerRange(0, 9)
 -flto-compression-level=<number>	Use zlib compression level <number> for IL.
 
 flto-odr-type-merging
@@ -2053,7 +2053,7 @@  Tell DSE that the storage for a C++ object is dead when the constructor
 starts and when the destructor finishes.
 
 flifetime-dse=
-Common Joined RejectNegative UInteger Var(flag_lifetime_dse) Optimization
+Common Joined RejectNegative UInteger Var(flag_lifetime_dse) Optimization IntegerRange(0, 2)
 
 flive-range-shrinkage
 Common Report Var(flag_live_range_shrinkage) Init(0) Optimization
diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
index 9384e29b1de..ef16cdefa5e 100644
--- a/gcc/config/i386/i386.opt
+++ b/gcc/config/i386/i386.opt
@@ -267,8 +267,8 @@  EnumValue
 Enum(asm_dialect) String(att) Value(ASM_ATT)
 
 mbranch-cost=
-Target RejectNegative Joined UInteger Var(ix86_branch_cost)
-Branches are this expensive (1-5, arbitrary units).
+Target RejectNegative Joined UInteger Var(ix86_branch_cost) IntegerRange(1, 5)
+Branches are this expensive (arbitrary units).
 
 mlarge-data-threshold=
 Target RejectNegative Joined UInteger Var(ix86_section_threshold) Init(DEFAULT_LARGE_SECTION_THRESHOLD)
diff --git a/gcc/opt-functions.awk b/gcc/opt-functions.awk
index 0736a6f3faf..90b263223b4 100644
--- a/gcc/opt-functions.awk
+++ b/gcc/opt-functions.awk
@@ -314,6 +314,17 @@  function search_var_name(name, opt_numbers, opts, flags, n_opts)
     return ""
 }
 
+function integer_range_info(range_option)
+{
+    if (range_option != "") {
+	start = nth_arg(0, range_option);
+	end = nth_arg(1, range_option);
+	return start ", " end
+    }
+    else
+        return "-1, -1"
+}
+
 # Handle LangEnabledBy(ENABLED_BY_LANGS, ENABLEDBY_NAME, ENABLEDBY_POSARG,
 # ENABLEDBY_NEGARG). This function does not return anything.
 function lang_enabled_by(enabledby_langs, enabledby_name, enabledby_posarg, enabledby_negarg)
diff --git a/gcc/optc-gen.awk b/gcc/optc-gen.awk
index 3b9126ccf97..5b9e12c03a1 100644
--- a/gcc/optc-gen.awk
+++ b/gcc/optc-gen.awk
@@ -399,8 +399,8 @@  for (i = 0; i < n_opts; i++) {
 		printf("    %s,\n" \
 		       "    0, %s,\n",
 		       cl_flags, cl_bit_fields)
-	printf("    %s, %s }%s\n", var_ref(opts[i], flags[i]),
-	       var_set(flags[i]), comma)
+	printf("    %s, %s, %s }%s\n", var_ref(opts[i], flags[i]),
+	       var_set(flags[i]), integer_range_info(opt_args("IntegerRange", flags[i])), comma)
 }
 
 print "};"
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index f2f7385a4c7..0cab42a021c 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -661,6 +661,11 @@  decode_cmdline_option (const char **argv, unsigned int lang_mask,
       value = integral_argument (arg);
       if (value == -1)
 	errors |= CL_ERR_UINT_ARG;
+
+      /* Reject value out of a range.  */
+      if (option->range_max != -1
+	  && (value < option->range_min || value > option->range_max))
+	errors |= CL_ERR_INT_RANGE_ARG;
     }
 
   /* If the switch takes an enumerated argument, convert it.  */
@@ -1137,6 +1142,13 @@  cmdline_handle_error (location_t loc, const struct cl_option *option,
       return true;
     }
 
+  if (errors & CL_ERR_INT_RANGE_ARG)
+    {
+      error_at (loc, "argument to %qs is not between %d and %d",
+		option->opt_text, option->range_min, option->range_max);
+      return true;
+    }
+
   if (errors & CL_ERR_ENUM_ARG)
     {
       const struct cl_enum *e = &cl_enums[option->var_enum];
diff --git a/gcc/opts.c b/gcc/opts.c
index 8274fab6661..aac099ad8b0 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1256,6 +1256,15 @@  print_filtered_help (unsigned int include_flags,
 	  help = new_help;
 	}
 
+      if (option->range_max != -1)
+	{
+	  char b[128];
+	  snprintf (b, sizeof (b), "<%d,%d>", option->range_min,
+		    option->range_max);
+	  opt = concat (opt, b, NULL);
+	  len += strlen (b);
+	}
+
       wrap_help (help, opt, len, columns);
       displayed = true;
 
diff --git a/gcc/opts.h b/gcc/opts.h
index eb626aa90ec..38a390bbebb 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -110,6 +110,10 @@  struct cl_option
   enum cl_var_type var_type;
   /* Value or bit-mask with which to set a field.  */
   HOST_WIDE_INT var_value;
+  /* Range info minimum, or -1.  */
+  int range_min;
+  /* Range info maximum, or -1.  */
+  int range_max;
 };
 
 /* Records that the state of an option consists of SIZE bytes starting
@@ -200,8 +204,9 @@  extern const unsigned int cl_enums_count;
 #define CL_ERR_MISSING_ARG	(1 << 1) /* Argument required but missing.  */
 #define CL_ERR_WRONG_LANG	(1 << 2) /* Option for wrong language.  */
 #define CL_ERR_UINT_ARG		(1 << 3) /* Bad unsigned integer argument.  */
-#define CL_ERR_ENUM_ARG		(1 << 4) /* Bad enumerated argument.  */
-#define CL_ERR_NEGATIVE		(1 << 5) /* Negative form of option
+#define CL_ERR_INT_RANGE_ARG	(1 << 4) /* Bad unsigned integer argument.  */
+#define CL_ERR_ENUM_ARG		(1 << 5) /* Bad enumerated argument.  */
+#define CL_ERR_NEGATIVE		(1 << 6) /* Negative form of option
 					    not permitted (together
 					    with OPT_SPECIAL_unknown).  */
 
diff --git a/gcc/testsuite/g++.dg/opt/pr79659.C b/gcc/testsuite/g++.dg/opt/pr79659.C
new file mode 100644
index 00000000000..132d5c1e7f7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/opt/pr79659.C
@@ -0,0 +1,5 @@ 
+// PR target/79659
+// { dg-do compile }
+// { dg-options "-flifetime-dse=123456" }
+
+// { dg-error "is not between 0 and 2" "" { target *-*-* } 0 }
-- 
2.11.1