Left shifts of negative values are defined if -fwrapv is set

[jmolloy]

Summary

This means we shouldn't emit ubsan detection code or warn.
Fixes PR25552.

[filcab]

test/CodeGen/wrapv-lshr-sanitize.c

2

Do you really need instnamer?

test/Sema/negative-shift-wrapv.c

10

It's a bit weird to have the test function be dead code, but I don't see a big problem anyway.

Do you need to do the whole WORD_BIT thing? I'd rather just shift -1 by 1 (which is already UB in non -fwrapv world, but defined in -fwrapv world).

[jmolloy]

Hi Filipe,

Thanks for the review! SGTM on both counts.

[jmolloy]

Hi Filipe,

Did this look good to you after those changes?

Cheers,

James

[davide]

The Sema bits look fine to me. I'll let Filipe comment on the sanitizer part.

[jmolloy]

Thanks, r278786!

lib/CodeGen/CGExprScalar.cpp

 Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) {
   // LLVM requires the LHS and RHS to be the same type: promote or truncate the
   // RHS to the same size as the LHS.
   Value *RHS = Ops.RHS;
   if (Ops.LHS->getType() != RHS->getType())
     RHS = Builder.CreateIntCast(RHS, Ops.LHS->getType(), false, "sh_prom");
 
   bool SanitizeBase = CGF.SanOpts.has(SanitizerKind::ShiftBase) &&
-                      Ops.Ty->hasSignedIntegerRepresentation();
+                      Ops.Ty->hasSignedIntegerRepresentation() &&
+                      !CGF.getLangOpts().isSignedOverflowDefined();
   bool SanitizeExponent = CGF.SanOpts.has(SanitizerKind::ShiftExponent);
   // OpenCL 6.3j: shift values are effectively % word size of LHS.
   if (CGF.getLangOpts().OpenCL)
     RHS =
         Builder.CreateAnd(RHS, GetWidthMinusOneValue(Ops.LHS, RHS), "shl.mask");
   else if ((SanitizeBase || SanitizeExponent) &&
            isa<llvm::IntegerType>(Ops.LHS->getType())) {
     CodeGenFunction::SanitizerScope SanScope(&CGF);

lib/Sema/SemaExpr.cpp

static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
   llvm::APSInt Left;
   if (LHS.get()->isValueDependent() ||
       LHSType->hasUnsignedIntegerRepresentation() ||
       !LHS.get()->EvaluateAsInt(Left, S.Context))
     return;
 
   // If LHS does not have a signed type and non-negative value
   // then, the behavior is undefined. Warn about it.
-  if (Left.isNegative()) {
+  if (Left.isNegative() && !S.getLangOpts().isSignedOverflowDefined()) {
     S.DiagRuntimeBehavior(Loc, LHS.get(),
                           S.PDiag(diag::warn_shift_lhs_negative)
                             << LHS.get()->getSourceRange());
     return;
   }
 
   llvm::APInt ResultBits =
       static_cast<llvm::APInt&>(Right) + Left.getMinSignedBits();

test/CodeGen/wrapv-lshr-sanitize.c

+// RUN: %clang_cc1 -fsanitize=shift-base -emit-llvm %s -o - -triple x86_64-linux-gnu -fwrapv | FileCheck %s
+
+// CHECK-LABEL: @lsh_overflow
+int lsh_overflow(int a, int b) {
+  // CHECK-NOT: br
+  // CHECK-NOT: call void @__ubsan_
+  // CHECK-NOT: call void @llvm.trap
+  
+  // CHECK:      %[[RET:.*]] = shl i32
+  // CHECK-NEXT: ret i32 %[[RET]]
+  return a << b;
+}

test/Sema/negative-shift-wrapv.c

+// RUN: %clang_cc1 -Wall -ffreestanding -fsyntax-only -fwrapv -verify %s
+
+int test() {
+  int i;
+  i = -1 << 1; // no-warning
+  return i;
+}
+
+// expected-no-diagnostics