Bug 96384

Summary: [11 Regression] bogus -Wstringop-overflow= storing into multidimensional array with index in range
Product: gcc Reporter: Martin Sebor <msebor>
Component: middle-endAssignee: Martin Sebor <msebor>
Status: RESOLVED FIXED    
Severity: normal Keywords: diagnostic, patch
Priority: P3    
Version: 11.0   
Target Milestone: 11.0   
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2020-07-29 00:00:00
Bug Depends on:    
Bug Blocks: 88443    

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>;
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)

}
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 ());
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.
Comment 3 GCC Commits 2020-10-12 15:07:24 UTC
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.
Comment 4 GCC Commits 2020-10-12 15:07:30 UTC
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.
Comment 5 Martin Sebor 2020-10-12 15:12:39 UTC
Fixed in r11-3826.