From: Richard Biener <richard.guenther@gmail.com>
To: Iskander Shakirzyanov <iskander@ispras.ru>
Cc: Franz Sirl <Franz.Sirl-kernel@lauterbach.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] Make Warray-bounds alias to Warray-bounds= [PR107787]
Date: Wed, 30 Nov 2022 08:25:03 +0100 [thread overview]
Message-ID: <CAFiYyc3DFr5HsAUefXAOOzLVFMW1JpcUBYeOhc+XUhC8oL=k5g@mail.gmail.com> (raw)
In-Reply-To: <87361B32-797C-4C7D-A82A-C0BB1B501DA8@ispras.ru>
On Thu, Nov 24, 2022 at 4:27 PM Iskander Shakirzyanov
<iskander@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 <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.)
OK if this passed bootstrap & regtest.
Thanks,
Richard.
> 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
>
prev parent reply other threads:[~2022-11-30 7:25 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-23 13:29 Искандер Шакирзянов
2022-11-23 14:08 ` Iskander Shakirzyanov
2022-11-23 20:11 ` Richard Biener
2022-11-24 9:54 ` Franz Sirl
2022-11-24 15:27 ` Iskander Shakirzyanov
2022-11-30 7:25 ` Richard Biener [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAFiYyc3DFr5HsAUefXAOOzLVFMW1JpcUBYeOhc+XUhC8oL=k5g@mail.gmail.com' \
--to=richard.guenther@gmail.com \
--cc=Franz.Sirl-kernel@lauterbach.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=iskander@ispras.ru \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).