[Bug middle-end/85602] -Wsizeof-pointer-memaccess for strncat with size of source
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85602 --- Comment #14 from Martin Sebor --- Author: msebor Date: Wed Jul 18 17:20:05 2018 New Revision: 262859 URL: https://gcc.gnu.org/viewcvs?rev=262859=gcc=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
[Bug middle-end/85602] -Wsizeof-pointer-memaccess for strncat with size of source
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85602 Martin Sebor changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED Target Milestone|8.2 |9.0 --- Comment #13 from Martin Sebor --- Fixed on trunk.
[Bug middle-end/85602] -Wsizeof-pointer-memaccess for strncat with size of source
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85602 --- Comment #12 from Martin Sebor --- Author: msebor Date: Tue Jun 19 22:35:45 2018 New Revision: 261774 URL: https://gcc.gnu.org/viewcvs?rev=261774=gcc=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
[Bug middle-end/85602] -Wsizeof-pointer-memaccess for strncat with size of source
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85602 --- Comment #11 from seurer at gcc dot gnu.org --- It is fixed now. Thanks!
[Bug middle-end/85602] -Wsizeof-pointer-memaccess for strncat with size of source
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85602 --- Comment #9 from Martin Sebor --- Thanks, r261751 should take care of it. --- Comment #10 from Martin Sebor --- Author: msebor Date: Tue Jun 19 17:30:47 2018 New Revision: 261751 URL: https://gcc.gnu.org/viewcvs?rev=261751=gcc=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
[Bug middle-end/85602] -Wsizeof-pointer-memaccess for strncat with size of source
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85602 --- Comment #9 from Martin Sebor --- Thanks, r261751 should take care of it.
[Bug middle-end/85602] -Wsizeof-pointer-memaccess for strncat with size of source
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85602 seurer at gcc dot gnu.org changed: What|Removed |Added CC||seurer at gcc dot gnu.org --- Comment #8 from seurer at gcc dot gnu.org --- The new test cases fails in some cases: make -k check-gcc RUNTESTFLAGS=dg.exp=c-c++-common/attr-nonstring-8.c # of expected passes17 # of expected passes45 # of unexpected failures6 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]
[Bug middle-end/85602] -Wsizeof-pointer-memaccess for strncat with size of source
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85602 --- Comment #7 from Martin Sebor --- Adjusted patch committed into trunk in r261718.
[Bug middle-end/85602] -Wsizeof-pointer-memaccess for strncat with size of source
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85602 --- Comment #6 from Martin Sebor --- Author: msebor Date: Mon Jun 18 22:17:57 2018 New Revision: 261718 URL: https://gcc.gnu.org/viewcvs?rev=261718=gcc=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
[Bug middle-end/85602] -Wsizeof-pointer-memaccess for strncat with size of source
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85602 Martin Sebor changed: What|Removed |Added Keywords||patch --- Comment #5 from Martin Sebor --- Patch: https://gcc.gnu.org/ml/gcc-patches/2018-05/msg00869.html
[Bug middle-end/85602] -Wsizeof-pointer-memaccess for strncat with size of source
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85602 --- Comment #4 from Paul Eggert --- Thanks, that workaround is much better for coreutils, and I installed it here: https://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=f6cb50cc991d461f443ea3afc517c9e1e37ef496
[Bug middle-end/85602] -Wsizeof-pointer-memaccess for strncat with size of source
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85602
--- Comment #3 from Martin Sebor ---
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.
[Bug middle-end/85602] -Wsizeof-pointer-memaccess for strncat with size of source
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85602 --- Comment #2 from Paul Eggert --- 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
[Bug middle-end/85602] -Wsizeof-pointer-memaccess for strncat with size of source
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85602
Martin Sebor changed:
What|Removed |Added
Status|UNCONFIRMED |ASSIGNED
Last reconfirmed||2018-05-14
CC||msebor at gcc dot gnu.org
Assignee|unassigned at gcc dot gnu.org |msebor at gcc dot
gnu.org
Summary|[8/9 Regression] regression |-Wsizeof-pointer-memaccess
|with strncat and -Wall in |for strncat with size of
|GCC 8 |source
Ever confirmed|0 |1
--- Comment #1 from Martin Sebor ---
(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
