[cxx2a] P1236R1: the validity of a left shift does not depend on the

value of the LHS operand.

llvm-svn: 364265

Author: zygoloid

clang/lib/AST/ExprConstant.cpp

@@ -2388,9 +2388,11 @@ static bool handleIntIntBinOp(EvalInfo &Info, const Expr *E, const APSInt &LHS,
     if (SA != RHS) {
       Info.CCEDiag(E, diag::note_constexpr_large_shift)
         << RHS << E->getType() << LHS.getBitWidth();
-    } else if (LHS.isSigned()) {
+    } else if (LHS.isSigned() && !Info.getLangOpts().CPlusPlus2a) {
       // C++11 [expr.shift]p2: A signed left shift must have a non-negative
       // operand, and must not overflow the corresponding unsigned type.
+      // C++2a [expr.shift]p2: E1 << E2 is the unique value congruent to
+      // E1 x 2^E2 module 2^N.
       if (LHS.isNegative())
         Info.CCEDiag(E, diag::note_constexpr_lshift_of_negative) << LHS;
       else if (LHS.countLeadingZeros() < SA)

clang/lib/CodeGen/CGExprScalar.cpp

@@ -3640,7 +3640,8 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) {
 
   bool SanitizeBase = CGF.SanOpts.has(SanitizerKind::ShiftBase) &&
                       Ops.Ty->hasSignedIntegerRepresentation() &&
-                      !CGF.getLangOpts().isSignedOverflowDefined();
+                      !CGF.getLangOpts().isSignedOverflowDefined() &&
+                      !CGF.getLangOpts().CPlusPlus2a;
   bool SanitizeExponent = CGF.SanOpts.has(SanitizerKind::ShiftExponent);
   // OpenCL 6.3j: shift values are effectively % word size of LHS.
   if (CGF.getLangOpts().OpenCL)

clang/lib/Sema/SemaExpr.cpp

@@ -9645,9 +9645,11 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
     return;
 
   // When left shifting an ICE which is signed, we can check for overflow which
-  // according to C++ has undefined behavior ([expr.shift] 5.8/2). Unsigned
-  // integers have defined behavior modulo one more than the maximum value
-  // representable in the result type, so never warn for those.
+  // according to C++ standards prior to C++2a has undefined behavior
+  // ([expr.shift] 5.8/2). Unsigned integers have defined behavior modulo one
+  // more than the maximum value representable in the result type, so never
+  // warn for those. (FIXME: Unsigned left-shift overflow in a constant
+  // expression is still probably a bug.)
   Expr::EvalResult LHSResult;
   if (LHS.get()->isValueDependent() ||
       LHSType->hasUnsignedIntegerRepresentation() ||
@@ -9656,8 +9658,9 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
   llvm::APSInt Left = LHSResult.Val.getInt();
 
   // If LHS does not have a signed type and non-negative value
-  // then, the behavior is undefined. Warn about it.
-  if (Left.isNegative() && !S.getLangOpts().isSignedOverflowDefined()) {
+  // then, the behavior is undefined before C++2a. Warn about it.
+  if (Left.isNegative() && !S.getLangOpts().isSignedOverflowDefined() &&
+      !S.getLangOpts().CPlusPlus2a) {
     S.DiagRuntimeBehavior(Loc, LHS.get(),
                           S.PDiag(diag::warn_shift_lhs_negative)
                             << LHS.get()->getSourceRange());

clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp

@@ -231,9 +231,6 @@ BasicValueFactory::evalAPSInt(BinaryOperator::Opcode Op,
       // FIXME: This logic should probably go higher up, where we can
       // test these conditions symbolically.
 
-      if (V1.isSigned() && V1.isNegative())
-        return nullptr;
-
       if (V2.isSigned() && V2.isNegative())
         return nullptr;
 
@@ -242,8 +239,13 @@ BasicValueFactory::evalAPSInt(BinaryOperator::Opcode Op,
       if (Amt >= V1.getBitWidth())
         return nullptr;
 
-      if (V1.isSigned() && Amt > V1.countLeadingZeros())
+      if (!Ctx.getLangOpts().CPlusPlus2a) {
+        if (V1.isSigned() && V1.isNegative())
+          return nullptr;
+
+        if (V1.isSigned() && Amt > V1.countLeadingZeros())
           return nullptr;
+      }
 
       return &getValue( V1.operator<<( (unsigned) Amt ));
     }

clang/test/Analysis/left-shift-cxx2a.cpp

@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple x86_64-apple-darwin13 -Wno-shift-count-overflow -verify=expected,cxx17 -std=c++17 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple x86_64-apple-darwin13 -Wno-shift-count-overflow -verify=expected,cxx2a -std=c++2a %s
+
+int testNegativeShift(int a) {
+  if (a == -5) {
+    return 1 << a; // expected-warning{{The result of the left shift is undefined because the right operand is negative}}
+  }
+  return 0;
+}
+
+int testNegativeLeftShift(int a) {
+  if (a == -3) {
+    return a << 1; // cxx17-warning{{The result of the left shift is undefined because the left operand is negative}}
+  }
+  return 0;
+}
+
+int testUnrepresentableLeftShift(int a) {
+  if (a == 8)
+    return a << 30; // cxx17-warning{{The result of the left shift is undefined due to shifting '8' by '30', which is unrepresentable in the unsigned version of the return type 'int'}}
+  return 0;
+}

clang/test/CodeGenCXX/cxx2a-left-shift.cpp

@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++2a -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,REGULAR
+// RUN: %clang_cc1 -std=c++2a -fsanitize=shift-base,shift-exponent -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,SANITIZED
+
+// CHECK-LABEL: @_Z12lsh_overflow
+int lsh_overflow(int a, int b) {
+  // SANITIZED: %[[RHS_INBOUNDS:.*]] = icmp ule i32 %[[RHS:.*]], 31
+  // SANITIZED-NEXT: br i1 %[[RHS_INBOUNDS]], label %[[VALID:.*]], label
+
+  // SANITIZED: call void @__ubsan_handle_shift_out_of_bounds
+
+  // No check for the LHS here.
+  // SANITIZED: [[VALID]]:
+  // SANITIZED-NEXT: shl i32 %
+  // SANITIZED-NEXT: ret i32
+
+  // Just ensure there's no nsw nuw flags here.
+  // REGULAR: shl i32 %
+  return a << b;
+}

clang/test/SemaCXX/constant-expression-cxx2a.cpp

@@ -532,3 +532,19 @@ namespace Union {
     s.n = 0;
   }
 }
+
+namespace TwosComplementShifts {
+  using uint32 = __UINT32_TYPE__;
+  using int32 = __INT32_TYPE__;
+  static_assert(uint32(int32(0x1234) << 16) == 0x12340000);
+  static_assert(uint32(int32(0x1234) << 19) == 0x91a00000);
+  static_assert(uint32(int32(0x1234) << 20) == 0x23400000); // expected-warning {{requires 34 bits}}
+  static_assert(uint32(int32(0x1234) << 24) == 0x34000000); // expected-warning {{requires 38 bits}}
+  static_assert(uint32(int32(-1) << 31) == 0x80000000);
+
+  static_assert(-1 >> 1 == -1);
+  static_assert(-1 >> 31 == -1);
+  static_assert(-2 >> 1 == -1);
+  static_assert(-3 >> 1 == -2);
+  static_assert(-4 >> 1 == -2);
+}

clang/test/SemaCXX/shift.cpp

@@ -1,14 +1,84 @@
-// RUN: %clang_cc1 -Wall -Wshift-sign-overflow -ffreestanding -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wall -Wshift-sign-overflow -ffreestanding -fsyntax-only -verify=expected,cxx17 -std=c++17 %s
+// RUN: %clang_cc1 -Wall -Wshift-sign-overflow -ffreestanding -fsyntax-only -verify=expected,cxx2a -std=c++2a %s
 
 #include <limits.h>
 
 #define WORD_BIT (sizeof(int) * CHAR_BIT)
 
-template <int N> void f() {
-  (void)(N << 30); // expected-warning {{bits to represent, but 'int' only has}}
-  (void)(30 << N); // expected-warning {{bits to represent, but 'int' only has}}
-}
+enum {
+  X = 1 << 0,
+  Y = 1 << 1,
+  Z = 1 << 2
+};
 
 void test() {
-  f<30>(); // expected-note {{instantiation}}
+  char c;
+
+  c = 0 << 0;
+  c = 0 << 1;
+  c = 1 << 0;
+  c = 1 << -0;
+  c = 1 >> -0;
+  c = 1 << -1; // expected-warning {{shift count is negative}}
+  c = 1 >> -1; // expected-warning {{shift count is negative}}
+  c = 1 << (unsigned)-1; // expected-warning {{shift count >= width of type}}
+                         // expected-warning@-1 {{implicit conversion}}
+  c = 1 >> (unsigned)-1; // expected-warning {{shift count >= width of type}}
+  c = 1 << c;
+  c <<= 0;
+  c >>= 0;
+  c <<= 1;
+  c >>= 1;
+  c <<= -1; // expected-warning {{shift count is negative}}
+  c >>= -1; // expected-warning {{shift count is negative}}
+  c <<= 999999; // expected-warning {{shift count >= width of type}}
+  c >>= 999999; // expected-warning {{shift count >= width of type}}
+  c <<= CHAR_BIT; // expected-warning {{shift count >= width of type}}
+  c >>= CHAR_BIT; // expected-warning {{shift count >= width of type}}
+  c <<= CHAR_BIT+1; // expected-warning {{shift count >= width of type}}
+  c >>= CHAR_BIT+1; // expected-warning {{shift count >= width of type}}
+  (void)((long)c << CHAR_BIT);
+
+  int i;
+  i = 1 << (WORD_BIT - 2);
+  i = 2 << (WORD_BIT - 1); // expected-warning {{bits to represent, but 'int' only has}}
+  i = 1 << (WORD_BIT - 1); // expected-warning {{sets the sign bit of the shift expression}}
+  i = -1 << (WORD_BIT - 1); // cxx17-warning {{shifting a negative signed value is undefined}}
+  i = -1 << 0; // cxx17-warning {{shifting a negative signed value is undefined}}
+  i = 0 << (WORD_BIT - 1);
+  i = (char)1 << (WORD_BIT - 2);
+
+  unsigned u;
+  u = 1U << (WORD_BIT - 1);
+  u = 5U << (WORD_BIT - 1);
+
+  long long int lli;
+  lli = INT_MIN << 2; // cxx17-warning {{shifting a negative signed value is undefined}} cxx2a-warning {{requires 34 bits to represent}}
+  lli = 1LL << (sizeof(long long) * CHAR_BIT - 2);
+}
+
+#define a 0
+#define ashift 8
+enum { b = (a << ashift) };
+
+// Don't warn for negative shifts in code that is unreachable.
+void test_pr5544() {
+  (void) (((1) > 63 && (1) < 128 ? (((unsigned long long) 1)<<((1)-64)) : (unsigned long long) 0)); // no-warning
+}
+
+void test_shift_too_much(char x) {
+  if (0)
+    (void) (x >> 80); // no-warning
+  (void) (x >> 80); // expected-warning {{shift count >= width of type}}
+}
+
+typedef unsigned vec16 __attribute__((vector_size(16)));
+typedef unsigned vec8 __attribute__((vector_size(8)));
+
+void vect_shift_1(vec16 *x) { *x = *x << 4; }
+
+void vect_shift_2(vec16 *x, vec16 y) { *x = *x << y; }
+
+void vect_shift_3(vec16 *x, vec8 y) {
+  *x = *x << y; // expected-error {{vector operands do not have the same number of elements}}
 }

clang/www/cxx_status.html

@@ -1008,7 +1008,7 @@ <h2 id="cxx20">C++2a implementation status</h2>
     <tr>
       <td>Signed integers are two's complement</td>
       <td><a href="http://wg21.link/p1236r1">P1236R1</a></td>
-      <td class="none" align="center">No</td>
+      <td class="svn" align="center">SVN</td>
     </tr>
     <tr>
       <td><tt>char8_t</tt></td>