[Bug middle-end/96384] New: [11 Regression] bogus -Wstringop-overflow= storing into multidimensional array with index in range

[Bug middle-end/96384] New: [11 Regression] bogus -Wstringop-overflow= storing into multidimensional array with index in range

[msebor at gcc dot gnu.org]

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

            Bug ID: 96384
           Summary: [11 Regression] bogus -Wstringop-overflow= storing
                    into multidimensional array with index in range
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: msebor at gcc dot gnu.org
  Target Milestone: ---

The following test case triggers a false positive -Wstringop-overflow:

$ cat z.c && gcc -O2 -S -Wall -fdump-tree-strlen-all=/dev/stdout z.c
char a[2][3];

void f (unsigned i)
{
  if (i > ~0U - 1)
    i = ~0U - 1;
  a[i][0] = 0;
}

;; Function f (f, funcdef_no=0, decl_uid=1932, cgraph_uid=1, symbol_order=1)


Pass statistics of "strlen": ----------------

;; 1 loops found
;;
;; Loop 0
;;  header 0, latch 1
;;  depth 0, outer -1
;;  nodes: 0 1 2
;; 2 succs { 1 }

extract_range_from_stmt visiting:
# RANGE [0, 4294967294]
_2 = MIN_EXPR <i_1(D), 4294967294>;
Intersecting
  unsigned int [0, 4294967294]
and
  unsigned int [0, 4294967294]
to
  unsigned int [0, 4294967294]
z.c: In function ‘f’:
z.c:7:11: warning: writing 1 byte into a region of size 0
[-Wstringop-overflow=]
    7 |   a[i][0] = 0;
      |   ~~~~~~~~^~~
z.c:1:6: note: at offset 0 to object ‘a’ with size 6 declared here
    1 | char a[2][3];
      |      ^
maybe_invalidate called for a[_2][0] = 0;
maybe_invalidate returns 0

Pass statistics of "strlen": ----------------

f (unsigned intD.9 iD.1931)
{
  unsigned intD.9 i_1(D) = iD.1931;
  unsigned intD.9 _2;

;;   basic block 2, loop depth 0, count 1073741824 (estimated locally), maybe
hot
;;    prev block 0, next block 1, flags: (NEW, REACHABLE, VISITED)
;;    pred:       ENTRY [always]  count:1073741824 (estimated locally)
(FALLTHRU,EXECUTABLE)
  # RANGE [0, 4294967294]
  _2 = MIN_EXPR <i_1(D), 4294967294>;
  # .MEM_4 = VDEF <.MEM_3(D)>
  aD.1930[_2][0] = 0;
  # VUSE <.MEM_4>
  return;
;;    succ:       EXIT [always]  count:1073741824 (estimated locally)
(EXECUTABLE)

}

[Bug middle-end/96384] [11 Regression] bogus -Wstringop-overflow= storing into multidimensional array with index in range

[msebor at gcc dot gnu.org]

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

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2020-07-29
           Assignee|unassigned at gcc dot gnu.org      |msebor at gcc dot gnu.org
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
             Blocks|                            |88443
           Keywords|                            |diagnostic


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88443
[Bug 88443] [meta-bug] bogus/missing -Wstringop-overflow warnings

[Bug middle-end/96384] [11 Regression] bogus -Wstringop-overflow= storing into multidimensional array with index in range

[msebor at gcc dot gnu.org]

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

--- Comment #1 from Martin Sebor <msebor at gcc dot gnu.org> ---
It looks like a false positive caused by the weirdo signed vs unsigned
conversions between wide_int and offset_int.  It happens in this piece of code
in compute_objsize:

      offset_int orng[2];
      tree off = TREE_OPERAND (ptr, 1);
      if (!get_range (off, SIGNED, orng, rvals))
    /* Fail unless the size of the object is zero.  */
    return pref->sizrng[0] == 0 && pref->sizrng[0] == pref->sizrng[1];
      ...

      if (ostype && TREE_CODE (eltype) == ARRAY_TYPE)
        {
          /* Execpt for the permissive raw memory functions which
         use the size of the whole object determined above,
         use the size of the referenced array.  */
          pref->sizrng[0] = pref->offrng[0] + orng[0] + sz;
          pref->sizrng[1] = pref->offrng[1] + orng[1] + sz;
        }
    }

