Bug 77784 - duplicate warning for snprintf when n > object size
Summary: duplicate warning for snprintf when n > object size
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 7.0
: P3 minor
Target Milestone: ---
Assignee: Martin Sebor
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2016-09-28 16:48 UTC by Martin Sebor
Modified: 2016-12-08 00:24 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2016-09-28 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2016-09-28 16:48:26 UTC
As mentioned in bug 77762#2, with both the -Wformat-length option used and _FORTIFY_SOURCE defined (and with optimization enabled), GCC now prints two warnings for calls to functions like snprintf (which normally calls __builtin___snprintf_chk).  Only one warning should be printed.

$ cat zzz.c && /build/gcc-trunk-svn/gcc/xgcc -B /build/gcc-trunk-svn/gcc -S -Wall -Wformat-length=1 zzz.c
char d [2];

void f (const char *s)
{
  __builtin___snprintf_chk (d, 4, 0, 2, s);
}
zzz.c: In function ‘f’:
zzz.c:5:3: warning: specified size 4 exceeds the size 2 of the destination object [-Wformat-length=]
   __builtin___snprintf_chk (d, 4, 0, 2, s);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
zzz.c:5:3: warning: call to __builtin___snprintf_chk will always overflow destination buffer


As also pointed out in bug 77762 and as the following program shows, the latter warning is misleading because a buffer overflow need not actually take place and the call may be valid.  This is just a minor detail when the default implementation of the __snprintf_chk function is used that calls abort in this case, but printing a more accurate message becomes important when an alternate implementation is used that does not abort (as is sometimes done in operating system kernels, for example).

$ cat zzz.c && /build/gcc-trunk-svn/gcc/xgcc -B /build/gcc-trunk-svn/gcc -Wall -Wformat -Wextra -Wpedantic -Wformat-length=1 zzz.c && ./a.out 
typedef __SIZE_TYPE__ size_t;

char d [2];

int main (void)
{
  char s[] = "x";
  __builtin___snprintf_chk (d, 4, 0, 2, s);
}

int __snprintf_chk (char *s, size_t maxlen, int flag, size_t os,
                    const char *fmt, ...)
{
  __builtin_printf ("%s(%p, %zu, %i, %zu, \"%s\"...)\n",
                    __func__, s, maxlen, flag, os, fmt);
  return __builtin_puts (fmt);
}

zzz.c: In function ‘main’:
zzz.c:8:3: warning: specified size 4 exceeds the size 2 of the destination object [-Wformat-length=]
   __builtin___snprintf_chk (d, 4, 0, 2, s);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
zzz.c:8:3: warning: call to __builtin___snprintf_chk will always overflow destination buffer
__snprintf_chk(0x601049, 4, 0, 2, "x"...)
x
Comment 1 Jakub Jelinek 2016-09-28 20:08:53 UTC
The preexisting warning is not that misleading, because even if there is not any buffer overflow, the program is still terminated whenever the user provided buffer size is bigger than the __builtin_object_size provided size, without actually trying to print anything into the buffer.  If you want to suggest different wording for the *snprintf* case, it could be changed, but I believe it is the right spot for the warning - after all the GIMPLE optimizations, regardless of what warning options are enabled or not.
Comment 2 Martin Sebor 2016-11-07 21:18:10 UTC
I'll handle this in an update on my cumulative patch for bug 53562 initially posted for review here:
https://gcc.gnu.org/ml/gcc-patches/2016-10/msg02308.html
Comment 3 Martin Sebor 2016-12-08 00:02:06 UTC
Author: msebor
Date: Thu Dec  8 00:01:33 2016
New Revision: 243419

URL: https://gcc.gnu.org/viewcvs?rev=243419&root=gcc&view=rev
Log:
PR c/53562 - Add -Werror= support for -D_FORTIFY_SOURCE / __builtin___memcpy_chk
PR middle-end/77784 - duplicate warning for snprintf when n > object size
PR middle-end/78149 - missing warning on strncpy buffer overflow due to an excessive bound
PR middle-end/78138 - missing warnings on buffer overflow with non-constant source length

gcc/c-family/ChangeLog:

	PR c/53562
	PR middle-end/77784
	PR middle-end/78149
	PR middle-end/78138
	* c.opt (-Wstringop-overflow): New option.

