[Bug c/24542] potential integer overflow should be warned on assignment to wider variable

[Bug c/24542] potential integer overflow should be warned on assignment to wider variable

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24542

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |zhangboyang.id at gmail dot com

--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 109352 has been marked as a duplicate of this bug. ***

[Bug c/24542] potential integer overflow should be warned on assignment to wider variable

[zhangboyang.id at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24542

--- Comment #12 from Zhang Boyang <zhangboyang.id at gmail dot com> ---
Hi,

Sorry for filled a duplicate bug. But I'd like to suggest reconsider this
feature request. Here are two reasons:

1) "u64 = 1 << u32", "u64 = u32 * u32" are common mistakes in beginners, 

2) These expressions may introduce vulnerability especially on now-widely-used
64-bit machines:
  On a typical 64-bit machine, it's ok to write:
    unsigned x = ...;
    malloc(sizeof(...) + x)
  but it will introduce vulnerability with a trivial change of "*2", i.e.:
    malloc(sizeof(...) + x * 2)
If expression is very long, it's very hard to find out where is the bug.

Instead of warn on multiplys, I suggest a new "-Wexpr-conversion", it will
detect and warn on implicit conversions if and only if: 1) convert to wider
variable, and 2) value is real expression (i.e. result of operands, like a*b;
but not variable or function call or explicit cast)

For example, it should warn on:

  uint64_t u64 = ...;
  uint32_t u32 = ...;
  u64 = 1 << u32;
    //  ^^^^^^^^
    //   suggests "u64 = (uint64_t)1 << (uint64_t)u32"
    //   suppressed by "u64 = (uint32_t)(1 << u32)"

But not on:
  u64 = u32;
  u64 = (u32)(...);
  u64 = f(...);

This might be a kind of noisy warning like "-Wconversion" but I believe it will
help some people (we can just disable it by default).

Zhang Boyang

[Bug c/24542] potential unwanted truncation of operation overflow should be warned on assignment to wider variable

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24542

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-03-31
             Status|RESOLVED                    |NEW
         Resolution|WONTFIX                     |---
            Summary|potential integer overflow  |potential unwanted
                   |should be warned on         |truncation of operation
                   |assignment to wider         |overflow should be warned
                   |variable                    |on assignment to wider
                   |                            |variable
     Ever confirmed|0                           |1

--- Comment #13 from Richard Biener <rguenth at gcc dot gnu.org> ---
Let me re-open this.  I agree that it sounds useful to have a diagnostic that
would catch these cases but I also think it might have many false positives.
But that's similar to diagnosing if (a || b && c).

That said, the burden is on whoever is going to prototype patch with
extensive enough test coverage.

The question is whether to diagnose

 int x1, x2;
 long y1;
 y1 = x1 * x2;

since when x1 * x2 overflows that even invokes undefined behavior (so it's
even worse than the unsigned case).

The description is misleading, there's no "overflow on assignment" but the
operation itself might overflow and the truncated value is then widened
on assignment.  The assignment is a mere hint that a wider result might
have been intended (and a good enough hint IMHO).

[Bug c/24542] potential unwanted truncation of operation overflow should be warned on assignment to wider variable

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24542

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rsandifo at gcc dot gnu.org

--- Comment #14 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Heh, was in the process of reopening this too, but Richard beat
me too it.

FWIW, I agree this is worth providing as an option.  Another justification
is the different promotion handling between u32 = u16 op u16 and
u64 = u32 op u32.

"auto" (which wasn't a thing when the PR was first filed) might also
increase the chances of accidentally pushing promotions to the root of
a multi-statement calculation.

I don't think the false positive/negative ratio matters too much for
the option itself.  If it works then I think it's worth having.
IMO the ratio only becomes important if we're considering enabling
this by default (unlikely), -Wall (unsure) or -Wextra (seems feasible).

[Bug c/24542] potential unwanted truncation of operation overflow should be warned on assignment to wider variable

[mail+gcc at nh2 dot me]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24542

--- Comment #15 from Niklas Hambüchen <mail+gcc at nh2 dot me> ---
Another common integer overflow bug type is the "for (u32 i = 0; i < u64; ++i)"
pattern, as well as general widening comparisons.

I filed bug 110933 for those; just linking it here for people interested in
integer overflows.

[Bug c/24542] potential unwanted truncation of operation overflow should be warned on assignment to wider variable

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24542

--- Comment #16 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Niklas Hambüchen from comment #15)
> Another common integer overflow bug type is the "for (u32 i = 0; i < u64;
> ++i)" pattern, as well as general widening comparisons.
> 
> I filed bug 110933 for those; just linking it here for people interested in
> integer overflows.

There is no integer overflow here rather there has been wrapping happening. Yes
there is a huge difference between the two. Wrapping is defined behavior while
overflow is undefined behavior.