Created attachment 44051 [details] test program illustrating the regression I ran into some problems building GNU Coreutils with gcc (GCC) 8.0.1 20180324 (Red Hat 8.0.1-0.20) and isolated it to the attached program w.c. The command "gcc -Wall w.c" reports: w.c: In function ‘main’: w.c:11:35: warning: argument to ‘sizeof’ in ‘strncat’ call is the same expression as the source; did you mean to use the size of the destination? [-Wsizeof-pointer-memaccess] strncat (buf, u.ut_user, sizeof u.ut_user); This diagnostic is quite wrong, and propagates confusion about strncat. Unlike strncpy, strncat does not zero-fill the destination, and it is almost always incorrect to do as GCC suggests which is to specify the size of the destination as the third argument. The source code is correct as-is: it is one of the few places where strncat is the right function to use, as strncat was designed for struct utmp and similar data structures. GCC 7 does not warn about this usage, and GCC 8 should not warn either.
(The warning behaves as designed so this is not a regression. I've changed the Summary to reflect that.) The code in the test case is correct and safe, and reflects one of the original and intended uses of the function. Unfortunately, due to a poor understanding of the various string functions[1], many calls to strncat() with either the size of the source or (to a lesser extent) the size of the destination have been mistakes that have led to either buffer overflow or inadvertent truncation. To avoid these mistakes the recommended way to call the function is to pass it the amount of space remaining in the destination, e.g., like so[2]: strncat (dst, src, dstsize - strlen (d) - 1); The warning is designed to help detect these mistakes and encourage this idiom. Ideally, the warning would detect the safe calls (like the one in the test case) and avoid triggering for them, but it's not always possible. In the test case, it's impossible to detect the length of the destination string at the point where GCC sees the use of 'sizeof source' in the strncat() call; at the point where the length is known the fact that the bound is the result of 'sizeof source' has been lost. Since in the test case strncat() is being used to constrain the copy to avoid reading past the end of a non-string (i.e., an array of characters not necessarily terminated by a NUL) I'll see if GCC can be taught to recognize the attribute in this context to suppress the warning. Until then (or if annotating the member isn't an option), the warning can be suppressed either by a #pragma GCC diagnostic, or by introducing a temporary pointer pointing to the member array and passing the pointer to strncat, or by introducing a temporary for the size of the source and passing it to the function as the bound: char* __attribute__ ((nonstring)) p = u.ut_user; enum { N = sizeof u.ut_user }; strncat (buf, u.ut_user, N); [1] https://www.usenix.org/legacy/event/usenix99/full_papers/millert/millert.pdf [2] https://www.us-cert.gov/bsi/articles/knowledge/coding-practices/strncpy-and-strncat
Thanks for looking into it. For what it's worth, the practical effect of this new warning was that I changed that part of coreutils to not use strncat, causing 3 lines of code to grow to 8 lines. See the end of: https://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=9781fcd532f30afe174239a22816a83c40528b27 Another part of coreutils still uses strncat (also correctly). Let's hope GCC doesn't start warning about it too....
I agree that's not an improvement. Would something like this be better? (at least until utmp_ent is marked nonstring and GCC taught to suppress the diagnostic) size_t utmpsize = sizeof UT_ID (utmp_ent); char *comment = xmalloc (strlen (_("id=")) + utmpsize + 1); strcpy (comment, _("id=")); strncat (comment, UT_ID (utmp_ent), utmpsize); I'll try to remember to test coreutils with new warnings in the future.
Thanks, that workaround is much better for coreutils, and I installed it here: https://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=f6cb50cc991d461f443ea3afc517c9e1e37ef496
Patch: https://gcc.gnu.org/ml/gcc-patches/2018-05/msg00869.html
Author: msebor Date: Mon Jun 18 22:17:57 2018 New Revision: 261718 URL: https://gcc.gnu.org/viewcvs?rev=261718&root=gcc&view=rev Log: PR middle-end/85602 - -Wsizeof-pointer-memaccess for strncat with size of source gcc/c-family/ChangeLog: PR middle-end/85602 * c-warn.c (sizeof_pointer_memaccess_warning): Check for attribute nonstring. gcc/ChangeLog: PR middle-end/85602 * calls.c (maybe_warn_nonstring_arg): Handle strncat. * tree-ssa-strlen.c (is_strlen_related_p): Make extern. Handle integer subtraction. (maybe_diag_stxncpy_trunc): Handle nonstring source arguments. * tree-ssa-strlen.h (is_strlen_related_p): Declare. gcc/testsuite/ChangeLog: PR middle-end/85602 * gcc.dg/attr-nonstring-2.c: Adjust text of expected warning. * c-c++-common/attr-nonstring-8.c: New test. Added: trunk/gcc/testsuite/c-c++-common/attr-nonstring-8.c Modified: trunk/gcc/ChangeLog trunk/gcc/c-family/ChangeLog trunk/gcc/c-family/c-warn.c trunk/gcc/calls.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/attr-nonstring-2.c trunk/gcc/tree-ssa-strlen.c trunk/gcc/tree-ssa-strlen.h
Adjusted patch committed into trunk in r261718.
The new test cases fails in some cases: make -k check-gcc RUNTESTFLAGS=dg.exp=c-c++-common/attr-nonstring-8.c # of expected passes 17 # of expected passes 45 # of unexpected failures 6 FAIL: c-c++-common/attr-nonstring-8.c -std=gnu++98 (test for warnings, line 63) FAIL: c-c++-common/attr-nonstring-8.c -std=gnu++98 (test for excess errors) FAIL: c-c++-common/attr-nonstring-8.c -std=gnu++11 (test for warnings, line 63) FAIL: c-c++-common/attr-nonstring-8.c -std=gnu++11 (test for excess errors) FAIL: c-c++-common/attr-nonstring-8.c -std=gnu++14 (test for warnings, line 63) FAIL: c-c++-common/attr-nonstring-8.c -std=gnu++14 (test for excess errors) FAIL: c-c++-common/attr-nonstring-8.c -std=gnu++98 (test for excess errors) Excess errors: /home/seurer/gcc/gcc-test2/gcc/testsuite/c-c++-common/attr-nonstring-8.c:63:3: warning: argument to 'sizeof' in 'char* strncat(char*, const char*, long unsigned int)' call is the same expression as the source; did you mean to provide an explicit length? [-Wsizeof-pointer-memaccess]
Thanks, r261751 should take care of it.
Author: msebor Date: Tue Jun 19 17:30:47 2018 New Revision: 261751 URL: https://gcc.gnu.org/viewcvs?rev=261751&root=gcc&view=rev Log: gcc/testsuite/ChangeLog: PR middle-end/85602 * c-c++-common/attr-nonstring-8.c: Adjust text of expected warning to also match C++. Modified: trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/c-c++-common/attr-nonstring-8.c
It is fixed now. Thanks!
Author: msebor Date: Tue Jun 19 22:35:45 2018 New Revision: 261774 URL: https://gcc.gnu.org/viewcvs?rev=261774&root=gcc&view=rev Log: PR middle-end/85602 - -Warray-bounds fails to detect the out of bound array access gcc/testsuite/ChangeLog: * c-c++-common/attr-nonstring-8.c: Adjust text of expected warning to also match C++. Added: trunk/gcc/testsuite/gcc.dg/Warray-bounds-28.c Modified: trunk/gcc/testsuite/ChangeLog
Fixed on trunk.
Author: msebor Date: Wed Jul 18 17:20:05 2018 New Revision: 262859 URL: https://gcc.gnu.org/viewcvs?rev=262859&root=gcc&view=rev Log: Backport from trunk. PR middle-end/85602 - -Wsizeof-pointer-memaccess for strncat with size of source gcc/c-family/ChangeLog: PR middle-end/85602 * c-warn.c (sizeof_pointer_memaccess_warning): Check for attribute nonstring. gcc/ChangeLog: PR middle-end/85602 * calls.c (maybe_warn_nonstring_arg): Handle strncat. * tree-ssa-strlen.c (is_strlen_related_p): Make extern. Handle integer subtraction. (maybe_diag_stxncpy_trunc): Handle nonstring source arguments. * tree-ssa-strlen.h (is_strlen_related_p): Declare. * doc/invoke.texi (-Wstringop-truncation): Update. gcc/testsuite/ChangeLog: PR middle-end/85602 * gcc.dg/attr-nonstring-2.c: Adjust text of expected warning. * c-c++-common/attr-nonstring-8.c: New test. Added: branches/gcc-8-branch/gcc/testsuite/c-c++-common/attr-nonstring-8.c Modified: branches/gcc-8-branch/gcc/ChangeLog branches/gcc-8-branch/gcc/c-family/ChangeLog branches/gcc-8-branch/gcc/c-family/c-warn.c branches/gcc-8-branch/gcc/calls.c branches/gcc-8-branch/gcc/doc/invoke.texi branches/gcc-8-branch/gcc/testsuite/ChangeLog branches/gcc-8-branch/gcc/tree-ssa-strlen.c branches/gcc-8-branch/gcc/tree-ssa-strlen.h