We get orng indirectly by converting the get_range_info() result in get_range()
in tree-ssa-strlen.c to wide_int first, and then converting it to offset_int. 
We do this dance because some clients use wide_int and others offset_int.

We get vr->max ():

  <integer_cst 0x7fffe8f3bbe8 type <integer_type 0x7fffea0f7d20 guint> constant
4294967294>

the result of converting it to wide_int:

      minmax[1] = wi::to_wide (vr->max ());
is

(gdb) p minmax[1]
$39 = {<wide_int_storage> = {val = {-2, 24874380, 0}, len = 1, precision = 32},
static is_sign_extended = true}

We then take it and convert it to offset_int in the get_range() helper in
builtins.c:

  r[1] = offset_int::from (wr[1], sgn);

Because it's an offset, sgn is SIGNED so we end up with -2 instead of the
original positive 4294967294.

We use that -2 to compute the upper bound on the size of the object and store
it in pref->sizrng[1], which becomes negative.  In the end, we compare the
lower bound of the overall offset into the object (it's 0) to the upper bound
of the size (which is -6) to see if it's less.  It's not so the result is zero
bytes of space.

[Bug middle-end/96384] [11 Regression] bogus -Wstringop-overflow= storing into multidimensional array with index in range

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.0

[Bug middle-end/96384] [11 Regression] bogus -Wstringop-overflow= storing into multidimensional array with index in range

[msebor at gcc dot gnu.org]

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

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch

--- Comment #2 from Martin Sebor <msebor at gcc dot gnu.org> ---
Patch: https://gcc.gnu.org/pipermail/gcc-patches/2020-August/551783.html

[Bug middle-end/96384] [11 Regression] bogus -Wstringop-overflow= storing into multidimensional array with index in range

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>:

https://gcc.gnu.org/g:de05c19d5fd661ae16dd75a895b49d32d12f5edc

commit r11-3826-gde05c19d5fd661ae16dd75a895b49d32d12f5edc
Author: Martin Sebor <msebor@redhat.com>
Date:   Fri Oct 9 13:56:53 2020 -0600

    Correct handling of indices into arrays with elements larger than 1 (PR
c++/96511)

    Resolves:
    PR c++/96511 - Incorrect -Wplacement-new on POINTER_PLUS into an array with
4-byte elements
    PR middle-end/96384 - bogus -Wstringop-overflow= storing into
multidimensional array with index in range

    gcc/ChangeLog:

            PR c++/96511
            PR middle-end/96384
            * builtins.c (get_range): Return full range of type when neither
            value nor its range is available.  Fail for ranges inverted due
            to the signedness of offsets.
            (compute_objsize): Handle more special array members.  Handle
            POINTER_PLUS_EXPR and VIEW_CONVERT_EXPR that come up in front end
            code.
            (access_ref::offset_bounded): Define new member function.
            * builtins.h (access_ref::eval): New data member.
            (access_ref::offset_bounded): New member function.
            (access_ref::offset_zero): New member function.
            (compute_objsize): Declare a new overload.
            * gimple-array-bounds.cc (array_bounds_checker::check_array_ref):
Use
            enum special_array_member.
            * tree.c (component_ref_size): Use special_array_member.
            * tree.h (special_array_member): Define a new type.
            (component_ref_size): Change signature.

    gcc/cp/ChangeLog:

            PR c++/96511
            PR middle-end/96384
            * init.c (warn_placement_new_too_small): Call builtin_objsize
instead
            of duplicating what it does.

    gcc/testsuite/ChangeLog:

            PR c++/96511
            PR middle-end/96384
            * g++.dg/init/strlen.C: Add expected warning.
            * g++.dg/warn/Wplacement-new-size-1.C: Relax warnings.
            * g++.dg/warn/Wplacement-new-size-2.C: Same.
            * g++.dg/warn/Wplacement-new-size-6.C: Same.
            * gcc.dg/Warray-bounds-58.c: Adjust
            * gcc.dg/Wstringop-overflow-37.c: Same.
            * g++.dg/warn/Wplacement-new-size-7.C: New test.

[Bug middle-end/96384] [11 Regression] bogus -Wstringop-overflow= storing into multidimensional array with index in range

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>:

https://gcc.gnu.org/g:83685efd5fd1623cfc4e4c435ce2773d95d458d1

commit r11-3827-g83685efd5fd1623cfc4e4c435ce2773d95d458d1
Author: Martin Sebor <msebor@redhat.com>
Date:   Fri Oct 9 14:48:43 2020 -0600

    Generalize compute_objsize to return maximum size/offset instead of failing
(PR middle-end/97023).

    Also resolves:
    PR middle-end/97342 - bogus -Wstringop-overflow with nonzero signed and
unsigned offsets
    PR middle-end/97023 - missing warning on buffer overflow in chained mempcpy
    PR middle-end/96384 - bogus -Wstringop-overflow= storing into
multidimensional array with index in range

    gcc/ChangeLog:

            PR middle-end/97342
            PR middle-end/97023
            PR middle-end/96384
            * builtins.c (access_ref::access_ref): Initialize new member.  Use
            new enum.
            (access_ref::size_remaining): Define new member function.
            (inform_access): Handle expressions referencing objects.
            (gimple_call_alloc_size): Call get_size_range instead of get_range.
            (gimple_call_return_array): New function.
            (get_range): Rename...
            (get_offset_range): ...to this.  Improve detection of ranges from
            types of expressions.
            (gimple_call_return_array): Adjust calls to get_range per above.
            (compute_objsize): Same.  Set maximum size or offset instead of
            failing for unknown objects and handle more kinds of expressions.
            (compute_objsize): Call access_ref::size_remaining.
            (compute_objsize): Have transitional wrapper fail for pointers
            into unknown objects.
            (expand_builtin_strncmp): Call access_ref::size_remaining and
            handle new cases.
            * builtins.h (access_ref::size_remaining): Declare new member
function.
            (access_ref::set_max_size_range): Define new member function.
            (access_ref::add_ofset, access_ref::add_max_ofset): Same.
            (access_ref::add_base0): New data member.
            * calls.c (get_size_range): Change argument type.  Handle new
            condition.
            * calls.h (get_size_range): Adjust signature.
            (enum size_range_flags): Define new type.
            * gimple-ssa-warn-restrict.c (builtin_memref::builtin_memref):
Correct
            argument to get_size_range.
            * tree-ssa-strlen.c (get_range): Handle anti-ranges.
            (maybe_warn_overflow): Check DECL_P before assuming it's one.

    gcc/testsuite/ChangeLog:

            PR middle-end/97342
            PR middle-end/97023
            PR middle-end/96384
            * c-c++-common/Wrestrict.c: Adjust comment.
            * gcc.dg/Wstringop-overflow-34.c: Remove xfail.
            * gcc.dg/Wstringop-overflow-43.c: Remove xfails.  Adjust regex
patterns.
            * gcc.dg/pr51683.c: Prune out expected warning.
            * gcc.target/i386/pr60693.c: Same.
            * g++.dg/warn/Wplacement-new-size-8.C: New test.
            * gcc.dg/Wstringop-overflow-41.c: New test.
            * gcc.dg/Wstringop-overflow-44.s: New test.
            * gcc.dg/Wstringop-overflow-45.c: New test.
            * gcc.dg/Wstringop-overflow-46.c: New test.
            * gcc.dg/Wstringop-overflow-47.c: New test.
            * gcc.dg/Wstringop-overflow-49.c: New test.
            * gcc.dg/Wstringop-overflow-50.c: New test.
            * gcc.dg/Wstringop-overflow-51.c: New test.
            * gcc.dg/Wstringop-overflow-52.c: New test.
            * gcc.dg/Wstringop-overflow-53.c: New test.
            * gcc.dg/Wstringop-overflow-54.c: New test.
            * gcc.dg/Wstringop-overflow-55.c: New test.
            * gcc.dg/Wstringop-overread-5.c: New test.

[Bug middle-end/96384] [11 Regression] bogus -Wstringop-overflow= storing into multidimensional array with index in range

[msebor at gcc dot gnu.org]

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

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #5 from Martin Sebor <msebor at gcc dot gnu.org> ---
Fixed in r11-3826.