[Bug c/104711] New: Unnecessary -Wshift-negative-value warning

[Bug c/104711] New: Unnecessary -Wshift-negative-value warning

[arnd at linaro dot org]

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

            Bug ID: 104711
           Summary: Unnecessary -Wshift-negative-value warning
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: arnd at linaro dot org
  Target Milestone: ---

During the discussion of increasing the C standard version of the Linux kernel
fro m gnu89 to gnu99 or higher, it turned out that gcc warns about code that
shifts negative signed integers [2].

This is undefined behavior in standard C99, but defined as a GNU extension in
GCC.[3]. This warning is enabled by default at the -Wextra level for C99/GNU99
or higher, but disabled for C89/GNU89. In clang, the warning is enabled by
default at the -Wall level but in turn disabled when building with -fwrapv or
-fno-strict-overflow (as the Linux kernel does).

It would be nice if future compiler releases could either demote the warning
from being enabled at -Wextra to -Wpedantic, or follow clang and disable it
when used with -fwrapv/-fno-strict-overflow.

[1] https://lore.kernel.org/lkml/20220227010956.GW614@gate.crashing.org/
[2] https://www.godbolt.org/z/s1TzxrGz4
[3] https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gcc/Integers-implementation.html

[Bug c/104711] Unnecessary -Wshift-negative-value warning

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |documentation

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
However, -fsanitize=shift (and -fsanitize=undefined) will diagnose such cases.
They are also diagnosed where constant expressions are required.

So they are still caught by the undefined sanitizers. Maybe the documentation
should mention the warning too.

[Bug c/104711] Unnecessary -Wshift-negative-value warning

[segher at gcc dot gnu.org]

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

Segher Boessenkool <segher at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-02-27
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #2 from Segher Boessenkool <segher at gcc dot gnu.org> ---
Our documentation says in
<https://gcc.gnu.org/onlinedocs/gcc/Integers-implementation.html>

  As an extension to the C language, GCC does not use the latitude given in C99
  and C11 only to treat certain aspects of signed ‘<<’ as undefined. However,
  -fsanitize=shift (and -fsanitize=undefined) will diagnose such cases. They
are
  also diagnosed where constant expressions are required.

It would be much saner / much more practical if we actually implemented this,
i.e. don't have -Wshift-negative-value in -Wextra (the above text does not make
much sense if that was the design!)

This warning does have a good enough balance between amount of false positives,
detection of serious problems, and usefulness to be included in -Wextra.  The
considerations for -Wall and -W are exactly the same, just the bar is lower for
the latter.

Confirmed.

[Bug c/104711] Unnecessary -Wshift-negative-value warning

[segher at gcc dot gnu.org]

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

--- Comment #3 from Segher Boessenkool <segher at gcc dot gnu.org> ---
... does NOT have a good enough balance ...

Sorry :-)

[Bug c/104711] Unnecessary -Wshift-negative-value warning

[segher at gcc dot gnu.org]

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

--- Comment #4 from Segher Boessenkool <segher at gcc dot gnu.org> ---
Created attachment 52522
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52522&action=edit
testcase

[Bug c/104711] Unnecessary -Wshift-negative-value warning

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
I agree that -fwrapv should make left-shift of positive values when the result
is not representable well-defined but I'm not sure about the negative value
case - the standard does not provide enough reasoning to suggest the
undefinedness is because of actual overflow - in fact the standard allows E1 <<
0 and there's
definitely no overflow for negative E1 in that case.  Supposedly the
standard simply chickened out for non-twos-complement archs here again
and unfortunately didn't leave the door open for implementation-defined
behavior.

I suppose you are asking for an option to turn all left shifts into logical
shifts?

[Bug c/104711] Unnecessary -Wshift-negative-value warning

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |jason at gcc dot gnu.org,
                   |                            |jsm28 at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