gcc/ChangeLog:

	PR middle-end/77784
	PR middle-end/78149
	PR middle-end/78138
	
	* builtins.c (expand_builtin_strcat, expand_builtin_strncat): New
	functions.
	(compute_dest_size, get_size_range, check_sizes, check_strncat_sizes)
	(check_memop_sizes): Same.
	(expand_builtin_memcpy): Call check memop_sizes.
	(expand_builtin_mempcpy): Same.
	(expand_builtin_memset): Same,
	(expand_builtin_bzero): Same.
	(expand_builtin_memory_chk): Call check_sizes.
	(expand_builtin_strcpy): Same.
	(expand_builtin_strncpy): Same.
	(maybe_emit_sprintf_chk_warning): Same.
	(expand_builtin): Handle strcat and strncat.
	(fini_object_sizes): Reset pointers.
	(compute_object_size): New function.
	* gimple-ssa-sprintf.c (pass_sprintf_length::handle_gimple_call):
	Avoid issuing warnings also issued during built-in expansion.
	* doc/invoke.texi (Warning Options): Document -Wstringop-overflow.

gcc/testsuite/ChangeLog:

	PR middle-end/77784
	PR middle-end/78149
	PR middle-end/78138

	* c-c++-common/Wsizeof-pointer-memaccess2.c: Adjust expected diagnostic.
	* g++.dg/ext/builtin-object-size3.C (bar): Same.
	* g++.dg/ext/strncpy-chk1.C: Same.
	* g++.dg/opt/memcpy1.C: Same.
	* g++.dg/torture/Wsizeof-pointer-memaccess1.C: Same.
	* gcc.c-torture/compile/pr55569.c: Disable -Wstringop-overflow.
	* gcc.dg/Wobjsize-1.c: Adjust expected diagnostic.
	* gcc.dg/attr-alloc_size.c: Same.
	* gcc.dg/builtin-stringop-chk-1.c: Adjust expected diagnostic.
	* gcc.dg/builtin-stringop-chk-2.c: Same.
	* gcc.dg/builtin-stringop-chk-4.c: New test.
	* gcc.dg/builtin-strncat-chk-1.c: Adjust expected diagnostic.
	* gcc.dg/memcpy-2.c: Same.
	* gcc.dg/pr40340-1.c: Same.
	* gcc.dg/pr40340-2.c (main): Same.
	* gcc.dg/pr40340-5.c (main): Same.
	* gcc.dg/torture/Wsizeof-pointer-memaccess1.c: Same.
	* gcc.dg/torture/pr71132.c: Disable -Wstringop-overflow.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Adjust text of expected
	warning.
	* gfortran.dg/char_length_3.f90: Prune expected warnings.
	* gfortran.dg/pr38868.f: Add expected warnings.


Added:
    trunk/gcc/testsuite/gcc.dg/builtin-stringop-chk-4.c
    trunk/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c
    trunk/gcc/testsuite/gcc.dg/builtin-stringop-chk-6.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/builtins.c
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c.opt
    trunk/gcc/doc/invoke.texi
    trunk/gcc/gimple-ssa-sprintf.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c
    trunk/gcc/testsuite/g++.dg/ext/builtin-object-size3.C
    trunk/gcc/testsuite/g++.dg/ext/strncpy-chk1.C
    trunk/gcc/testsuite/g++.dg/opt/memcpy1.C
    trunk/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess1.C
    trunk/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess2.C
    trunk/gcc/testsuite/gcc.c-torture/compile/pr55569.c
    trunk/gcc/testsuite/gcc.dg/Wobjsize-1.c
    trunk/gcc/testsuite/gcc.dg/attr-alloc_size.c
    trunk/gcc/testsuite/gcc.dg/builtin-stringop-chk-1.c
    trunk/gcc/testsuite/gcc.dg/builtin-stringop-chk-2.c
    trunk/gcc/testsuite/gcc.dg/builtin-strncat-chk-1.c
    trunk/gcc/testsuite/gcc.dg/fstack-protector-strong.c
    trunk/gcc/testsuite/gcc.dg/memcpy-2.c
    trunk/gcc/testsuite/gcc.dg/pr40340-1.c
    trunk/gcc/testsuite/gcc.dg/pr40340-2.c
    trunk/gcc/testsuite/gcc.dg/pr40340-5.c
    trunk/gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c
    trunk/gcc/testsuite/gcc.dg/torture/pr71132.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c
    trunk/gcc/testsuite/gfortran.dg/char_length_3.f90
    trunk/gcc/testsuite/gfortran.dg/pr38868.f
Comment 4 Martin Sebor 2016-12-08 00:24:17 UTC
Fixed in r243419.