sberg updated this revision to Diff 124154.
sberg added a comment.

Oh, I'd meant to upload the version of the patch where the newly added calls to 
EmitTypeCheck use the default Alignment and SkippedChecks arguments, instead of 
explicitly skipping Alignment and ObjectSize checks.   (That's the version I 
had successfully used in a LibreOffice UBSan 'make check' test build.  But I 
still don't know which version is better.)


https://reviews.llvm.org/D40295

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  compiler-rt/lib/ubsan/ubsan_handlers.cc
  compiler-rt/test/ubsan/TestCases/TypeCheck/vptr.cpp

Index: compiler-rt/test/ubsan/TestCases/TypeCheck/vptr.cpp
===================================================================
--- compiler-rt/test/ubsan/TestCases/TypeCheck/vptr.cpp
+++ compiler-rt/test/ubsan/TestCases/TypeCheck/vptr.cpp
@@ -1,41 +1,53 @@
-// RUN: %clangxx -frtti -fsanitize=null,vptr -fno-sanitize-recover=null,vptr -g %s -O3 -o %t -mllvm -enable-tail-merge=false
-// RUN: %run %t rT && %run %t mT && %run %t fT && %run %t cT
-// RUN: %run %t rU && %run %t mU && %run %t fU && %run %t cU
-// RUN: %run %t rS && %run %t rV && %run %t oV
-// RUN: %env_ubsan_opts=print_stacktrace=1 not %run %t mS 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER --check-prefix=CHECK-%os-MEMBER --strict-whitespace
-// RUN: %env_ubsan_opts=print_stacktrace=1 not %run %t fS 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN --strict-whitespace
-// RUN: %env_ubsan_opts=print_stacktrace=1 not %run %t cS 2>&1 | FileCheck %s --check-prefix=CHECK-DOWNCAST --check-prefix=CHECK-%os-DOWNCAST --strict-whitespace
-// RUN: %env_ubsan_opts=print_stacktrace=1 not %run %t mV 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER --check-prefix=CHECK-%os-MEMBER --strict-whitespace
-// RUN: %env_ubsan_opts=print_stacktrace=1 not %run %t fV 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN --strict-whitespace
-// RUN: %env_ubsan_opts=print_stacktrace=1 not %run %t cV 2>&1 | FileCheck %s --check-prefix=CHECK-DOWNCAST --check-prefix=CHECK-%os-DOWNCAST --strict-whitespace
-// RUN: %env_ubsan_opts=print_stacktrace=1 not %run %t oU 2>&1 | FileCheck %s --check-prefix=CHECK-OFFSET --check-prefix=CHECK-%os-OFFSET --strict-whitespace
-// RUN: %env_ubsan_opts=print_stacktrace=1 not %run %t m0 2>&1 | FileCheck %s --check-prefix=CHECK-INVALID-MEMBER --check-prefix=CHECK-%os-NULL-MEMBER --strict-whitespace
-// RUN: %env_ubsan_opts=print_stacktrace=1 not %run %t m0 2>&1 | FileCheck %s --check-prefix=CHECK-INVALID-MEMBER --check-prefix=CHECK-%os-NULL-MEMBER --strict-whitespace
-// RUN: not %run %t nN 2>&1 | FileCheck %s --check-prefix=CHECK-NULL-MEMFUN --strict-whitespace
+// RUN: %clangxx -frtti -fsanitize=null,vptr -g %s -O3 -o %t -mllvm -enable-tail-merge=false
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t rT
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t mT
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t fT
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t cT
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t rU
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t mU
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t fU
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t cU
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t rS
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t rV
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t oV
+// RUN: %env_ubsan_opts=halt_on_error=1 %run %t zN
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1 not %run %t mS 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER --check-prefix=CHECK-%os-MEMBER --strict-whitespace
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1 not %run %t fS 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN --strict-whitespace
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1 not %run %t cS 2>&1 | FileCheck %s --check-prefix=CHECK-DOWNCAST --check-prefix=CHECK-%os-DOWNCAST --strict-whitespace
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1 not %run %t mV 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER --check-prefix=CHECK-%os-MEMBER --strict-whitespace
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1 not %run %t fV 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN --strict-whitespace
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1 not %run %t cV 2>&1 | FileCheck %s --check-prefix=CHECK-DOWNCAST --check-prefix=CHECK-%os-DOWNCAST --strict-whitespace
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1 not %run %t oU 2>&1 | FileCheck %s --check-prefix=CHECK-OFFSET --check-prefix=CHECK-%os-OFFSET --strict-whitespace
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1 not %run %t m0 2>&1 | FileCheck %s --check-prefix=CHECK-INVALID-MEMBER --check-prefix=CHECK-%os-NULL-MEMBER --strict-whitespace
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1 not %run %t m0 2>&1 | FileCheck %s --check-prefix=CHECK-INVALID-MEMBER --check-prefix=CHECK-%os-NULL-MEMBER --strict-whitespace
+// RUN: %env_ubsan_opts=halt_on_error=1 not %run %t nN 2>&1 | FileCheck %s --check-prefix=CHECK-NULL-MEMFUN --strict-whitespace
+// RUN: %env_ubsan_opts=print_stacktrace=1 %run %t dT 2>&1 | FileCheck %s --check-prefix=CHECK-DYNAMIC --check-prefix=CHECK-%os-DYNAMIC --strict-whitespace
 
 // RUN: (echo "vptr_check:S"; echo "vptr_check:T"; echo "vptr_check:U") > %t.supp
-// RUN: %env_ubsan_opts=suppressions='"%t.supp"' %run %t mS
-// RUN: %env_ubsan_opts=suppressions='"%t.supp"' %run %t fS
-// RUN: %env_ubsan_opts=suppressions='"%t.supp"' %run %t cS
-// RUN: %env_ubsan_opts=suppressions='"%t.supp"' %run %t mV
-// RUN: %env_ubsan_opts=suppressions='"%t.supp"' %run %t fV
-// RUN: %env_ubsan_opts=suppressions='"%t.supp"' %run %t cV
-// RUN: %env_ubsan_opts=suppressions='"%t.supp"' %run %t oU
+// RUN: %env_ubsan_opts=halt_on_error=1:suppressions='"%t.supp"' %run %t mS
+// RUN: %env_ubsan_opts=halt_on_error=1:suppressions='"%t.supp"' %run %t fS
+// RUN: %env_ubsan_opts=halt_on_error=1:suppressions='"%t.supp"' %run %t cS
+// RUN: %env_ubsan_opts=halt_on_error=1:suppressions='"%t.supp"' %run %t mV
+// RUN: %env_ubsan_opts=halt_on_error=1:suppressions='"%t.supp"' %run %t fV
+// RUN: %env_ubsan_opts=halt_on_error=1:suppressions='"%t.supp"' %run %t cV
+// RUN: %env_ubsan_opts=halt_on_error=1:suppressions='"%t.supp"' %run %t oU
+// RUN: %env_ubsan_opts=halt_on_error=1:suppressions='"%t.supp"' %run %t dT
 
 // RUN: echo "vptr_check:S" > %t.loc-supp
-// RUN: %env_ubsan_opts=suppressions='"%t.loc-supp"' not %run %t x- 2>&1 | FileCheck %s --check-prefix=CHECK-LOC-SUPPRESS
+// RUN: %env_ubsan_opts=halt_on_error=1:suppressions='"%t.loc-supp"' not %run %t x- 2>&1 | FileCheck %s --check-prefix=CHECK-LOC-SUPPRESS
 
 // REQUIRES: stable-runtime, cxxabi
 // UNSUPPORTED: win32
 // Suppressions file not pushed to the device.
 // UNSUPPORTED: android
 #include <new>
+#include <typeinfo>
 #include <assert.h>
 #include <stdio.h>
 
 struct S {
   S() : a(0) {}
-  ~S() {}
+  ~S();
   int a;
   int f() { return 0; }
   virtual int v() { return 0; }
@@ -52,13 +64,23 @@
 
 struct V : S {};
 
-// Make p global so that lsan does not complain.
+namespace {
+  struct W {};
+}
+
 T *p = 0;
 
+bool dtorCheck = false;
+
 volatile void *sink1, *sink2;
 
 int access_p(T *p, char type);
 
+S::~S() {
+  if (dtorCheck)
+    access_p(p, '~');
+}
+
 int main(int argc, char **argv) {
   assert(argc > 1);
   fprintf(stderr, "Test case: %s\n", argv[1]);
@@ -180,6 +202,51 @@
   case 'n':
     // CHECK-NULL-MEMFUN: vptr.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'T'
     return p->g();
+
+  case 'd':
+    dtorCheck = true;
+    delete p;
+    dtorCheck = false;
+    return 0;
+  case '~':
+    // CHECK-DYNAMIC: vptr.cpp:[[@LINE+6]]:11: runtime error: dynamic operation on address [[PTR:0x[0-9a-f]*]] which does not point to an object of type 'T'
+    // CHECK-DYNAMIC-NEXT: [[PTR]]: note: object is of type 'S'
+    // CHECK-DYNAMIC-NEXT: {{^ .. .. .. ..  .. .. .. .. .. .. .. ..  }}
+    // CHECK-DYNAMIC-NEXT: {{^              \^~~~~~~~~~~(~~~~~~~~~~~~)? *$}}
+    // CHECK-DYNAMIC-NEXT: {{^              vptr for}} 'S'
+    // CHECK-Linux-DYNAMIC: #0 {{.*}}access_p{{.*}}vptr.cpp:[[@LINE+1]]
+    (void)dynamic_cast<V*>(p);
+    // CHECK-DYNAMIC: vptr.cpp:[[@LINE+6]]:11: runtime error: dynamic operation on address [[PTR:0x[0-9a-f]*]] which does not point to an object of type 'T'
+    // CHECK-DYNAMIC-NEXT: [[PTR]]: note: object is of type 'S'
+    // CHECK-DYNAMIC-NEXT: {{^ .. .. .. ..  .. .. .. .. .. .. .. ..  }}
+    // CHECK-DYNAMIC-NEXT: {{^              \^~~~~~~~~~~(~~~~~~~~~~~~)? *$}}
+    // CHECK-DYNAMIC-NEXT: {{^              vptr for}} 'S'
+    // CHECK-Linux-DYNAMIC: #0 {{.*}}access_p{{.*}}vptr.cpp:[[@LINE+1]]
+    (void)dynamic_cast<W*>(p);
+    try {
+      // CHECK-DYNAMIC: vptr.cpp:[[@LINE+6]]:13: runtime error: dynamic operation on address [[PTR:0x[0-9a-f]*]] which does not point to an object of type 'T'
+      // CHECK-DYNAMIC-NEXT: [[PTR]]: note: object is of type 'S'
+      // CHECK-DYNAMIC-NEXT: {{^ .. .. .. ..  .. .. .. .. .. .. .. ..  }}
+      // CHECK-DYNAMIC-NEXT: {{^              \^~~~~~~~~~~(~~~~~~~~~~~~)? *$}}
+      // CHECK-DYNAMIC-NEXT: {{^              vptr for}} 'S'
+      // CHECK-Linux-DYNAMIC: #0 {{.*}}access_p{{.*}}vptr.cpp:[[@LINE+1]]
+      (void)dynamic_cast<V&>(*p);
+    } catch (std::bad_cast &) {}
+    // CHECK-DYNAMIC: vptr.cpp:[[@LINE+6]]:18: runtime error: dynamic operation on address [[PTR:0x[0-9a-f]*]] which does not point to an object of type 'T'
+    // CHECK-DYNAMIC-NEXT: [[PTR]]: note: object is of type 'S'
+    // CHECK-DYNAMIC-NEXT: {{^ .. .. .. ..  .. .. .. .. .. .. .. ..  }}
+    // CHECK-DYNAMIC-NEXT: {{^              \^~~~~~~~~~~(~~~~~~~~~~~~)? *$}}
+    // CHECK-DYNAMIC-NEXT: {{^              vptr for}} 'S'
+    // CHECK-Linux-DYNAMIC: #0 {{.*}}access_p{{.*}}vptr.cpp:[[@LINE+1]]
+    (void)typeid(*p);
+    return 0;
+
+  case 'z':
+    (void)dynamic_cast<V*>(p);
+    try {
+      (void)typeid(*p);
+    } catch (std::bad_typeid &) {}
+    return 0;
   }
   return 0;
 }
Index: compiler-rt/lib/ubsan/ubsan_handlers.cc
===================================================================
--- compiler-rt/lib/ubsan/ubsan_handlers.cc
+++ compiler-rt/lib/ubsan/ubsan_handlers.cc
@@ -38,7 +38,8 @@
 const char *TypeCheckKinds[] = {
     "load of", "store to", "reference binding to", "member access within",
     "member call on", "constructor call on", "downcast of", "downcast of",
-    "upcast of", "cast to virtual base of", "_Nonnull binding to"};
+    "upcast of", "cast to virtual base of", "_Nonnull binding to",
+    "dynamic operation on"};
 }
 
 static void handleTypeMismatchImpl(TypeMismatchData *Data, ValueHandle Pointer,
Index: clang/lib/CodeGen/CodeGenFunction.h
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2367,7 +2367,10 @@
     /// object within its lifetime.
     TCK_UpcastToVirtualBase,
     /// Checking the value assigned to a _Nonnull pointer. Must not be null.
-    TCK_NonnullAssign
+    TCK_NonnullAssign,
+    /// Checking the operand of a dynamic_cast or a typeid expression.  Must be
+    /// null or an object within its lifetime.
+    TCK_DynamicOperation
   };
 
   /// Determine whether the pointer type check \p TCK permits null pointers.
