public inbox for gcc-bugs@sourceware.org help / color / mirror / Atom feed
* [Bug c/103835] New: Bogus sprintf warnings @ 2021-12-26 16:13 lavr at ncbi dot nlm.nih.gov 2021-12-26 21:02 ` [Bug tree-optimization/103835] " pinskia at gcc dot gnu.org ` (4 more replies) 0 siblings, 5 replies; 6+ messages in thread From: lavr at ncbi dot nlm.nih.gov @ 2021-12-26 16:13 UTC (permalink / raw) To: gcc-bugs https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103835 Bug ID: 103835 Summary: Bogus sprintf warnings Product: gcc Version: 11.2.0 Status: UNCONFIRMED Severity: normal Priority: P3 Component: c Assignee: unassigned at gcc dot gnu.org Reporter: lavr at ncbi dot nlm.nih.gov Target Milestone: --- Please address these warnings because they create more noise than they help! $ cat test.c #include <stdio.h> #include <stdlib.h> #include <string.h> const char* fun(char* buf, const char* pfx, int a, int b) { sprintf(buf, "%sa = %d\n" "%sb = %d\n", pfx, a, pfx, b); return buf; } int main(int argc, char* argv[]) { char buf[500]; const char* str; strcpy(buf, "\t"); str = fun(buf + strlen(buf) + 1, buf, atoi(argv[1]), atoi(argv[2])); printf("%s\n", str); return 0; } $ gcc --version gcc (GCC) 11.2.0 Copyright (C) 2021 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. $ gcc -Wall -O6 test.c test.c: In function ‘main’: test.c:8:21: warning: ‘a = ’ directive writing 4 bytes into a region of size between 0 and 499 [-Wformat-overflow=] 8 | sprintf(buf, "%sa = %d\n" | ^~~~ test.c:8:5: note: ‘sprintf’ output between 13 and 1031 bytes into a destination of size 499 8 | sprintf(buf, "%sa = %d\n" | ^~~~~~~~~~~~~~~~~~~~~~~~~ 9 | "%sb = %d\n", | ~~~~~~~~~~~~~ 10 | pfx, a, pfx, b); | ~~~~~~~~~~~~~~~ test.c:8:5: warning: ‘sprintf’ arguments 3, 5 may overlap destination object ‘buf’ [-Wrestrict] test.c:17:10: note: destination object referenced by ‘restrict’-qualified argument 1 was declared here 17 | char buf[500]; | ^~~ It's clear that the destination buffer will NOT overlap with anything related to "pfx" in the fun() function. Is also clear that output will NOT contain that many characters that the warning claims (up to 1031). If GCC can't estimate the length for sure, it's better NOT to emit any warnings, rather than printing this annoying noise. Please be mindful of your users -- and their time to re-analyze the code that suddenly is now flagged with these senseless warnings, only to realize that it's all red herring. Thank you ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug tree-optimization/103835] Bogus sprintf warnings 2021-12-26 16:13 [Bug c/103835] New: Bogus sprintf warnings lavr at ncbi dot nlm.nih.gov @ 2021-12-26 21:02 ` pinskia at gcc dot gnu.org 2021-12-27 15:09 ` lavr at ncbi dot nlm.nih.gov ` (3 subsequent siblings) 4 siblings, 0 replies; 6+ messages in thread From: pinskia at gcc dot gnu.org @ 2021-12-26 21:02 UTC (permalink / raw) To: gcc-bugs https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103835 Andrew Pinski <pinskia at gcc dot gnu.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Last reconfirmed| |2021-12-26 Keywords| |diagnostic, | |missed-optimization Ever confirmed|0 |1 Component|c |tree-optimization Status|UNCONFIRMED |NEW --- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> --- Looks like atoi is not know and GCC thinks atoi will write into buf. But that does not fix the restrict warning which does not use string length analysis to figure out it does not overlap. Confirmed. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug tree-optimization/103835] Bogus sprintf warnings 2021-12-26 16:13 [Bug c/103835] New: Bogus sprintf warnings lavr at ncbi dot nlm.nih.gov 2021-12-26 21:02 ` [Bug tree-optimization/103835] " pinskia at gcc dot gnu.org @ 2021-12-27 15:09 ` lavr at ncbi dot nlm.nih.gov 2022-01-05 0:52 ` [Bug tree-optimization/103835] bogus sprintf warnings due to missing strlen propagation msebor at gcc dot gnu.org ` (2 subsequent siblings) 4 siblings, 0 replies; 6+ messages in thread From: lavr at ncbi dot nlm.nih.gov @ 2021-12-27 15:09 UTC (permalink / raw) To: gcc-bugs https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103835 --- Comment #2 from lavr at ncbi dot nlm.nih.gov --- Irrespective of whether atoi() is known, printing an "int" (or two) will never produce this many characters... This, however, also seems to have triggered some weird logic that took the entire size of buf[] as a possible size of "pfx" on output; hence, the warning noted 1000+ characters to be printed. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug tree-optimization/103835] bogus sprintf warnings due to missing strlen propagation 2021-12-26 16:13 [Bug c/103835] New: Bogus sprintf warnings lavr at ncbi dot nlm.nih.gov 2021-12-26 21:02 ` [Bug tree-optimization/103835] " pinskia at gcc dot gnu.org 2021-12-27 15:09 ` lavr at ncbi dot nlm.nih.gov @ 2022-01-05 0:52 ` msebor at gcc dot gnu.org 2022-01-05 13:59 ` lavr at ncbi dot nlm.nih.gov 2022-01-06 23:24 ` msebor at gcc dot gnu.org 4 siblings, 0 replies; 6+ messages in thread From: msebor at gcc dot gnu.org @ 2022-01-05 0:52 UTC (permalink / raw) To: gcc-bugs https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103835 Martin Sebor <msebor at gcc dot gnu.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Bogus sprintf warnings |bogus sprintf warnings due | |to missing strlen | |propagation Blocks| |83819 --- Comment #3 from Martin Sebor <msebor at gcc dot gnu.org> --- The warnings in comment #0 are caused by a limitation of the strlen/sprintf passes: their less than perfect integration with the range (or constant) propagation machinery. The issue can be seen in the simplified test case below. In f() the constant strlen() result isn't propagated into the addition in d_5 = &a + n_4; at the time the warning runs (within the strlen pass), and so it triggers as expected. In g() it is propagated and the warning doesn't trigger. To avoid the warnings the strlen pass needs to set the range of strlen results and propagate them through the IL so they can be determined in computations in subsequent statements (like sprintf). $ cat pr103835.c && gcc -O2 -S -Wall -fdump-tree-strlen=/dev/stdout pr103835.c char a[8]; int f (void) { __builtin_strcpy (a, "f"); __SIZE_TYPE__ n = __builtin_strlen (a) + 1; char *d = a + n; __builtin_sprintf (d, "%s", a); return n; } int g (void) { __builtin_strcpy (a, "g"); __SIZE_TYPE__ n = __builtin_strlen ("g") + 1; char *d = a + n; __builtin_sprintf (d, "%s", a); return n; } ;; Function f (f, funcdef_no=0, decl_uid=1980, cgraph_uid=1, symbol_order=1) ;; 1 loops found ;; ;; Loop 0 ;; header 0, latch 1 ;; depth 0, outer -1 ;; nodes: 0 1 2 ;; 2 succs { 1 } pr103835.c:11: __builtin_sprintf: objsize = 6, fmtstr = "%s" Directive 1 at offset 0: "%s" Result: 1, 1, 1, 1 (1, 1, 1, 1) Directive 2 at offset 2: "", length = 1 pr103835.c: In function ‘f’: pr103835.c:11:2: warning: ‘__builtin_sprintf’ argument 3 may overlap destination object ‘a’ [-Wrestrict] 11 | __builtin_sprintf (d, "%s", a); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ pr103835.c:1:6: note: destination object referenced by ‘restrict’-qualified argument 1 was declared here 1 | char a[8]; | ^ int f () { char * d; long unsigned int n; long unsigned int _1; int _7; <bb 2> [local count: 1073741824]: __builtin_memcpy (&a, "f", 2); _1 = 1; n_4 = _1 + 1; d_5 = &a + n_4; __builtin_strcpy (d_5, &a); _7 = (int) n_4; return _7; } ;; Function g (g, funcdef_no=1, decl_uid=1985, cgraph_uid=2, symbol_order=2) ;; 1 loops found ;; ;; Loop 0 ;; header 0, latch 1 ;; depth 0, outer -1 ;; nodes: 0 1 2 ;; 2 succs { 1 } pr103835.c:24: __builtin_sprintf: objsize = 6, fmtstr = "%s" Directive 1 at offset 0: "%s" Result: 1, 1, 1, 1 (1, 1, 1, 1) Directive 2 at offset 2: "", length = 1 Substituting 1 for return value. int g () { <bb 2> [local count: 1073741824]: __builtin_memcpy (&a, "g", 2); __builtin_strcpy (&MEM <char[8]> [(void *)&a + 2B], &a); return 2; } Referenced Bugs: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83819 [Bug 83819] [meta-bug] missing strlen optimizations ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug tree-optimization/103835] bogus sprintf warnings due to missing strlen propagation 2021-12-26 16:13 [Bug c/103835] New: Bogus sprintf warnings lavr at ncbi dot nlm.nih.gov ` (2 preceding siblings ...) 2022-01-05 0:52 ` [Bug tree-optimization/103835] bogus sprintf warnings due to missing strlen propagation msebor at gcc dot gnu.org @ 2022-01-05 13:59 ` lavr at ncbi dot nlm.nih.gov 2022-01-06 23:24 ` msebor at gcc dot gnu.org 4 siblings, 0 replies; 6+ messages in thread From: lavr at ncbi dot nlm.nih.gov @ 2022-01-05 13:59 UTC (permalink / raw) To: gcc-bugs https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103835 --- Comment #4 from lavr at ncbi dot nlm.nih.gov --- Thank you for investigating this! Like I said, it's better NOT to emit any warnings if some machinery in the compiler is not perfect enough to recognize the danger correctly (as it used to be the case in previous versions and so they were silent in this respect). There is the second warning there, too, which seems not being addressed by the discussion. Not only the count (499) is not given correctly (but maybe it's because of the constant propagation that was mentioned), there's the second part (the note), which follows, and which looks extremely bad and unintelligent w.r.t. the output size calculation: test.c:8:21: warning: ‘a = ’ directive writing 4 bytes into a region of size between 0 and 499 [-Wformat-overflow=] 8 | sprintf(buf, "%sa = %d\n" | ^~~~ test.c:8:5: note: ‘sprintf’ output between 13 and 1031 bytes into a destination of size 499 Is this the same cause, too? ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug tree-optimization/103835] bogus sprintf warnings due to missing strlen propagation 2021-12-26 16:13 [Bug c/103835] New: Bogus sprintf warnings lavr at ncbi dot nlm.nih.gov ` (3 preceding siblings ...) 2022-01-05 13:59 ` lavr at ncbi dot nlm.nih.gov @ 2022-01-06 23:24 ` msebor at gcc dot gnu.org 4 siblings, 0 replies; 6+ messages in thread From: msebor at gcc dot gnu.org @ 2022-01-06 23:24 UTC (permalink / raw) To: gcc-bugs https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103835 --- Comment #5 from Martin Sebor <msebor at gcc dot gnu.org> --- It's the same cause. When the -Wformat-overflow and -truncation warnings are unable to compute the exact length of a string argument to a directive like %s they use the size of the array the string is stored in as a worst case estimate (if they can determine the size of the array). In other words, they assume the string is as long as fits in the array (minus 1 for the terminating nul). See the example below. In simple cases specifying a precision (e.g., "%.4s") to constrain the amount of output can be used to avoid the potential overflow and suppress the warning. This is difficult to do with multiple string directives; with those, using snprintf is the preferred alternative. To avoid the -Wformat-truncation warning for the equivalent snprintf calls it's necessary to check the function's return value. The sprintf pass keeps a running total of the ranges of the bytes formatted by each directive, and the warning tries to strike a balance between providing too little and too much detail to understand why it triggers. This is tricky as the range of total bytes on output includes the sum of more and more directives. We discussed various approaches to improve this some time ago (see pr77696) but didn't converge on one that made everyone happy. $ cat a.c && gcc -O2 -S -Wall a.c char a[8], b[8]; void f (void) { __builtin_sprintf (a, "%s%i", b, 123); } a.c: In function ‘f’: a.c:5:28: warning: ‘%i’ directive writing 3 bytes into a region of size between 1 and 8 [-Wformat-overflow=] 5 | __builtin_sprintf (a, "%s%i", b, 123); | ^~ a.c:5:3: note: ‘__builtin_sprintf’ output between 4 and 11 bytes into a destination of size 8 5 | __builtin_sprintf (a, "%s%i", b, 123); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The decision when to warn and when not is difficult. All these warnings depend on various optimizations, and those are never perfect. The warnings make a few basic assumptions (e.g., unreachable code is eliminated) and sometimes apply heuristics like the string length above. Both of these can lead to false positives in addition to false negatives. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-06 23:24 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-26 16:13 [Bug c/103835] New: Bogus sprintf warnings lavr at ncbi dot nlm.nih.gov 2021-12-26 21:02 ` [Bug tree-optimization/103835] " pinskia at gcc dot gnu.org 2021-12-27 15:09 ` lavr at ncbi dot nlm.nih.gov 2022-01-05 0:52 ` [Bug tree-optimization/103835] bogus sprintf warnings due to missing strlen propagation msebor at gcc dot gnu.org 2022-01-05 13:59 ` lavr at ncbi dot nlm.nih.gov 2022-01-06 23:24 ` msebor at gcc dot gnu.org
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).