>From what I can see, -fsanitize=undefined -fwrapv already treats shifts the
C++20  https://wg21.link/p0907 way, where only the last operand is checked for
being out of bounds (negative or greater or equal to bitsize).
N2731 still contains the old (C11 etc.) shift wording.
Then we have this -Wshift-negative-value warning which doesn't seem to care
about details, complains about left shifts of negative constant always, doesn't
care about -fwrapv and is enabled in -Wextra for C99 and later and C++11 and
later.
I'd say we should not warn if TYPE_OVERFLOW_WRAPS (type) aka -fwrapv and we
shouldn't enable the warning in -Wextra for C++20 or later.

Note, what -fsanitize=undefined actually instruments is beyond the out of
bounds y is for signed x << y:
C99-C2x ((unsigned) x >> (uprecm1 - y)) != 0 then UB
C++11-C++17 x < 0 || ((unsigned) x >> (uprecm1 - y)) > 1 then UB

So, I think we want something like:
--- gcc/doc/invoke.texi.jj      2022-02-25 10:46:53.085181500 +0100
+++ gcc/doc/invoke.texi 2022-03-01 09:59:15.040855224 +0100
@@ -5809,7 +5809,7 @@ name is still supported, but the newer n
 -Wredundant-move @r{(only for C++)}  @gol
 -Wtype-limits  @gol
 -Wuninitialized  @gol
--Wshift-negative-value @r{(in C++03 and in C99 and newer)}  @gol
+-Wshift-negative-value @r{(in C++11 to C++17 and in C99 and newer)}  @gol
 -Wunused-parameter @r{(only with} @option{-Wunused} @r{or} @option{-Wall}@r{)}
@gol
 -Wunused-but-set-parameter @r{(only with} @option{-Wunused} @r{or}
@option{-Wall}@r{)}}

@@ -6839,7 +6839,7 @@ of the type.  This warning is enabled by
 @opindex Wshift-negative-value
 @opindex Wno-shift-negative-value
 Warn if left shifting a negative value.  This warning is enabled by
-@option{-Wextra} in C99 and C++11 modes (and newer).
+@option{-Wextra} in C99 (and newer) and C++11 to C++17 modes.

 @item -Wno-shift-overflow
 @itemx -Wshift-overflow=@var{n}
