Bug 96384 - [11 Regression] bogus -Wstringop-overflow= storing into multidimensional array with index in range
Summary: [11 Regression] bogus -Wstringop-overflow= storing into multidimensional arra...
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 11.0
: P3 normal
Target Milestone: 11.0
Assignee: Martin Sebor
Keywords: diagnostic, patch
Depends on:
Blocks: Wstringop-overflow
  Show dependency treegraph
Reported: 2020-07-29 21:46 UTC by Martin Sebor
Modified: 2020-10-12 15:12 UTC (History)
0 users

See Also:
Known to work:
Known to fail:
Last reconfirmed: 2020-07-29 00:00:00


Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2020-07-29 21:46:48 UTC
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>;
  unsigned int [0, 4294967294]
  unsigned int [0, 4294967294]
  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>
;;    succ:       EXIT [always]  count:1073741824 (estimated locally) (EXECUTABLE)

Comment 1 Martin Sebor 2020-07-29 22:07:12 UTC
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 ());

(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.
Comment 3 CVS Commits 2020-10-12 15:07:24 UTC
The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>:


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)
    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
            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
            (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.
            PR c++/96511
            PR middle-end/96384
            * init.c (warn_placement_new_too_small): Call builtin_objsize instead
            of duplicating what it does.
            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.
Comment 4 CVS Commits 2020-10-12 15:07:30 UTC
The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>:


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
            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
            * 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.
            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.
Comment 5 Martin Sebor 2020-10-12 15:12:39 UTC
Fixed in r11-3826.