krasin updated this revision to Diff 77941.
krasin added a comment.
Do better job with destructors.
https://reviews.llvm.org/D26559
Files:
lib/CodeGen/CGExprCXX.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGenCXX/ubsan-vtable-checks.cpp
Index: test/CodeGenCXX/ubsan-vtable-checks.cpp
===================================================================
--- /dev/null
+++ test/CodeGenCXX/ubsan-vtable-checks.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=MSABI
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=MSABI
+struct T {
+ virtual ~T() {}
+ virtual int v() { return 1; }
+};
+
+struct U : T {
+ ~U();
+ virtual int v() { return 2; }
+};
+
+U::~U() {}
+
+// ITANIUM: define i32 @_Z5get_vP1T
+// MSABI: define i32 @"\01?get_v
+int get_v(T* t) {
+ // First, we check that vtable is not loaded before a type check.
+ // CHECK-NULL-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+ // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* %{{[_a-z0-9]+}}, null
+ // CHECK-NULL-NEXT: br i1 [[UBSAN_CMP_RES]], label %cont, label %handler.type_mismatch
+ // CHECK-NULL: call void @__ubsan_handle_type_mismatch_abort
+ // Second, we check that vtable is actually loaded once the type check is done.
+ // CHECK-NULL: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+ return t->v();
+}
+
+// ITANIUM: define void @_Z9delete_itP1T
+// MSABI: define void @"\01?delete_it
+void delete_it(T *t) {
+ // First, we check that vtable is not loaded before a type check.
+ // CHECK-VPTR-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+ // CHECK-VPTR: br i1 {{.*}} label %handler.dynamic_type_cache_miss
+ // CHECK-VPTR: call void @__ubsan_handle_dynamic_type_cache_miss_abort
+ // Second, we check that vtable is actually loaded once the type check is done.
+ // CHECK-VPTR: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+ delete t;
+}
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===================================================================
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1817,16 +1817,21 @@
assert(CE == nullptr || CE->arg_begin() == CE->arg_end());
assert(DtorType == Dtor_Deleting || DtorType == Dtor_Complete);
+ ASTContext &Context = getContext();
+ SourceLocation CallLoc = CE ? CE->getLocStart() : SourceLocation();
+ CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall,
+ CallLoc, This.getPointer(),
+ Context.getRecordType(Dtor->getParent()));
+
// We have only one destructor in the vftable but can get both behaviors
// by passing an implicit int parameter.
GlobalDecl GD(Dtor, Dtor_Deleting);
const CGFunctionInfo *FInfo = &CGM.getTypes().arrangeCXXStructorDeclaration(
Dtor, StructorType::Deleting);
llvm::Type *Ty = CGF.CGM.getTypes().GetFunctionType(*FInfo);
CGCallee Callee = getVirtualFunctionPointer(
- CGF, GD, This, Ty, CE ? CE->getLocStart() : SourceLocation());
+ CGF, GD, This, Ty, CallLoc);
- ASTContext &Context = getContext();
llvm::Value *ImplicitParam = llvm::ConstantInt::get(
llvm::IntegerType::getInt32Ty(CGF.getLLVMContext()),
DtorType == Dtor_Deleting);
Index: lib/CodeGen/CGExprCXX.cpp
===================================================================
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -35,17 +35,6 @@
"Trying to emit a member or operator call expr on a static method!");
ASTContext &C = CGF.getContext();
- // C++11 [class.mfct.non-static]p2:
- // If a non-static member function of a class X is called for an object that
- // is not of type X, or of a type derived from X, the behavior is undefined.
- SourceLocation CallLoc;
- if (CE)
- CallLoc = CE->getExprLoc();
- CGF.EmitTypeCheck(isa<CXXConstructorDecl>(MD)
- ? CodeGenFunction::TCK_ConstructorCall
- : CodeGenFunction::TCK_MemberCall,
- CallLoc, This, C.getRecordType(MD->getParent()));
-
// Push the this ptr.
const CXXRecordDecl *RD =
CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD);
@@ -293,6 +282,19 @@
llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo);
+ // C++11 [class.mfct.non-static]p2:
+ // If a non-static member function of a class X is called for an object that
+ // is not of type X, or of a type derived from X, the behavior is undefined.
+ SourceLocation CallLoc;
+ ASTContext &C = getContext();
+ if (CE)
+ CallLoc = CE->getExprLoc();
+
+ EmitTypeCheck(isa<CXXConstructorDecl>(CalleeDecl)
+ ? CodeGenFunction::TCK_ConstructorCall
+ : CodeGenFunction::TCK_MemberCall,
+ CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent()));
+
// FIXME: Uses of 'MD' past this point need to be audited. We may need to use
// 'CalleeDecl' instead.
@@ -1923,6 +1925,12 @@
if (E->isArrayForm()) {
EmitArrayDelete(*this, E, Ptr, DeleteTy);
} else {
+ SourceLocation CallLoc;
+ if (E)
+ CallLoc = E->getExprLoc();
+ EmitTypeCheck(CodeGenFunction::TCK_MemberCall,
+ CallLoc, Ptr.getPointer(),
+ DeleteTy);
EmitObjectDelete(*this, E, Ptr, DeleteTy);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
