[libvirt] Non-zero constant warning on RHEL 6.1 with 0.9.7
Hi guys, Just checking 0.9.7 on RHEL 6.1 x86_64. Noticed this when compiling with make -j 3: make[3]: Entering directory `/home/jc/git_repos/libvirt/src' CC libvirt_util_la-bitmap.lo CC libvirt_util_la-authhelper.lo CC libvirt_util_la-bridge.lo CC libvirt_util_la-buf.lo CC libvirt_util_la-command.lo util/buf.c: In function 'virBufferEscape': util/buf.c:469: warning: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op] ... CC libvirt_lxc-command.o util/buf.c: In function 'virBufferEscape': util/buf.c:469: warning: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op] Obviously not fatal, but it figured someone might want to keep things warning free. :) Regards and best wishes, Justin Clift -- Aeolus Community Manager http://www.aeolusproject.org
On 09.11.2011 07:46, Justin Clift wrote:
Hi guys,
Just checking 0.9.7 on RHEL 6.1 x86_64. Noticed this when compiling with make -j 3:
make[3]: Entering directory `/home/jc/git_repos/libvirt/src' CC libvirt_util_la-bitmap.lo CC libvirt_util_la-authhelper.lo CC libvirt_util_la-bridge.lo CC libvirt_util_la-buf.lo CC libvirt_util_la-command.lo util/buf.c: In function 'virBufferEscape': util/buf.c:469: warning: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op]
...
I've ran gcc -E on util/buf.c and interesting things are happening there: the original code: while (*cur != 0) { if (strchr(toescape, *cur)) *out++ = '\\'; *out++ = *cur; cur++; } [line 469 is: if (strchr(...))] after preprocessing stage: while (*cur != 0) { if ((__extension__ (__builtin_constant_p (*cur) && !__builtin_constant_p (toescape) && (*cur) == '\0' ? (char *) __rawmemchr (toescape, *cur) : __builtin_strchr (toescape, *cur)))) *out++ = '\\'; *out++ = *cur; cur++; } So I think the problem lies within !__builtin_constant_p (toescape) but i am not sure how to fix that. Michal
On 11/08/2011 11:46 PM, Justin Clift wrote:
Hi guys,
Just checking 0.9.7 on RHEL 6.1 x86_64. Noticed this when compiling with make -j 3:
CC libvirt_lxc-command.o util/buf.c: In function 'virBufferEscape': util/buf.c:469: warning: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op]
Obviously not fatal, but it figured someone might want to keep things warning free. :)
This is a bug in gcc, although I'm not sure if anyone has raised a bug report against the gcc folks yet: https://www.redhat.com/archives/libvir-list/2011-October/msg00837.html I'm open to suggestions on how to work around it; perhaps we just need to disable -Wlogical-op if glibc's headers are turning on the optimized strchr (is that -D_FORTIFY_SOURCE that does it?) :( -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 12/11/2011, at 8:06 AM, Eric Blake wrote:
On 11/08/2011 11:46 PM, Justin Clift wrote:
Hi guys,
Just checking 0.9.7 on RHEL 6.1 x86_64. Noticed this when compiling with make -j 3:
CC libvirt_lxc-command.o util/buf.c: In function 'virBufferEscape': util/buf.c:469: warning: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op]
Obviously not fatal, but it figured someone might want to keep things warning free. :)
This is a bug in gcc, although I'm not sure if anyone has raised a bug report against the gcc folks yet: https://www.redhat.com/archives/libvir-list/2011-October/msg00837.html
I'm open to suggestions on how to work around it; perhaps we just need to disable -Wlogical-op if glibc's headers are turning on the optimized strchr (is that -D_FORTIFY_SOURCE that does it?) :(
Hmmmm, we might as well report it, so it gets looked at. Next question is where? Upstream gcc, or on the RG BZ as a Fedora15(+?)/RHEL6 specific gcc bug? Regards and best wishes, Justin Clift -- Aeolus Community Manager http://www.aeolusproject.org
On 11/11/2011 04:54 PM, Justin Clift wrote:
On 12/11/2011, at 8:06 AM, Eric Blake wrote:
On 11/08/2011 11:46 PM, Justin Clift wrote:
Hi guys,
Just checking 0.9.7 on RHEL 6.1 x86_64. Noticed this when compiling with make -j 3:
CC libvirt_lxc-command.o util/buf.c: In function 'virBufferEscape': util/buf.c:469: warning: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op]
Obviously not fatal, but it figured someone might want to keep things warning free. :) This is a bug in gcc, although I'm not sure if anyone has raised a bug report against the gcc folks yet: https://www.redhat.com/archives/libvir-list/2011-October/msg00837.html
I'm open to suggestions on how to work around it; perhaps we just need to disable -Wlogical-op if glibc's headers are turning on the optimized strchr (is that -D_FORTIFY_SOURCE that does it?) :( Hmmmm, we might as well report it, so it gets looked at.
Next question is where? Upstream gcc, or on the RG BZ as a Fedora15(+?)/RHEL6 specific gcc bug? I am seeing it in this gcc version: Using built-in specs. Target: ppc64-redhat-linux Configured with: ../configure --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-bootstrap --enable-shared --enable-threads=posix --enable-checking=release --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-languages=c,c++,objc,obj-c++,java,fortran,ada --enable-java-awt=gtk --disable-dssi --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre --enable-libgcj-multifile --enable-java-maintainer-mode --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --disable-libjava-multilib --with-ppl --with-cloog --enable-secureplt --with-long-double-128 --with-cpu-32=power4 --with-tune-32=power6 --with-cpu-64=power4 --with-tune-64=power6 --build=ppc64-redhat-linux Thread model: posix gcc version 4.4.5 20110214 (Red Hat 4.4.5-6) (GCC)
Stefan
On 11/11/2011 04:06 PM, Eric Blake wrote:
On 11/08/2011 11:46 PM, Justin Clift wrote:
Hi guys,
Just checking 0.9.7 on RHEL 6.1 x86_64. Noticed this when compiling with make -j 3:
CC libvirt_lxc-command.o util/buf.c: In function 'virBufferEscape': util/buf.c:469: warning: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op]
Obviously not fatal, but it figured someone might want to keep things warning free. :) This is a bug in gcc, although I'm not sure if anyone has raised a bug report against the gcc folks yet: https://www.redhat.com/archives/libvir-list/2011-October/msg00837.html
I'm open to suggestions on how to work around it; perhaps we just need to disable -Wlogical-op if glibc's headers are turning on the optimized strchr (is that -D_FORTIFY_SOURCE that does it?) :( This should work, no?
--- a/src/util/buf.c +++ b/src/util/buf.c @@ -466,7 +466,8 @@ virBufferEscape(virBufferPtr buf, const char *toescape, cur = str; out = escaped; while (*cur != 0) { - if (strchr(toescape, *cur)) + char x[2] = { *cur, 0 }; + if (strstr(toescape, x)) *out++ = '\\'; *out++ = *cur; cur++; Stefan
On 11/14/2011 05:54 PM, Stefan Berger wrote:
On 11/11/2011 04:06 PM, Eric Blake wrote:
On 11/08/2011 11:46 PM, Justin Clift wrote:
Hi guys,
Just checking 0.9.7 on RHEL 6.1 x86_64. Noticed this when compiling with make -j 3:
CC libvirt_lxc-command.o util/buf.c: In function 'virBufferEscape': util/buf.c:469: warning: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op]
Obviously not fatal, but it figured someone might want to keep things warning free. :) This is a bug in gcc, although I'm not sure if anyone has raised a bug report against the gcc folks yet: https://www.redhat.com/archives/libvir-list/2011-October/msg00837.html
I'm open to suggestions on how to work around it; perhaps we just need to disable -Wlogical-op if glibc's headers are turning on the optimized strchr (is that -D_FORTIFY_SOURCE that does it?) :( This should work, no?
--- a/src/util/buf.c +++ b/src/util/buf.c @@ -466,7 +466,8 @@ virBufferEscape(virBufferPtr buf, const char *toescape, cur = str; out = escaped; while (*cur != 0) { - if (strchr(toescape, *cur)) + char x[2] = { *cur, 0 }; + if (strstr(toescape, x))
The glibc headers are expanding the pre-patch code to: if ((__extension__ (__builtin_constant_p (*cur) && !__builtin_constant_p (toescape) && (*cur) == '\0' ? (char *) __rawmemchr (toescape, *cur) : __builtin_strchr (toescape, *cur)))) but warning about: util/buf.c:469: warning: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op] I see two constants in that expression: !__builtin_constant_p (toescape) is always 1 (*cur) == '\0' is always 0 (since the containing while loop already guaranteed *cur != 0) There's definitely at least one gcc bug: that of an incorrect error message. After all, the expression 'expra && exprb' is constant in only three cases: 'expra' is false, 'exprb' is false, or 'expra' is true and 'exprb' is true. But the gcc message isn't clear on whether it is warning about 'expra' or 'exprb', not to mention that it claims the overall expression is true even though we just proved the overall expression is always false by virtue of exprb being false. Whether there is a secondary bug, about gcc being noisy with -Wlogical-op inside __extension__, is also worth arguing. At any rate, I agree that your proposed patch skirts the issue by using strstr() instead of strchr(), so it would indeed do the trick (while under the hood, strstr() on a single-byte needle tends to defer to strchr() anyways). It feels kind of gross, without at least a comment explaining that we are working around a gcc bug in -Wlogical-op (and even better, a URL of the gcc bug report); but I'm inclined to ACK it if we can come up with an appropriate comment. An even more aggressive patch would use strpbrk() to locate each character that needs escaping and memcpy() to quickly copy chunks of non-escaped text, rather than processing things one byte at a time, but I'm not sure whether that would be a micro-optimization or whether this function is called often enough to make it worth the extra complexity. What does 'make V=1' show as the exact compiler line used when triggering the error? I still haven't been able to reproduce the bogus compiler warning on a much smaller test case, but suspect I may be missing a particular flag that makes it apparent. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 11/14/2011 10:36 PM, Eric Blake wrote:
On 11/14/2011 05:54 PM, Stefan Berger wrote:
On 11/11/2011 04:06 PM, Eric Blake wrote:
On 11/08/2011 11:46 PM, Justin Clift wrote:
Hi guys,
Just checking 0.9.7 on RHEL 6.1 x86_64. Noticed this when compiling with make -j 3:
CC libvirt_lxc-command.o util/buf.c: In function 'virBufferEscape': util/buf.c:469: warning: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op]
Obviously not fatal, but it figured someone might want to keep things warning free. :) This is a bug in gcc, although I'm not sure if anyone has raised a bug report against the gcc folks yet: https://www.redhat.com/archives/libvir-list/2011-October/msg00837.html
I'm open to suggestions on how to work around it; perhaps we just need to disable -Wlogical-op if glibc's headers are turning on the optimized strchr (is that -D_FORTIFY_SOURCE that does it?) :(
At any rate, I agree that your proposed patch skirts the issue by using strstr() instead of strchr(), so it would indeed do the trick (while under the hood, strstr() on a single-byte needle tends to defer to strchr() anyways). It feels kind of gross, without at least a comment explaining that we are working around a gcc bug in -Wlogical-op (and even better, a URL of the gcc bug report); but I'm inclined to ACK it if we can come up with an appropriate comment.
What about this here: --- a/src/util/buf.c +++ b/src/util/buf.c @@ -466,7 +466,11 @@ virBufferEscape(virBufferPtr buf, const char *toescape, cur = str; out = escaped; while (*cur != 0) { - if (strchr(toescape, *cur)) + /* strchr work-around for gcc 4.3 & 4.4 bug with -Wlogical-op + * http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36513 + */ + char needle[2] = { *cur, 0 }; + if (strstr(toescape, needle)) *out++ = '\\'; *out++ = *cur; cur++;
An even more aggressive patch would use strpbrk() to locate each character that needs escaping and memcpy() to quickly copy chunks of non-escaped text, rather than processing things one byte at a time, but I'm not sure whether that would be a micro-optimization or whether this function is called often enough to make it worth the extra complexity.
What does 'make V=1' show as the exact compiler line used when triggering the error? I still haven't been able to reproduce the bogus compiler warning on a much smaller test case, but suspect I may be missing a particular flag that makes it apparent.
libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -I../gnulib/lib -I../gnulib/lib -I../include -I../src/util -I../include -DIN_LIBVIRT -I/usr/include/libxml2 -Wall -W -Wformat-y2k -Wformat-security -Winit-self -Wmissing-include-dirs -Wunused -Wunknown-pragmas -Wstrict-aliasing -Wshadow -Wpointer-arith -Wbad-function-cast -Wcast-align -Wwrite-strings -Wlogical-op -Waggregate-return -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-declarations -Wmissing-noreturn -Wmissing-format-attribute -Wredundant-decls -Wnested-externs -Winline -Winvalid-pch -Wvolatile-register-var -Wdisabled-optimization -Wbuiltin-macro-redefined -Wmudflap -Wpacked-bitfield-compat -Wsync-nand -Wattributes -Wcoverage-mismatch -Wmultichar -Wno-missing-field-initializers -Wno-sign-compare -Wframe-larger-than=4096 -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-all --param=ssp-buffer-size=4 -fexceptions -fasynchronous-unwind-tables -fdiagnostics-show-option -funit-at-a-time -g -O2 -MT libvirt_util_la-buf.lo -MD -MP -MF .deps/libvirt_util_la-buf.Tpo -c util/buf.c -fPIC -DPIC -o .libs/libvirt_util_la-buf.o util/buf.c: In function 'virBufferEscape': util/buf.c:469: warning: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op] Stefan
On 15/11/2011, at 10:34 PM, Stefan Berger wrote:
+ /* strchr work-around for gcc 4.3 & 4.4 bug with -Wlogical-op + * http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36513 + */
Cool, that's good investigation Stefan. :) + Justin -- Aeolus Community Manager http://www.aeolusproject.org
On Tue, Nov 15, 2011 at 06:34:27AM -0500, Stefan Berger wrote:
On 11/14/2011 10:36 PM, Eric Blake wrote:
On 11/14/2011 05:54 PM, Stefan Berger wrote:
On 11/11/2011 04:06 PM, Eric Blake wrote:
On 11/08/2011 11:46 PM, Justin Clift wrote:
Hi guys,
Just checking 0.9.7 on RHEL 6.1 x86_64. Noticed this when compiling with make -j 3:
CC libvirt_lxc-command.o util/buf.c: In function 'virBufferEscape': util/buf.c:469: warning: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op]
Obviously not fatal, but it figured someone might want to keep things warning free. :) This is a bug in gcc, although I'm not sure if anyone has raised a bug report against the gcc folks yet: https://www.redhat.com/archives/libvir-list/2011-October/msg00837.html
I'm open to suggestions on how to work around it; perhaps we just need to disable -Wlogical-op if glibc's headers are turning on the optimized strchr (is that -D_FORTIFY_SOURCE that does it?) :(
At any rate, I agree that your proposed patch skirts the issue by using strstr() instead of strchr(), so it would indeed do the trick (while under the hood, strstr() on a single-byte needle tends to defer to strchr() anyways). It feels kind of gross, without at least a comment explaining that we are working around a gcc bug in -Wlogical-op (and even better, a URL of the gcc bug report); but I'm inclined to ACK it if we can come up with an appropriate comment.
What about this here:
--- a/src/util/buf.c +++ b/src/util/buf.c @@ -466,7 +466,11 @@ virBufferEscape(virBufferPtr buf, const char *toescape, cur = str; out = escaped; while (*cur != 0) { - if (strchr(toescape, *cur)) + /* strchr work-around for gcc 4.3 & 4.4 bug with -Wlogical-op + * http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36513 + */ + char needle[2] = { *cur, 0 }; + if (strstr(toescape, needle)) *out++ = '\\';
I'm puzzelled that we need this change here, but we have 175 other uses of strchr without trouble $ grep strchr src/*/*.c | wc -l 174 Is the difference just that we're deferencing a char * for the second arg rather than using a constant value ? Would it suffice to do int c = *cur; if (strchr(toescape, c)) Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Tue, Nov 15, 2011 at 06:34:27AM -0500, Stefan Berger wrote:
Is the difference just that we're deferencing a char * for the second arg rather than using a constant value ?
Would it suffice to do
int c = *cur; if (strchr(toescape, c)) I still get the same error with this. It's the two variables that cause
On 11/15/2011 06:41 AM, Daniel P. Berrange wrote: the trouble here. Stefan
On 11/15/2011 04:41 AM, Daniel P. Berrange wrote:
while (*cur != 0) { - if (strchr(toescape, *cur)) + /* strchr work-around for gcc 4.3 & 4.4 bug with -Wlogical-op + * http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36513 + */ + char needle[2] = { *cur, 0 }; + if (strstr(toescape, needle)) *out++ = '\\';
I'm puzzelled that we need this change here, but we have 175 other uses of strchr without trouble
$ grep strchr src/*/*.c | wc -l 174
Rather, look for instances of strchr that have a constant second argument: $ git grep strchr".*'" src/*/*.c | wc -l 173 vs. those with a variable second argument: $ git grep strchr"[^']*\$" src/*/*.c | wc -l 3 further, 2 of those 3 have a constant first argument. So this really was the only instance of using strchr() with two variable arguments, and that is the only use-case that can trigger the gcc bug. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 11/15/2011 04:34 AM, Stefan Berger wrote:
What about this here:
--- a/src/util/buf.c +++ b/src/util/buf.c @@ -466,7 +466,11 @@ virBufferEscape(virBufferPtr buf, const char *toescape, cur = str; out = escaped; while (*cur != 0) { - if (strchr(toescape, *cur)) + /* strchr work-around for gcc 4.3 & 4.4 bug with -Wlogical-op + * http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36513 + */ + char needle[2] = { *cur, 0 }; + if (strstr(toescape, needle)) *out++ = '\\'; *out++ = *cur; cur++;
ACK. And I finally reproduced the issue with gcc 4.4.6, as well as confirmed that gcc 4.6.2 is immune; it involves both -O1 (or greater) and -Wlogical-op at the same time. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (5)
-
Daniel P. Berrange -
Eric Blake -
Justin Clift -
Michal Privoznik -
Stefan Berger