On Thu, Nov 24, 2022 at 4:27 PM Iskander Shakirzyanov <iskan...@ispras.ru> wrote: > > 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 <iskan...@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.)
OK if this passed bootstrap & regtest. Thanks, Richard. > PR driver/107787 > > Co-authored-by: Franz Sirl <franz.sirl-ker...@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 >