[Bug c/101645] New: -Wsign-conversion misses negation of unsigned int

[Bug c/101645] New: -Wsign-conversion misses negation of unsigned int

[matthew at wil dot cx]

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

            Bug ID: 101645
           Summary: -Wsign-conversion misses negation of unsigned int
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: matthew at wil dot cx
  Target Milestone: ---

Test case:

unsigned long a(void);
void b(long);
void c(void) {
    unsigned int x = a();
    b(-x);
}

There is a missed warning in the call to b().  x is negated, but as an int.  It
is then passed to b() which sees a very large positive number instead of a
small negative number.  This has recently affected the Linux codebase, and it's
disappointing that gcc doesn't have a warning that we could enable to find it.

https://godbolt.org/z/xvaxh1rqr

shows that Clang does spot this with -Wsign-conversion, but GCC currently
doesn't (it only warns for line 4 and not line 5).  Admittedly the Clang
diagnostic message isn't the greatest, but at least it says _something_.

[Bug c/101645] -Wsign-conversion misses negation of unsigned int

[jrtc27 at jrtc27 dot com]

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

Jessica Clarke <jrtc27 at jrtc27 dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jrtc27 at jrtc27 dot com

--- Comment #1 from Jessica Clarke <jrtc27 at jrtc27 dot com> ---
Correction: "x is negated but as an *unsigned* int", which is key here

[Bug c/101645] warn about neg of unsigned type should be added to -Wsign-conversion

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|-Wsign-conversion misses    |warn about neg of unsigned
                   |negation of unsigned int    |type should be added to
                   |                            |-Wsign-conversion
           Keywords|                            |diagnostic
           Severity|normal                      |enhancement

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I don't know if this is useful to even to add to -Wsign-conversion.
The documentation is clear here:
Warn for implicit conversions that may change the sign of an integer value,
like assigning a signed integer expression to an unsigned integer variable. 

- is just two comp not rather than neg :)

[Bug c/101645] warn about neg of unsigned type should be added to -Wsign-conversion

[jhubbard at nvidia dot com]

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

John Hubbard <jhubbard at nvidia dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jhubbard at nvidia dot com

--- Comment #3 from John Hubbard <jhubbard at nvidia dot com> ---
Here are some data points in favor of adding this warning:

1) It's already desirable in a real project (Linux kernel, as Matthew pointed
out), and

2) Clang has already gone there, presumably for good reasons as well.

3) Anecdote: as an experienced C programmer, I can tell you that the "b(-x)"
results are rarely what a programmer would want, nor intend. (Of course, the
ideal is a perfect type-safe match between input and args, but that doesn't
always happen in real code.)

The compiler knows enough to significantly help here, by warning. It's also
possible to update the -Wsign-conversion documentation to match, if necessary,
let's not forget that.

[Bug c/101645] warn about neg of unsigned type should be added to -Wsign-conversion

[matthew at wil dot cx]

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

--- Comment #4 from Matthew Wilcox <matthew at wil dot cx> ---
On second thoughts -Wsign-conversion is useless.  Consider that it warns on
this:

unsigned long f(unsigned long x, int y) {
    return x + y;
}

Both gcc and clang emit a warning, which makes it useless.  That code obviously
does what the programmer intended.

What would be useful is a warning which diagnoses code which behaves
differently in infinite-precision arithmetic vs the C promotion rules.

For example,

unsigned long g(unsigned char v, int s) {
    return v << s;
}

By the C standard, v is promoted to int, shifted by s and then promoted to
unsigned long.  This is a fertile source of bugs as it's easy to overlook that
v will not be promoted to unsigned long first.

(by the way, this is another example where clang diagnoses with -Wsign-compare
and gcc doesn't.)

Another example plucked from real life:

long long h(unsigned int x) {
    return x * 4096;
}

(slightly changed to demonstrate the problem on LP64; the original was from
unsigned long to long long, and it is only a bug on ILP32)

[Bug c/101645] warn about neg of unsigned type should be added to -Wsign-conversion

[redi at gcc dot gnu.org]

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

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(I wrote this before comment 4 was added ...)

What would the precise semantics of the suggested warning be? What kind of
expressions do you expect to warn, and which should not?

How would I suppress the warning if -x is exactly what I intended to write and
it does exactly what I expect? For example, the implementation of std::align
uses:

inline void*
align(size_t __align, size_t __size, void*& __ptr, size_t& __space) noexcept
{
  if (__space < __size)
    return nullptr;
  const auto __intptr = reinterpret_cast<uintptr_t>(__ptr);
  const auto __aligned = (__intptr - 1u + __align) & -__align;
  // ...

How would I prevent -__align from warning?