public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "aldyh at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug tree-optimization/107561] [13 Regression] g++.dg/pr71488.C  and [g++.dg/warn/Warray-bounds-16.C -m32] regression due to -Wstringop-overflow problem
Date: Mon, 27 Feb 2023 16:16:24 +0000	[thread overview]
Message-ID: <bug-107561-4-LSI4s7uk5z@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-107561-4@http.gcc.gnu.org/bugzilla/>

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107561

--- Comment #14 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #11)
> So I've missed the VR_ANTI_RANGE handling in get_size_range where we run into
> 
>           wide_int maxsize = wi::to_wide (max_object_size ());
>           min = wide_int::from (min, maxsize.get_precision (), UNSIGNED);
>           max = wide_int::from (max, maxsize.get_precision (), UNSIGNED);
>           if (wi::eq_p (0, min - 1))
>             {
>               /* EXP is unsigned and not in the range [1, MAX].  That means
>                  it's either zero or greater than MAX.  Even though 0 would
>                  normally be detected by -Walloc-zero, unless ALLOW_ZERO
>                  is set, set the range to [MAX, TYPE_MAX] so that when MAX
>                  is greater than the limit the whole range is diagnosed.  */
>               wide_int maxsize = wi::to_wide (max_object_size ());
>               if (flags & SR_ALLOW_ZERO)
>                 {
>                   if (wi::leu_p (maxsize, max + 1)
>                       || !(flags & SR_USE_LARGEST))
>                     min = max = wi::zero (expprec);
>                   else
>                     {
>                       min = max + 1;
>                       max = wi::to_wide (TYPE_MAX_VALUE (exptype));
>                     }
>                 }
>               else
>                 {
>                   min = max + 1;
>                   max = wi::to_wide (TYPE_MAX_VALUE (exptype));
>                 }
> 
> and from [0,0] [8, +INF] pick [8, +INF] based on the comments reasoning.

Ughh, you're reaching all the problematic cases I ran into while trying to
remove legacy.

> 
> This all wouldn't happen if we'd be able to CSE the zero size ...
> 
> We can now try to put additional heuristic ontop of the above heuristic,
> namely when the object we write to is of size zero set SR_ALLOW_ZERO.
> Or try to "undo" the multiplication trick which would probably make us
> end up with VARYING.
> 
> I'll note that with the earlier proposed change we regress the following,
> that's an observation I make a lot of times - all "weirdness" in the code
> is backed by (artificial) testcases verifying it works exactly as coded ...

+1

> 
>     FAIL: gcc.dg/Wstringop-overflow-15.c pr82608 (test for warnings, line 37)
>     FAIL: gcc.dg/Wstringop-overflow-15.c pr82608 (test for warnings, line 38)
>     FAIL: gcc.dg/Wstringop-overflow-15.c pr82608 (test for warnings, line 69)
>     FAIL: gcc.dg/Wstringop-overflow-15.c pr82608 (test for warnings, line 70)
>     FAIL: gcc.dg/Wstringop-overflow-56.c  (test for warnings, line 64)
>     FAIL: gcc.dg/Wstringop-overflow-56.c  (test for warnings, line 75)
>     FAIL: gcc.dg/Wstringop-overflow-56.c  (test for warnings, line 86)
>     FAIL: gcc.dg/Wstringop-overflow-56.c  (test for warnings, line 97)
>     FAIL: gcc.dg/Wstringop-overflow-56.c  (test for warnings, line 108)
>     FAIL: gcc.dg/Wstringop-overflow-56.c  (test for warnings, line 148)
>     FAIL: gcc.dg/Wstringop-overflow-56.c  (test for warnings, line 159)
>     FAIL: gcc.dg/attr-alloc_size-11.c  (test for warnings, line 52)
>     FAIL: gcc.dg/attr-alloc_size-11.c  (test for warnings, line 53)
>     FAIL: gcc.dg/attr-alloc_size-11.c  (test for warnings, line 54)
>     FAIL: gcc.dg/attr-alloc_size-11.c  (test for warnings, line 55)
>     FAIL: gcc.dg/attr-alloc_size-11.c  (test for warnings, line 56)
>     FAIL: gcc.dg/attr-alloc_size-11.c  (test for warnings, line 57)
>     FAIL: gcc.dg/attr-alloc_size-11.c  (test for warnings, line 58)
>     FAIL: gcc.dg/attr-alloc_size-11.c  (test for warnings, line 64)
>     FAIL: gcc.dg/attr-alloc_size-11.c  (test for warnings, line 65)
>     FAIL: gcc.dg/attr-alloc_size-3.c  (test for warnings, line 438)
>     FAIL: gcc.dg/attr-alloc_size-4.c  (test for warnings, line 138)
>     FAIL: gcc.dg/attr-alloc_size-4.c  (test for warnings, line 143)
>     FAIL: gcc.dg/attr-alloc_size-4.c  (test for warnings, line 187)
>     FAIL: gcc.dg/pr98721-1.c  (test for warnings, line 11)
>     FAIL: gcc.dg/pr98721-1.c  (test for warnings, line 12)
> 
> For example gcc.dg/pr98721-1.c has
> 
> int
> foo (int n)
> {
>   if (n <= 0)
>     {
>       char vla[n];                      /* { dg-message "source object 'vla'
> of size 0" } */
>       return __builtin_strlen (vla);    /* { dg-warning "'__builtin_strlen'
> reading 1 or more bytes from a region of size 0" } */
> 
> but of course we do not diagnose
> 
> int
> foo (int n)
> {
>   if (n < 0)
>     {
>       char vla[n];
> 
> or when no condition is present or a n > 32 condition is present.

Yup, ran into that too.

> 
> I fear it's not possible to "fix" this testcase without changing the
> expectation on a bunch of other testcases.  But the messaging to the
> user is quite unhelpful because it doesn't actually inform him about
> above reasoning.

Agreed.

  parent reply	other threads:[~2023-02-27 16:16 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-07 20:06 [Bug tree-optimization/107561] New: g++.dg/pr17488.C " aldyh at gcc dot gnu.org
2022-11-07 20:10 ` [Bug tree-optimization/107561] [13 Regression] " pinskia at gcc dot gnu.org
2022-11-08  8:31 ` aldyh at gcc dot gnu.org
2022-11-08  8:48 ` [Bug tree-optimization/107561] [13 Regression] g++.dg/pr17488.C and [g++.dg/warn/Warray-bounds-16.C -m32] " aldyh at gcc dot gnu.org
2022-11-08  9:34 ` rguenth at gcc dot gnu.org
2022-11-08 13:43 ` aldyh at gcc dot gnu.org
2022-11-08 18:27 ` pinskia at gcc dot gnu.org
2022-12-14 15:43 ` [Bug tree-optimization/107561] [13 Regression] g++.dg/pr71488.C " danglin at gcc dot gnu.org
2023-01-13 12:42 ` rguenth at gcc dot gnu.org
2023-02-02 18:53 ` hp at gcc dot gnu.org
2023-02-02 21:03 ` hp at gcc dot gnu.org
2023-02-10  0:42 ` cvs-commit at gcc dot gnu.org
2023-02-27  9:56 ` rguenth at gcc dot gnu.org
2023-02-27 11:18 ` rguenth at gcc dot gnu.org
2023-02-27 12:49 ` rguenth at gcc dot gnu.org
2023-02-27 13:45 ` aldyh at gcc dot gnu.org
2023-02-27 14:38 ` redi at gcc dot gnu.org
2023-02-27 16:16 ` aldyh at gcc dot gnu.org [this message]
2023-03-01 14:22 ` rguenth at gcc dot gnu.org
2023-03-01 14:34 ` jakub at gcc dot gnu.org
2023-03-01 14:38 ` rguenth at gcc dot gnu.org
2023-03-01 14:55 ` jakub at gcc dot gnu.org
2023-03-01 14:57 ` jakub at gcc dot gnu.org
2023-03-01 15:55 ` redi at gcc dot gnu.org
2023-03-02  7:51 ` rguenth at gcc dot gnu.org
2023-03-02  7:53 ` rguenth at gcc dot gnu.org
2023-03-29 11:38 ` rguenth at gcc dot gnu.org
2023-03-29 11:41 ` rguenth at gcc dot gnu.org
2023-03-30 11:16 ` cvs-commit at gcc dot gnu.org
2023-03-30 11:17 ` rguenth at gcc dot gnu.org

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=bug-107561-4-LSI4s7uk5z@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.org \
    /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).