Bug 79234 - warn on past the end reads by library functions
Summary: warn on past the end reads by library functions
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 7.0
: P3 enhancement
Target Milestone: ---
Assignee: Martin Sebor
URL:
Keywords: diagnostic, patch
Depends on:
Blocks:
 
Reported: 2017-01-25 20:14 UTC by Martin Sebor
Modified: 2017-05-04 23:51 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-04-20 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2017-01-25 20:14:21 UTC
The -Wstringop-overflow option can detect calls to some standard library functions that write past the end of a destination object, but it doesn't detect calls that attempt to read beyond the end of an object.  As the following test case shows, even though all three functions access memory beyond the end of an object only the first one that writes past the end is diagnosed.

This is an enhancement request to add an option to also detect and diagnose past the end reads.  The feature should be a straightforward extension of the -Wstringop-overflow approach (though under it own option).

$ cat t.c && gcc  -O2 -S -Wall -Wextra -Wpedantic t.c
#include <string.h>

char a[5];

void f (size_t n)
{
  memcpy (a, "01234567", n < 7 ? 7 : n);
}

void g (void *d, size_t n)
{
  memcpy (d, a, n < 7 ? 7 : n);
}

int h (size_t n)
{
  return memcmp (a, "01234567", n < 7 ? 7 : n);
}

t.c: In function ‘f’:
t.c:7:3: warning: ‘memcpy’ writing between 7 and 18446744073709551615 bytes into a region of size 5 overflows the destination [-Wstringop-overflow=]
   memcpy (a, "01234567", n < 7 ? 7 : n);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tmp$
Comment 1 Martin Sebor 2017-03-29 19:16:45 UTC
See also bug 71501.

For another example consider the test case below.  Both functions attempt to read a character array that isn't (necessarily) nul-terminated.  In f, the function ends up reading the uninitialized contents of the local array and so it should, arguably, be diagnosed by -Wuninitialized.  In g, the function reads past the end of the local array so it could also be diagnosed by -Wuninitialized, or 


$ cat z.c && gcc -O2 -S -Wall -Wextra -Wpedantic z.c
unsigned f (void)
{
  char d[4];
  __builtin_memcpy (d, "12", 2);
  return __builtin_strlen (d);   // missing warning
}

unsigned g (void)
{
  char d[4];
  __builtin_memcpy (d, "1234", 4);
  return __builtin_strlen (d);   // missing warning
}
Comment 2 Martin Sebor 2017-04-20 22:52:14 UTC
Patch posted for review:
https://gcc.gnu.org/ml/gcc-patches/2017-04/msg00925.html
Comment 3 Martin Sebor 2017-05-04 23:50:54 UTC
Author: msebor
Date: Thu May  4 23:50:21 2017
New Revision: 247622

URL: https://gcc.gnu.org/viewcvs?rev=247622&root=gcc&view=rev
Log:
PR libstdc++/54924 - Warn for std::string constructor with wrong size
PR middle-end/79234 - warn on past the end reads by library functions

gcc/ChangeLog:

	PR middle-end/79234
	* builtins.c (check_sizes): Adjust to handle reading past the end.
	Avoid printing excessive upper bound of ranges.  Use %E to print
	tree nodes instead of converting them to %wu.
	(expand_builtin_memchr): New function.
	(compute_dest_size): Rename...
	(compute_objsize): ...to this.
	(expand_builtin_memcpy): Adjust.
	(expand_builtin_mempcpy): Adjust.
	(expand_builtin_strcat): Adjust.
	(expand_builtin_strcpy): Adjust.
	(check_strncat_sizes): Adjust.
	(expand_builtin_strncat): Adjust.
	(expand_builtin_strncpy): Adjust and simplify.
	(expand_builtin_memset): Adjust.
	(expand_builtin_bzero): Adjust.
	(expand_builtin_memcmp): Adjust.
	(expand_builtin): Handle memcmp.
	(maybe_emit_chk_warning): Check strncat just once.

gcc/testsuite/ChangeLog:

	PR middle-end/79234
	* gcc.dg/builtin-stringop-chk-8.c: New test.
	* gcc.dg/builtin-stringop-chk-1.c: Adjust.
	* gcc.dg/builtin-stringop-chk-4.c: Same.
	* gcc.dg/builtin-strncat-chk-1.c: Same.
	* g++.dg/ext/strncpy-chk1.C: Same.
	* g++.dg/torture/Wsizeof-pointer-memaccess1.C: Same.
	* gcc.dg/out-of-bounds-1.c: Same.
	* gcc.dg/pr78138.c: Same.
	* gcc.dg/torture/Wsizeof-pointer-memaccess1.c: Same.
	* gfortran.dg/mvbits_7.f90: Same.


Added:
    trunk/gcc/testsuite/gcc.dg/builtin-stringop-chk-8.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/builtins.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/ext/strncpy-chk1.C
    trunk/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess1.C
    trunk/gcc/testsuite/gcc.dg/builtin-stringop-chk-1.c
    trunk/gcc/testsuite/gcc.dg/builtin-stringop-chk-4.c
    trunk/gcc/testsuite/gcc.dg/builtin-strncat-chk-1.c
    trunk/gcc/testsuite/gcc.dg/out-of-bounds-1.c
    trunk/gcc/testsuite/gcc.dg/pr78138.c
    trunk/gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
    trunk/gcc/testsuite/gfortran.dg/mvbits_7.f90
Comment 4 Martin Sebor 2017-05-04 23:51:52 UTC
Patch committed in r247622.