Created attachment 45424 [details] test-case I originally reported that to elfutils bugzilla. The warning looks as follows: $ gcc -m32 /tmp/readelf.i -c -O2 -Werror=format-overflow readelf.c: In function ‘print_debug_str_section’: readelf.c:10152:15: error: ‘%*llx’ directive output between 4 and 2147483647 bytes may cause result to exceed ‘INT_MAX’ [-Werror=format-overflow=] 10152 | printf (" [%*" PRIx64 "] \"%s\"\n", digits, (uint64_t) offset, str); | ^~~~~~ readelf.c:10152:15: note: directive argument in the range [0, 18446744073709551614]
The warning can be reproduced in the (over)simplified test case below. There, it isn't strictly incorrect -- the upper bound of the width does allow the output to exceed INT_MAX. But it arguably is overly pedantic. The root cause in the readelf.i test case is the same: the width the warning sees is unbounded. But the warning there is a false positive because the width is clearly bounded. The width is computed in a loop like the one in pr88844 where GCC should be but is unable to determine the maximum number of iterations (bounded by the size of the integer argument). As a result of the width's upper bound being unknown it is taken as INT_MAX. Short of examining the loop and computing the range itself there isn't much the warning code can do to compensate for the poor range info it relies on. What it could and maybe should do is avoid using the upper bound at level 1 and instead use the lower bound. GCC 8 issues the same warning for calls to sprintf. It doesn't check printf and fprintf, so it doesn't diagnose the Binutils test case. r265648 extended the checking to those functions and exposed the underlying issues. As a result, even though I agree that the warning is overly aggressive, I don't consider this bug a regression. Let me look into relaxing it. $ cat pr88835.c && gcc -O2 -S -Wall -fdump-tree-printf-return-value=/dev/stdout pr88835.c void f (int n, int i) { if (n < 4) n = 4; if (i < 4) i = 4; __builtin_printf (" %*i", n, i); } ;; Function f (f, funcdef_no=0, decl_uid=1907, cgraph_uid=1, symbol_order=0) pr88835.c:8: __builtin_printf: objsize = 18446744073709551615, fmtstr = " %*i" Directive 1 at offset 0: " ", length = 1 Result: 1, 1, 1, 1 (1, 1, 1, 1) Directive 2 at offset 1: "%*i", width in range [4, 2147483647] pr88835.c: In function ‘f’: pr88835.c:8:23: warning: ‘%*i’ directive output between 4 and 2147483647 bytes may cause result to exceed ‘INT_MAX’ [-Wformat-overflow=] 8 | __builtin_printf (" %*i", n, i); | ^~~ pr88835.c:8:21: note: directive argument in the range [4, 2147483647] 8 | __builtin_printf (" %*i", n, i); | ^~~~~~ Result: 4, 2147483647, 2147483647, 2147483647 (5, 2147483648, 2147483648, 2147483648) Directive 3 at offset 4: "", length = 1 f (int n, int i) { <bb 2> [local count: 1073741824]: n_4 = MAX_EXPR <n_1(D), 4>; i_2 = MAX_EXPR <i_3(D), 4>; __builtin_printf (" %*i", n_4, i_2); return; }
Created attachment 45598 [details] One another-test case I see one another test-case that comes from here: https://github.com/systemd/systemd/issues/11369 $ gcc load-fragment.i -c -Werror=format-overflow -O load-fragment.i: In function ‘load_from_path’: load-fragment.i:30349:36: error: ‘%s’ directive argument is null [-Werror=format-overflow=] 30349 | ) ? log_internal_realm(((_realm) << 10 | (_level)), _e, "../src/core/load-fragment.c", 4301, __func__, "Cannot access \"%s\": %m", filename) : -abs(_e); }); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: some warnings being treated as errors I must say that it's really misleading warning and is probably caused by ethread pass. Martin can you please take a look?
The case in comment #2 is entirely dissimilar to the the one in comment #0 so it would be better deal with under its own bug. But unless I'm misreading the code in the translation unit, the warning looks justified: filename is cleared by the assignment from the result of mfree() but then used as an argument to log_internal_realm() right after it. The function is declared with attribute printf. I copied the context of the warning from the translation unit below. It doesn't seem to correspond to the code pointed to in the discussion (https://github.com/systemd/systemd/issues/11369#issuecomment-453044884): the mfree() call appears after the test for filename being null. If I'm missing something please open a new bug for it. filename = path_make_absolute(path, *p); if (!filename) return - 12 ; ... if (r >= 0) break; filename = mfree(filename); if (r == - 13 ) ({ int _level = (( 7 )), _e = ((r)), _realm = (LOG_REALM_SYSTEMD); (log_get_max_level_realm(_realm) >= (( _level ) & 0x07) ) ? log_internal_realm(((_realm) << 10 | (_level)), _e, "../src/core/load-fragment.c", 4301, __func__, "Cannot access \"%s\": %m", filename) : -abs(_e); });
Martin, please also let me know what specifically about the warning you find misleading so I can make it better.
> warning from the translation unit below. It doesn't seem to correspond to > the code pointed to in the discussion > (https://github.com/systemd/systemd/issues/11369#issuecomment-453044884): > the mfree() call appears after the test for filename being null. If I'm > missing something please open a new bug for it. > You are right, it was fixed here: https://github.com/systemd/systemd/commit/baa162cecd00e122a626656d25b8eae92b767519 Thus, the warning was correct. Thanks for help.
Thanks for looking into the systemd warning. Indeed, the gcc warning was fine, but the code was alread fixed in https://github.com/systemd/systemd/commit/baa162cecd before the gcc warning was reported, and everybody was confused.
Is this a regression that will probably be fixed for GCC 9.1 or should we be adding workaround to the code. Currently elfutils won't build on distros that started using the GCC 9.0 prerelease on 32bit architectures (i686, arm32): BUILDSTDERR: readelf.c: In function 'print_debug_str_section': BUILDSTDERR: readelf.c:10152:15: error: '%*llx' directive output between 4 and 2147483647 bytes may cause result to exceed 'INT_MAX' [-Werror=format-overflow=] BUILDSTDERR: 10152 | printf (" [%*" PRIx64 "] \"%s\"\n", digits, (uint64_t) offset, str); BUILDSTDERR: | ^~~~~~ BUILDSTDERR: readelf.c:10152:18: note: format string is defined here BUILDSTDERR: 10152 | printf (" [%*" PRIx64 "] \"%s\"\n", digits, (uint64_t) offset, str); BUILDSTDERR: readelf.c:10152:15: note: directive argument in the range [0, 18446744073709551614] BUILDSTDERR: 10152 | printf (" [%*" PRIx64 "] \"%s\"\n", digits, (uint64_t) offset, str); BUILDSTDERR: |
The patch I posted for the related pr88993 also relaxes this warning for printf and fprintf: https://gcc.gnu.org/ml/gcc-patches/2019-02/msg00224.html Like in the case of pr88993, the warning is by design and (in my view) makes sense for sprintf but it's not very useful for the other functions where very little code worries about exceeding these limits (or even cares about the functions failing as they can for many other reasons).
(In reply to Martin Sebor from comment #8) > The patch I posted for the related pr88993 also relaxes this warning for > printf and fprintf: https://gcc.gnu.org/ml/gcc-patches/2019-02/msg00224.html > > Like in the case of pr88993, the warning is by design and (in my view) makes > sense for sprintf but it's not very useful for the other functions where > very little code worries about exceeding these limits (or even cares about > the functions failing as they can for many other reasons). I tried that patch, but it does not fix this issue. This case now triggers the following warning, which isn't guarded by is_string_func: /* The directive output or either causes or may cause the total length of output to exceed INT_MAX bytes. */ if (likelyximax && fmtres.range.min == fmtres.range.max) warned = fmtwarn (dirloc, argloc, NULL, info.warnopt (), "%<%.*s%> directive output of %wu bytes causes " "result to exceed %<INT_MAX%>", dirlen, target_to_host (hostdir, sizeof hostdir, dir.beg), fmtres.range.min); So with that patch we get: /home/mark/src/elfutils/src/readelf.c: In function ‘print_debug_str_section’: /home/mark/src/elfutils/src/readelf.c:10167:15: error: ‘] "’ directive output of 4 bytes causes result to exceed ‘INT_MAX’ [-Werror=format-overflow=] 10167 | printf (" [%*" PRIx64 "] \"%s\"\n", digits, (uint64_t) offset, str); | ^~~~~~ /home/mark/src/elfutils/src/readelf.c:10167:30: note: format string is defined here 10167 | printf (" [%*" PRIx64 "] \"%s\"\n", digits, (uint64_t) offset, str); | ^~~~~ cc1: all warnings being treated as errors Which is actually more mysterious than the previous warning (without the patch as shown in description).
I've built the top of binutils-gdb with the patch referenced in comment #8 applied and with -Wformat-overflow=2 and -Wformat-truncation=2 and got the following breakdown: Diagnostic Count Unique Files -Wformat-overflow= 19 19 10 -Wformat-truncation= 12 12 7 -Wconflicts-sr 7 7 7 -Wmaybe-uninitialized 3 3 3 -Wstringop-truncation 2 2 2 -Wconflicts-rr 2 2 2 -Wsign-compare 1 1 1 -Wother 1 1 1 The -Wformat-overflow warnings are: /src/binutils-gdb/gas/macro.c:386:18: warning: ‘sprintf’ may write a terminating nul past the end of the destination [-Wformat-overflow=] /src/binutils-gdb/binutils/arsup.c:158:20: warning: ‘%.*s’ directive output between 0 and 2147483647 bytes may exceed minimum required size of 4095 [-Wformat-overflow=] /src/binutils-gdb/binutils/wrstabs.c:426:21: warning: ‘sprintf’ may write a terminating nul past the end of the destination [-Wformat-overflow=] /src/binutils-gdb/binutils/wrstabs.c:739:27: warning: ‘%u’ directive writing between 1 and 10 bytes into a region of size between 7 and 45 [-Wformat-overflow=] /src/binutils-gdb/binutils/wrstabs.c:620:26: warning: ‘%ld’ directive writing between 1 and 20 bytes into a region of size between 19 and 38 [-Wformat-overflow=] /src/binutils-gdb/binutils/wrstabs.c:595:26: warning: ‘%ld’ directive writing between 1 and 20 bytes into a region of size between 19 and 38 [-Wformat-overflow=] eelf_iamcu.c:635:25: warning: ‘%.*s’ directive output between 0 and 2147483647 bytes may exceed minimum required size of 4095 [-Wformat-overflow=] eelf_iamcu.c:628:25: warning: ‘%.*s’ directive output between 0 and 2147483647 bytes may exceed minimum required size of 4095 [-Wformat-overflow=] eelf_x86_64.c:638:25: warning: ‘%.*s’ directive output between 0 and 2147483647 bytes may exceed minimum required size of 4095 [-Wformat-overflow=] eelf_x86_64.c:631:25: warning: ‘%.*s’ directive output between 0 and 2147483647 bytes may exceed minimum required size of 4095 [-Wformat-overflow=] eelf_i386.c:638:25: warning: ‘%.*s’ directive output between 0 and 2147483647 bytes may exceed minimum required size of 4095 [-Wformat-overflow=] eelf_i386.c:631:25: warning: ‘%.*s’ directive output between 0 and 2147483647 bytes may exceed minimum required size of 4095 [-Wformat-overflow=] eelf32_x86_64.c:638:25: warning: ‘%.*s’ directive output between 0 and 2147483647 bytes may exceed minimum required size of 4095 [-Wformat-overflow=] eelf32_x86_64.c:631:25: warning: ‘%.*s’ directive output between 0 and 2147483647 bytes may exceed minimum required size of 4095 [-Wformat-overflow=] eelf_k1om.c:638:25: warning: ‘%.*s’ directive output between 0 and 2147483647 bytes may exceed minimum required size of 4095 [-Wformat-overflow=] eelf_k1om.c:631:25: warning: ‘%.*s’ directive output between 0 and 2147483647 bytes may exceed minimum required size of 4095 [-Wformat-overflow=] eelf_l1om.c:638:25: warning: ‘%.*s’ directive output between 0 and 2147483647 bytes may exceed minimum required size of 4095 [-Wformat-overflow=] eelf_l1om.c:631:25: warning: ‘%.*s’ directive output between 0 and 2147483647 bytes may exceed minimum required size of 4095 [-Wformat-overflow=] /src/binutils-gdb/gdb/gdbserver/remote-utils.c:1188:21: warning: ‘%s’ directive output between 0 and 8191 bytes may exceed minimum required size of 4095 [-Wformat-overflow=] None for readelf.c. I think they are all for sprintf (and not printf or fprintf), so I'm not sure where the ones you are seeing are coming from.
Ah, but you mentioned elfutilts, not binutils. I've now downloaded and built elfutils-0.175. It took a bit more effort because --disable-werror doesn't work there but once I got past that I just got the three -Wformat-truncation instances below: Diagnostic Count Unique Files -Wformat-truncation= 3 3 2 -Wformat-truncation Instances: /src/elfutils-0.175/src/ar.c:1468 /src/elfutils-0.175/src/ar.c:859 /src/elfutils-0.175/src/arlib.c:63 The code at line 10167 in readelf.c looks like this: static void print_debug_str_offsets_section (Dwfl_Module *dwflmod __attribute__ ((unused)), Ebl *ebl, GElf_Ehdr *ehdr __attribute__ ((unused)), Elf_Scn *scn, GElf_Shdr *shdr, Dwarf *dbg) { printf (gettext ("\ \nDWARF section [%2zu] '%s' at offset %#" PRIx64 ":\n"), elf_ndxscn (scn), section_name (ebl, shdr), (uint64_t) shdr->sh_offset); I'm probably not using the same sources as you, but shouldn't I be seeing at least some of the warnings? (I couldn't find an easy way build the top of trunk -- there's no configure script.)
(In reply to Martin Sebor from comment #11) > Ah, but you mentioned elfutilts, not binutils. I've now downloaded and > built elfutils-0.175. It took a bit more effort because --disable-werror > doesn't work there but once I got past that I just got the three > -Wformat-truncation instances below: > > Diagnostic Count Unique Files > -Wformat-truncation= 3 3 2 > > -Wformat-truncation Instances: > /src/elfutils-0.175/src/ar.c:1468 > /src/elfutils-0.175/src/ar.c:859 > /src/elfutils-0.175/src/arlib.c:63 I am not seeing these, but they might have been fixed in git. We like to keep the code warning free since we always build with -Werror. > I'm probably not using the same sources as you, but shouldn't I be seeing at > least some of the warnings? (I couldn't find an easy way build the top of > trunk -- there's no configure script.) I am using git master. git://sourceware.org/git/elfutils.git See the README for build instructions: To build a git checkout do: autoreconf -i -f && \ ./configure --enable-maintainer-mode && \ make && make check Note that the warning/error only occurs on 32bit systems, like these fedora koji arm32 and i686 builds: https://koji.fedoraproject.org/koji/taskinfo?taskID=32816136 To get the same on a 64bit system build with CFLAGS="-g -O2 -m32"
(In reply to Martin Sebor from comment #10) > I've built the top of binutils-gdb with the patch referenced in comment #8 > applied and with -Wformat-overflow=2 and -Wformat-truncation=2 and got the > following breakdown: > > Diagnostic Count Unique Files > -Wformat-overflow= 19 19 10 > -Wformat-truncation= 12 12 7 > -Wconflicts-sr 7 7 7 > -Wmaybe-uninitialized 3 3 3 > -Wstringop-truncation 2 2 2 > -Wconflicts-rr 2 2 2 > -Wsign-compare 1 1 1 > -Wother 1 1 1 Tangent, but -Wconflicts-sr, -Wconflicts-rr, and -Wother are from bison, not gcc, so you might want to update whatever tool you use to generate these pretty tables to filter them out
(In reply to Mark Wielaard from comment #12) > (In reply to Martin Sebor from comment #11) > > Ah, but you mentioned elfutilts, not binutils. I've now downloaded and > > built elfutils-0.175. It took a bit more effort because --disable-werror > > doesn't work there but once I got past that I just got the three > > -Wformat-truncation instances below: > > > > Diagnostic Count Unique Files > > -Wformat-truncation= 3 3 2 > > > > -Wformat-truncation Instances: > > /src/elfutils-0.175/src/ar.c:1468 > > /src/elfutils-0.175/src/ar.c:859 > > /src/elfutils-0.175/src/arlib.c:63 > > I am not seeing these, but they might have been fixed in git. We like to > keep the code warning free since we always build with -Werror. Aha, I now see, you are using -Wformat-truncation=2. Then yes, these snprintfs formats could produce more characters than would fit in the given buffer/size. But that is kind of the point of the code, that we don't overflow the given buffer. The snprintf is supposed to truncate to what would fit in these cases. I can see if I could come up with something smarter to detect this without using snprintf, but that seems to defeat the point of using snprintf. So for now we just don't use -Wformat-truncation=2. (Background, ar files are weird, they use fixed size character fields for numbers as decimal strings without a zero terminator, but right padded with spaces.) The specific warnings which we enable can be found in config/eu.am and depend on some configure checks to make sure gcc supports them: AM_CFLAGS = -std=gnu99 -Wall -Wshadow -Wformat=2 \ -Wold-style-definition -Wstrict-prototypes -Wtrampolines \ $(LOGICAL_OP_WARNING) $(DUPLICATED_COND_WARNING) \ $(NULL_DEREFERENCE_WARNING) $(IMPLICIT_FALLTHROUGH_WARNING) \ $(if $($(*F)_no_Werror),,-Werror) \ $(if $($(*F)_no_Wunused),,-Wunused -Wextra) \ $(if $($(*F)_no_Wstack_usage),,$(STACK_USAGE_WARNING)) \ $(if $($(*F)_no_Wpacked_not_aligned),-Wno-packed-not-aligned,) \ $($(*F)_CFLAGS) With the following (if supported): STACK_USAGE_WARNING=-Wstack-usage=262144 LOGICAL_OP_WARNING=-Wlogical-op DUPLICATED_COND_WARNING=-Wduplicated-cond NULL_DEREFERENCE_WARNING=-Wnull-dereference IMPLICIT_FALLTHROUGH_WARNING=-Wimplicit-fallthrough=5 As you can see individual files can turn off some of these if necessary in the Makefile by adding file_no_Wxxx. So the easiest way to see which warnings are used it by running make V=1 which for this specific case gives (note the -m32 since I am running this on x86_64): gcc -D_GNU_SOURCE -DHAVE_CONFIG_H -DLOCALEDIR='"/usr/local/share/locale"' -DDEBUGPRED=0 -DSRCDIR=\"/home/mark/src/elfutils/src\" -DOBJDIR=\"/opt/local/build/elfutils-obj/src\" -I. -I/home/mark/src/elfutils/src -I.. -I. -I/home/mark/src/elfutils/src -I/home/mark/src/elfutils/lib -I.. -I/home/mark/src/elfutils/src/../libelf -I/home/mark/src/elfutils/src/../libebl -I/home/mark/src/elfutils/src/../libdw -I/home/mark/src/elfutils/src/../libdwelf -I/home/mark/src/elfutils/src/../libdwfl -I/home/mark/src/elfutils/src/../libasm -std=gnu99 -Wall -Wshadow -Wformat=2 -Wold-style-definition -Wstrict-prototypes -Wtrampolines -Wlogical-op -Wduplicated-cond -Wnull-dereference -Wimplicit-fallthrough=5 -Werror -Wunused -Wextra -D_FORTIFY_SOURCE=2 -m32 -g -O2 -DBAD_FTS=1 -MT readelf.o -MD -MP -MF .deps/readelf.Tpo -c -o readelf.o /home/mark/src/elfutils/src/readelf.c /home/mark/src/elfutils/src/readelf.c: In function ‘print_debug_str_section’: /home/mark/src/elfutils/src/readelf.c:10167:15: error: ‘%*llx’ directive output between 4 and 2147483647 bytes may cause result to exceed ‘INT_MAX’ [-Werror=format-overflow=] 10167 | printf (" [%*" PRIx64 "] \"%s\"\n", digits, (uint64_t) offset, str); | ^~~~~~ /home/mark/src/elfutils/src/readelf.c:10167:18: note: format string is defined here 10167 | printf (" [%*" PRIx64 "] \"%s\"\n", digits, (uint64_t) offset, str); /home/mark/src/elfutils/src/readelf.c:10167:15: note: directive argument in the range [0, 18446744073709551614] 10167 | printf (" [%*" PRIx64 "] \"%s\"\n", digits, (uint64_t) offset, str); | ^~~~~~ cc1: all warnings being treated as errors
Author: msebor Date: Fri Feb 22 17:38:11 2019 New Revision: 269125 URL: https://gcc.gnu.org/viewcvs?rev=269125&root=gcc&view=rev Log: PR tree-optimization/88993 - GCC 9 -Wformat-overflow=2 should reflect real libc limits PR tree-optimization/88835 - overly aggressive -Werror=format-overflow for printf gcc/ChangeLog: PR tree-optimization/88993 PR tree-optimization/88853 * gimple-ssa-sprintf.c (sprintf_dom_walker::call_info::is_file_func): New helper. (sprintf_dom_walker::call_info::is_string_func): New helper. (format_directive): Only issue "may exceed" 4095/INT_MAX warnings for formatted string functions. (sprintf_dom_walker::handle_gimple_call): Fix a typo in a comment. gcc/testsuite/ChangeLog: PR tree-optimization/88993 PR tree-optimization/88853 * gcc.dg/tree-ssa/builtin-fprintf-warn-2.c: New test. * gcc.dg/tree-ssa/builtin-printf-warn-2.c: New test. * gcc.dg/tree-ssa/builtin-snprintf-warn-3.c: Adjust. * gcc.dg/tree-ssa/builtin-sprintf-warn-18.c: Same. Added: trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-fprintf-warn-2.c trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-printf-warn-2.c Modified: trunk/gcc/gimple-ssa-sprintf.c trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-warn-3.c trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-18.c
The warning has been relaxed for GCC 9 in r269125.
(In reply to Martin Sebor from comment #16) > The warning has been relaxed for GCC 9 in r269125. Thanks, I can confirm elfutils builds fine without warnings with GCC 9 now.