--- gcc/c-family/c-warn.cc.jj   2022-01-18 11:58:58.922991486 +0100
+++ gcc/c-family/c-warn.cc      2022-03-01 10:02:41.634971050 +0100
@@ -2605,7 +2605,7 @@ maybe_warn_shift_overflow (location_t lo
   unsigned int prec0 = TYPE_PRECISION (type0);

   /* Left-hand operand must be signed.  */
-  if (TYPE_UNSIGNED (type0) || cxx_dialect >= cxx20)
+  if (TYPE_OVERFLOW_WRAPS (type0) || cxx_dialect >= cxx20)
     return false;

   unsigned int min_prec = (wi::min_precision (wi::to_wide (op0), SIGNED)
--- gcc/c-family/c-ubsan.cc.jj  2022-02-09 15:15:59.288840032 +0100
+++ gcc/c-family/c-ubsan.cc     2022-03-01 09:55:51.779693845 +0100
@@ -173,7 +173,7 @@ ubsan_instrument_shift (location_t loc,
       || cxx_dialect >= cxx20)
     ;

-  /* For signed x << y, in C99/C11, the following:
+  /* For signed x << y, in C99 and later, the following:
      (unsigned) x >> (uprecm1 - y)
      if non-zero, is undefined.  */
   else if (code == LSHIFT_EXPR && flag_isoc99 && cxx_dialect < cxx11)
@@ -186,7 +186,7 @@ ubsan_instrument_shift (location_t loc,
                        build_int_cst (TREE_TYPE (tt), 0));
     }

-  /* For signed x << y, in C++11 and later, the following:
+  /* For signed x << y, in C++11 to C++17, the following:
      x < 0 || ((unsigned) x >> (uprecm1 - y))
      if > 1, is undefined.  */
   else if (code == LSHIFT_EXPR && cxx_dialect >= cxx11)
--- gcc/c-family/c-opts.cc.jj   2022-01-18 11:58:58.884992028 +0100
+++ gcc/c-family/c-opts.cc      2022-03-01 09:57:34.880253831 +0100
@@ -934,10 +934,12 @@ c_common_post_options (const char **pfil
   if (warn_shift_overflow == -1)
     warn_shift_overflow = cxx_dialect >= cxx11 || flag_isoc99;

-  /* -Wshift-negative-value is enabled by -Wextra in C99 and C++11 modes.  */
+  /* -Wshift-negative-value is enabled by -Wextra in C99 and C++11 to C++17
+     modes.  */
   if (warn_shift_negative_value == -1)
     warn_shift_negative_value = (extra_warnings
-                                && (cxx_dialect >= cxx11 || flag_isoc99));
+                                && (cxx_dialect >= cxx11 || flag_isoc99)
+                                && cxx_dialect < cxx20);

   /* -Wregister is enabled by default in C++17.  */
   SET_OPTION_IF_UNSET (&global_options, &global_options_set, warn_register,
--- gcc/c/c-typeck.cc.jj        2022-02-11 00:19:22.135067293 +0100
+++ gcc/c/c-typeck.cc   2022-03-01 10:04:20.925584897 +0100
@@ -12213,7 +12213,8 @@ build_binary_op (location_t location, en
        {
          doing_shift = true;
          if (TREE_CODE (op0) == INTEGER_CST
-             && tree_int_cst_sgn (op0) < 0)
+             && tree_int_cst_sgn (op0) < 0
+             && !TYPE_OVERFLOW_WRAPS (type0))
            {
              /* Don't reject a left shift of a negative value in a context
                 where a constant expression is needed in C90.  */
--- gcc/cp/typeck.cc.jj 2022-02-18 12:38:06.065393230 +0100
+++ gcc/cp/typeck.cc    2022-03-01 10:04:57.726071137 +0100
@@ -5382,6 +5382,7 @@ cp_build_binary_op (const op_location_t
          doing_shift = true;
          if (TREE_CODE (const_op0) == INTEGER_CST
              && tree_int_cst_sgn (const_op0) < 0
+             && !TYPE_OVERFLOW_WRAPS (type0)
              && (complain & tf_warning)
              && c_inhibit_evaluation_warnings == 0)
            warning_at (location, OPT_Wshift_negative_value,
plus testsuite adjustments.

[Bug c/104711] Unnecessary -Wshift-negative-value warning

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52536
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52536&action=edit
gcc12-pr104711.patch

Full untested patch.

[Bug c/104711] Unnecessary -Wshift-negative-value warning

[segher at gcc dot gnu.org]

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

--- Comment #8 from Segher Boessenkool <segher at gcc dot gnu.org> ---
Arnd's request was to not have -Wshift-negative-value implied by -W, or at
least not if -fwrapv (-pedantic would be wrong btw, the standard does not
require a diagnostic here, and that is what -pedantic does / is for).

My opinion is it does not belong in -W at all.

X << 0 is undefined behaviour for negative X (in C.  This whole PR is about C).

> I suppose you are asking for an option to turn all left shifts into logical shifts?

The request is to not warn for left shift of a negative number with -W.  It
is fine to do that with -Wshift-negative-value, but that option should not be
implied by -W.

[Bug c/104711] Unnecessary -Wshift-negative-value warning

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:d76511138dc816ef66fd16f71531f48c37dac3b4

commit r12-7557-gd76511138dc816ef66fd16f71531f48c37dac3b4
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Mar 9 09:15:28 2022 +0100

    c, c++, c-family: -Wshift-negative-value and -Wshift-overflow* tweaks for
-fwrapv and C++20+ [PR104711]

    As mentioned in the PR, different standards have different definition
    on what is an UB left shift.  They all agree on out of bounds (including
    negative) shift count.
    The rules used by ubsan are:
    C99-C2x ((unsigned) x >> (uprecm1 - y)) != 0 then UB
    C++11-C++17 x < 0 || ((unsigned) x >> (uprecm1 - y)) > 1 then UB
    C++20 and later everything is well defined
    Now, for C++20, I've in the P1236R1 implementation added an early
    exit for -Wshift-overflow* warning so that it never warns, but apparently
    -Wshift-negative-value remained as is.  As it is well defined in C++20,
    the following patch doesn't enable -Wshift-negative-value from -Wextra
    anymore for C++20 and later, if users want for compatibility with C++17
    and earlier get the warning, they still can by using -Wshift-negative-value
    explicitly.
    Another thing is -fwrapv, that is an extension to the standards, so it is
up
    to us how exactly we define that case.  Our ubsan code treats
    TYPE_OVERFLOW_WRAPS (type0) and cxx_dialect >= cxx20 the same as only
    diagnosing out of bounds shift count and nothing else and IMHO it is most
    sensical to treat -fwrapv signed left shifts the same as C++20 treats
    them, https://eel.is/c++draft/expr.shift#2
    "The value of E1 << E2 is the unique value congruent to E1×2^E2 modulo 2^N,
    where N is the width of the type of the result.
    [Note 1: E1 is left-shifted E2 bit positions; vacated bits are zero-filled.
    â end note]"
    with no UB dependent on the E1 values.  The UB is only
    "The behavior is undefined if the right operand is negative, or greater
    than or equal to the width of the promoted left operand."
    Under the hood (except for FEs and ubsan from FEs) GCC middle-end doesn't
    consider UB in left shifts dependent on the first operand's value, only
    the out of bounds shifts.

    While this change isn't a regression, I'd think it is useful for GCC 12,
    it doesn't add new warnings, but just removes warnings that aren't
    appropriate.

    2022-03-09  Jakub Jelinek  <jakub@redhat.com>

            PR c/104711
    gcc/
            * doc/invoke.texi (-Wextra): Document that -Wshift-negative-value
            is enabled by it only for C++11 to C++17 rather than for C++03 or
            later.
            (-Wshift-negative-value): Similarly (except here we stated
            that it is enabled for C++11 or later).
    gcc/c-family/
            * c-opts.cc (c_common_post_options): Don't enable
            -Wshift-negative-value from -Wextra for C++20 or later.
            * c-ubsan.cc (ubsan_instrument_shift): Adjust comments.
            * c-warn.cc (maybe_warn_shift_overflow): Use TYPE_OVERFLOW_WRAPS
            instead of TYPE_UNSIGNED.
    gcc/c/
            * c-fold.cc (c_fully_fold_internal): Don't emit
            -Wshift-negative-value warning if TYPE_OVERFLOW_WRAPS.
            * c-typeck.cc (build_binary_op): Likewise.
    gcc/cp/
            * constexpr.cc (cxx_eval_check_shift_p): Use TYPE_OVERFLOW_WRAPS
            instead of TYPE_UNSIGNED.
            * typeck.cc (cp_build_binary_op): Don't emit
            -Wshift-negative-value warning if TYPE_OVERFLOW_WRAPS.
    gcc/testsuite/
            * c-c++-common/Wshift-negative-value-1.c: Remove
            dg-additional-options, instead in target selectors of each
diagnostic
            check for exact C++ versions where it should be diagnosed.
            * c-c++-common/Wshift-negative-value-2.c: Likewise.
            * c-c++-common/Wshift-negative-value-3.c: Likewise.
            * c-c++-common/Wshift-negative-value-4.c: Likewise.
            * c-c++-common/Wshift-negative-value-7.c: New test.
            * c-c++-common/Wshift-negative-value-8.c: New test.
            * c-c++-common/Wshift-negative-value-9.c: New test.
            * c-c++-common/Wshift-negative-value-10.c: New test.
            * c-c++-common/Wshift-overflow-1.c: Remove
            dg-additional-options, instead in target selectors of each
diagnostic
            check for exact C++ versions where it should be diagnosed.
            * c-c++-common/Wshift-overflow-2.c: Likewise.
            * c-c++-common/Wshift-overflow-5.c: Likewise.
            * c-c++-common/Wshift-overflow-6.c: Likewise.
            * c-c++-common/Wshift-overflow-7.c: Likewise.
            * c-c++-common/Wshift-overflow-8.c: New test.
            * c-c++-common/Wshift-overflow-9.c: New test.
            * c-c++-common/Wshift-overflow-10.c: New test.
            * c-c++-common/Wshift-overflow-11.c: New test.
            * c-c++-common/Wshift-overflow-12.c: New test.

[Bug c/104711] Unnecessary -Wshift-negative-value warning

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:ddc0d2593fb4d2eb432e24018d36dd3f337a8138

commit r11-9726-gddc0d2593fb4d2eb432e24018d36dd3f337a8138
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Mar 9 09:15:28 2022 +0100

    c, c++, c-family: -Wshift-negative-value and -Wshift-overflow* tweaks for
-fwrapv and C++20+ [PR104711]

    As mentioned in the PR, different standards have different definition
    on what is an UB left shift.  They all agree on out of bounds (including
    negative) shift count.
    The rules used by ubsan are:
    C99-C2x ((unsigned) x >> (uprecm1 - y)) != 0 then UB
    C++11-C++17 x < 0 || ((unsigned) x >> (uprecm1 - y)) > 1 then UB
    C++20 and later everything is well defined
    Now, for C++20, I've in the P1236R1 implementation added an early
    exit for -Wshift-overflow* warning so that it never warns, but apparently
    -Wshift-negative-value remained as is.  As it is well defined in C++20,
    the following patch doesn't enable -Wshift-negative-value from -Wextra
    anymore for C++20 and later, if users want for compatibility with C++17
    and earlier get the warning, they still can by using -Wshift-negative-value
    explicitly.
    Another thing is -fwrapv, that is an extension to the standards, so it is
up
    to us how exactly we define that case.  Our ubsan code treats
    TYPE_OVERFLOW_WRAPS (type0) and cxx_dialect >= cxx20 the same as only
    diagnosing out of bounds shift count and nothing else and IMHO it is most
    sensical to treat -fwrapv signed left shifts the same as C++20 treats
    them, https://eel.is/c++draft/expr.shift#2
    "The value of E1 << E2 is the unique value congruent to E1×2^E2 modulo 2^N,
    where N is the width of the type of the result.
    [Note 1: E1 is left-shifted E2 bit positions; vacated bits are zero-filled.
    â end note]"
    with no UB dependent on the E1 values.  The UB is only
    "The behavior is undefined if the right operand is negative, or greater
    than or equal to the width of the promoted left operand."
    Under the hood (except for FEs and ubsan from FEs) GCC middle-end doesn't
    consider UB in left shifts dependent on the first operand's value, only
    the out of bounds shifts.

    While this change isn't a regression, I'd think it is useful for GCC 12,
    it doesn't add new warnings, but just removes warnings that aren't
    appropriate.

    2022-03-09  Jakub Jelinek  <jakub@redhat.com>

            PR c/104711
    gcc/
            * doc/invoke.texi (-Wextra): Document that -Wshift-negative-value
            is enabled by it only for C++11 to C++17 rather than for C++03 or
            later.
            (-Wshift-negative-value): Similarly (except here we stated
            that it is enabled for C++11 or later).
    gcc/c-family/
            * c-opts.c (c_common_post_options): Don't enable
            -Wshift-negative-value from -Wextra for C++20 or later.
            * c-ubsan.c (ubsan_instrument_shift): Adjust comments.
            * c-warn.c (maybe_warn_shift_overflow): Use TYPE_OVERFLOW_WRAPS
            instead of TYPE_UNSIGNED.
    gcc/c/
            * c-fold.c (c_fully_fold_internal): Don't emit
            -Wshift-negative-value warning if TYPE_OVERFLOW_WRAPS.
            * c-typeck.c (build_binary_op): Likewise.
    gcc/cp/
            * constexpr.c (cxx_eval_check_shift_p): Use TYPE_OVERFLOW_WRAPS
            instead of TYPE_UNSIGNED.
            * typeck.c (cp_build_binary_op): Don't emit
            -Wshift-negative-value warning if TYPE_OVERFLOW_WRAPS.
    gcc/testsuite/
            * c-c++-common/Wshift-negative-value-1.c: Remove
            dg-additional-options, instead in target selectors of each
diagnostic
            check for exact C++ versions where it should be diagnosed.
            * c-c++-common/Wshift-negative-value-2.c: Likewise.
            * c-c++-common/Wshift-negative-value-3.c: Likewise.
            * c-c++-common/Wshift-negative-value-4.c: Likewise.
            * c-c++-common/Wshift-negative-value-7.c: New test.
            * c-c++-common/Wshift-negative-value-8.c: New test.
            * c-c++-common/Wshift-negative-value-9.c: New test.
            * c-c++-common/Wshift-negative-value-10.c: New test.
            * c-c++-common/Wshift-overflow-1.c: Remove
            dg-additional-options, instead in target selectors of each
diagnostic
            check for exact C++ versions where it should be diagnosed.
            * c-c++-common/Wshift-overflow-2.c: Likewise.
            * c-c++-common/Wshift-overflow-5.c: Likewise.
            * c-c++-common/Wshift-overflow-6.c: Likewise.
            * c-c++-common/Wshift-overflow-7.c: Likewise.
            * c-c++-common/Wshift-overflow-8.c: New test.
            * c-c++-common/Wshift-overflow-9.c: New test.
            * c-c++-common/Wshift-overflow-10.c: New test.
            * c-c++-common/Wshift-overflow-11.c: New test.
            * c-c++-common/Wshift-overflow-12.c: New test.

    (cherry picked from commit d76511138dc816ef66fd16f71531f48c37dac3b4)

[Bug c/104711] Unnecessary -Wshift-negative-value warning

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:7a4db01ba8171e5f7b82cb5a36e0a6cbb6e996a0

commit r10-10693-g7a4db01ba8171e5f7b82cb5a36e0a6cbb6e996a0
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Mar 9 09:15:28 2022 +0100

    c, c++, c-family: -Wshift-negative-value and -Wshift-overflow* tweaks for
-fwrapv and C++20+ [PR104711]

    As mentioned in the PR, different standards have different definition
    on what is an UB left shift.  They all agree on out of bounds (including
    negative) shift count.
    The rules used by ubsan are:
    C99-C2x ((unsigned) x >> (uprecm1 - y)) != 0 then UB
    C++11-C++17 x < 0 || ((unsigned) x >> (uprecm1 - y)) > 1 then UB
    C++20 and later everything is well defined
    Now, for C++20, I've in the P1236R1 implementation added an early
    exit for -Wshift-overflow* warning so that it never warns, but apparently
    -Wshift-negative-value remained as is.  As it is well defined in C++20,
    the following patch doesn't enable -Wshift-negative-value from -Wextra
    anymore for C++20 and later, if users want for compatibility with C++17
    and earlier get the warning, they still can by using -Wshift-negative-value
    explicitly.
    Another thing is -fwrapv, that is an extension to the standards, so it is
up
    to us how exactly we define that case.  Our ubsan code treats
    TYPE_OVERFLOW_WRAPS (type0) and cxx_dialect >= cxx20 the same as only
    diagnosing out of bounds shift count and nothing else and IMHO it is most
    sensical to treat -fwrapv signed left shifts the same as C++20 treats
    them, https://eel.is/c++draft/expr.shift#2
    "The value of E1 << E2 is the unique value congruent to E1×2^E2 modulo 2^N,
    where N is the width of the type of the result.
    [Note 1: E1 is left-shifted E2 bit positions; vacated bits are zero-filled.
    â end note]"
    with no UB dependent on the E1 values.  The UB is only
    "The behavior is undefined if the right operand is negative, or greater
    than or equal to the width of the promoted left operand."
    Under the hood (except for FEs and ubsan from FEs) GCC middle-end doesn't
    consider UB in left shifts dependent on the first operand's value, only
    the out of bounds shifts.

    While this change isn't a regression, I'd think it is useful for GCC 12,
    it doesn't add new warnings, but just removes warnings that aren't
    appropriate.

    2022-03-09  Jakub Jelinek  <jakub@redhat.com>

            PR c/104711
    gcc/
            * doc/invoke.texi (-Wextra): Document that -Wshift-negative-value
            is enabled by it only for C++11 to C++17 rather than for C++03 or
            later.
            (-Wshift-negative-value): Similarly (except here we stated
            that it is enabled for C++11 or later).
    gcc/c-family/
            * c-opts.c (c_common_post_options): Don't enable
            -Wshift-negative-value from -Wextra for C++20 or later.
            * c-ubsan.c (ubsan_instrument_shift): Adjust comments.
            * c-warn.c (maybe_warn_shift_overflow): Use TYPE_OVERFLOW_WRAPS
            instead of TYPE_UNSIGNED.
    gcc/c/
            * c-fold.c (c_fully_fold_internal): Don't emit
            -Wshift-negative-value warning if TYPE_OVERFLOW_WRAPS.
            * c-typeck.c (build_binary_op): Likewise.
    gcc/cp/
            * constexpr.c (cxx_eval_check_shift_p): Use TYPE_OVERFLOW_WRAPS
            instead of TYPE_UNSIGNED.
            * typeck.c (cp_build_binary_op): Don't emit
            -Wshift-negative-value warning if TYPE_OVERFLOW_WRAPS.
    gcc/testsuite/
            * c-c++-common/Wshift-negative-value-1.c: Remove
            dg-additional-options, instead in target selectors of each
diagnostic
            check for exact C++ versions where it should be diagnosed.
            * c-c++-common/Wshift-negative-value-2.c: Likewise.
            * c-c++-common/Wshift-negative-value-3.c: Likewise.
            * c-c++-common/Wshift-negative-value-4.c: Likewise.
            * c-c++-common/Wshift-negative-value-7.c: New test.
            * c-c++-common/Wshift-negative-value-8.c: New test.
            * c-c++-common/Wshift-negative-value-9.c: New test.
            * c-c++-common/Wshift-negative-value-10.c: New test.
            * c-c++-common/Wshift-overflow-1.c: Remove
            dg-additional-options, instead in target selectors of each
diagnostic
            check for exact C++ versions where it should be diagnosed.
            * c-c++-common/Wshift-overflow-2.c: Likewise.
            * c-c++-common/Wshift-overflow-5.c: Likewise.
            * c-c++-common/Wshift-overflow-6.c: Likewise.
            * c-c++-common/Wshift-overflow-7.c: Likewise.
            * c-c++-common/Wshift-overflow-8.c: New test.
            * c-c++-common/Wshift-overflow-9.c: New test.
            * c-c++-common/Wshift-overflow-10.c: New test.
            * c-c++-common/Wshift-overflow-11.c: New test.
            * c-c++-common/Wshift-overflow-12.c: New test.

    (cherry picked from commit d76511138dc816ef66fd16f71531f48c37dac3b4)

[Bug c/104711] Unnecessary -Wshift-negative-value warning

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:0e02b8468be6b655c43c6d64fef7724444678681

commit r9-10139-g0e02b8468be6b655c43c6d64fef7724444678681
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Mar 9 09:15:28 2022 +0100

    c, c++, c-family: -Wshift-negative-value and -Wshift-overflow* tweaks for
-fwrapv and C++20+ [PR104711]

    As mentioned in the PR, different standards have different definition
    on what is an UB left shift.  They all agree on out of bounds (including
    negative) shift count.
    The rules used by ubsan are:
    C99-C2x ((unsigned) x >> (uprecm1 - y)) != 0 then UB
    C++11-C++17 x < 0 || ((unsigned) x >> (uprecm1 - y)) > 1 then UB
    C++20 and later everything is well defined
    Now, for C++20, I've in the P1236R1 implementation added an early
    exit for -Wshift-overflow* warning so that it never warns, but apparently
    -Wshift-negative-value remained as is.  As it is well defined in C++20,
    the following patch doesn't enable -Wshift-negative-value from -Wextra
    anymore for C++20 and later, if users want for compatibility with C++17
    and earlier get the warning, they still can by using -Wshift-negative-value
    explicitly.
    Another thing is -fwrapv, that is an extension to the standards, so it is
up
    to us how exactly we define that case.  Our ubsan code treats
    TYPE_OVERFLOW_WRAPS (type0) and cxx_dialect >= cxx20 the same as only
    diagnosing out of bounds shift count and nothing else and IMHO it is most
    sensical to treat -fwrapv signed left shifts the same as C++20 treats
    them, https://eel.is/c++draft/expr.shift#2
    "The value of E1 << E2 is the unique value congruent to E1×2^E2 modulo 2^N,
    where N is the width of the type of the result.
    [Note 1: E1 is left-shifted E2 bit positions; vacated bits are zero-filled.
    â end note]"
    with no UB dependent on the E1 values.  The UB is only
    "The behavior is undefined if the right operand is negative, or greater
    than or equal to the width of the promoted left operand."
    Under the hood (except for FEs and ubsan from FEs) GCC middle-end doesn't
    consider UB in left shifts dependent on the first operand's value, only
    the out of bounds shifts.

    While this change isn't a regression, I'd think it is useful for GCC 12,
    it doesn't add new warnings, but just removes warnings that aren't
    appropriate.

    2022-03-09  Jakub Jelinek  <jakub@redhat.com>

            PR c/104711
    gcc/
            * doc/invoke.texi (-Wextra): Document that -Wshift-negative-value
            is enabled by it only for C++11 to C++17 rather than for C++03 or
            later.
            (-Wshift-negative-value): Similarly (except here we stated
            that it is enabled for C++11 or later).
    gcc/c-family/
            * c-opts.c (c_common_post_options): Don't enable
            -Wshift-negative-value from -Wextra for C++20 or later.
            * c-ubsan.c (ubsan_instrument_shift): Adjust comments.
            * c-warn.c (maybe_warn_shift_overflow): Use TYPE_OVERFLOW_WRAPS
            instead of TYPE_UNSIGNED.
    gcc/c/
            * c-fold.c (c_fully_fold_internal): Don't emit
            -Wshift-negative-value warning if TYPE_OVERFLOW_WRAPS.
            * c-typeck.c (build_binary_op): Likewise.
    gcc/cp/
            * constexpr.c (cxx_eval_check_shift_p): Use TYPE_OVERFLOW_WRAPS
            instead of TYPE_UNSIGNED.
            * typeck.c (cp_build_binary_op): Don't emit
            -Wshift-negative-value warning if TYPE_OVERFLOW_WRAPS.
    gcc/testsuite/
            * c-c++-common/Wshift-negative-value-1.c: Remove
            dg-additional-options, instead in target selectors of each
diagnostic
            check for exact C++ versions where it should be diagnosed.
            * c-c++-common/Wshift-negative-value-2.c: Likewise.
            * c-c++-common/Wshift-negative-value-3.c: Likewise.
            * c-c++-common/Wshift-negative-value-4.c: Likewise.
            * c-c++-common/Wshift-negative-value-7.c: New test.
            * c-c++-common/Wshift-negative-value-8.c: New test.
            * c-c++-common/Wshift-negative-value-9.c: New test.
            * c-c++-common/Wshift-negative-value-10.c: New test.
            * c-c++-common/Wshift-overflow-1.c: Remove
            dg-additional-options, instead in target selectors of each
diagnostic
            check for exact C++ versions where it should be diagnosed.
            * c-c++-common/Wshift-overflow-2.c: Likewise.
            * c-c++-common/Wshift-overflow-5.c: Likewise.
            * c-c++-common/Wshift-overflow-6.c: Likewise.
            * c-c++-common/Wshift-overflow-7.c: Likewise.
            * c-c++-common/Wshift-overflow-8.c: New test.
            * c-c++-common/Wshift-overflow-9.c: New test.
            * c-c++-common/Wshift-overflow-10.c: New test.
            * c-c++-common/Wshift-overflow-11.c: New test.
            * c-c++-common/Wshift-overflow-12.c: New test.

    (cherry picked from commit d76511138dc816ef66fd16f71531f48c37dac3b4)

[Bug c/104711] Unnecessary -Wshift-negative-value warning

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.