Index: clang/lib/CodeGen/CGExprCXX.cpp
===================================================================
--- clang/lib/CodeGen/CGExprCXX.cpp
+++ clang/lib/CodeGen/CGExprCXX.cpp
@@ -2053,15 +2053,23 @@
   // Get the vtable pointer.
   Address ThisPtr = CGF.EmitLValue(E).getAddress();
 
+  QualType SrcRecordTy = E->getType();
+
+  // C++ [class.cdtor]p4:
+  //   If the operand of typeid refers to the object under construction or
+  //   destruction and the static type of the operand is neither the constructor
+  //   or destructor’s class nor one of its bases, the behavior is undefined.
+  CGF.EmitTypeCheck(CodeGenFunction::TCK_DynamicOperation, E->getExprLoc(),
+                    ThisPtr.getPointer(), SrcRecordTy);
+
   // C++ [expr.typeid]p2:
   //   If the glvalue expression is obtained by applying the unary * operator to
   //   a pointer and the pointer is a null pointer value, the typeid expression
   //   throws the std::bad_typeid exception.
   //
   // However, this paragraph's intent is not clear.  We choose a very generous
   // interpretation which implores us to consider comma operators, conditional
   // operators, parentheses and other such constructs.
-  QualType SrcRecordTy = E->getType();
   if (CGF.CGM.getCXXABI().shouldTypeidBeNullChecked(
           isGLValueFromPointerDeref(E), SrcRecordTy)) {
     llvm::BasicBlock *BadTypeidBlock =
@@ -2124,10 +2132,6 @@
   CGM.EmitExplicitCastExprType(DCE, this);
   QualType DestTy = DCE->getTypeAsWritten();
 
-  if (DCE->isAlwaysNull())
-    if (llvm::Value *T = EmitDynamicCastToNull(*this, DestTy))
-      return T;
-
   QualType SrcTy = DCE->getSubExpr()->getType();
 
   // C++ [expr.dynamic.cast]p7:
@@ -2148,6 +2152,18 @@
     DestRecordTy = DestTy->castAs<ReferenceType>()->getPointeeType();
   }
 
