Bug 97023 - missing warning on buffer overflow in chained mempcpy
Summary: missing warning on buffer overflow in chained mempcpy
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 11.0
: P3 normal
Target Milestone: 11.0
Assignee: Martin Sebor
URL:
Keywords: diagnostic, patch
Depends on:
Blocks: Wstringop-overflow
  Show dependency treegraph
 
Reported: 2020-09-11 17:35 UTC by Martin Sebor
Modified: 2020-10-12 15:11 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-09-11 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2020-09-11 17:35:10 UTC
-Wstringop-overflow successfully diagnoses a buffer overflow by the second in a chain of calls to stpcpy but fails to detect the same bug in calls to mempcpy because it doesn't track function arguments that are returned from builti-ins like mempcpy or memchr.

$ cat z.c && gcc -O2 -S z.c
char a[7];

void* f (void)
{
  void *p = __builtin_stpcpy (a, "123");
  p = __builtin_stpcpy (p, "4567");        // warning (good)
  return p;
}

void* g (void)
{
  void *p = __builtin_mempcpy (a, "123", 3);
  p = __builtin_mempcpy (p, "4567", 5);    // missing warning
  return p;
}
z.c: In function ‘f’:
z.c:5:13: warning: ‘__builtin_memcpy’ writing 5 bytes into a region of size 4 [-Wstringop-overflow=]
    5 |   void *p = __builtin_stpcpy (a, "123");
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~
z.c:1:6: note: at offset 3 to object ‘a’ with size 7 declared here
    1 | char a[7];
      |      ^
Comment 1 Martin Sebor 2020-09-11 17:35:46 UTC
I'm testing a fix.
Comment 3 CVS Commits 2020-10-12 15:07:29 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 4 Martin Sebor 2020-10-12 15:11:17 UTC
Implemented in r11-3827.  GCC 11 diagnoses both invalid calls.  With -Warray-bounds:

$ gcc -O2 -S -Wall pr97023.c
pr97023.c: In function ‘f’:
pr97023.c:5:13: warning: ‘__builtin_memcpy’ writing 5 bytes into a region of size 4 [-Wstringop-overflow=]
    5 |   void *p = __builtin_stpcpy (a, "123");
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~
pr97023.c:1:6: note: at offset 3 to object ‘a’ with size 7 declared here
    1 | char a[7];
      |      ^
pr97023.c: In function ‘g’:
pr97023.c:13:7: warning: ‘__builtin_mempcpy’ forming offset 4 is out of the bounds [0, 4] [-Warray-bounds]
   13 |   p = __builtin_mempcpy (p, "4567", 5);    // missing warning
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


and without:

$ gcc -O2 -S -Wall -Wno-array-bounds pr97023.c
pr97023.c: In function ‘f’:
pr97023.c:5:13: warning: ‘__builtin_memcpy’ writing 5 bytes into a region of size 4 [-Wstringop-overflow=]
    5 |   void *p = __builtin_stpcpy (a, "123");
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~
pr97023.c:1:6: note: at offset 3 to object ‘a’ with size 7 declared here
    1 | char a[7];
      |      ^
pr97023.c: In function ‘g’:
pr97023.c:13:7: warning: ‘__builtin_mempcpy’ writing 5 bytes into a region of size 4 overflows the destination [-Wstringop-overflow=]
   13 |   p = __builtin_mempcpy (p, "4567", 5);    // missing warning
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pr97023.c:1:6: note: at offset 3 into destination object ‘a’
    1 | char a[7];
      |      ^