From 2c5868c334646d4cdc92589cea393a5012cdd813 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Wed, 13 Feb 2013 21:18:23 +0000 Subject: [PATCH] ubsan: Add checking for invalid downcasts. Per [expr.static.cast]p2 and p11, base-to-derived casts have undefined behavior if the object is not actually an instance of the derived type. llvm-svn: 175078 --- clang/lib/CodeGen/CGClass.cpp | 2 +- clang/lib/CodeGen/CGExpr.cpp | 27 +++++++++++++-- clang/lib/CodeGen/CGExprScalar.cpp | 10 +++++- clang/lib/CodeGen/CodeGenFunction.h | 8 ++++- clang/test/CodeGenCXX/catch-undef-behavior.cpp | 46 ++++++++++++++++++++++++++ 5 files changed, 88 insertions(+), 5 deletions(-) diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp index 4e4acc8..d847469 100644 --- a/clang/lib/CodeGen/CGClass.cpp +++ b/clang/lib/CodeGen/CGClass.cpp @@ -232,7 +232,7 @@ CodeGenFunction::GetAddressOfDerivedClass(llvm::Value *Value, QualType DerivedTy = getContext().getCanonicalType(getContext().getTagDeclType(Derived)); llvm::Type *DerivedPtrTy = ConvertType(DerivedTy)->getPointerTo(); - + llvm::Value *NonVirtualOffset = CGM.GetNonVirtualBaseClassOffset(Derived, PathBegin, PathEnd); diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 41518fb..ba400b8 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -501,11 +501,22 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc, return; llvm::Value *Cond = 0; + llvm::BasicBlock *Done = 0; if (SanOpts->Null) { // 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. + // Skip the remaining checks in that case. + Done = createBasicBlock("null"); + llvm::BasicBlock *Rest = createBasicBlock("not.null"); + Builder.CreateCondBr(Cond, Rest, Done); + EmitBlock(Rest); + Cond = 0; + } } if (SanOpts->ObjectSize && !Ty->isIncompleteType()) { @@ -561,7 +572,8 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc, // or call a non-static member function CXXRecordDecl *RD = Ty->getAsCXXRecordDecl(); if (SanOpts->Vptr && - (TCK == TCK_MemberAccess || TCK == TCK_MemberCall) && + (TCK == TCK_MemberAccess || TCK == TCK_MemberCall || + TCK == TCK_DowncastPointer || TCK == TCK_DowncastReference) && RD && RD->hasDefinition() && RD->isDynamicClass()) { // Compute a hash of the mangled name of the type. // @@ -611,6 +623,11 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc, "dynamic_type_cache_miss", StaticData, DynamicData, CRK_AlwaysRecoverable); } + + if (Done) { + Builder.CreateBr(Done); + EmitBlock(Done); + } } @@ -2638,7 +2655,13 @@ LValue CodeGenFunction::EmitCastLValue(const CastExpr *E) { cast(DerivedClassTy->getDecl()); LValue LV = EmitLValue(E->getSubExpr()); - + + // C++11 [expr.static.cast]p2: Behavior is undefined if a downcast is + // performed and the object is not of the derived type. + if (SanitizePerformTypeCheck) + EmitTypeCheck(TCK_DowncastReference, E->getExprLoc(), + LV.getAddress(), E->getType()); + // Perform the base-to-derived conversion llvm::Value *Derived = GetAddressOfDerivedClass(LV.getAddress(), DerivedClassDecl, diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 49494be..7b86c04 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -1220,7 +1220,15 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) { const CXXRecordDecl *DerivedClassDecl = DestTy->getPointeeCXXRecordDecl(); assert(DerivedClassDecl && "BaseToDerived arg isn't a C++ object pointer!"); - return CGF.GetAddressOfDerivedClass(Visit(E), DerivedClassDecl, + llvm::Value *V = Visit(E); + + // C++11 [expr.static.cast]p11: Behavior is undefined if a downcast is + // performed and the object is not of the derived type. + if (CGF.SanitizePerformTypeCheck) + CGF.EmitTypeCheck(CodeGenFunction::TCK_DowncastPointer, CE->getExprLoc(), + V, DestTy->getPointeeType()); + + return CGF.GetAddressOfDerivedClass(V, DerivedClassDecl, CE->path_begin(), CE->path_end(), ShouldNullCheckClassCastValue(CE)); } diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index ea5e873..e85c23e 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -1895,7 +1895,13 @@ public: /// Must be an object within its lifetime. TCK_MemberCall, /// Checking the 'this' pointer for a constructor call. - TCK_ConstructorCall + TCK_ConstructorCall, + /// Checking the operand of a static_cast to a derived pointer type. Must be + /// null or an object within its lifetime. + TCK_DowncastPointer, + /// Checking the operand of a static_cast to a derived reference type. Must + /// be an object within its lifetime. + TCK_DowncastReference }; /// \brief Emit a check that \p V is the address of storage of the diff --git a/clang/test/CodeGenCXX/catch-undef-behavior.cpp b/clang/test/CodeGenCXX/catch-undef-behavior.cpp index 3a3536b..05621cc 100644 --- a/clang/test/CodeGenCXX/catch-undef-behavior.cpp +++ b/clang/test/CodeGenCXX/catch-undef-behavior.cpp @@ -6,6 +6,8 @@ struct S { virtual int f(); }; +struct T : S {}; + // CHECK: @_Z17reference_binding void reference_binding(int *p, S *q) { // C++ core issue 453: If an lvalue to which a reference is directly bound @@ -173,3 +175,47 @@ int bad_enum_value() { int c = e3; return a + b + c; } + +// CHECK: @_Z20bad_downcast_pointer +void bad_downcast_pointer(S *p) { + // CHECK: %[[NONNULL:.*]] = icmp ne {{.*}}, null + // CHECK: br i1 %[[NONNULL]], + + // CHECK: %[[SIZE:.*]] = call i64 @llvm.objectsize.i64( + // CHECK: %[[E1:.*]] = icmp uge i64 %[[SIZE]], 24 + // CHECK: %[[MISALIGN:.*]] = and i64 %{{.*}}, 7 + // CHECK: %[[E2:.*]] = icmp eq i64 %[[MISALIGN]], 0 + // CHECK: %[[E12:.*]] = and i1 %[[E1]], %[[E2]] + // CHECK: br i1 %[[E12]], + + // CHECK: call void @__ubsan_handle_type_mismatch + // CHECK: br label + + // CHECK: br i1 %{{.*}}, + + // CHECK: call void @__ubsan_handle_dynamic_type_cache_miss + // CHECK: br label + (void) static_cast(p); +} + +// CHECK: @_Z22bad_downcast_reference +void bad_downcast_reference(S &p) { + // CHECK: %[[E1:.*]] = icmp ne {{.*}}, null + // CHECK-NOT: br i1 + // CHECK: %[[SIZE:.*]] = call i64 @llvm.objectsize.i64( + // CHECK: %[[E2:.*]] = icmp uge i64 %[[SIZE]], 24 + // CHECK: %[[E12:.*]] = and i1 %[[E1]], %[[E2]] + // CHECK: %[[MISALIGN:.*]] = and i64 %{{.*}}, 7 + // CHECK: %[[E3:.*]] = icmp eq i64 %[[MISALIGN]], 0 + // CHECK: %[[E123:.*]] = and i1 %[[E12]], %[[E3]] + // CHECK: br i1 %[[E123]], + + // CHECK: call void @__ubsan_handle_type_mismatch + // CHECK: br label + + // CHECK: br i1 %{{.*}}, + + // CHECK: call void @__ubsan_handle_dynamic_type_cache_miss + // CHECK: br label + (void) static_cast(p); +} -- 2.7.4