[Bug c/103881] Wconversion false positive when using |= and &= with two rvalues in binary op
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103881 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org Resolution|--- |FIXED Status|UNCONFIRMED |RESOLVED --- Comment #9 from Jakub Jelinek --- Fixed for 10.4 as well, not going to backport it further.
[Bug c/103881] Wconversion false positive when using |= and &= with two rvalues in binary op
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103881 --- Comment #8 from CVS Commits --- The releases/gcc-10 branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:4e8c32ecfb87e723229dc6c08df8ea602e947f1e commit r10-10666-g4e8c32ecfb87e723229dc6c08df8ea602e947f1e Author: Jakub Jelinek Date: Tue Jan 11 19:11:51 2022 +0100 c-family: Fix up -W*conversion on bitwise &/|/^ [PR101537] The following testcases emit a bogus -Wconversion warning. This is because conversion_warning function doesn't handle BIT_*_EXPR (only unsafe_conversion_p that is called during the default: case, and that one doesn't handle SAVE_EXPRs added because the unsigned char & or | operands promoted to int have side-effects and =| or =& is used. The patch handles BIT_IOR_EXPR/BIT_XOR_EXPR like the last 2 operands of COND_EXPR by recursing on the two operands, if either of them doesn't fit into the narrower type, complain. BIT_AND_EXPR too, but first it needs to handle some special cases that unsafe_conversion_p does, namely when one of the two operands is a constant. This fixes completely the pr101537.c test and for C also pr103881.c and doesn't regress anything in the testsuite, for C++ pr103881.c still emits the bogus warnings. This is because while the C FE emits in that case a SAVE_EXPR that conversion_warning can handle already, C++ FE emits TARGET_EXPR , something | D.whatever etc. and conversion_warning handles COMPOUND_EXPR by "recursing" on the rhs. To handle that case, we'd need for TARGET_EXPR on the lhs remember in some hash map the mapping from D.whatever to the TARGET_EXPR and when we see D.whatever, use corresponding TARGET_EXPR initializer instead. 2022-01-11 Jakub Jelinek PR c/101537 PR c/103881 gcc/c-family/ * c-warn.c (conversion_warning): Handle BIT_AND_EXPR, BIT_IOR_EXPR and BIT_XOR_EXPR. gcc/testsuite/ * c-c++-common/pr101537.c: New test. * c-c++-common/pr103881.c: New test. (cherry picked from commit 20e4a5e573e76f4379b353cc736215a5f10cdb84)
[Bug c/103881] Wconversion false positive when using |= and &= with two rvalues in binary op
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103881 --- Comment #7 from CVS Commits --- The releases/gcc-11 branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:777b73e45983b11e010bd5192185ad01af444de4 commit r11-9500-g777b73e45983b11e010bd5192185ad01af444de4 Author: Jakub Jelinek Date: Tue Jan 11 19:11:51 2022 +0100 c-family: Fix up -W*conversion on bitwise &/|/^ [PR101537] The following testcases emit a bogus -Wconversion warning. This is because conversion_warning function doesn't handle BIT_*_EXPR (only unsafe_conversion_p that is called during the default: case, and that one doesn't handle SAVE_EXPRs added because the unsigned char & or | operands promoted to int have side-effects and =| or =& is used. The patch handles BIT_IOR_EXPR/BIT_XOR_EXPR like the last 2 operands of COND_EXPR by recursing on the two operands, if either of them doesn't fit into the narrower type, complain. BIT_AND_EXPR too, but first it needs to handle some special cases that unsafe_conversion_p does, namely when one of the two operands is a constant. This fixes completely the pr101537.c test and for C also pr103881.c and doesn't regress anything in the testsuite, for C++ pr103881.c still emits the bogus warnings. This is because while the C FE emits in that case a SAVE_EXPR that conversion_warning can handle already, C++ FE emits TARGET_EXPR , something | D.whatever etc. and conversion_warning handles COMPOUND_EXPR by "recursing" on the rhs. To handle that case, we'd need for TARGET_EXPR on the lhs remember in some hash map the mapping from D.whatever to the TARGET_EXPR and when we see D.whatever, use corresponding TARGET_EXPR initializer instead. 2022-01-11 Jakub Jelinek PR c/101537 PR c/103881 gcc/c-family/ * c-warn.c (conversion_warning): Handle BIT_AND_EXPR, BIT_IOR_EXPR and BIT_XOR_EXPR. gcc/testsuite/ * c-c++-common/pr101537.c: New test. * c-c++-common/pr103881.c: New test. (cherry picked from commit 20e4a5e573e76f4379b353cc736215a5f10cdb84)
[Bug c/103881] Wconversion false positive when using |= and &= with two rvalues in binary op
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103881
--- Comment #6 from thomas at habets dot se ---
The sequence point is a bit off topic, but I can't imagine that it's the
intention to warn about `f() & f()`.
Per gcc docs it's supposed to "Warn about code that may have undefined
semantics because of violations of sequence point rules in the C and C++
standards […] Programs whose behavior depends on this have undefined behavior".
I'm not a language lawyer, but I can't imagine that it's the intention to warn
about e.g. `printf("called with %f: %s", sqrt(2), strerror(errno));`. Nor does
it sound helpful to do so.
Nor `x = get_value() & admin_bits();`. There's just no wrong but technically
conforming way to miscompile that.
[Bug c/103881] Wconversion false positive when using |= and &= with two rvalues in binary op
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103881 Eric Gallager changed: What|Removed |Added See Also||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=40752, ||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=12411 CC||egallager at gcc dot gnu.org --- Comment #5 from Eric Gallager --- (In reply to thomas from comment #3) > Interesting. > > So the difference between "x |= a & a" and "x |= f() & f()" is that the > latter has passed a somewhat arbitrary level of complexity after which GCC > is not able to prove that it's safe, and therefore warns as it being > potentially losing precision? > > It's understandable, but unfortunate. It means that I have no hope of having > real world programs be free of false positives for conversion warnings. The latter looks like something that ought to get a -Wsequence-point warning anyways, at least per bug 12411... but then again that one was closed as WONTFIX, so never mind...
[Bug c/103881] Wconversion false positive when using |= and &= with two rvalues in binary op
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103881 --- Comment #4 from CVS Commits --- The master branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:20e4a5e573e76f4379b353cc736215a5f10cdb84 commit r12-6486-g20e4a5e573e76f4379b353cc736215a5f10cdb84 Author: Jakub Jelinek Date: Tue Jan 11 19:11:51 2022 +0100 c-family: Fix up -W*conversion on bitwise &/|/^ [PR101537] The following testcases emit a bogus -Wconversion warning. This is because conversion_warning function doesn't handle BIT_*_EXPR (only unsafe_conversion_p that is called during the default: case, and that one doesn't handle SAVE_EXPRs added because the unsigned char & or | operands promoted to int have side-effects and =| or =& is used. The patch handles BIT_IOR_EXPR/BIT_XOR_EXPR like the last 2 operands of COND_EXPR by recursing on the two operands, if either of them doesn't fit into the narrower type, complain. BIT_AND_EXPR too, but first it needs to handle some special cases that unsafe_conversion_p does, namely when one of the two operands is a constant. This fixes completely the pr101537.c test and for C also pr103881.c and doesn't regress anything in the testsuite, for C++ pr103881.c still emits the bogus warnings. This is because while the C FE emits in that case a SAVE_EXPR that conversion_warning can handle already, C++ FE emits TARGET_EXPR , something | D.whatever etc. and conversion_warning handles COMPOUND_EXPR by "recursing" on the rhs. To handle that case, we'd need for TARGET_EXPR on the lhs remember in some hash map the mapping from D.whatever to the TARGET_EXPR and when we see D.whatever, use corresponding TARGET_EXPR initializer instead. 2022-01-11 Jakub Jelinek PR c/101537 PR c/103881 gcc/c-family/ * c-warn.c (conversion_warning): Handle BIT_AND_EXPR, BIT_IOR_EXPR and BIT_XOR_EXPR. gcc/testsuite/ * c-c++-common/pr101537.c: New test. * c-c++-common/pr103881.c: New test.
[Bug c/103881] Wconversion false positive when using |= and &= with two rvalues in binary op
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103881 --- Comment #3 from thomas at habets dot se --- Interesting. So the difference between "x |= a & a" and "x |= f() & f()" is that the latter has passed a somewhat arbitrary level of complexity after which GCC is not able to prove that it's safe, and therefore warns as it being potentially losing precision? It's understandable, but unfortunate. It means that I have no hope of having real world programs be free of false positives for conversion warnings.
[Bug c/103881] Wconversion false positive when using |= and &= with two rvalues in binary op
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103881 --- Comment #2 from Jakub Jelinek --- See C99 6.3.1.1/2 and 6.3.1.8, yes, in all cases of |=, &=, |, & the uint8_t operands are promoted to int. And the reason why we don't warn for the simple binary operations is that those common cases have special cases for the warnings, while if you have more complex expressions you get warnings. This is a FE warning, value range propagation etc. doesn't happen at that point...
[Bug c/103881] Wconversion false positive when using |= and &= with two rvalues in binary op
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103881 --- Comment #1 from thomas at habets dot se --- I just reproduced this from git HEAD, as seen on github (commit e3cbb8c66c930ba738674b0fcf29848dc3ecfef2), with latest commit today, 2021-12-31.
