In C++11 it is undefined to shift into the sign bit

[filcab]

Summary

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

[rsmith]

Core issue 1457 is a DR against C++11, so it applies to our C++11 mode.

[filcab]

That means that our C++11 mode will have some fixes, right?
How can we call what out C++11 mode is? Are there updated versions of
the standard? Are there lists of defects that we have fixed and others
we haven't?

Sorry, I'm not too familiar with how the standard+fixes usually work.

Thank you,

Filipe

[rsmith]

In D18654#388419, @filcab wrote:

That means that our C++11 mode will have some fixes, right?

Right. The standard's rules are often incoherent or unimplementable without the fixes in DRs, so it's not meaningful to implement ISO C++ as standardized. In order to avoid making judgement calls ourselves, we (and many other compiler vendors) enable *all* relevant DR fixes for each language mode.

How can we call what out C++11 mode is?

It's ISO C++11, plus all relevant defect reports. (Incidentally, this is why C++98 and C++03 modes are identical across typical compilers -- the difference is only DRs.)

Are there updated versions of the standard?

No. (As the C++ standard editor, I've been considering maintaining such an alternative standard, but it's a large amount of work, and it would give the false impression that the document containing the standard + DRs is in some way an official product of the ISO C++ committee.)

Are there lists of defects that we have fixed and others we haven't?

clang.llvm.org/cxx_dr_status.html is has a complete list of core issues, along with an incomplete list of Clang's implementation status for them. If you want to get involved with that, there are tests in clang's test/CXX/drs/... for each such issue, containing special comments describing our implementation status (the HTML status page is then generated by scraping those comments); tests for more DRs would be appreciated!

[filcab]

Thanks for clarifying, Richard.
I'll think about getting some fixes/tests in for our DRs.

lib/CodeGen/CGExprScalar.cpp

if (SanitizeBase) {
       llvm::BasicBlock *CheckShiftBase = CGF.createBasicBlock("check");
       Builder.CreateCondBr(ValidExponent, CheckShiftBase, Cont);
       CGF.EmitBlock(CheckShiftBase);
       llvm::Value *BitsShiftedOff =
         Builder.CreateLShr(Ops.LHS,
                            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);
       }
       llvm::Value *Zero = llvm::ConstantInt::get(BitsShiftedOff->getType(), 0);
       llvm::Value *ValidBase = Builder.CreateICmpEQ(BitsShiftedOff, Zero);
       CGF.EmitBlock(Cont);
       llvm::PHINode *BaseCheck = Builder.CreatePHI(ValidBase->getType(), 2);
       BaseCheck->addIncoming(Builder.getTrue(), Orig);

test/CodeGen/cxx-signed-shift-overflow.cpp

+// 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;
+}

test/CodeGenCXX/catch-undef-behavior.cpp

 int lsh_overflow(int a, int b) {
   // CHECK: %[[RHS_INBOUNDS:.*]] = icmp ule i32 %[[RHS:.*]], 31
   // CHECK-NEXT: br i1 %[[RHS_INBOUNDS]], label %[[CHECK_BB:.*]], label %[[CONT_BB:.*]],
 
   // CHECK:      [[CHECK_BB]]:
   // 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]]:
   // CHECK-NEXT: %[[VALID_BASE:.*]] = phi i1 [ true, {{.*}} ], [ %[[NO_OVERFLOW]], %[[CHECK_BB]] ]
   // CHECK-NEXT: %[[VALID:.*]] = and i1 %[[RHS_INBOUNDS]], %[[VALID_BASE]]
   // CHECK-NEXT: br i1 %[[VALID]]
 
   // CHECK: call void @__ubsan_handle_shift_out_of_bounds