OK, thanks.
On Wed, Nov 14, 2018 at 7:12 AM Jakub Jelinek <ja...@redhat.com> wrote:
>
> Hi!
>
> This paper from what I can see mostly just codifies our
> implementation-defined behavior as standard (two's complement as the only
> possible representation of signed integers, but still keeping UB signed
> integer overflows), the only changes I found that IMHO need changing
> in GCC are that left shifts of signed integers are now always well defined
> (well, like for unsigned shifts out of bounds shift count is still UB),
> so we need to accept such shifts in constant expressions, don't warn about
> those and don't sanitize it in ubsan when in -std=c++2a or -std=gnu++2a
> modes.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2018-11-14 Jakub Jelinek <ja...@redhat.com>
>
> P1236R1 - Signed integers are two's complement
> gcc/cp/
> * constexpr.c (cxx_eval_check_shift_p): Disable the signed LSHIFT_EXPR
> checks for c++2a.
> gcc/c-family/
> * c-warn.c (maybe_warn_shift_overflow): Don't warn for c++2a.
> * c-ubsan.c (ubsan_instrument_shift): Make signed shifts
> with in-range second operand well defined for -std=c++2a.
> gcc/
> * doc/invoke.texi (Wshift-overflow): Adjust documentation for
> c++2a.
> gcc/testsuite/
> * g++.dg/cpp2a/constexpr-shift1.C: New test.
> * g++.dg/warn/permissive-1.C (enum A, enum D): Don't expect
> diagnostics here for c++2a.
> * g++.dg/cpp0x/constexpr-shift1.C (fn3, i3, fn4, i4): Don't expect
> diagnostics here for c++2a.
> * g++.dg/cpp0x/constexpr-60049.C (f3, x3, y3): Likewise.
> * g++.dg/ubsan/cxx11-shift-1.C (main): Add some further tests.
> * g++.dg/ubsan/cxx11-shift-2.C (main): Likewise.
> * g++.dg/ubsan/cxx2a-shift-1.C: New test.
> * g++.dg/ubsan/cxx2a-shift-2.C: New test.
>
> --- gcc/cp/constexpr.c.jj 2018-11-08 18:07:56.071075133 +0100
> +++ gcc/cp/constexpr.c 2018-11-14 08:56:23.705641740 +0100
> @@ -1920,9 +1920,14 @@ cxx_eval_check_shift_p (location_t loc,
> if E1 has a signed type and non-negative value, and E1x2^E2 is
> representable in the corresponding unsigned type of the result type,
> then that value, converted to the result type, is the resulting value;
> - otherwise, the behavior is undefined. */
> - if (code == LSHIFT_EXPR && !TYPE_UNSIGNED (lhstype)
> - && (cxx_dialect >= cxx11))
> + otherwise, the behavior is undefined.
> + For C++2a:
> + The value of E1 << E2 is the unique value congruent to E1 x 2^E2 modulo
> + 2^N, where N is the range exponent of the type of the result. */
> + if (code == LSHIFT_EXPR
> + && !TYPE_UNSIGNED (lhstype)
> + && cxx_dialect >= cxx11
> + && cxx_dialect < cxx2a)
> {
> if (tree_int_cst_sgn (lhs) == -1)
> {
> --- gcc/c-family/c-warn.c.jj 2018-08-27 17:50:41.649525026 +0200
> +++ gcc/c-family/c-warn.c 2018-11-14 08:59:02.424022594 +0100
> @@ -2286,6 +2286,8 @@ diagnose_mismatched_attributes (tree old
> /* Warn if signed left shift overflows. We don't warn
> about left-shifting 1 into the sign bit in C++14; cf.
> <http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3367.html#1457>
> + and don't warn for C++2a at all, as signed left shifts never
> + overflow.
> LOC is a location of the shift; OP0 and OP1 are the operands.
> Return true if an overflow is detected, false otherwise. */
>
> @@ -2300,7 +2302,7 @@ maybe_warn_shift_overflow (location_t lo
> unsigned int prec0 = TYPE_PRECISION (type0);
>
> /* Left-hand operand must be signed. */
> - if (TYPE_UNSIGNED (type0))
> + if (TYPE_UNSIGNED (type0) || cxx_dialect >= cxx2a)
> return false;
>
> unsigned int min_prec = (wi::min_precision (wi::to_wide (op0), SIGNED)
> @@ -2309,7 +2311,7 @@ maybe_warn_shift_overflow (location_t lo
> * However, shifting 1 _out_ of the sign bit, as in
> * INT_MIN << 1, is considered an overflow.
> */
> - if (!tree_int_cst_sign_bit(op0) && min_prec == prec0 + 1)
> + if (!tree_int_cst_sign_bit (op0) && min_prec == prec0 + 1)
> {
> /* Never warn for C++14 onwards. */
> if (cxx_dialect >= cxx14)
> --- gcc/c-family/c-ubsan.c.jj 2018-11-14 00:55:32.351645152 +0100
> +++ gcc/c-family/c-ubsan.c 2018-11-14 08:35:13.131600599 +0100
> @@ -134,7 +134,10 @@ ubsan_instrument_shift (location_t loc,
> if (TYPE_OVERFLOW_WRAPS (type0)
> || maybe_ne (GET_MODE_BITSIZE (TYPE_MODE (type0)),
> TYPE_PRECISION (type0))
> - || !sanitize_flags_p (SANITIZE_SHIFT_BASE))
> + || !sanitize_flags_p (SANITIZE_SHIFT_BASE)
> + /* In C++2a and later, shifts are well defined except when
> + the second operand is not within bounds. */
> + || cxx_dialect >= cxx2a)
> ;
>
> /* For signed x << y, in C99/C11, the following:
> --- gcc/doc/invoke.texi.jj 2018-11-14 00:55:36.636576201 +0100
> +++ gcc/doc/invoke.texi 2018-11-14 13:05:54.055418228 +0100
> @@ -5074,11 +5074,12 @@ This is the warning level of @option{-Ws
> by default in C99 and C++11 modes (and newer). This warning level does
> not warn about left-shifting 1 into the sign bit. (However, in C, such
> an overflow is still rejected in contexts where an integer constant
> expression
> -is required.)
> +is required.) No warning is emitted in C++2A mode (and newer), as signed
> left
> +shifts always wrap.
>
> @item -Wshift-overflow=2
> This warning level also warns about left-shifting 1 into the sign bit,
> -unless C++14 mode is active.
> +unless C++14 mode (or newer) is active.
> @end table
>
> @item -Wswitch
> --- gcc/testsuite/g++.dg/cpp2a/constexpr-shift1.C.jj 2018-11-14
> 09:11:57.355234640 +0100
> +++ gcc/testsuite/g++.dg/cpp2a/constexpr-shift1.C 2018-11-14
> 09:10:58.382207816 +0100
> @@ -0,0 +1,25 @@
> +// { dg-do compile { target c++11 } }
> +
> +constexpr int a = -42 << 0; // { dg-error "left operand of shift
> expression '\\(-42 << 0\\)' is negative" "" { target c++17_down } }
> +constexpr int b = -42 << 1; // { dg-error "left operand of shift
> expression '\\(-42 << 1\\)' is negative" "" { target c++17_down } }
> +constexpr int c = -42 << (__SIZEOF_INT__ * __CHAR_BIT__ - 1); // { dg-error
> "left operand of shift expression '\\(-42 << \[0-9]*\\)' is negative" "" {
> target c++17_down } }
> + // { dg-warning "result of '\\(-42 <<
> \[0-9]*\\)' requires \[0-9]* bits to represent, but 'int' only has \[0-9]*
> bits" "" { target c++17_down } .-1 }
> +constexpr int d = 42 << (__SIZEOF_INT__ * __CHAR_BIT__ - 1); // { dg-error
> "shift expression '\\(42 << \[0-9]*\\)' overflows" "" { target c++17_down } }
> + // { dg-warning "result of '\\(42 <<
> \[0-9]*\\)' requires \[0-9]* bits to represent, but 'int' only has \[0-9]*
> bits" "" { target c++17_down } .-1 }
> +constexpr int e = 32 << (__SIZEOF_INT__ * __CHAR_BIT__ - 5); // { dg-error
> "shift expression '\\(32 << \[0-9]*\\)' overflows" "" { target c++17_down } }
> + // { dg-warning "result of '\\(32 <<
> \[0-9]*\\)' requires \[0-9]* bits to represent, but 'int' only has \[0-9]*
> bits" "" { target c++17_down } .-1 }
> +constexpr int f = 32 << (__SIZEOF_INT__ * __CHAR_BIT__ - 6);
> +constexpr int g = -42U << 0;
> +constexpr int h = -42U << 1;
> +constexpr int i = -42U << (__SIZEOF_INT__ * __CHAR_BIT__ - 1);
> +constexpr int j = 42U << (__SIZEOF_INT__ * __CHAR_BIT__ - 1);
> +constexpr int k = 32U << (__SIZEOF_INT__ * __CHAR_BIT__ - 5);
> +constexpr int l = 32U << (__SIZEOF_INT__ * __CHAR_BIT__ - 6);
> +#if __cplusplus > 201703L
> +static_assert (a == g);
> +static_assert (b == h);
> +static_assert (c == i);
> +static_assert (d == j);
> +static_assert (e == k);
> +static_assert (f == l);
> +#endif
> --- gcc/testsuite/g++.dg/warn/permissive-1.C.jj 2016-01-12 19:22:56.760037087
> +0100
> +++ gcc/testsuite/g++.dg/warn/permissive-1.C 2018-11-14 09:36:47.658634223
> +0100
> @@ -2,7 +2,7 @@
> // { dg-do compile { target int32 } }
> // { dg-options "-fpermissive -Wno-shift-overflow -Wno-shift-count-overflow
> -Wno-shift-count-negative" }
>
> -enum A { AA = -1 << 4 }; // { dg-warning "operand of shift expression" "" {
> target c++11 } }
> +enum A { AA = -1 << 4 }; // { dg-warning "operand of shift expression" "" {
> target { c++11 && c++17_down } } }
> enum B { BB = 1 << -4 }; // { dg-warning "operand of shift expression" }
> enum C { CC = 1 << __SIZEOF_INT__ * 4 * __CHAR_BIT__ - 4 }; // { dg-warning
> "operand of shift expression" }
> -enum D { DD = 10 << __SIZEOF_INT__ * __CHAR_BIT__ - 2 }; // { dg-warning
> "shift expression" "" { target c++11 } }
> +enum D { DD = 10 << __SIZEOF_INT__ * __CHAR_BIT__ - 2 }; // { dg-warning
> "shift expression" "" { target { c++11 && c++17_down } } }
> --- gcc/testsuite/g++.dg/cpp0x/constexpr-shift1.C.jj 2017-11-21
> 20:22:57.851043093 +0100
> +++ gcc/testsuite/g++.dg/cpp0x/constexpr-shift1.C 2018-11-14
> 09:14:31.240695295 +0100
> @@ -19,18 +19,18 @@ constexpr int i2 = fn2 (1, 200); // { dg
> constexpr int
> fn3 (int i, int j)
> {
> - return i << j; // { dg-error "is negative" }
> + return i << j; // { dg-error "is negative" "" { target c++17_down } }
> }
>
> -constexpr int i3 = fn3 (-1, 2); // { dg-message "in .constexpr. expansion of
> " }
> +constexpr int i3 = fn3 (-1, 2); // { dg-message "in .constexpr. expansion of
> " "" { target c++17_down } }
>
> constexpr int
> fn4 (int i, int j)
> {
> - return i << j; // { dg-error "overflows" }
> + return i << j; // { dg-error "overflows" "" { target c++17_down } }
> }
>
> -constexpr int i4 = fn4 (__INT_MAX__, 2); // { dg-message "in .constexpr.
> expansion of " }
> +constexpr int i4 = fn4 (__INT_MAX__, 2); // { dg-message "in .constexpr.
> expansion of " "" { target c++17_down } }
>
> constexpr int
> fn5 (int i, int j)
> --- gcc/testsuite/g++.dg/cpp0x/constexpr-60049.C.jj 2017-11-21
> 20:22:57.870042857 +0100
> +++ gcc/testsuite/g++.dg/cpp0x/constexpr-60049.C 2018-11-14
> 09:35:29.599922164 +0100
> @@ -5,7 +5,7 @@
>
> constexpr int f1 (int n) { return 1 << n; } // { dg-error "shift
> expression" }
> constexpr int f2 (int n) { return 1 << n; } // { dg-error "shift
> expression" }
> -constexpr int f3 (int n) { return n << 1; } // { dg-error "shift
> expression" }
> +constexpr int f3 (int n) { return n << 1; } // { dg-error "shift
> expression" "" { target c++17_down } }
> constexpr int f4 (int n) { return 1 >> n; } // { dg-error "shift
> expression" }
> constexpr int f5 (int n) { return 1 >> n; } // { dg-error "shift
> expression" }
>
> @@ -13,12 +13,12 @@ constexpr int X = __CHAR_BIT__ * sizeof
>
> constexpr int x1 = f1 (X); // { dg-message "in .constexpr. expansion of" }
> constexpr int x2 = f2 (-1); // { dg-message "in .constexpr. expansion of" }
> -constexpr int x3 = f3 (-1); // { dg-message "in .constexpr. expansion of" }
> +constexpr int x3 = f3 (-1); // { dg-message "in .constexpr. expansion of"
> "" { target c++17_down } }
> constexpr int x4 = f4 (X); // { dg-message "in .constexpr. expansion of" }
> constexpr int x5 = f5 (-1); // { dg-message "in .constexpr. expansion of" }
>
> constexpr int y1 = 1 << X; // { dg-error "shift expression" }
> constexpr int y2 = 1 << -1; // { dg-error "shift expression" }
> -constexpr int y3 = -1 << 1; // { dg-error "shift expression" }
> +constexpr int y3 = -1 << 1; // { dg-error "shift expression" "" { target
> c++17_down } }
> constexpr int y4 = 1 >> X; // { dg-error "shift expression" }
> constexpr int y5 = 1 >> -1; // { dg-error "shift expression" }
> --- gcc/testsuite/g++.dg/ubsan/cxx11-shift-1.C.jj 2014-10-22
> 15:52:16.527879247 +0200
> +++ gcc/testsuite/g++.dg/ubsan/cxx11-shift-1.C 2018-11-14 08:43:15.806640709
> +0100
> @@ -2,9 +2,10 @@
> /* { dg-options "-fsanitize=shift -w -fno-sanitize-recover=shift -std=c++11"
> } */
>
> int
> -main (void)
> +main ()
> {
> int a = 1;
> - a <<= 31;
> - return 0;
> + a <<= (__SIZEOF_INT__ * __CHAR_BIT__ - 1);
> + a = 16;
> + a <<= (__SIZEOF_INT__ * __CHAR_BIT__ - 5);
> }
> --- gcc/testsuite/g++.dg/ubsan/cxx11-shift-2.C.jj 2014-04-23
> 10:18:41.089330171 +0200
> +++ gcc/testsuite/g++.dg/ubsan/cxx11-shift-2.C 2018-11-14 08:43:07.947770370
> +0100
> @@ -2,9 +2,18 @@
> /* { dg-options "-fsanitize=shift -w -std=c++11" } */
>
> int
> -main (void)
> +main ()
> {
> int a = -42;
> a <<= 1;
> + a = -43;
> + a <<= 0;
> + a = -44;
> + a <<= (__SIZEOF_INT__ * __CHAR_BIT__ - 1);
> + a = 32;
> + a <<= (__SIZEOF_INT__ * __CHAR_BIT__ - 3);
> }
> -/* { dg-output "left shift of negative value -42" } */
> +/* { dg-output "left shift of negative value -42.*" } */
> +/* { dg-output "left shift of negative value -43.*" } */
> +/* { dg-output "left shift of negative value -44.*" } */
> +/* { dg-output "left shift of 32 by \[0-9]* places cannot be represented in
> type 'int'" } */
> --- gcc/testsuite/g++.dg/ubsan/cxx2a-shift-1.C.jj 2018-11-14
> 08:35:13.132600583 +0100
> +++ gcc/testsuite/g++.dg/ubsan/cxx2a-shift-1.C 2018-11-14 08:42:56.229963690
> +0100
> @@ -0,0 +1,11 @@
> +/* { dg-do run } */
> +/* { dg-options "-fsanitize=shift -w -fno-sanitize-recover=shift -std=c++2a"
> } */
> +
> +int
> +main ()
> +{
> + int a = 1;
> + a <<= 31;
> + a = 16;
> + a <<= (__SIZEOF_INT__ * __CHAR_BIT__ - 5);
> +}
> --- gcc/testsuite/g++.dg/ubsan/cxx2a-shift-2.C.jj 2018-11-14
> 08:35:13.132600583 +0100
> +++ gcc/testsuite/g++.dg/ubsan/cxx2a-shift-2.C 2018-11-14 08:42:48.803086219
> +0100
> @@ -0,0 +1,15 @@
> +/* { dg-do run } */
> +/* { dg-options "-fsanitize=shift -w -fno-sanitize-recover=shift -std=c++2a"
> } */
> +
> +int
> +main ()
> +{
> + int a = -42;
> + a <<= 1;
> + a = -43;
> + a <<= 0;
> + a = -44;
> + a <<= (__SIZEOF_INT__ * __CHAR_BIT__ - 1);
> + a = 32;
> + a <<= (__SIZEOF_INT__ * __CHAR_BIT__ - 3);
> +}
>
> Jakub
