Bug 89428 - missing -Wstringop-overflow on a PHI with variable offset
Summary: missing -Wstringop-overflow on a PHI with variable offset
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 9.0
: P3 normal
Target Milestone: 11.0
Assignee: Martin Sebor
URL:
Keywords: diagnostic, patch
Depends on:
Blocks: Wstringop-overflow
  Show dependency treegraph
 
Reported: 2019-02-21 01:28 UTC by Martin Sebor
Modified: 2020-11-29 22:38 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 10.0, 9.2.0
Last reconfirmed: 2020-04-22 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2019-02-21 01:28:59 UTC
GCC diagnoses the 1-byte overflow in the call to memset in f() but fails to diagnose the much bigger overflow in g().  The problem is the PHI that the compute_objsize() function doesn't handle.

$ cat t.c && gcc -O2 -S -Wall -Wextra -fdump-tree-optimized=/dev/stdout t.c
char a[7];

void f (__SIZE_TYPE__ i)
{
  if (i < 1 || 2 < i) i = 1;

  __builtin_memset (a + i, 0, 7);   // warning (good)
}

void g (__SIZE_TYPE__ i)
{
  if (2 < i) i = 0;

  __builtin_memset (a + i, 0, 99);   // missing warning (bug)
}

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

Removing basic block 3
f (long unsigned int i)
{
  long unsigned int _1;
  void * _2;

  <bb 2> [local count: 1073741824]:
  _1 = i_4(D) + 18446744073709551615;
  if (_1 > 1)
    goto <bb 3>; [59.00%]
  else
    goto <bb 4>; [41.00%]

  <bb 3> [local count: 633507680]:

  <bb 4> [local count: 1073741824]:
  # i_3 = PHI <i_4(D)(2), 1(3)>
  _2 = &a + i_3;
  __builtin_memset (_2, 0, 7); [tail call]
  return;

}


t.c: In function ‘f’:
t.c:7:3: warning: ‘__builtin_memset’ writing 7 bytes into a region of size 6 overflows the destination [-Wstringop-overflow=]
    7 |   __builtin_memset (a + i, 0, 7);   // warning (good)
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

;; Function g (g, funcdef_no=1, decl_uid=1910, cgraph_uid=2, symbol_order=2)

Removing basic block 5
g (long unsigned int i)
{
  char[7] * _6;
  char[7] * prephitmp_7;

  <bb 2> [local count: 1073741824]:
  if (i_3(D) > 2)
    goto <bb 4>; [50.00%]
  else
    goto <bb 3>; [50.00%]

  <bb 3> [local count: 536870912]:
  _6 = &a + i_3(D);

  <bb 4> [local count: 1073741824]:
  # prephitmp_7 = PHI <_6(3), &a(2)>
  __builtin_memset (prephitmp_7, 0, 99); [tail call]
  return;

}
Comment 1 Martin Sebor 2020-04-22 21:55:35 UTC
No progress in GCC 10.
Comment 2 Nicholas Krause 2020-04-26 08:01:46 UTC
Changing i equal to 1 in the second function gives the warning and gets similar GIMPLE like this to the first function:

<bb 2> [local count: 1073741824]:
  i_3 = MAX_EXPR <i_2(D), 1>;
  _1 = &a + i_3;                                                               
  __builtin_memset (_1, 0, 99); [tail call]                                    
  return;  

Are we assuming somewhere that zero size offsets should for the first argument of __builtin_memset to be forgotten about? Because I tried it with multiple numbers other than zero and they all worked to get GIMPLE similar to the above.
Comment 3 Martin Sebor 2020-10-29 19:27:42 UTC
See also pr92936.
Comment 4 Martin Sebor 2020-11-03 15:32:30 UTC
Fixed in the following patch for pr92936:
https://gcc.gnu.org/pipermail/gcc-patches/2020-November/557807.html
Comment 5 CVS Commits 2020-11-29 22:13:33 UTC
The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>:

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

