* [Bug c/104711] Unnecessary -Wshift-negative-value warning
2022-02-27 21:27 [Bug c/104711] New: Unnecessary -Wshift-negative-value warning arnd at linaro dot org
@ 2022-02-27 21:40 ` pinskia at gcc dot gnu.org
2022-02-27 22:37 ` segher at gcc dot gnu.org
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-02-27 21:40 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug c/104711] Unnecessary -Wshift-negative-value warning
2022-02-27 21:27 [Bug c/104711] New: Unnecessary -Wshift-negative-value warning arnd at linaro dot org
2022-02-27 21:40 ` [Bug c/104711] " pinskia at gcc dot gnu.org
@ 2022-02-27 22:37 ` segher at gcc dot gnu.org
2022-02-27 22:38 ` segher at gcc dot gnu.org
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: segher at gcc dot gnu.org @ 2022-02-27 22:37 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug c/104711] Unnecessary -Wshift-negative-value warning
2022-02-27 21:27 [Bug c/104711] New: Unnecessary -Wshift-negative-value warning arnd at linaro dot org
2022-02-27 21:40 ` [Bug c/104711] " pinskia at gcc dot gnu.org
2022-02-27 22:37 ` segher at gcc dot gnu.org
@ 2022-02-27 22:38 ` segher at gcc dot gnu.org
2022-02-27 22:42 ` segher at gcc dot gnu.org
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: segher at gcc dot gnu.org @ 2022-02-27 22:38 UTC (permalink / raw)
To: gcc-bugs
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 :-)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug c/104711] Unnecessary -Wshift-negative-value warning
2022-02-27 21:27 [Bug c/104711] New: Unnecessary -Wshift-negative-value warning arnd at linaro dot org
` (2 preceding siblings ...)
2022-02-27 22:38 ` segher at gcc dot gnu.org
@ 2022-02-27 22:42 ` segher at gcc dot gnu.org
2022-03-01 7:58 ` rguenth at gcc dot gnu.org
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: segher at gcc dot gnu.org @ 2022-02-27 22:42 UTC (permalink / raw)
To: gcc-bugs
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug c/104711] Unnecessary -Wshift-negative-value warning
2022-02-27 21:27 [Bug c/104711] New: Unnecessary -Wshift-negative-value warning arnd at linaro dot org
` (3 preceding siblings ...)
2022-02-27 22:42 ` segher at gcc dot gnu.org
@ 2022-03-01 7:58 ` rguenth at gcc dot gnu.org
2022-03-01 9:07 ` jakub at gcc dot gnu.org
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-03-01 7:58 UTC (permalink / raw)
To: gcc-bugs
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?
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug c/104711] Unnecessary -Wshift-negative-value warning
2022-02-27 21:27 [Bug c/104711] New: Unnecessary -Wshift-negative-value warning arnd at linaro dot org
` (4 preceding siblings ...)
2022-03-01 7:58 ` rguenth at gcc dot gnu.org
@ 2022-03-01 9:07 ` jakub at gcc dot gnu.org
2022-03-01 14:11 ` jakub at gcc dot gnu.org
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-01 9:07 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug c/104711] Unnecessary -Wshift-negative-value warning
2022-02-27 21:27 [Bug c/104711] New: Unnecessary -Wshift-negative-value warning arnd at linaro dot org
` (5 preceding siblings ...)
2022-03-01 9:07 ` jakub at gcc dot gnu.org
@ 2022-03-01 14:11 ` jakub at gcc dot gnu.org
2022-03-01 17:51 ` segher at gcc dot gnu.org
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-01 14:11 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug c/104711] Unnecessary -Wshift-negative-value warning
2022-02-27 21:27 [Bug c/104711] New: Unnecessary -Wshift-negative-value warning arnd at linaro dot org
` (6 preceding siblings ...)
2022-03-01 14:11 ` jakub at gcc dot gnu.org
@ 2022-03-01 17:51 ` segher at gcc dot gnu.org
2022-03-09 8:16 ` cvs-commit at gcc dot gnu.org
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: segher at gcc dot gnu.org @ 2022-03-01 17:51 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug c/104711] Unnecessary -Wshift-negative-value warning
2022-02-27 21:27 [Bug c/104711] New: Unnecessary -Wshift-negative-value warning arnd at linaro dot org
` (7 preceding siblings ...)
2022-03-01 17:51 ` segher at gcc dot gnu.org
@ 2022-03-09 8:16 ` cvs-commit at gcc dot gnu.org
2022-03-29 5:53 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-03-09 8:16 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug c/104711] Unnecessary -Wshift-negative-value warning
2022-02-27 21:27 [Bug c/104711] New: Unnecessary -Wshift-negative-value warning arnd at linaro dot org
` (8 preceding siblings ...)
2022-03-09 8:16 ` cvs-commit at gcc dot gnu.org
@ 2022-03-29 5:53 ` cvs-commit at gcc dot gnu.org
2022-05-10 8:25 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-03-29 5:53 UTC (permalink / raw)
To: gcc-bugs
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)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug c/104711] Unnecessary -Wshift-negative-value warning
2022-02-27 21:27 [Bug c/104711] New: Unnecessary -Wshift-negative-value warning arnd at linaro dot org
` (9 preceding siblings ...)
2022-03-29 5:53 ` cvs-commit at gcc dot gnu.org
@ 2022-05-10 8:25 ` cvs-commit at gcc dot gnu.org
2022-05-11 6:25 ` cvs-commit at gcc dot gnu.org
2022-05-11 6:36 ` jakub at gcc dot gnu.org
12 siblings, 0 replies; 14+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-05-10 8:25 UTC (permalink / raw)
To: gcc-bugs
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)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug c/104711] Unnecessary -Wshift-negative-value warning
2022-02-27 21:27 [Bug c/104711] New: Unnecessary -Wshift-negative-value warning arnd at linaro dot org
` (10 preceding siblings ...)
2022-05-10 8:25 ` cvs-commit at gcc dot gnu.org
@ 2022-05-11 6:25 ` cvs-commit at gcc dot gnu.org
2022-05-11 6:36 ` jakub at gcc dot gnu.org
12 siblings, 0 replies; 14+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-05-11 6:25 UTC (permalink / raw)
To: gcc-bugs
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)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug c/104711] Unnecessary -Wshift-negative-value warning
2022-02-27 21:27 [Bug c/104711] New: Unnecessary -Wshift-negative-value warning arnd at linaro dot org
` (11 preceding siblings ...)
2022-05-11 6:25 ` cvs-commit at gcc dot gnu.org
@ 2022-05-11 6:36 ` jakub at gcc dot gnu.org
12 siblings, 0 replies; 14+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-05-11 6:36 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 14+ messages in thread