[PATCH] D18654: In C++11 it is undefined to shift into the sign bit

[Filipe Cabecinhas via cfe-commits]

filcab created this revision.
filcab added reviewers: rsmith, samsonov.
filcab added a subscriber: cfe-commits.

Clang was using C++14 rules for shifts into the sign bit with any C++
standard. Now it has the C++ <14 special case (same as C).

Using this C++11 standard draft as reference (before CWG 1457):
https://github.com/cplusplus/draft/blob/672675e01528fba6ca02f5340eee747566ebdca8/papers/N3376.pdf

http://reviews.llvm.org/D18654

Files:
  lib/CodeGen/CGExprScalar.cpp
  test/CodeGen/cxx-signed-shift-overflow.cpp
  test/CodeGenCXX/catch-undef-behavior.cpp

Index: test/CodeGenCXX/catch-undef-behavior.cpp
===================================================================
--- test/CodeGenCXX/catch-undef-behavior.cpp
+++ test/CodeGenCXX/catch-undef-behavior.cpp
@@ -137,11 +137,7 @@
   // CHECK-NEXT: %[[SHIFTED_OUT_WIDTH:.*]] = sub nuw nsw i32 31, %[[RHS]]
   // CHECK-NEXT: %[[SHIFTED_OUT:.*]] = lshr i32 %[[LHS:.*]], %[[SHIFTED_OUT_WIDTH]]
 
-  // This is present for C++11 but not for C: C++ core issue 1457 allows a '1'
-  // to be shifted into the sign bit, but not out of it.
-  // CHECK-NEXT: %[[SHIFTED_OUT_NOT_SIGN:.*]] = lshr i32 %[[SHIFTED_OUT]], 1
-
-  // CHECK-NEXT: %[[NO_OVERFLOW:.*]] = icmp eq i32 %[[SHIFTED_OUT_NOT_SIGN]], 0
+  // CHECK-NEXT: %[[NO_OVERFLOW:.*]] = icmp eq i32 %[[SHIFTED_OUT]], 0
   // CHECK-NEXT: br label %[[CONT_BB]]
 
   // CHECK:      [[CONT_BB]]:
Index: test/CodeGen/cxx-signed-shift-overflow.cpp
===================================================================
--- /dev/null
+++ test/CodeGen/cxx-signed-shift-overflow.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin10 -fsanitize=shift-base %s -emit-llvm -o - | FileCheck %s -check-prefix=CXX11 -check-prefix=BOTH
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-apple-darwin10 -fsanitize=shift-base %s -emit-llvm -o - | FileCheck %s -check-prefix=CXX14 -check-prefix=BOTH
+
+int shiftoverflow(int b, int e) {
+  // BOTH-LABEL: shiftoverflow
+  // CXX14-LABEL: check:
+  // CXX14: %shl.check = lshr
+  // CXX14: lshr i{{[^ ]*}} %shl.check, 1
+  // CXX11-LABEL: check:
+  // CXX11: %shl.check = lshr
+  // CXX11-NOT: lshr i{{[^ ]*}} %shl.check, 1
+  return b << e;
+}
Index: lib/CodeGen/CGExprScalar.cpp
===================================================================
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -2746,11 +2746,12 @@
                            Builder.CreateSub(WidthMinusOne, RHS, "shl.zeros",
                                              /*NUW*/true, /*NSW*/true),
                            "shl.check");
-      if (CGF.getLangOpts().CPlusPlus) {
-        // In C99, we are not permitted to shift a 1 bit into the sign bit.
-        // Under C++11's rules, shifting a 1 bit into the sign bit is
-        // OK, but shifting a 1 bit out of it is not. (C89 and C++03 don't
-        // define signed left shifts, so we use the C99 and C++11 rules there).
+      if (CGF.getLangOpts().CPlusPlus && CGF.getLangOpts().CPlusPlus14) {
+        // In C99 and C++11, we are not permitted to shift a 1 bit into the sign
+        // bit.
+        // Under C++14's rules, shifting a 1 bit into the sign bit is OK, but
+        // shifting a 1 bit out of it is not. (C89 and C++03 don't define signed
+        // left shifts, so we use the C99 and C++11 rules there).
         llvm::Value *One = llvm::ConstantInt::get(BitsShiftedOff->getType(), 1);
         BitsShiftedOff = Builder.CreateLShr(BitsShiftedOff, One);
       }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D18654.52233.patch
Type: text/x-patch
Size: 2930 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160331/48ccab12/attachment.bin>