[UBSan] Split -fsanitize=shift into -fsanitize=shift-base and -fsanitize=shift-exponent.
-fsanitize=shift is now a group that includes both these checks, so
exisiting users should not be affected.
This change introduces two new UBSan kinds that sanitize only left-hand
side and right-hand side of shift operation. In practice, invalid
exponent value (negative or too large) tends to cause more portability
problems, including inconsistencies between different compilers, crashes
and inadequeate results on non-x86 architectures etc. That is,
-fsanitize=shift-exponent failures should generally be addressed first.
As a bonus, this change simplifies CodeGen implementation for emitting left
shift (separate checks for base and exponent are now merged by the
existing generic logic in EmitCheck()), and LLVM IR for these checks
(the number of basic blocks is reduced).
llvm-svn: 231150
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index dc12dd8..3544ce5 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -2664,21 +2664,30 @@
if (Ops.LHS->getType() != RHS->getType())
RHS = Builder.CreateIntCast(RHS, Ops.LHS->getType(), false, "sh_prom");
- if (CGF.SanOpts.has(SanitizerKind::Shift) && !CGF.getLangOpts().OpenCL &&
- isa<llvm::IntegerType>(Ops.LHS->getType())) {
+ // 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 ((CGF.SanOpts.has(SanitizerKind::ShiftBase) ||
+ CGF.SanOpts.has(SanitizerKind::ShiftExponent)) &&
+ isa<llvm::IntegerType>(Ops.LHS->getType())) {
CodeGenFunction::SanitizerScope SanScope(&CGF);
- llvm::Value *WidthMinusOne = GetWidthMinusOneValue(Ops.LHS, RHS);
- llvm::Value *Valid = Builder.CreateICmpULE(RHS, WidthMinusOne);
+ SmallVector<std::pair<Value *, SanitizerKind>, 2> Checks;
+ llvm::Value *WidthMinusOne = nullptr;
- 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);
+ if (CGF.SanOpts.has(SanitizerKind::ShiftExponent)) {
+ WidthMinusOne = GetWidthMinusOneValue(Ops.LHS, RHS);
+ llvm::Value *ValidExponent = Builder.CreateICmpULE(RHS, WidthMinusOne);
+ Checks.push_back(
+ std::make_pair(ValidExponent, SanitizerKind::ShiftExponent));
+ }
+ if (CGF.SanOpts.has(SanitizerKind::ShiftBase) &&
+ Ops.Ty->hasSignedIntegerRepresentation()) {
+ if (WidthMinusOne == nullptr)
+ WidthMinusOne = GetWidthMinusOneValue(Ops.LHS, RHS);
// 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",
@@ -2693,19 +2702,14 @@
BitsShiftedOff = Builder.CreateLShr(BitsShiftedOff, One);
}
llvm::Value *Zero = llvm::ConstantInt::get(BitsShiftedOff->getType(), 0);
- 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;
+ llvm::Value *ValidBase = Builder.CreateICmpEQ(BitsShiftedOff, Zero);
+
+ Checks.push_back(std::make_pair(ValidBase, SanitizerKind::ShiftBase));
}
- EmitBinOpCheck(std::make_pair(Valid, SanitizerKind::Shift), Ops);
+ if (!Checks.empty())
+ EmitBinOpCheck(Checks, Ops);
}
- // 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");
return Builder.CreateShl(Ops.LHS, RHS, "shl");
}
@@ -2717,18 +2721,18 @@
if (Ops.LHS->getType() != RHS->getType())
RHS = Builder.CreateIntCast(RHS, Ops.LHS->getType(), false, "sh_prom");
- if (CGF.SanOpts.has(SanitizerKind::Shift) && !CGF.getLangOpts().OpenCL &&
- isa<llvm::IntegerType>(Ops.LHS->getType())) {
+ // OpenCL 6.3j: shift values are effectively % word size of LHS.
+ if (CGF.getLangOpts().OpenCL)
+ RHS =
+ Builder.CreateAnd(RHS, GetWidthMinusOneValue(Ops.LHS, RHS), "shr.mask");
+ else if (CGF.SanOpts.has(SanitizerKind::ShiftExponent) &&
+ isa<llvm::IntegerType>(Ops.LHS->getType())) {
CodeGenFunction::SanitizerScope SanScope(&CGF);
llvm::Value *Valid =
Builder.CreateICmpULE(RHS, GetWidthMinusOneValue(Ops.LHS, RHS));
- EmitBinOpCheck(std::make_pair(Valid, SanitizerKind::Shift), Ops);
+ EmitBinOpCheck(std::make_pair(Valid, SanitizerKind::ShiftExponent), Ops);
}
- // OpenCL 6.3j: shift values are effectively % word size of LHS.
- if (CGF.getLangOpts().OpenCL)
- RHS = Builder.CreateAnd(RHS, GetWidthMinusOneValue(Ops.LHS, RHS), "shr.mask");
-
if (Ops.Ty->hasUnsignedIntegerRepresentation())
return Builder.CreateLShr(Ops.LHS, RHS, "shr");
return Builder.CreateAShr(Ops.LHS, RHS, "shr");