Re: [PATCH] Make Warray-bounds alias to Warray-bounds= [PR107787]

[Iskander Shakirzyanov <iskander@ispras.ru>]

[-- Attachment #1: Type: text/plain, Size: 16228 bytes --]

>> How did you test the patch? If you bootstrapped it and ran the
>> testsuite then it's OK.
Yes, i ran testsuite and  bootstrapped and everything seemed OK, but i missed fail of tests gcc.dg/Warray-bounds-34.c and gcc.dg/Warray-bounds-43.c, so Franz is right. After that I fixed the regexps in dg directives and now everything seems OK.  

> I'm pretty sure the testsuite will have regressions, as I have a very similar patch lying around that needs these testsuite changes.

You are right, thank you. I missed this, attaching corrected version of patch.

> This also shows nicely why I don't like warnings with levels, what if I want -Werror=array-bounds=2 + -Warray-bounds=1?

I completely agree with you, because I also thought about using -Werror=opt=X + -Wopt=Y, this functionality looks useful. As I know, gcc, while parsing an option with the same OPT,  overwrites the old config of OPT. 

> Because I think at least -Wuse-after-free= and Wattributes= have the same problem.

Yes, looks like this, probably should be fixed too.  

> BTW, is the duplicated warning description "Warn if an array is accessed out of bounds." needed or not with Alias()?

According to other examples in common.opt, duplicated description is not necessary, you are right.

> I've attached my patch, feel free to integrate the testsuite changes.

Thanks, but it seems to me that duplicating existing tests seems redundant to test functionality of -Werror=array-bounds=X.


From bf047e36392dab138db10be2ec257d08c376ada5 Mon Sep 17 00:00:00 2001
From: Iskander Shakirzyanov <iskander@ispras.ru>
Date: Thu, 24 Nov 2022 14:26:59 +0000
Subject: [PATCH] Make Warray-bounds alias to Warray-bounds= [PR107787]

According to documentation the -Werror= option makes the specified warning
into an error and also automatically implies this option. Then it seems that
the behavior of the compiler when specifying -Werror=array-bounds=X should be
the same as specifying "-Werror=array-bounds -Warray-bounds=X", so we expect to
receive array-bounds pass triggers and they must be processed as errors.
In practice, we observe that the array-bounds pass is indeed called, but
its responses are processed as warnings, not errors.
As I understand, this happens because Warray-bounds and Warray-bounds= are
declared as 2 different options in common.opt, so when
diagnostic_classify_diagnostic() is called, DK_ERROR is set for
the Warray-bounds= option, but in diagnostic_report_diagnostic() through
warning_at() passes opt_index of Warray-bounds, so information about
DK_ERROR is lost. Fixed by using Alias() in declaration of
Warray-bounds (similarly as in Wattribute-alias etc.)

PR driver/107787

Co-authored-by: Franz Sirl <Franz.Sirl-kernel@lauterbach.com>

gcc/ChangeLog:

	* common.opt (Warray-bounds): Turn into alias to
	-Warray-bounds=1.
	* builtins.cc (warn_array_bounds): Use OPT_Warray_bounds_
	instead of OPT_Warray_bounds.
	* diagnostic-spec.cc: Likewise.
	* gimple-array-bounds.cc: Likewise.
	* gimple-ssa-warn-restrict.cc: Likewise.

gcc/testsuite/ChangeLog:

	* gcc.dg/Warray-bounds-34.c: Correct the regular
	expression for -Warray-bounds=.
	* gcc.dg/Warray-bounds-43.c: Likewise.
	* gcc.dg/pr107787.c: New test.

gcc/c-family/ChangeLog:

	* c-common.cc (warn_array_bounds): Use OPT_Warray_bounds_
	instead of OPT_Warray_bounds.
---
 gcc/builtins.cc                         |  6 ++--
 gcc/c-family/c-common.cc                |  4 +--
 gcc/common.opt                          |  3 +-
 gcc/diagnostic-spec.cc                  |  1 -
 gcc/gimple-array-bounds.cc              | 38 ++++++++++++-------------
 gcc/gimple-ssa-warn-restrict.cc         |  2 +-
 gcc/testsuite/gcc.dg/Warray-bounds-34.c |  2 +-
 gcc/testsuite/gcc.dg/Warray-bounds-43.c |  6 ++--
 gcc/testsuite/gcc.dg/pr107787.c         | 13 +++++++++
 9 files changed, 43 insertions(+), 32 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr107787.c

diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index 4dc1ca672b2..02c4fefa86f 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -696,14 +696,14 @@ c_strlen (tree arg, int only_value, c_strlen_data *data, unsigned eltsize)
     {
       /* Suppress multiple warnings for propagated constant strings.  */
       if (only_value != 2
-	  && !warning_suppressed_p (arg, OPT_Warray_bounds)
-	  && warning_at (loc, OPT_Warray_bounds,
+	  && !warning_suppressed_p (arg, OPT_Warray_bounds_)
+	  && warning_at (loc, OPT_Warray_bounds_,
 			 "offset %qwi outside bounds of constant string",
 			 eltoff))
 	{
 	  if (decl)
 	    inform (DECL_SOURCE_LOCATION (decl), "%qE declared here", decl);
-	  suppress_warning (arg, OPT_Warray_bounds);
+	  suppress_warning (arg, OPT_Warray_bounds_);
 	}
       return NULL_TREE;
     }
diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index 6f1f21bc4c1..b0da6886ccf 100644
--- a/gcc/c-family/c-common.cc
+++ b/gcc/c-family/c-common.cc
@@ -6811,7 +6811,7 @@ fold_offsetof (tree expr, tree type, enum tree_code ctx)
 		     definition thereof.  */
 		  if (TREE_CODE (v) == ARRAY_REF
 		      || TREE_CODE (v) == COMPONENT_REF)
-		    warning (OPT_Warray_bounds,
+		    warning (OPT_Warray_bounds_,
 			     "index %E denotes an offset "
 			     "greater than size of %qT",
 			     t, TREE_TYPE (TREE_OPERAND (expr, 0)));
@@ -8534,7 +8534,7 @@ convert_vector_to_array_for_subscript (location_t loc,
       if (TREE_CODE (index) == INTEGER_CST)
         if (!tree_fits_uhwi_p (index)
 	    || maybe_ge (tree_to_uhwi (index), TYPE_VECTOR_SUBPARTS (type)))
-          warning_at (loc, OPT_Warray_bounds, "index value is out of bound");
+          warning_at (loc, OPT_Warray_bounds_, "index value is out of bound");
 
       /* We are building an ARRAY_REF so mark the vector as addressable
          to not run into the gimplifiers premature setting of DECL_GIMPLE_REG_P
diff --git a/gcc/common.opt b/gcc/common.opt
index 26e9d1cc4e7..f91b631f257 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -539,8 +539,7 @@ Common Var(warn_aggressive_loop_optimizations) Init(1) Warning
 Warn if a loop with constant number of iterations triggers undefined behavior.
 
 Warray-bounds
-Common Var(warn_array_bounds) Warning
-Warn if an array is accessed out of bounds.
+Common Alias(Warray-bounds=, 1, 0) Warning
 
 Warray-bounds=
 Common Joined RejectNegative UInteger Var(warn_array_bounds) Warning IntegerRange(0, 2)
diff --git a/gcc/diagnostic-spec.cc b/gcc/diagnostic-spec.cc
index aece89619e7..7a03fc493e6 100644
--- a/gcc/diagnostic-spec.cc
+++ b/gcc/diagnostic-spec.cc
@@ -79,7 +79,6 @@ nowarn_spec_t::nowarn_spec_t (opt_code opt)
       break;
 
       /* Access warning group.  */
-    case OPT_Warray_bounds:
     case OPT_Warray_bounds_:
     case OPT_Wformat_overflow_:
     case OPT_Wformat_truncation_:
diff --git a/gcc/gimple-array-bounds.cc b/gcc/gimple-array-bounds.cc
index fbf448e045d..7af85b86f75 100644
--- a/gcc/gimple-array-bounds.cc
+++ b/gcc/gimple-array-bounds.cc
@@ -182,7 +182,7 @@ bool
 array_bounds_checker::check_array_ref (location_t location, tree ref,
 				       gimple *stmt, bool ignore_off_by_one)
 {
-  if (warning_suppressed_p (ref, OPT_Warray_bounds))
+  if (warning_suppressed_p (ref, OPT_Warray_bounds_))
     /* Return true to have the caller prevent warnings for enclosing
        refs.  */
     return true;
@@ -287,7 +287,7 @@ array_bounds_checker::check_array_ref (location_t location, tree ref,
 
   /* Empty array.  */
   if (up_bound && tree_int_cst_equal (low_bound, up_bound_p1))
-    warned = warning_at (location, OPT_Warray_bounds,
+    warned = warning_at (location, OPT_Warray_bounds_,
 			 "array subscript %E is outside array bounds of %qT",
 			 low_sub, artype);
 
@@ -313,7 +313,7 @@ array_bounds_checker::check_array_ref (location_t location, tree ref,
 	      : tree_int_cst_le (up_bound, up_sub))
 	  && TREE_CODE (low_sub) == INTEGER_CST
 	  && tree_int_cst_le (low_sub, low_bound))
-	warned = warning_at (location, OPT_Warray_bounds,
+	warned = warning_at (location, OPT_Warray_bounds_,
 			     "array subscript [%E, %E] is outside "
 			     "array bounds of %qT",
 			     low_sub, up_sub, artype);
@@ -323,12 +323,12 @@ array_bounds_checker::check_array_ref (location_t location, tree ref,
 	   && (ignore_off_by_one
 	       ? !tree_int_cst_le (up_sub, up_bound_p1)
 	       : !tree_int_cst_le (up_sub, up_bound)))
-    warned = warning_at (location, OPT_Warray_bounds,
+    warned = warning_at (location, OPT_Warray_bounds_,
 			 "array subscript %E is above array bounds of %qT",
 			 up_sub, artype);
   else if (TREE_CODE (low_sub) == INTEGER_CST
 	   && tree_int_cst_lt (low_sub, low_bound))
-    warned = warning_at (location, OPT_Warray_bounds,
+    warned = warning_at (location, OPT_Warray_bounds_,
 			 "array subscript %E is below array bounds of %qT",
 			 low_sub, artype);
 
@@ -353,7 +353,7 @@ array_bounds_checker::check_array_ref (location_t location, tree ref,
       /* Avoid more warnings when checking more significant subscripts
 	 of the same expression.  */
       ref = TREE_OPERAND (ref, 0);
-      suppress_warning (ref, OPT_Warray_bounds);
+      suppress_warning (ref, OPT_Warray_bounds_);
 
       if (decl)
 	ref = decl;
@@ -393,7 +393,7 @@ bool
 array_bounds_checker::check_mem_ref (location_t location, tree ref,
 				     bool ignore_off_by_one)
 {
-  if (warning_suppressed_p (ref, OPT_Warray_bounds))
+  if (warning_suppressed_p (ref, OPT_Warray_bounds_))
     return false;
 
   /* The statement used to allocate the array or null.  */
@@ -493,12 +493,12 @@ array_bounds_checker::check_mem_ref (location_t location, tree ref,
   if (lboob)
     {
       if (offrange[0] == offrange[1])
-	warned = warning_at (location, OPT_Warray_bounds,
+	warned = warning_at (location, OPT_Warray_bounds_,
 			     "array subscript %wi is outside array bounds "
 			     "of %qT",
 			     offrange[0].to_shwi (), reftype);
       else
-	warned = warning_at (location, OPT_Warray_bounds,
+	warned = warning_at (location, OPT_Warray_bounds_,
 			     "array subscript [%wi, %wi] is outside "
 			     "array bounds of %qT",
 			     offrange[0].to_shwi (),
@@ -513,7 +513,7 @@ array_bounds_checker::check_mem_ref (location_t location, tree ref,
 	backtype = build_array_type_nelts (unsigned_char_type_node,
 					   aref.sizrng[1].to_uhwi ());
 
-      warned = warning_at (location, OPT_Warray_bounds,
+      warned = warning_at (location, OPT_Warray_bounds_,
 			   "array subscript %<%T[%wi]%> is partly "
 			   "outside array bounds of %qT",
 			   axstype, offrange[0].to_shwi (), backtype);
@@ -523,7 +523,7 @@ array_bounds_checker::check_mem_ref (location_t location, tree ref,
     {
       /* TODO: Determine the access from the statement and use it.  */
       aref.inform_access (access_none);
-      suppress_warning (ref, OPT_Warray_bounds);
+      suppress_warning (ref, OPT_Warray_bounds_);
       return true;
     }
 
@@ -536,11 +536,11 @@ array_bounds_checker::check_mem_ref (location_t location, tree ref,
     {
       HOST_WIDE_INT tmpidx = (aref.offmax[i] / eltsize).to_shwi ();
 
-      if (warning_at (location, OPT_Warray_bounds,
+      if (warning_at (location, OPT_Warray_bounds_,
 		      "intermediate array offset %wi is outside array bounds "
 		      "of %qT", tmpidx, reftype))
 	{
-	  suppress_warning (ref, OPT_Warray_bounds);
+	  suppress_warning (ref, OPT_Warray_bounds_);
 	  return true;
 	}
     }
@@ -572,7 +572,7 @@ array_bounds_checker::check_addr_expr (location_t location, tree t,
 	warned = check_mem_ref (location, t, ignore_off_by_one);
 
       if (warned)
-	suppress_warning (t, OPT_Warray_bounds);
+	suppress_warning (t, OPT_Warray_bounds_);
 
       t = TREE_OPERAND (t, 0);
     }
@@ -580,7 +580,7 @@ array_bounds_checker::check_addr_expr (location_t location, tree t,
 
   if (TREE_CODE (t) != MEM_REF
       || TREE_CODE (TREE_OPERAND (t, 0)) != ADDR_EXPR
-      || warning_suppressed_p (t, OPT_Warray_bounds))
+      || warning_suppressed_p (t, OPT_Warray_bounds_))
     return;
 
   tree tem = TREE_OPERAND (TREE_OPERAND (t, 0), 0);
@@ -615,7 +615,7 @@ array_bounds_checker::check_addr_expr (location_t location, tree t,
 	  dump_generic_expr (MSG_NOTE, TDF_SLIM, t);
 	  fprintf (dump_file, "\n");
 	}
-      warned = warning_at (location, OPT_Warray_bounds,
+      warned = warning_at (location, OPT_Warray_bounds_,
 			   "array subscript %wi is below "
 			   "array bounds of %qT",
 			   idx.to_shwi (), TREE_TYPE (tem));
@@ -629,7 +629,7 @@ array_bounds_checker::check_addr_expr (location_t location, tree t,
 	  dump_generic_expr (MSG_NOTE, TDF_SLIM, t);
 	  fprintf (dump_file, "\n");
 	}
-      warned = warning_at (location, OPT_Warray_bounds,
+      warned = warning_at (location, OPT_Warray_bounds_,
 			   "array subscript %wu is above "
 			   "array bounds of %qT",
 			   idx.to_uhwi (), TREE_TYPE (tem));
@@ -640,7 +640,7 @@ array_bounds_checker::check_addr_expr (location_t location, tree t,
       if (DECL_P (t))
 	inform (DECL_SOURCE_LOCATION (t), "while referencing %qD", t);
 
-      suppress_warning (t, OPT_Warray_bounds);
+      suppress_warning (t, OPT_Warray_bounds_);
     }
 }
 
@@ -740,7 +740,7 @@ array_bounds_checker::check_array_bounds (tree *tp, int *walk_subtree,
   /* Propagate the no-warning bit to the outer statement to avoid also
      issuing -Wstringop-overflow/-overread for the out-of-bounds accesses.  */
   if (warned)
-    suppress_warning (wi->stmt, OPT_Warray_bounds);
+    suppress_warning (wi->stmt, OPT_Warray_bounds_);
 
   return NULL_TREE;
 }
diff --git a/gcc/gimple-ssa-warn-restrict.cc b/gcc/gimple-ssa-warn-restrict.cc
index 832456ba6fc..107ba278798 100644
--- a/gcc/gimple-ssa-warn-restrict.cc
+++ b/gcc/gimple-ssa-warn-restrict.cc
@@ -1734,7 +1734,7 @@ maybe_diag_access_bounds (gimple *call, tree func, int strict,
   if (!oobref)
     return no_warning;
 
-  const opt_code opt = OPT_Warray_bounds;
+  const opt_code opt = OPT_Warray_bounds_;
   /* Return true without issuing a warning.  */
   if (!do_warn)
     return opt;
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-34.c b/gcc/testsuite/gcc.dg/Warray-bounds-34.c
index cea7c4b32a2..1b2a098ad86 100644
--- a/gcc/testsuite/gcc.dg/Warray-bounds-34.c
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-34.c
@@ -7,7 +7,7 @@ int x;
 
 inline void foo (int i)
 {
-  a[i + 1] = 123;   /* { dg-warning "\\\[-Warray-bounds]" } */
+  a[i + 1] = 123;   /* { dg-warning "\\\[-Warray-bounds" } */
 }
 
 int bar (void)
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-43.c b/gcc/testsuite/gcc.dg/Warray-bounds-43.c
index 0f521a7250d..43a83315787 100644
--- a/gcc/testsuite/gcc.dg/Warray-bounds-43.c
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-43.c
@@ -19,7 +19,7 @@ NOIPA int g2 (int i)
 
   sink (p0, p1, p2);
 
-  return p2[8];     // { dg-warning "\\\[-Warray-bounds]" }
+  return p2[8];     // { dg-warning "\\\[-Warray-bounds" }
 }
 
 NOIPA int g3 (int i)
@@ -33,7 +33,7 @@ NOIPA int g3 (int i)
 
   sink (p0, p1, p2, p3);
 
-  return p3[7];     // { dg-warning "\\\[-Warray-bounds]" }
+  return p3[7];     // { dg-warning "\\\[-Warray-bounds" }
 }
 
 NOIPA int g4 (int i)
@@ -48,7 +48,7 @@ NOIPA int g4 (int i)
 
   sink (p0, p1, p2, p3, p4);
 
-  return p4[6];     // { dg-warning "\\\[-Warray-bounds]" }
+  return p4[6];     // { dg-warning "\\\[-Warray-bounds" }
 }
 
 NOIPA int g5 (int i)
diff --git a/gcc/testsuite/gcc.dg/pr107787.c b/gcc/testsuite/gcc.dg/pr107787.c
new file mode 100644
index 00000000000..922dbff8b2a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr107787.c
@@ -0,0 +1,13 @@
+/* PR driver/107787 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -Werror=array-bounds=1" } */
+/* { dg-message "some warnings being treated as errors" "" {target "*-*-*"} 0 } */
+
+int a[10];     /* { dg-note "while referencing" } */
+
+int* f(void) {
+
+    a[-1] = 0; /* { dg-error "is below array bounds" } */
+
+    return a;
+}
-- 
2.25.1