Insert a type check before reading vtable.

Summary:
this is to prevent a situation when a pointer is invalid or null,
but we get to reading from vtable before we can check that
(possibly causing a segfault without a good diagnostics).

Reviewers: pcc

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D26559

llvm-svn: 287181

2 files changed, 62 insertions, 11 deletions

diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp
index 88a5f35d2dc..ab2468ef61e 100644
--- a/clang/lib/CodeGen/CGExprCXX.cpp
+++ b/clang/lib/CodeGen/CGExprCXX.cpp
@@ -35,17 +35,6 @@ commonEmitCXXMemberOrOperatorCall(CodeGenFunction &CGF, const CXXMethodDecl *MD,
          "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 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr(
 
   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.
 
@@ -1763,6 +1765,15 @@ static void EmitObjectDelete(CodeGenFunction &CGF,
                              const CXXDeleteExpr *DE,
                              Address Ptr,
                              QualType ElementType) {
+  // C++11 [expr.delete]p3:
+  //   If the static type of the object to be deleted is different from its
+  //   dynamic type, the static type shall be a base class of the dynamic type
+  //   of the object to be deleted and the static type shall have a virtual
+  //   destructor or the behavior is undefined.
+  CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall,
+                    DE->getExprLoc(), Ptr.getPointer(),
+                    ElementType);
+
   // Find the destructor for the type, if applicable.  If the
   // destructor is virtual, we'll just emit the vcall and return.
   const CXXDestructorDecl *Dtor = nullptr;
diff --git a/clang/test/CodeGenCXX/ubsan-vtable-checks.cpp b/clang/test/CodeGenCXX/ubsan-vtable-checks.cpp
new file mode 100644
index 00000000000..932390cfb24
--- /dev/null
+++ b/clang/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 %{{.*}}, label %{{.*}}
+  // 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 %{{.*}}
+  // 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;
+}