[Bug c++/113852] -Wsign-compare doesn't warn on unsigned result types
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113852 --- Comment #8 from rguenther at suse dot de --- On Mon, 12 Feb 2024, admin at computerquip dot com wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113852 > > --- Comment #7 from Zachary L --- > (In reply to Richard Biener from comment #6) > > Well, given athat a1 * a2 is carried out in 'int' you are invoking undefined > > behavior if it overflows. GCC assumes that doesn't happen so it's correct > > to elide the diagnostic. Unless you make overflow well-defined with > > -fwrapv. > > > > I think that errors on the right side for the purpose of -Wsign-compare. > > Removing a diagnostic because the program could be ill-formed seems backwards > to me, especially since it seems like there's already logic in performing this > diagnostic. Perhaps I've misunderstood the situation? Well, we could, for each int * int diagnose a "may invoke undefined behavior if overflow happens at runtime" but that's hardly useful. So we want to diagnose cases where it's _likely_ that there's an issue. For the case at hand it's not likely that ushort * ushort invokes undefined behavior (because invoking undefined should be unlikely, very much so). So we choose to not diagnose it as "maybe". Yes, with constexpr evaluation we actually _see_ we invoke undefined behavior and diagnose that. The subsequent diagnostic of -Wsign-compare is then spurious though and IMO should be not given. The error is the integer overflow invoking undefined behavior even - if that didn't happen there would be no issue with comparing signed/unsigned values.
[Bug c++/113852] -Wsign-compare doesn't warn on unsigned result types
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113852 --- Comment #7 from Zachary L --- (In reply to Richard Biener from comment #6) > Well, given athat a1 * a2 is carried out in 'int' you are invoking undefined > behavior if it overflows. GCC assumes that doesn't happen so it's correct > to elide the diagnostic. Unless you make overflow well-defined with -fwrapv. > > I think that errors on the right side for the purpose of -Wsign-compare. Removing a diagnostic because the program could be ill-formed seems backwards to me, especially since it seems like there's already logic in performing this diagnostic. Perhaps I've misunderstood the situation?
[Bug c++/113852] -Wsign-compare doesn't warn on unsigned result types
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113852 Richard Biener changed: What|Removed |Added CC||rguenth at gcc dot gnu.org --- Comment #6 from Richard Biener --- Well, given athat a1 * a2 is carried out in 'int' you are invoking undefined behavior if it overflows. GCC assumes that doesn't happen so it's correct to elide the diagnostic. Unless you make overflow well-defined with -fwrapv. I think that errors on the right side for the purpose of -Wsign-compare.
[Bug c++/113852] -Wsign-compare doesn't warn on unsigned result types
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113852 --- Comment #5 from Jonathan Wakely --- The behaviour changed with r247495 common.opt (fstrict-overflow): Alias negative to fwrapv. 2017-05-02 Richard Biener * common.opt (fstrict-overflow): Alias negative to fwrapv. * doc/invoke.texi (fstrict-overflow): Remove all traces of -fstrict-overflow documentation. * tree.h (TYPE_OVERFLOW_UNDEFINED): Do not test flag_strict_overflow. (POINTER_TYPE_OVERFLOW_UNDEFINED): Test !flag_wrapv instead of flag_strict_overflow. * ipa-inline.c (can_inline_edge_p): Do not test flag_strict_overflow. * lto-opts.c (lto_write_options): Do not stream it. * lto-wrapper.c (merge_and_complain): Do not handle it. * opts.c (default_options_table): Do not set -fstrict-overflow. (finish_options): Likewise do not clear it when sanitizing. * simplify-rtx.c (simplify_const_relational_operation): Do not test flag_strict_overflow.
[Bug c++/113852] -Wsign-compare doesn't warn on unsigned result types
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113852
--- Comment #4 from Jonathan Wakely ---
Reduced:
int main()
{
unsigned short a1 = (1u << 16) - 1;
unsigned short a2 = a1;
/* a1 * a2 should be 4294836225 in math terms */
unsigned long long a3 = 4294836225;
/*
* The result of (a1 * a2) is of type int and the result is negative.
* (a1 * a2) ends up as some bogus high number because the common
* type here ends up as uint64_t and sign-extension occurs.
*/
if ((a1 * a2) > a3) {
__builtin_abort();
}
}
[Bug c++/113852] -Wsign-compare doesn't warn on unsigned result types
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113852 --- Comment #3 from Jonathan Wakely --- (In reply to Jonathan Wakely from comment #2) > I assume what's happening is that GCC assumes integer promotion from > uint16_t to int is value preserving and so we get two positive values, and > therefore comparison with an unsigned value is fine - there are no negative > values involved and so it doesn't matter that we're comparing int with > unsigned long. But of course that's not true here. We have two positive ints > but their product overflows to produce a negative int. I guess we're also > assuming no overflow happens, because that would be undefined and we assume > no UB. Ah yes, if you add -fwrapv then you get the -Wsign-compare warning, because now a negative product can occur without undefined overflow.
[Bug c++/113852] -Wsign-compare doesn't warn on unsigned result types
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113852
Jonathan Wakely changed:
What|Removed |Added
Keywords||diagnostic
Ever confirmed|0 |1
Status|UNCONFIRMED |NEW
Last reconfirmed||2024-02-09
--- Comment #2 from Jonathan Wakely ---
If a1 and a2 are const then GCC also notices the undefined overflow:
: In function 'int main()':
:18:13: warning: integer overflow in expression of type 'int' results
in '-131071' [-Woverflow]
18 | if ((a1 * a2) > a3) {
| ~~~^~~~
:18:19: warning: comparison of integer expressions of different
signedness: 'int' and 'uint64_t' {aka 'long unsigned int'} [-Wsign-compare]
18 | if ((a1 * a2) > a3) {
| ~~^~~~
I don't know why the -Wsign-compare warning is only given when we've detected
the overflow. The types are always the same, whether the values are known to
overflow or not.
I assume what's happening is that GCC assumes integer promotion from uint16_t
to int is value preserving and so we get two positive values, and therefore
comparison with an unsigned value is fine - there are no negative values
involved and so it doesn't matter that we're comparing int with unsigned long.
But of course that's not true here. We have two positive ints but their product
overflows to produce a negative int. I guess we're also assuming no overflow
happens, because that would be undefined and we assume no UB.
When the values are constant we can tell the overflow happens, and no longer
assume it doesn't happen.
[Bug c++/113852] -Wsign-compare doesn't warn on unsigned result types
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113852 --- Comment #1 from Zachary L --- Sorry, that should say "If *both* a1 or a2 are constexpr, the warning will occur."
