From: Alexey Samsonov Date: Mon, 13 Oct 2014 23:59:00 +0000 (+0000) Subject: Sanitize upcasts and conversion to virtual base. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=52de81f78d920e5b65954f34119345ba485ef60c;p=clang Sanitize upcasts and conversion to virtual base. This change adds UBSan check to upcasts. Namely, when we perform derived-to-base conversion, we: 1) check that the pointer-to-derived has suitable alignment and underlying storage, if this pointer is non-null. 2) if vptr-sanitizer is enabled, and we perform conversion to virtual base, we check that pointer-to-derived has a matching vptr. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@219642 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/CodeGen/CGClass.cpp b/lib/CodeGen/CGClass.cpp index 49760853dc..d7010c7e91 100644 --- a/lib/CodeGen/CGClass.cpp +++ b/lib/CodeGen/CGClass.cpp @@ -134,12 +134,11 @@ ApplyNonVirtualAndVirtualOffset(CodeGenFunction &CGF, llvm::Value *ptr, return ptr; } -llvm::Value * -CodeGenFunction::GetAddressOfBaseClass(llvm::Value *Value, - const CXXRecordDecl *Derived, - CastExpr::path_const_iterator PathBegin, - CastExpr::path_const_iterator PathEnd, - bool NullCheckValue) { +llvm::Value *CodeGenFunction::GetAddressOfBaseClass( + llvm::Value *Value, const CXXRecordDecl *Derived, + CastExpr::path_const_iterator PathBegin, + CastExpr::path_const_iterator PathEnd, bool NullCheckValue, + SourceLocation Loc) { assert(PathBegin != PathEnd && "Base path should not be empty!"); CastExpr::path_const_iterator Start = PathBegin; @@ -176,9 +175,16 @@ CodeGenFunction::GetAddressOfBaseClass(llvm::Value *Value, llvm::Type *BasePtrTy = ConvertType((PathEnd[-1])->getType())->getPointerTo(); + QualType DerivedTy = getContext().getRecordType(Derived); + CharUnits DerivedAlign = getContext().getTypeAlignInChars(DerivedTy); + // If the static offset is zero and we don't have a virtual step, // just do a bitcast; null checks are unnecessary. if (NonVirtualOffset.isZero() && !VBase) { + if (sanitizePerformTypeCheck()) { + EmitTypeCheck(TCK_Upcast, Loc, Value, DerivedTy, DerivedAlign, + !NullCheckValue); + } return Builder.CreateBitCast(Value, BasePtrTy); } @@ -197,6 +203,11 @@ CodeGenFunction::GetAddressOfBaseClass(llvm::Value *Value, EmitBlock(notNullBB); } + if (sanitizePerformTypeCheck()) { + EmitTypeCheck(VBase ? TCK_UpcastToVirtualBase : TCK_Upcast, Loc, Value, + DerivedTy, DerivedAlign, true); + } + // Compute the virtual offset. llvm::Value *VirtualOffset = nullptr; if (VBase) { diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp index 54c2fd2647..b7504ca122 100644 --- a/lib/CodeGen/CGExpr.cpp +++ b/lib/CodeGen/CGExpr.cpp @@ -377,7 +377,7 @@ LValue CodeGenFunction::EmitMaterializeTemporaryExpr( GetAddressOfBaseClass(Object, Adjustment.DerivedToBase.DerivedClass, Adjustment.DerivedToBase.BasePath->path_begin(), Adjustment.DerivedToBase.BasePath->path_end(), - /*NullCheckValue=*/ false); + /*NullCheckValue=*/ false, E->getExprLoc()); break; case SubobjectAdjustment::FieldAdjustment: { @@ -448,8 +448,8 @@ bool CodeGenFunction::sanitizePerformTypeCheck() const { } void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc, - llvm::Value *Address, - QualType Ty, CharUnits Alignment) { + llvm::Value *Address, QualType Ty, + CharUnits Alignment, bool SkipNullCheck) { if (!sanitizePerformTypeCheck()) return; @@ -464,13 +464,15 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc, llvm::Value *Cond = nullptr; llvm::BasicBlock *Done = nullptr; - if (SanOpts->Null || TCK == TCK_DowncastPointer) { + bool AllowNullPointers = TCK == TCK_DowncastPointer || TCK == TCK_Upcast || + TCK == TCK_UpcastToVirtualBase; + if ((SanOpts->Null || AllowNullPointers) && !SkipNullCheck) { // The glvalue must not be an empty glvalue. Cond = Builder.CreateICmpNE( Address, llvm::Constant::getNullValue(Address->getType())); - if (TCK == TCK_DowncastPointer) { - // When performing a pointer downcast, it's OK if the value is null. + if (AllowNullPointers) { + // When performing pointer casts, it's OK if the value is null. // Skip the remaining checks in that case. Done = createBasicBlock("null"); llvm::BasicBlock *Rest = createBasicBlock("not.null"); @@ -536,7 +538,8 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc, CXXRecordDecl *RD = Ty->getAsCXXRecordDecl(); if (SanOpts->Vptr && (TCK == TCK_MemberAccess || TCK == TCK_MemberCall || - TCK == TCK_DowncastPointer || TCK == TCK_DowncastReference) && + TCK == TCK_DowncastPointer || TCK == TCK_DowncastReference || + TCK == TCK_UpcastToVirtualBase) && RD && RD->hasDefinition() && RD->isDynamicClass()) { // Compute a hash of the mangled name of the type. // @@ -2874,10 +2877,9 @@ LValue CodeGenFunction::EmitCastLValue(const CastExpr *E) { llvm::Value *This = LV.getAddress(); // Perform the derived-to-base conversion - llvm::Value *Base = - GetAddressOfBaseClass(This, DerivedClassDecl, - E->path_begin(), E->path_end(), - /*NullCheckValue=*/false); + llvm::Value *Base = GetAddressOfBaseClass( + This, DerivedClassDecl, E->path_begin(), E->path_end(), + /*NullCheckValue=*/false, E->getExprLoc()); return MakeAddrLValue(Base, E->getType()); } diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp index 5ca2414efa..8d8fd39cb0 100644 --- a/lib/CodeGen/CGExprScalar.cpp +++ b/lib/CodeGen/CGExprScalar.cpp @@ -1388,9 +1388,9 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) { E->getType()->getPointeeCXXRecordDecl(); assert(DerivedClassDecl && "DerivedToBase arg isn't a C++ object pointer!"); - return CGF.GetAddressOfBaseClass(Visit(E), DerivedClassDecl, - CE->path_begin(), CE->path_end(), - ShouldNullCheckClassCastValue(CE)); + return CGF.GetAddressOfBaseClass( + Visit(E), DerivedClassDecl, CE->path_begin(), CE->path_end(), + ShouldNullCheckClassCastValue(CE), CE->getExprLoc()); } case CK_Dynamic: { Value *V = Visit(const_cast(E)); diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h index f20b758a3b..865f1c9668 100644 --- a/lib/CodeGen/CodeGenFunction.h +++ b/lib/CodeGen/CodeGenFunction.h @@ -1681,7 +1681,7 @@ public: const CXXRecordDecl *Derived, CastExpr::path_const_iterator PathBegin, CastExpr::path_const_iterator PathEnd, - bool NullCheckValue); + bool NullCheckValue, SourceLocation Loc); llvm::Value *GetAddressOfDerivedClass(llvm::Value *Value, const CXXRecordDecl *Derived, @@ -1776,7 +1776,13 @@ public: TCK_DowncastPointer, /// Checking the operand of a static_cast to a derived reference type. Must /// be an object within its lifetime. - TCK_DowncastReference + TCK_DowncastReference, + /// Checking the operand of a cast to a base object. Must be suitably sized + /// and aligned. + TCK_Upcast, + /// Checking the operand of a cast to a virtual base object. Must be an + /// object within its lifetime. + TCK_UpcastToVirtualBase }; /// \brief Whether any type-checking sanitizers are enabled. If \c false, @@ -1786,7 +1792,8 @@ public: /// \brief Emit a check that \p V is the address of storage of the /// appropriate size and alignment for an object of type \p Type. void EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc, llvm::Value *V, - QualType Type, CharUnits Alignment = CharUnits::Zero()); + QualType Type, CharUnits Alignment = CharUnits::Zero(), + bool SkipNullCheck = false); /// \brief Emit a check that \p Base points into an array object, which /// we can access at index \p Index. \p Accessed should be \c false if we diff --git a/test/CodeGenCXX/catch-undef-behavior.cpp b/test/CodeGenCXX/catch-undef-behavior.cpp index 333855d0ba..5bf31c1ca4 100644 --- a/test/CodeGenCXX/catch-undef-behavior.cpp +++ b/test/CodeGenCXX/catch-undef-behavior.cpp @@ -404,6 +404,40 @@ void indirect_function_call(void (*p)(int)) { p(42); } +namespace UpcastPointerTest { +struct S {}; +struct T : S { double d; }; +struct V : virtual S {}; + +// CHECK-LABEL: upcast_pointer +S* upcast_pointer(T* t) { + // Check for null pointer + // CHECK: %[[NONNULL:.*]] = icmp ne {{.*}}, null + // CHECK: br i1 %[[NONNULL]] + + // Check alignment + // CHECK: %[[MISALIGN:.*]] = and i64 %{{.*}}, 7 + // CHECK: icmp eq i64 %[[MISALIGN]], 0 + + // CHECK: call void @__ubsan_handle_type_mismatch + return t; +} + +V getV(); + +// CHECK-LABEL: upcast_to_vbase +void upcast_to_vbase() { + // No need to check for null here, as we have a temporary here. + + // CHECK-NOT: br i1 + + // CHECK: call i64 @llvm.objectsize + // CHECK: call void @__ubsan_handle_type_mismatch + // CHECK: call void @__ubsan_handle_dynamic_type_cache_miss + const S& s = getV(); +} +} + namespace CopyValueRepresentation { // CHECK-LABEL: define {{.*}} @_ZN23CopyValueRepresentation2S3aSERKS0_ // CHECK-NOT: call {{.*}} @__ubsan_handle_load_invalid_value