public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "rguenth 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 12:49:00 +0000	[thread overview]
Message-ID: <bug-107561-4-LwKWmTm1B0@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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jwakely.gcc at gmail dot com
           Keywords|testsuite-fail              |missed-optimization

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
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.

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 ...

    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.

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.

That we are not allowed to optimize the code is not of help either.

We can again work around this in libstdc++ by CSEing ->_M_size ourselves.
The following helps:

diff --git a/libstdc++-v3/include/std/valarray
b/libstdc++-v3/include/std/valarray
index 7a23c27a0ce..7383071f98d 100644
--- a/libstdc++-v3/include/std/valarray
+++ b/libstdc++-v3/include/std/valarray
@@ -647,8 +647,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     inline
     valarray<_Tp>::valarray(const valarray<_Tp>& __v)
     : _M_size(__v._M_size), _M_data(__valarray_get_storage<_Tp>(__v._M_size))
-    { std::__valarray_copy_construct(__v._M_data, __v._M_data + _M_size,
-                                    _M_data); }
+    {
+      auto __v_M_size = __v._M_size;
+      _M_size = __v_M_size;
+      _M_data = __valarray_get_storage<_Tp>(__v_M_size);
+      std::__valarray_copy_construct(__v._M_data, __v._M_data + __v_M_size,
+                                    _M_data);
+    }

 #if __cplusplus >= 201103L
   template<typename _Tp>

  parent reply	other threads:[~2023-02-27 12:49 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 [this message]
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
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-LwKWmTm1B0@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).