Bug 92936 - missing warning on a past-the-end store to a PHI
Summary: missing warning on a past-the-end store to a PHI
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 10.0
: P3 normal
Target Milestone: 11.0
Assignee: Martin Sebor
URL:
Keywords: diagnostic, patch
Depends on:
Blocks: Warray-bounds Wstringop-overflow
  Show dependency treegraph
 
Reported: 2019-12-13 23:06 UTC by Martin Sebor
Modified: 2024-10-31 01:44 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-11-03 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-12-13 23:06:41 UTC
Out of bounds stores to destinations that are the results of conditionals like the one in g() below are not diagnosed.  They should be both by -Warray-bounds and by -Wstringop-overflow (without duplicates) but they're not because PHI expressions aren't handled.

$ cat b.c && gcc -O2 -S -Wall b.c
void sink (void*);

void f (int n)
{
  char a3[3], a5[5], *p;

  if (n <= 3)
    {
      n = 3;
      p = a3;
      p[n] = 0;   // -Warray-bounds
    }
  else
    {
      n = 5;
      p = a5;
      p[n] = 0;   // -Warray-bounds
    }

  sink (p);
}

void g (int n)
{
  char a3[3], a5[5], *p;

  if (n <= 3)
    {
      n = 3;
      p = a3;
    }
  else
    {
      n = 5;
      p = a5;
    }

  p[n] = 0;       // missing warning
  p[n + 1] = 1;   // missing warning
  p[n + 9] = 2;   // missing warning
  p[12345] = 3;   // missing warning

  sink (p);
}

b.c: In function ‘f’:
b.c:17:8: warning: array subscript 5 is outside array bounds of ‘char[5]’ [-Warray-bounds]
   17 |       p[n] = 0;   // -Warray-bounds
      |       ~^~~
b.c:5:15: note: while referencing ‘a5’
    5 |   char a3[3], a5[5], *p;
      |               ^~
b.c:11:8: warning: array subscript 3 is outside array bounds of ‘char[3]’ [-Warray-bounds]
   11 |       p[n] = 0;   // -Warray-bounds
      |       ~^~~
b.c:5:8: note: while referencing ‘a3’
    5 |   char a3[3], a5[5], *p;
      |        ^~
Comment 1 Richard Biener 2020-01-08 13:00:55 UTC
In full generality it would require inspecting all possible program paths
(and then decide which ones are "impossible" to take).  We're not doing that.
Still we should have n = [3, 5] and yes, no info for p (just walking defs
isn't appropriate and too simple, consider a p += i; inbetween)
Comment 3 GCC 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 4 Martin Sebor 2020-11-29 22:35:36 UTC
Implemented for GCC 11.  GCC 11 issues the following warnings.   Only accesses that exceed the size of the largest object are diagnosed for now.  Doing otherwise would lead to false positives for some common idioms (accessing either a small buffer or a dynamically allocated larger buffer).  Those cases will require a predicate analysis to handle in a more robust way.

$ gcc -O2 -S -Wall pr92936.c  
pr92936.c: In function ‘f’:
pr92936.c:17:8: warning: array subscript 5 is outside array bounds of ‘char[5]’ [-Warray-bounds]
   17 |       p[n] = 0;   // -Warray-bounds
      |       ~^~~
pr92936.c:5:15: note: while referencing ‘a5’
    5 |   char a3[3], a5[5], *p;
      |               ^~
pr92936.c:11:8: warning: array subscript 3 is outside array bounds of ‘char[3]’ [-Warray-bounds]
   11 |       p[n] = 0;   // -Warray-bounds
      |       ~^~~
pr92936.c:5:8: note: while referencing ‘a3’
    5 |   char a3[3], a5[5], *p;
      |        ^~
pr92936.c: In function ‘g’:
pr92936.c:40:12: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
   40 |   p[n + 9] = 2;   // missing warning
      |   ~~~~~~~~~^~~
pr92936.c:25:8: note: at offset [12, 14] into destination object ‘a3’ of size 3
   25 |   char a3[3], a5[5], *p;
      |        ^~
pr92936.c:25:15: note: at offset [12, 14] into destination object ‘a5’ of size 5
   25 |   char a3[3], a5[5], *p;
      |               ^~
pr92936.c:41:12: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
   41 |   p[12345] = 3;   // missing warning
      |   ~~~~~~~~~^~~
pr92936.c:25:8: note: at offset 12345 into destination object ‘a3’ of size 3
   25 |   char a3[3], a5[5], *p;
      |        ^~
pr92936.c:25:15: note: at offset 12345 into destination object ‘a5’ of size 5
   25 |   char a3[3], a5[5], *p;
      |               ^~
Comment 5 GCC Commits 2024-10-31 01:42:38 UTC
The master branch has been updated by Sam James <sjames@gcc.gnu.org>:

https://gcc.gnu.org/g:2dcb174385fd366282bf34bf95adbf918d5befda

commit r15-4792-g2dcb174385fd366282bf34bf95adbf918d5befda
Author: Sam James <sam@gentoo.org>
Date:   Thu Oct 31 01:37:47 2024 +0000

    testsuite: fix syntax in Wstringop-overflow-59.c
    
    Fix quoting issues, escaping, and dg directive types.
    
    There were two issues here:
    1) The incorrect quoting in an earlier dg-message was covering up that the
    syntax in the next part was wrong;
    2) Fix dg-warning -> dg-message to correctly pick up the notes. Once 1) was
    fixed, this was exposed.
    
    With this, I get:
    ```
    +PASS: gcc.dg/Wstringop-overflow-59.c note (test for warnings, line 192)
    +PASS: gcc.dg/Wstringop-overflow-59.c note (test for warnings, line 193)
    ```
    
    gcc/testsuite/ChangeLog:
            PR middle-end/92936
    
            * gcc.dg/Wstringop-overflow-59.c: Fix dg-* syntax.