Similar to bug 79220, the -Wstringop-overflow option diagnoses the buffer overflow in the call to strcat in f() in the program below but fails to do the same for the strcat() overflow in g(). As in the referenced bug, GCC transforms the second strcat() to an assignment followed by a call to memcpy, defeating the overflow detection. GCC should avoid this transformation when the destination isn't big enough for the copy. I expect this bug will be resolved by a comprehensive fix for bug 79220 but it seems that keeping track of each of these troublesome -- even though not invalid -- transformations separately might help assure that the fix does, in fact, resolve all these related problems. (Both cases of overflow are diagnosed when _FORTIFY_SOURCE is defined.) $ cat t.c && gcc -O2 -S -Wall -Wextra -Wpedantic -fdump-tree-optimized=/dev/stdout t.c #include <string.h> char d[3]; void f (int i) { const char *s = i < 0 ? "01234567" : "89abcd"; strcat (d, s); } void gf (int i) { const char *s = i < 0 ? "12345678" : "87654321"; strcat (d, s); } ;; Function f (f, funcdef_no=14, decl_uid=2196, cgraph_uid=14, symbol_order=15) Removing basic block 3 f (int i) { const char * iftmp.0_1; <bb 2> [100.00%]: if (i_2(D) < 0) goto <bb 4>; [32.39%] else goto <bb 3>; [67.61%] <bb 3> [67.61%]: <bb 4> [100.00%]: # iftmp.0_1 = PHI <"01234567"(2), "89abcd"(3)> strcat (&d, iftmp.0_1); [tail call] return; } t.c: In function ‘f’: t.c:8:3: warning: ‘strcat’ writing 7 bytes into a region of size 3 overflows the destination [-Wstringop-overflow=] strcat (d, s); ^~~~~~~~~~~~~ ;; Function gf (gf, funcdef_no=15, decl_uid=2200, cgraph_uid=15, symbol_order=16) Removing basic block 3 gf (int i) { char[9] * iftmp.1_1; long unsigned int _5; char[3] * _6; <bb 2> [100.00%]: if (i_2(D) < 0) goto <bb 4>; [32.39%] else goto <bb 3>; [67.61%] <bb 3> [67.61%]: <bb 4> [100.00%]: # iftmp.1_1 = PHI <"12345678"(2), "87654321"(3)> _5 = __builtin_strlen (&d); _6 = &d + _5; __builtin_memcpy (_6, iftmp.1_1, 9); [tail call] return; }
(In reply to Martin Sebor from comment #0) > Similar to bug 79220, the -Wstringop-overflow option diagnoses the buffer > overflow in the call to strcat in f() in the program below but fails to do > the same for the strcat() overflow in g(). As in the referenced bug, GCC > transforms the second strcat() to an assignment followed by a call to > memcpy, defeating the overflow detection. GCC should avoid this > transformation when the destination isn't big enough for the copy. > > I expect this bug will be resolved by a comprehensive fix for bug 79220 but > it seems that keeping track of each of these troublesome -- even though not > invalid -- transformations separately might help assure that the fix does, > in fact, resolve all these related problems. > > (Both cases of overflow are diagnosed when _FORTIFY_SOURCE is defined.) > Confirmed that only 1 is diagnosed normally. Posting -D_FORTIFY_SOURCE=2 output for comparison: $ /usr/local/bin/gcc -c -O2 -S -Wall -Wextra -Wpedantic -fdump-tree-optimized=/dev/stdout -D_FORTIFY_SOURCE=2 79221.c ;; Function f (f, funcdef_no=8, decl_uid=2062, cgraph_uid=8, symbol_order=9) Removing basic block 3 f (int i) { const char * iftmp.0_1; <bb 2> [100.00%] [count: INV]: if (i_2(D) < 0) goto <bb 4>; [36.00%] [count: INV] else goto <bb 3>; [64.00%] [count: INV] <bb 3> [64.00%] [count: INV]: <bb 4> [100.00%] [count: INV]: # iftmp.0_1 = PHI <"01234567"(2), "89abcd"(3)> __builtin___strcat_chk (&d, iftmp.0_1, 3); [tail call] return; } In file included from /usr/include/string.h:148:0, from 79221.c:1: 79221.c: In function ‘f’: 79221.c:8:2: warning: ‘__builtin___strcat_chk’ writing between 7 and 9 bytes into a region of size 3 overflows the destination [-Wstringop-overflow=] strcat (d, s); ^~~~~~ ;; Function gf (gf, funcdef_no=9, decl_uid=2066, cgraph_uid=9, symbol_order=10) Removing basic block 3 gf (int i) { char[9] * iftmp.2_1; <bb 2> [100.00%] [count: INV]: if (i_2(D) < 0) goto <bb 4>; [36.00%] [count: INV] else goto <bb 3>; [64.00%] [count: INV] <bb 3> [64.00%] [count: INV]: <bb 4> [100.00%] [count: INV]: # iftmp.2_1 = PHI <"12345678"(2), "87654321"(3)> __builtin___strcat_chk (&d, iftmp.2_1, 3); [tail call] return; } 79221.c: In function ‘gf’: 79221.c:15:2: warning: ‘__builtin___strcat_chk’ writing 9 bytes into a region of size 3 overflows the destination [-Wstringop-overflow=] strcat (d, s); ^~~~~~ $
cc-ing diagnostics maintainers
(In reply to Martin Sebor from comment #0) > Similar to bug 79220, the -Wstringop-overflow option diagnoses the buffer > overflow in the call to strcat in f() in the program below but fails to do > the same for the strcat() overflow in g(). As in the referenced bug, GCC > transforms the second strcat() to an assignment followed by a call to > memcpy, defeating the overflow detection. GCC should avoid this > transformation when the destination isn't big enough for the copy. > > I expect this bug will be resolved by a comprehensive fix for bug 79220 bug 79220 was fixed... was the fix for it comprehensive enough?
Looks like this was resolved by r255755 (PR 78918). Both functions in the test case in comment #0 are now diagnosed, by different warnings. That's not unexpected but should at some point be changed to issue -Wstringop-overflow for both. $ gcc -O2 -S -Wall pr79221.c pr79221.c: In function ‘f’: pr79221.c:8:3: warning: ‘strcat’ writing between 7 and 9 bytes into a region of size 3 overflows the destination [-Wstringop-overflow=] 8 | strcat (d, s); | ^~~~~~~~~~~~~ pr79221.c: In function ‘gf’: pr79221.c:15:3: warning: ‘__builtin_memcpy’ forming offset [3, 8] is out of the bounds [0, 3] of object ‘d’ with type ‘char[3]’ [-Warray-bounds] 15 | strcat (d, s); | ^~~~~~~~~~~~~ pr79221.c:3:6: note: ‘d’ declared here 3 | char d[3]; | ^
Author: msebor Date: Wed Dec 11 15:59:55 2019 New Revision: 279227 URL: https://gcc.gnu.org/viewcvs?rev=279227&root=gcc&view=rev Log: PR middle-end/79221 - missing -Wstringop-overflow= on a strcat overflow gcc/testsuite/ChangeLog: * gcc.dg/Wstringop-overflow-26.c: New test. Added: trunk/gcc/testsuite/gcc.dg/Wstringop-overflow-26.c Modified: trunk/gcc/testsuite/ChangeLog
Test case added in r279227.