From: Will Dietz Date: Mon, 25 Feb 2013 22:37:49 +0000 (+0000) Subject: [ubsan] Emit single check for left shift. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=bb60fc6a71f280f8e19e7f1a784ef05c40f5aa39;p=clang [ubsan] Emit single check for left shift. Avoids warning twice on same shift. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176056 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp index d76cad2c56..3ec4040a64 100644 --- a/lib/CodeGen/CGExprScalar.cpp +++ b/lib/CodeGen/CGExprScalar.cpp @@ -2407,13 +2407,17 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) { if (CGF.SanOpts->Shift && !CGF.getLangOpts().OpenCL && isa(Ops.LHS->getType())) { llvm::Value *WidthMinusOne = GetWidthMinusOneValue(Ops.LHS, RHS); - // FIXME: Emit the branching explicitly rather than emitting the check - // twice. - EmitBinOpCheck(Builder.CreateICmpULE(RHS, WidthMinusOne), Ops); + llvm::Value *Valid = Builder.CreateICmpULE(RHS, WidthMinusOne); if (Ops.Ty->hasSignedIntegerRepresentation()) { + llvm::BasicBlock *Orig = Builder.GetInsertBlock(); + llvm::BasicBlock *Cont = CGF.createBasicBlock("cont"); + llvm::BasicBlock *CheckBitsShifted = CGF.createBasicBlock("check"); + Builder.CreateCondBr(Valid, CheckBitsShifted, Cont); + // Check whether we are shifting any non-zero bits off the top of the // integer. + CGF.EmitBlock(CheckBitsShifted); llvm::Value *BitsShiftedOff = Builder.CreateLShr(Ops.LHS, Builder.CreateSub(WidthMinusOne, RHS, "shl.zeros", @@ -2428,8 +2432,15 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) { BitsShiftedOff = Builder.CreateLShr(BitsShiftedOff, One); } llvm::Value *Zero = llvm::ConstantInt::get(BitsShiftedOff->getType(), 0); - EmitBinOpCheck(Builder.CreateICmpEQ(BitsShiftedOff, Zero), Ops); + llvm::Value *SecondCheck = Builder.CreateICmpEQ(BitsShiftedOff, Zero); + CGF.EmitBlock(Cont); + llvm::PHINode *P = Builder.CreatePHI(Valid->getType(), 2); + P->addIncoming(Valid, Orig); + P->addIncoming(SecondCheck, CheckBitsShifted); + Valid = P; } + + EmitBinOpCheck(Valid, Ops); } // OpenCL 6.3j: shift values are effectively % word size of LHS. if (CGF.getLangOpts().OpenCL) diff --git a/test/CodeGen/catch-undef-behavior.c b/test/CodeGen/catch-undef-behavior.c index cd86dd5a60..3e180a445b 100644 --- a/test/CodeGen/catch-undef-behavior.c +++ b/test/CodeGen/catch-undef-behavior.c @@ -8,8 +8,7 @@ // FIXME: When we only emit each type once, use [[INT]] more below. // CHECK: @[[LINE_100:.*]] = private unnamed_addr global {{.*}}, i32 100, i32 5 {{.*}} @[[INT]], i64 4, i8 1 // CHECK: @[[LINE_200:.*]] = {{.*}}, i32 200, i32 10 {{.*}}, i64 4, i8 0 -// CHECK: @[[LINE_300_A:.*]] = {{.*}}, i32 300, i32 12 {{.*}} @{{.*}}, {{.*}} @{{.*}} -// CHECK: @[[LINE_300_B:.*]] = {{.*}}, i32 300, i32 12 {{.*}} @{{.*}}, {{.*}} @{{.*}} +// CHECK: @[[LINE_300:.*]] = {{.*}}, i32 300, i32 12 {{.*}} @{{.*}}, {{.*}} @{{.*}} // CHECK: @[[LINE_400:.*]] = {{.*}}, i32 400, i32 12 {{.*}} @{{.*}}, {{.*}} @{{.*}} // CHECK: @[[LINE_500:.*]] = {{.*}}, i32 500, i32 10 {{.*}} @{{.*}}, i64 4, i8 0 } // CHECK: @[[LINE_600:.*]] = {{.*}}, i32 600, i32 3 {{.*}} @{{.*}}, i64 4, i8 1 } @@ -106,35 +105,36 @@ int addr_space(int __attribute__((address_space(256))) *a) { // CHECK-TRAP: @lsh_overflow int lsh_overflow(int a, int b) { // CHECK: %[[INBOUNDS:.*]] = icmp ule i32 %[[RHS:.*]], 31 - // CHECK-NEXT: br i1 %[[INBOUNDS]] + // CHECK-NEXT: br i1 %[[INBOUNDS]], label %[[CHECKBB:.*]], label %[[CONTBB:.*]] // CHECK-TRAP: %[[INBOUNDS:.*]] = icmp ule i32 %[[RHS:.*]], 31 - // CHECK-TRAP-NEXT: br i1 %[[INBOUNDS]] - - // FIXME: Only emit one trap block here. - // CHECK: %[[ARG1:.*]] = zext - // CHECK-NEXT: %[[ARG2:.*]] = zext - // CHECK-NEXT: call void @__ubsan_handle_shift_out_of_bounds(i8* bitcast ({{.*}} @[[LINE_300_A]] to i8*), i64 %[[ARG1]], i64 %[[ARG2]]) - - // CHECK-TRAP: call void @llvm.trap() [[NR_NUW]] - // CHECK-TRAP-NEXT: unreachable + // CHECK-TRAP-NEXT: br i1 %[[INBOUNDS]], label %[[CHECKBB:.*]], label %[[CONTBB:.*]] // CHECK: %[[SHIFTED_OUT_WIDTH:.*]] = sub nuw nsw i32 31, %[[RHS]] // CHECK-NEXT: %[[SHIFTED_OUT:.*]] = lshr i32 %[[LHS:.*]], %[[SHIFTED_OUT_WIDTH]] // CHECK-NEXT: %[[NO_OVERFLOW:.*]] = icmp eq i32 %[[SHIFTED_OUT]], 0 - // CHECK-NEXT: br i1 %[[NO_OVERFLOW]], {{.*}} !prof ![[WEIGHT_MD]] + // CHECK-NEXT: br label %[[CONTBB]] // CHECK-TRAP: %[[SHIFTED_OUT_WIDTH:.*]] = sub nuw nsw i32 31, %[[RHS]] // CHECK-TRAP-NEXT: %[[SHIFTED_OUT:.*]] = lshr i32 %[[LHS:.*]], %[[SHIFTED_OUT_WIDTH]] // CHECK-TRAP-NEXT: %[[NO_OVERFLOW:.*]] = icmp eq i32 %[[SHIFTED_OUT]], 0 - // CHECK-TRAP-NEXT: br i1 %[[NO_OVERFLOW]] + // CHECK-TRAP-NEXT: br label %[[CONTBB]] + + // CHECK: %[[VALID:.*]] = phi i1 [ %[[INBOUNDS]], {{.*}} ], [ %[[NO_OVERFLOW]], %[[CHECKBB]] ] + // CHECK-NEXT: br i1 %[[VALID]], {{.*}} !prof ![[WEIGHT_MD]] + + // CHECK-TRAP: %[[VALID:.*]] = phi i1 [ %[[INBOUNDS]], {{.*}} ], [ %[[NO_OVERFLOW]], %[[CHECKBB]] ] + // CHECK-TRAP-NEXT: br i1 %[[VALID]] + // CHECK: %[[ARG1:.*]] = zext // CHECK-NEXT: %[[ARG2:.*]] = zext - // CHECK-NEXT: call void @__ubsan_handle_shift_out_of_bounds(i8* bitcast ({{.*}} @[[LINE_300_B]] to i8*), i64 %[[ARG1]], i64 %[[ARG2]]) + // CHECK-NEXT: call void @__ubsan_handle_shift_out_of_bounds(i8* bitcast ({{.*}} @[[LINE_300]] to i8*), i64 %[[ARG1]], i64 %[[ARG2]]) + // CHECK-NOT: call void @__ubsan_handle_shift_out_of_bounds // CHECK-TRAP: call void @llvm.trap() [[NR_NUW]] - // CHECK-TRAP-NEXT: unreachable + // CHECK-TRAP: unreachable + // CHECK-TRAP-NOT: call void @llvm.trap() // CHECK: %[[RET:.*]] = shl i32 %[[LHS]], %[[RHS]] // CHECK-NEXT: ret i32 %[[RET]] diff --git a/test/CodeGenCXX/catch-undef-behavior.cpp b/test/CodeGenCXX/catch-undef-behavior.cpp index 31958a61e3..d6d0edfa1e 100644 --- a/test/CodeGenCXX/catch-undef-behavior.cpp +++ b/test/CodeGenCXX/catch-undef-behavior.cpp @@ -130,7 +130,12 @@ int lsh_overflow(int a, int b) { // 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: br i1 %[[NO_OVERFLOW]] + + // CHECK: %[[VALID:.*]] = phi i1 [ %[[INBOUNDS]], {{.*}} ], [ %[[NO_OVERFLOW]], {{.*}} ] + // CHECK-NEXT: br i1 %[[VALID]] + + // CHECK: call void @__ubsan_handle_shift_out_of_bounds + // CHECK-NOT: call void @__ubsan_handle_shift_out_of_bounds // CHECK: %[[RET:.*]] = shl i32 %[[LHS]], %[[RHS]] // CHECK-NEXT: ret i32 %[[RET]]