From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x22d.google.com (mail-lj1-x22d.google.com [IPv6:2a00:1450:4864:20::22d]) by sourceware.org (Postfix) with ESMTPS id 3BE3E3858D3C for ; Wed, 30 Nov 2022 07:25:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3BE3E3858D3C Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lj1-x22d.google.com with SMTP id h10so3176671ljk.11 for ; Tue, 29 Nov 2022 23:25:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=MNytaUj4AYM2gcjz4uVuReRSj4hJWzwSxzqZcnaco3s=; b=TBvhsZ2K4t9kw4jXWM8Bi49DvQqvPxJfqaJAepWA+Tzh/1GT6/Gf2ewAMsJI9tARl/ TZ+83I4r5dfPFmXs8gF0unoNIObKGji6eEOPB0TfOEy/zJO7YWt8ngN82uUDdfb6GWoa LbYa0TJ4jvFjpuMM2PzCJAt8ulEuGP1xY4FFNUAho2f1R/YzJ2N6AMD/Gxj70kcqeVgL tnybS0nSbPfCH+DMtesjE0bxZwsvKNOjs7iUYABuPNCmhqlsC8QuApRZn9qIgETo/k7J nn9vtTvaWMMQMeABSFrG1jdli9solUv7rY7+WPQJkKxadS17lGvITC4FFPKQDK25WSzH eq/A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=MNytaUj4AYM2gcjz4uVuReRSj4hJWzwSxzqZcnaco3s=; b=emytzWHZNKYEUnoE3mTOy/765jmSQp372mtc8OwQ0h+EztQlwbmIKPpF6MynGKmg6c Vb8YEOVok9VlYtX2O9YBAG/BSsVZEXQEquJYl/NTYYGcizYnt9yRTVh/4rAP/R1Cs8kV uV95+ZG0XvW66qwit9rvoNYXhzy3VZewZDJdp6ZtBxa7mlWza99wR33MV7qUriizOW84 FvYILFpKuMXEL0/UxH0vTTakF+OoQmuubsmgnyLdTokTQ/1Md57iWzEkFFYuQMwGnbWJ zpMCiMDSt35azYOPx/OLhf8lckfjPtQivoQ1cd5z6lT1o6KS1Eso3BHVVA/ruvevCtiQ 6fxg== X-Gm-Message-State: ANoB5pkOsOEBQ0co42itxt1hDE6i5S0cFtm6ml0nk2nEu5aSgvvZRqTq pOei2Ot32UTEsFamyzUtZK9PjS7W1h2Fmomko94= X-Google-Smtp-Source: AA0mqf5lYiKdSA46GBlLR5eQmYwR0undP2itN+6ong2ztT66l5Efn3dqKCa4MkrP5e6SuLKAnTqCkx593x/jbC9f7cI= X-Received: by 2002:a2e:b5b3:0:b0:279:86ec:a686 with SMTP id f19-20020a2eb5b3000000b0027986eca686mr9268178ljn.399.1669793116451; Tue, 29 Nov 2022 23:25:16 -0800 (PST) MIME-Version: 1.0 References: <9795E6AA-E646-4134-AABE-23F530F5219E@ispras.ru> <2552ab22-916f-d0fe-2c78-d482f6ad8412@lauterbach.com> <87361B32-797C-4C7D-A82A-C0BB1B501DA8@ispras.ru> In-Reply-To: <87361B32-797C-4C7D-A82A-C0BB1B501DA8@ispras.ru> From: Richard Biener Date: Wed, 30 Nov 2022 08:25:03 +0100 Message-ID: Subject: Re: [PATCH] Make Warray-bounds alias to Warray-bounds= [PR107787] To: Iskander Shakirzyanov Cc: Franz Sirl , gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, Nov 24, 2022 at 4:27 PM Iskander Shakirzyanov 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 > 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 > > 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 >