+  // C++ [class.cdtor]p5:
+  //   If the operand of the dynamic_cast refers to the object under
+  //   construction or destruction and the static type of the operand is not a
+  //   pointer to or object of the constructor or destructor’s own class or one
+  //   of its bases, the dynamic_cast results in undefined behavior.
+  EmitTypeCheck(TCK_DynamicOperation, DCE->getExprLoc(), ThisAddr.getPointer(),
+                SrcRecordTy);
+
+  if (DCE->isAlwaysNull())
+    if (llvm::Value *T = EmitDynamicCastToNull(*this, DestTy))
+      return T;
+
   assert(SrcRecordTy->isRecordType() && "source type must be a record type!");
 
   // C++ [expr.dynamic.cast]p4: 
Index: clang/lib/CodeGen/CGExpr.cpp
===================================================================
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -570,15 +570,15 @@
 
 bool CodeGenFunction::isNullPointerAllowed(TypeCheckKind TCK) {
   return TCK == TCK_DowncastPointer || TCK == TCK_Upcast ||
-         TCK == TCK_UpcastToVirtualBase;
+         TCK == TCK_UpcastToVirtualBase || TCK == TCK_DynamicOperation;
 }
 
 bool CodeGenFunction::isVptrCheckRequired(TypeCheckKind TCK, QualType Ty) {
   CXXRecordDecl *RD = Ty->getAsCXXRecordDecl();
   return (RD && RD->hasDefinition() && RD->isDynamicClass()) &&
          (TCK == TCK_MemberAccess || TCK == TCK_MemberCall ||
           TCK == TCK_DowncastPointer || TCK == TCK_DowncastReference ||
-          TCK == TCK_UpcastToVirtualBase);
+          TCK == TCK_UpcastToVirtualBase || TCK == TCK_DynamicOperation);
 }
 
 bool CodeGenFunction::sanitizePerformTypeCheck() const {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to