From 322ecd901b52650fcd0ec091a9a01c3483b6f57d Mon Sep 17 00:00:00 2001 From: Filipe Cabecinhas Date: Mon, 12 Dec 2016 16:18:40 +0000 Subject: [PATCH] [clang] Version support for UBSan handlers This adds a way for us to version any UBSan handler by itself. The patch overrides D21289 for a better implementation (we're able to rev up a single handler). After this, then we can land a slight modification of D19667+D19668. We probably don't want to keep all the versions in compiler-rt (maybe we want to deprecate on one release and remove the old handler on the next one?), but with this patch we will loudly fail to compile when mixing incompatible handler calls, instead of silently compiling and then providing bad error messages. Reviewers: kcc, samsonov, rsmith, vsk Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D21695 llvm-svn: 289444 --- clang/lib/CodeGen/CGBuiltin.cpp | 4 +-- clang/lib/CodeGen/CGCall.cpp | 4 +-- clang/lib/CodeGen/CGClass.cpp | 6 ++-- clang/lib/CodeGen/CGExpr.cpp | 54 ++++++++++++++++++++++++----------- clang/lib/CodeGen/CGExprScalar.cpp | 18 ++++++------ clang/lib/CodeGen/CodeGenFunction.cpp | 6 ++-- clang/lib/CodeGen/CodeGenFunction.h | 28 +++++++++++++++++- 7 files changed, 84 insertions(+), 36 deletions(-) diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index 7eaf0f7..8517040 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp @@ -959,8 +959,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD, SanitizerScope SanScope(this); EmitCheck(std::make_pair(static_cast(Builder.getFalse()), SanitizerKind::Unreachable), - "builtin_unreachable", EmitCheckSourceLocation(E->getExprLoc()), - None); + SanitizerHandler::BuiltinUnreachable, + EmitCheckSourceLocation(E->getExprLoc()), None); } else Builder.CreateUnreachable(); diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index 8f61f46..02cbdf0 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -2862,7 +2862,7 @@ void CodeGenFunction::EmitFunctionEpilog(const CGFunctionInfo &FI, EmitCheckSourceLocation(RetNNAttr->getLocation()), }; EmitCheck(std::make_pair(Cond, SanitizerKind::ReturnsNonnullAttribute), - "nonnull_return", StaticData, None); + SanitizerHandler::NonnullReturn, StaticData, None); } } Ret = Builder.CreateRet(RV); @@ -3202,7 +3202,7 @@ void CodeGenFunction::EmitNonNullArgCheck(RValue RV, QualType ArgType, llvm::ConstantInt::get(Int32Ty, ArgNo + 1), }; EmitCheck(std::make_pair(Cond, SanitizerKind::NonnullAttribute), - "nonnull_arg", StaticData, None); + SanitizerHandler::NonnullArg, StaticData, None); } void CodeGenFunction::EmitCallArgs( diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp index 635824e..709c4af 100644 --- a/clang/lib/CodeGen/CGClass.cpp +++ b/clang/lib/CodeGen/CGClass.cpp @@ -2802,8 +2802,8 @@ void CodeGenFunction::EmitVTablePtrCheck(const CXXRecordDecl *RD, llvm::MDString::get(CGM.getLLVMContext(), "all-vtables")); llvm::Value *ValidVtable = Builder.CreateCall( CGM.getIntrinsic(llvm::Intrinsic::type_test), {CastedVTable, AllVtables}); - EmitCheck(std::make_pair(TypeTest, M), "cfi_check_fail", StaticData, - {CastedVTable, ValidVtable}); + EmitCheck(std::make_pair(TypeTest, M), SanitizerHandler::CFICheckFail, + StaticData, {CastedVTable, ValidVtable}); } bool CodeGenFunction::ShouldEmitVTableTypeCheckedLoad(const CXXRecordDecl *RD) { @@ -2835,7 +2835,7 @@ llvm::Value *CodeGenFunction::EmitVTableTypeCheckedLoad( llvm::Value *CheckResult = Builder.CreateExtractValue(CheckedLoad, 1); EmitCheck(std::make_pair(CheckResult, SanitizerKind::CFIVCall), - "cfi_check_fail", nullptr, nullptr); + SanitizerHandler::CFICheckFail, nullptr, nullptr); return Builder.CreateBitCast( Builder.CreateExtractValue(CheckedLoad, 0), diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index dc147a1..a35b652 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -608,7 +608,7 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc, llvm::ConstantInt::get(SizeTy, AlignVal), llvm::ConstantInt::get(Int8Ty, TCK) }; - EmitCheck(Checks, "type_mismatch", StaticData, Ptr); + EmitCheck(Checks, SanitizerHandler::TypeMismatch, StaticData, Ptr); } // If possible, check that the vptr indicates that there is a subobject of @@ -676,7 +676,8 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc, }; llvm::Value *DynamicData[] = { Ptr, Hash }; EmitCheck(std::make_pair(EqualHash, SanitizerKind::Vptr), - "dynamic_type_cache_miss", StaticData, DynamicData); + SanitizerHandler::DynamicTypeCacheMiss, StaticData, + DynamicData); } } @@ -766,8 +767,8 @@ void CodeGenFunction::EmitBoundsCheck(const Expr *E, const Expr *Base, }; llvm::Value *Check = Accessed ? Builder.CreateICmpULT(IndexVal, BoundVal) : Builder.CreateICmpULE(IndexVal, BoundVal); - EmitCheck(std::make_pair(Check, SanitizerKind::ArrayBounds), "out_of_bounds", - StaticData, Index); + EmitCheck(std::make_pair(Check, SanitizerKind::ArrayBounds), + SanitizerHandler::OutOfBounds, StaticData, Index); } @@ -1339,8 +1340,8 @@ llvm::Value *CodeGenFunction::EmitLoadOfScalar(Address Addr, bool Volatile, EmitCheckTypeDescriptor(Ty) }; SanitizerMask Kind = NeedsEnumCheck ? SanitizerKind::Enum : SanitizerKind::Bool; - EmitCheck(std::make_pair(Check, Kind), "load_invalid_value", StaticArgs, - EmitCheckValue(Load)); + EmitCheck(std::make_pair(Check, Kind), SanitizerHandler::LoadInvalidValue, + StaticArgs, EmitCheckValue(Load)); } } else if (CGM.getCodeGenOpts().OptimizationLevel > 0) if (llvm::MDNode *RangeInfo = getRangeForLoadFromType(Ty)) @@ -2500,17 +2501,35 @@ static CheckRecoverableKind getRecoverableKind(SanitizerMask Kind) { } } +namespace { +struct SanitizerHandlerInfo { + char const *const Name; + unsigned Version; +}; +}; + +const SanitizerHandlerInfo SanitizerHandlers[] = { +#define SANITIZER_CHECK(Enum, Name, Version) {#Name, Version}, + LIST_SANITIZER_CHECKS +#undef SANITIZER_CHECK +}; + static void emitCheckHandlerCall(CodeGenFunction &CGF, llvm::FunctionType *FnType, ArrayRef FnArgs, - StringRef CheckName, + SanitizerHandler CheckHandler, CheckRecoverableKind RecoverKind, bool IsFatal, llvm::BasicBlock *ContBB) { assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable); bool NeedsAbortSuffix = IsFatal && RecoverKind != CheckRecoverableKind::Unrecoverable; - std::string FnName = ("__ubsan_handle_" + CheckName + - (NeedsAbortSuffix ? "_abort" : "")).str(); + const SanitizerHandlerInfo &CheckInfo = SanitizerHandlers[CheckHandler]; + const StringRef CheckName = CheckInfo.Name; + std::string FnName = + ("__ubsan_handle_" + CheckName + + (CheckInfo.Version ? "_v" + std::to_string(CheckInfo.Version) : "") + + (NeedsAbortSuffix ? "_abort" : "")) + .str(); bool MayReturn = !IsFatal || RecoverKind == CheckRecoverableKind::AlwaysRecoverable; @@ -2536,10 +2555,13 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF, void CodeGenFunction::EmitCheck( ArrayRef> Checked, - StringRef CheckName, ArrayRef StaticArgs, + SanitizerHandler CheckHandler, ArrayRef StaticArgs, ArrayRef DynamicArgs) { assert(IsSanitizerScope); assert(Checked.size() > 0); + assert(CheckHandler >= 0 && + CheckHandler < sizeof(SanitizerHandlers) / sizeof(*SanitizerHandlers)); + const StringRef CheckName = SanitizerHandlers[CheckHandler].Name; llvm::Value *FatalCond = nullptr; llvm::Value *RecoverableCond = nullptr; @@ -2619,7 +2641,7 @@ void CodeGenFunction::EmitCheck( if (!FatalCond || !RecoverableCond) { // Simple case: we need to generate a single handler call, either // fatal, or non-fatal. - emitCheckHandlerCall(*this, FnType, Args, CheckName, RecoverKind, + emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind, (FatalCond != nullptr), Cont); } else { // Emit two handler calls: first one for set of unrecoverable checks, @@ -2629,10 +2651,10 @@ void CodeGenFunction::EmitCheck( llvm::BasicBlock *FatalHandlerBB = createBasicBlock("fatal." + CheckName); Builder.CreateCondBr(FatalCond, NonFatalHandlerBB, FatalHandlerBB); EmitBlock(FatalHandlerBB); - emitCheckHandlerCall(*this, FnType, Args, CheckName, RecoverKind, true, + emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind, true, NonFatalHandlerBB); EmitBlock(NonFatalHandlerBB); - emitCheckHandlerCall(*this, FnType, Args, CheckName, RecoverKind, false, + emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind, false, Cont); } @@ -2757,7 +2779,7 @@ void CodeGenFunction::EmitCfiCheckFail() { llvm::Value *Cond = Builder.CreateICmpNE(CheckKind, llvm::ConstantInt::get(Int8Ty, Kind)); if (CGM.getLangOpts().Sanitize.has(Mask)) - EmitCheck(std::make_pair(Cond, Mask), "cfi_check_fail", {}, + EmitCheck(std::make_pair(Cond, Mask), SanitizerHandler::CFICheckFail, {}, {Data, Addr, ValidVtable}); else EmitTrapCheck(Cond); @@ -4127,7 +4149,7 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, const CGCallee &OrigCallee EmitCheckTypeDescriptor(CalleeType) }; EmitCheck(std::make_pair(CalleeRTTIMatch, SanitizerKind::Function), - "function_type_mismatch", StaticData, CalleePtr); + SanitizerHandler::FunctionTypeMismatch, StaticData, CalleePtr); Builder.CreateBr(Cont); EmitBlock(Cont); @@ -4160,7 +4182,7 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, const CGCallee &OrigCallee CastedCallee, StaticData); } else { EmitCheck(std::make_pair(TypeTest, SanitizerKind::CFIICall), - "cfi_check_fail", StaticData, + SanitizerHandler::CFICheckFail, StaticData, {CastedCallee, llvm::UndefValue::get(IntPtrTy)}); } } diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index c15e580..e9bdbda 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -731,7 +731,7 @@ void ScalarExprEmitter::EmitFloatConversionCheck( CGF.EmitCheckTypeDescriptor(OrigSrcType), CGF.EmitCheckTypeDescriptor(DstType)}; CGF.EmitCheck(std::make_pair(Check, SanitizerKind::FloatCastOverflow), - "float_cast_overflow", StaticArgs, OrigSrc); + SanitizerHandler::FloatCastOverflow, StaticArgs, OrigSrc); } /// Emit a conversion from the specified type to the specified destination type, @@ -934,7 +934,7 @@ Value *ScalarExprEmitter::EmitNullValue(QualType Ty) { void ScalarExprEmitter::EmitBinOpCheck( ArrayRef> Checks, const BinOpInfo &Info) { assert(CGF.IsSanitizerScope); - StringRef CheckName; + SanitizerHandler Check; SmallVector StaticData; SmallVector DynamicData; @@ -945,13 +945,13 @@ void ScalarExprEmitter::EmitBinOpCheck( StaticData.push_back(CGF.EmitCheckSourceLocation(Info.E->getExprLoc())); const UnaryOperator *UO = dyn_cast(Info.E); if (UO && UO->getOpcode() == UO_Minus) { - CheckName = "negate_overflow"; + Check = SanitizerHandler::NegateOverflow; StaticData.push_back(CGF.EmitCheckTypeDescriptor(UO->getType())); DynamicData.push_back(Info.RHS); } else { if (BinaryOperator::isShiftOp(Opcode)) { // Shift LHS negative or too large, or RHS out of bounds. - CheckName = "shift_out_of_bounds"; + Check = SanitizerHandler::ShiftOutOfBounds; const BinaryOperator *BO = cast(Info.E); StaticData.push_back( CGF.EmitCheckTypeDescriptor(BO->getLHS()->getType())); @@ -959,14 +959,14 @@ void ScalarExprEmitter::EmitBinOpCheck( CGF.EmitCheckTypeDescriptor(BO->getRHS()->getType())); } else if (Opcode == BO_Div || Opcode == BO_Rem) { // Divide or modulo by zero, or signed overflow (eg INT_MAX / -1). - CheckName = "divrem_overflow"; + Check = SanitizerHandler::DivremOverflow; StaticData.push_back(CGF.EmitCheckTypeDescriptor(Info.Ty)); } else { // Arithmetic overflow (+, -, *). switch (Opcode) { - case BO_Add: CheckName = "add_overflow"; break; - case BO_Sub: CheckName = "sub_overflow"; break; - case BO_Mul: CheckName = "mul_overflow"; break; + case BO_Add: Check = SanitizerHandler::AddOverflow; break; + case BO_Sub: Check = SanitizerHandler::SubOverflow; break; + case BO_Mul: Check = SanitizerHandler::MulOverflow; break; default: llvm_unreachable("unexpected opcode for bin op check"); } StaticData.push_back(CGF.EmitCheckTypeDescriptor(Info.Ty)); @@ -975,7 +975,7 @@ void ScalarExprEmitter::EmitBinOpCheck( DynamicData.push_back(Info.RHS); } - CGF.EmitCheck(Checks, CheckName, StaticData, DynamicData); + CGF.EmitCheck(Checks, Check, StaticData, DynamicData); } //===----------------------------------------------------------------------===// diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 1507e8e..de2537d 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -1149,8 +1149,8 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn, SanitizerScope SanScope(this); llvm::Value *IsFalse = Builder.getFalse(); EmitCheck(std::make_pair(IsFalse, SanitizerKind::Return), - "missing_return", EmitCheckSourceLocation(FD->getLocation()), - None); + SanitizerHandler::MissingReturn, + EmitCheckSourceLocation(FD->getLocation()), None); } else if (CGM.getCodeGenOpts().OptimizationLevel == 0) { EmitTrapCall(llvm::Intrinsic::trap); } @@ -1858,7 +1858,7 @@ void CodeGenFunction::EmitVariablyModifiedType(QualType type) { }; EmitCheck(std::make_pair(Builder.CreateICmpSGT(Size, Zero), SanitizerKind::VLABound), - "vla_bound_not_positive", StaticArgs, Size); + SanitizerHandler::VLABoundNotPositive, StaticArgs, Size); } // Always zexting here would be wrong if it weren't diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index b9b33a8..eddaf65 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -103,6 +103,32 @@ enum TypeEvaluationKind { TEK_Aggregate }; +#define LIST_SANITIZER_CHECKS \ + SANITIZER_CHECK(AddOverflow, add_overflow, 0) \ + SANITIZER_CHECK(BuiltinUnreachable, builtin_unreachable, 0) \ + SANITIZER_CHECK(CFICheckFail, cfi_check_fail, 0) \ + SANITIZER_CHECK(DivremOverflow, divrem_overflow, 0) \ + SANITIZER_CHECK(DynamicTypeCacheMiss, dynamic_type_cache_miss, 0) \ + SANITIZER_CHECK(FloatCastOverflow, float_cast_overflow, 0) \ + SANITIZER_CHECK(FunctionTypeMismatch, function_type_mismatch, 0) \ + SANITIZER_CHECK(LoadInvalidValue, load_invalid_value, 0) \ + SANITIZER_CHECK(MissingReturn, missing_return, 0) \ + SANITIZER_CHECK(MulOverflow, mul_overflow, 0) \ + SANITIZER_CHECK(NegateOverflow, negate_overflow, 0) \ + SANITIZER_CHECK(NonnullArg, nonnull_arg, 0) \ + SANITIZER_CHECK(NonnullReturn, nonnull_return, 0) \ + SANITIZER_CHECK(OutOfBounds, out_of_bounds, 0) \ + SANITIZER_CHECK(ShiftOutOfBounds, shift_out_of_bounds, 0) \ + SANITIZER_CHECK(SubOverflow, sub_overflow, 0) \ + SANITIZER_CHECK(TypeMismatch, type_mismatch, 0) \ + SANITIZER_CHECK(VLABoundNotPositive, vla_bound_not_positive, 0) + +enum SanitizerHandler { +#define SANITIZER_CHECK(Enum, Name, Version) Enum, + LIST_SANITIZER_CHECKS +#undef SANITIZER_CHECK +}; + /// CodeGenFunction - This class organizes the per-function state that is used /// while generating LLVM code. class CodeGenFunction : public CodeGenTypeCache { @@ -3376,7 +3402,7 @@ public: /// sanitizer runtime with the provided arguments, and create a conditional /// branch to it. void EmitCheck(ArrayRef> Checked, - StringRef CheckName, ArrayRef StaticArgs, + SanitizerHandler Check, ArrayRef StaticArgs, ArrayRef DynamicArgs); /// \brief Emit a slow path cross-DSO CFI check which calls __cfi_slowpath -- 2.7.4