Bug 1320175
| Summary: | "-Werror=format-nonliteral" failure but worked in F23 | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Bastien Nocera <bnocera> |
| Component: | gcc | Assignee: | Jakub Jelinek <jakub> |
| Status: | CLOSED NOTABUG | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | high | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | 24 | CC: | davejohansen, jakub, jwakely, law, mpolacek |
| Target Milestone: | --- | ||
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2016-03-22 15:16:02 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
I think this behavior is desirable. I think what has changed is this: given e.g.
void
foo (void)
{
const char *t = "%d\n";
__builtin_printf (t, 1);
}
$ xgcc-5 -c g.c -Werror=format=2
says nothing, but with gcc6:
$ xgcc -c g.c -Werror=format=2
g.c: In function ‘foo’:
g.c:5:3: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
__builtin_printf (t, 1);
^~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
Note that with -Wformat=2 both version warn as they should:
g.c: In function ‘foo’:
g.c:5:3: warning: format not a string literal, argument types not checked [-Wformat-nonliteral]
__builtin_printf (t, 1);
^
I think this is so since gcc.gnu.org/r231406.
Yeah, handling of -Werror= options which have arguments has been broken in GCC 5 and earlier, so -Werror=format=2 used to work as -Werror=format=1 instead, unless -Werror=format=2 -Wformat=2 has been used. (In reply to Marek Polacek from comment #1) > I think this behavior is desirable. Why is it desirable? In your example, if "t" is a constant, what does this fix? Similarly, something like: void foo (void) { const char * const t[] = { "%d\n", "%d\n" }; __builtin_printf (t[0], 1); } Also fails, whether t[0] is a constant or not. Are we trying to protect against somebody swapping out the pointer to the string? (In reply to Jakub Jelinek from comment #2) > Yeah, handling of -Werror= options which have arguments has been broken in > GCC 5 and earlier, so -Werror=format=2 used to work as -Werror=format=1 > instead, unless > -Werror=format=2 -Wformat=2 has been used. Again, what does it fix? See the documentation? -Werror=<WHATEVER>=<ARG> is supposed to enable -W<WHATEVER>=<ARG> and make sure that -W<WHATEVER> warnings are reported as errors. -Wformat (which is the same as -Wformat=1) is some set of format warning options, -Wformat=2 adds other warnings to that. If you don't want the -Wformat=2 warnings, use just -Werror=format or -Werror=format=1 instead of -Werror=format=2. (In reply to Jakub Jelinek from comment #5) > See the documentation? -Werror=<WHATEVER>=<ARG> is supposed to enable > -W<WHATEVER>=<ARG> and make sure that -W<WHATEVER> warnings are reported as > errors. > -Wformat (which is the same as -Wformat=1) is some set of format warning > options, -Wformat=2 adds other warnings to that. If you don't want the > -Wformat=2 warnings, use just -Werror=format or -Werror=format=1 instead of > -Werror=format=2. That's not what I'm asking. See comment 3. I'm asking why this ends up being a warning (and then an error) when we're stashing a constant into a variable, and reusing it without changes. -Wformat is a FE warning, it doesn't look at optimized code, and is not meant to. The point of the warning is to warn when the FE can't analyze the format string, because it is not simple enough. In the plugin, you have a structure that contains const char * fields, array of them and index it by some counter. What if the counter is out of bounds? In your #c3 testcase, what if somebody in between takes address of the automatic variable? Even when it is const, somebody (possibly an exploit) could have changed that. And so on and on. Or in other words, -Wformat-nonliteral warns about this code by design. That's what it's meant to do. If you don't like that, don't use the warning. The gcc5 behaviour was a bug, the use of -Werror=format=1 should have enabled -Werror=format-literal, but didn't. That's fixed now, so if you use the warning option you get the warning. If you don't want the warning, don't use the option. (In reply to Jonathan Wakely from comment #8) > The gcc5 behaviour was a bug, the use of -Werror=format=1 should have Oops, typo, I meant -Werror=format=2 (In reply to Jakub Jelinek from comment #7) > -Wformat is a FE warning, it doesn't look at optimized code, and is not > meant to. > The point of the warning is to warn when the FE can't analyze the format > string, because it is not simple enough. > In the plugin, you have a structure that contains const char * fields, array > of them and index it by some counter. What if the counter is out of bounds? > In your #c3 testcase, what if somebody in between takes address of the > automatic variable? Even when it is const, somebody (possibly an exploit) > could have changed that. And so on and on. Right. I know that software is magic, but I wish that the compiler could warn me about the possibly out-of-bounds counter, or not use a temporary variable when unrolling the code, to avoid those problems, instead of me having to refactor code which I can see is correct. Thanks for the help. (In reply to Jonathan Wakely from comment #8) > If you don't want the warning, don't > use the option. I didn't choose that warning. I'm fairly certain it's one of the default RPM CFLAGS in Fedora. |
Trying to build grilo-plugins 0.3.1 in Fedora 24 and rawhide fails: make[3]: Entering directory '/builddir/build/BUILD/grilo-plugins-0.3.1/src/jamendo' CC libgrljamendo_la-grl-jamendo.lo grl-jamendo.c: In function 'grl_jamendo_source_browse': grl-jamendo.c:1180:32: error: format not a string literal, argument types not checked [-Werror=format-nonliteral] page_number); ^~~~~~~~~~~ grl-jamendo.c: In function 'grl_jamendo_source_query': grl-jamendo.c:1281:26: error: format not a string literal, argument types not checked [-Werror=format-nonliteral] term); ^~~~ with gcc-6.0.0-0.15.fc24 Compiling on F23 with gcc-5.3.1-2.fc23.x86_64 works: V=1 make /bin/sh ../../libtool --tag=CC --mode=compile gcc -DHAVE_CONFIG_H -I. -I../.. -pthread -I/home/hadess/Projects/gnome-install/include/grilo-0.3 -I/home/hadess/Projects/gnome-install/include/glib-2.0 -I/home/hadess/Projects/gnome-install/lib/glib-2.0/include -I/usr/include/libxml2 -pthread -I/home/hadess/Projects/gnome-install/include/grilo-0.3 -I/home/hadess/Projects/gnome-install/include/glib-2.0 -I/home/hadess/Projects/gnome-install/lib/glib-2.0/include -I/usr/include/libxml2 -I/usr/include/libxml2 -DG_LOG_DOMAIN=\"GrlJamendo\" -DLOCALEDIR=\"/home/hadess/Projects/gnome-install/share/locale\" -g3 -ggdb -Wformat -Werror=format-nonliteral -Wall -Wstrict-prototypes -Wnested-externs -Werror=missing-prototypes -Werror=implicit-function-declaration -Werror=pointer-arith -Werror=init-self -Werror=format-security -Werror=format=2 -Werror=missing-include-dirs -Werror=return-type -Wmissing-declarations -std=c99 -MT libgrljamendo_la-grl-jamendo.lo -MD -MP -MF .deps/libgrljamendo_la-grl-jamendo.Tpo -c -o libgrljamendo_la-grl-jamendo.lo `test -f 'grl-jamendo.c' || echo './'`grl-jamendo.c libtool: compile: gcc -DHAVE_CONFIG_H -I. -I../.. -pthread -I/home/hadess/Projects/gnome-install/include/grilo-0.3 -I/home/hadess/Projects/gnome-install/include/glib-2.0 -I/home/hadess/Projects/gnome-install/lib/glib-2.0/include -I/usr/include/libxml2 -pthread -I/home/hadess/Projects/gnome-install/include/grilo-0.3 -I/home/hadess/Projects/gnome-install/include/glib-2.0 -I/home/hadess/Projects/gnome-install/lib/glib-2.0/include -I/usr/include/libxml2 -I/usr/include/libxml2 -DG_LOG_DOMAIN=\"GrlJamendo\" -DLOCALEDIR=\"/home/hadess/Projects/gnome-install/share/locale\" -g3 -ggdb -Wformat -Werror=format-nonliteral -Wall -Wstrict-prototypes -Wnested-externs -Werror=missing-prototypes -Werror=implicit-function-declaration -Werror=pointer-arith -Werror=init-self -Werror=format-security -Werror=format=2 -Werror=missing-include-dirs -Werror=return-type -Wmissing-declarations -std=c99 -MT libgrljamendo_la-grl-jamendo.lo -MD -MP -MF .deps/libgrljamendo_la-grl-jamendo.Tpo -c grl-jamendo.c -fPIC -DPIC -o .libs/libgrljamendo_la-grl-jamendo.o mv -f .deps/libgrljamendo_la-grl-jamendo.Tpo .deps/libgrljamendo_la-grl-jamendo.Plo /bin/sh ../../libtool --tag=CC --mode=link gcc -pthread -I/home/hadess/Projects/gnome-install/include/grilo-0.3 -I/home/hadess/Projects/gnome-install/include/glib-2.0 -I/home/hadess/Projects/gnome-install/lib/glib-2.0/include -I/usr/include/libxml2 -pthread -I/home/hadess/Projects/gnome-install/include/grilo-0.3 -I/home/hadess/Projects/gnome-install/include/glib-2.0 -I/home/hadess/Projects/gnome-install/lib/glib-2.0/include -I/usr/include/libxml2 -I/usr/include/libxml2 -DG_LOG_DOMAIN=\"GrlJamendo\" -DLOCALEDIR=\"/home/hadess/Projects/gnome-install/share/locale\" -g3 -ggdb -Wformat -Werror=format-nonliteral -Wall -Wstrict-prototypes -Wnested-externs -Werror=missing-prototypes -Werror=implicit-function-declaration -Werror=pointer-arith -Werror=init-self -Werror=format-security -Werror=format=2 -Werror=missing-include-dirs -Werror=return-type -Wmissing-declarations -std=c99 -no-undefined -module -avoid-version -L/home/hadess/Projects/gnome-install/lib -o libgrljamendo.la -rpath /home/hadess/Projects/gnome-install/lib/grilo-0.3 libgrljamendo_la-grl-jamendo.lo -L/home/hadess/Projects/gnome-install/lib -lgrilo-0.3 -Wl,--export-dynamic -lgmodule-2.0 -pthread -lgio-2.0 -lgobject-2.0 -lglib-2.0 -lxml2 -L/home/hadess/Projects/gnome-install/lib -lgrlnet-0.3 -lgrilo-0.3 -Wl,--export-dynamic -lgmodule-2.0 -pthread -lgio-2.0 -lgobject-2.0 -lglib-2.0 -lxml2 -lxml2 libtool: link: gcc -shared -fPIC -DPIC .libs/libgrljamendo_la-grl-jamendo.o -Wl,-rpath -Wl,/home/hadess/Projects/gnome-install/lib -Wl,-rpath -Wl,/home/hadess/Projects/gnome-install/lib -L/home/hadess/Projects/gnome-install/lib -lgrlnet-0.3 /home/hadess/Projects/gnome-install/lib/libgrilo-0.3.so -lgmodule-2.0 -lgio-2.0 -lgobject-2.0 -lglib-2.0 -lxml2 -pthread -pthread -g3 -ggdb -Wl,--export-dynamic -pthread -Wl,--export-dynamic -pthread -pthread -Wl,-soname -Wl,libgrljamendo.so -o .libs/libgrljamendo.so libtool: link: ( cd ".libs" && rm -f "libgrljamendo.la" && ln -s "../libgrljamendo.la" "libgrljamendo.la" ) The code for that file is here: https://git.gnome.org/browse/grilo-plugins/tree/src/jamendo/grl-jamendo.c And while the code around line 1281 is easy enough to fix (move the url printing to the switch statement, rather than using a temporary variable), but the first one would make us lose the benefits of using a structure array to fit the corresponding values.