commit r11-5523-geafe8ee7af13c39805ea09bbf5b4f9ab2a48304a
Author: Martin Sebor <msebor@redhat.com>
Date:   Sun Nov 29 15:09:30 2020 -0700

    Handle PHIs in compute_objsize.
    
    PR middle-end/92936 - missing warning on a past-the-end store to a PHI
    PR middle-end/92940 - incorrect offset and size in -Wstringop-overflow for out-of-bounds store into VLA and two offset ranges
    PR middle-end/89428 - missing -Wstringop-overflow on a PHI with variable offset
    
    gcc/ChangeLog:
    
            PR middle-end/92936
            PR middle-end/92940
            PR middle-end/89428
            * builtins.c (access_ref::access_ref): Initialize member.
            (access_ref::phi): New function.
            (access_ref::get_ref): New function.
            (access_ref::add_offset): Remove duplicate assignment.
            (maybe_warn_for_bound): Add "maybe" kind of warning messages.
            (warn_for_access): Same.
            (inform_access): Rename...
            (access_ref::inform_access): ...to this.  Print PHI arguments.  Format
            offset the same as size and simplify.  Improve printing of allocation
            functions and VLAs.
            (check_access): Adjust to the above.
            (gimple_parm_array_size): Change argument.
            (handle_min_max_size): New function.
            * builtins.h (class ssa_name_limit_t): Move class here from
            tree-ssa-strlen.c.
            (struct access_ref): Declare new members.
            (gimple_parm_array_size): Change argument.
            * tree-ssa-strlen.c (maybe_warn_overflow): Use access_ref and simplify.
            (handle_builtin_memcpy): Correct argument passed to maybe_warn_overflow.
            (handle_builtin_memset): Same.
            (class ssa_name_limit_t): Move class to builtins.{h,c}.
    
    gcc/testsuite/ChangeLog:
    
            PR middle-end/92936
            PR middle-end/92940
            PR middle-end/89428
            * c-c++-common/Wstringop-overflow-2.c: Adjust text of expected
            informational notes.
            * g++.dg/warn/Wstringop-overflow-3.C: Same.
            * g++.dg/warn/Wplacement-new-size.C: Remove a test for a no longer
            issued warning.
            * gcc.dg/Warray-bounds-43.c: Removed unused declarations.
            * gcc.dg/Wstringop-overflow-11.c: Remove xfails.
            * gcc.dg/Wstringop-overflow-12.c: Same.
            * gcc.dg/Wstringop-overflow-17.c: Adjust text of expected messages.
            * gcc.dg/Wstringop-overflow-27.c: Same.  Remove xfails.
            * gcc.dg/Wstringop-overflow-28.c: Adjust text of expected messages.
            * gcc.dg/Wstringop-overflow-29.c: Same.
            * gcc.dg/Wstringop-overflow-37.c: Same.
            * gcc.dg/Wstringop-overflow-46.c: Same.
            * gcc.dg/Wstringop-overflow-47.c: Same.
            * gcc.dg/Wstringop-overflow-54.c: Same.
            * gcc.dg/warn-strnlen-no-nul.c: Add expected warning.
            * gcc.dg/Wstringop-overflow-7.c: New test.
            * gcc.dg/Wstringop-overflow-58.c: New test.
            * gcc.dg/Wstringop-overflow-59.c: New test.
            * gcc.dg/Wstringop-overflow-60.c: New test.
            * gcc.dg/Wstringop-overflow-61.c: New test.
            * gcc.dg/Wstringop-overflow-62.c: New test.
            * gcc.dg/Wstringop-overflow-63.c: New test.
            * gcc.dg/Wstringop-overflow-64.c: New test.
Comment 6 Martin Sebor 2020-11-29 22:38:52 UTC
Resolved by  r11-5523 for GCC 11 which now issues the following warnings for the test case in comment #0 (the duplicate note needs to be fixed):

$ gcc -O2 -S pr89428.c
pr89428.c: In function ‘f’:
pr89428.c:7:3: warning: ‘__builtin_memset’ writing 7 bytes into a region of size 6 overflows the destination [-Wstringop-overflow=]
    7 |   __builtin_memset (a + i, 0, 7);   // warning (good)
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pr89428.c:1:6: note: at offset [1, 2] into destination object ‘a’ of size 7
    1 | char a[7];
      |      ^
pr89428.c: In function ‘g’:
pr89428.c:14:3: warning: ‘__builtin_memset’ writing 99 bytes into a region of size 7 overflows the destination [-Wstringop-overflow=]
   14 |   __builtin_memset (a + i, 0, 99);   // missing warning (bug)
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pr89428.c:1:6: note: destination object ‘a’ of size 7
    1 | char a[7];
      |      ^
pr89428.c:1:6: note: destination object ‘a’ of size 7