eugenis created this revision.
eugenis added reviewers: pcc, kcc.
eugenis added a subscriber: cfe-commits.
eugenis set the repository for this revision to rL LLVM.

Avoid crashing when printing diagnostics for vtable-related CFI
errors. In diagnostic mode, the frontend does an additional check of
the vtable pointer against the set of all known vtable addresses and
lets the runtime handler know if it is safe to inspect the vtable.

Repository:
  rL LLVM

http://reviews.llvm.org/D16823

Files:
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.cpp
  test/CodeGen/cfi-check-fail.c
  test/CodeGenCXX/cfi-cast.cpp
  test/CodeGenCXX/cfi-vcall.cpp

Index: test/CodeGenCXX/cfi-vcall.cpp
===================================================================
--- test/CodeGenCXX/cfi-vcall.cpp
+++ test/CodeGenCXX/cfi-vcall.cpp
@@ -55,7 +55,7 @@
 
 // DIAG: @[[SRC:.*]] = private unnamed_addr constant [{{.*}} x i8] c"{{.*}}cfi-vcall.cpp\00", align 1
 // DIAG: @[[TYPE:.*]] = private unnamed_addr constant { i16, i16, [4 x i8] } { i16 -1, i16 0, [4 x i8] c"'A'\00" }
-// DIAG: @[[BADTYPESTATIC:.*]] = private unnamed_addr global { i8, { [{{.*}} x i8]*, i32, i32 }, { i16, i16, [4 x i8] }* } { i8 0, { [{{.*}} x i8]*, i32, i32 } { [{{.*}} x i8]* @[[SRC]], i32 [[@LINE+21]], i32 3 }, { i16, i16, [4 x i8] }* @[[TYPE]] }
+// DIAG: @[[BADTYPESTATIC:.*]] = private unnamed_addr global { i8, { [{{.*}} x i8]*, i32, i32 }, { i16, i16, [4 x i8] }* } { i8 0, { [{{.*}} x i8]*, i32, i32 } { [{{.*}} x i8]* @[[SRC]], i32 [[@LINE+23]], i32 3 }, { i16, i16, [4 x i8] }* @[[TYPE]] }
 
 // ITANIUM: define void @_Z2afP1A
 // MS: define void @"\01?af@@YAXPEAUA@@@Z"
@@ -68,10 +68,12 @@
   // CHECK: [[TRAPBB]]
   // NDIAG-NEXT: call void @llvm.trap()
   // NDIAG-NEXT: unreachable
+  // DIAG-NEXT: [[VTVALID0:%[^ ]*]] = call i1 @llvm.bitset.test(i8* [[VT]], metadata !"all-vtables")
   // DIAG-NEXT: [[VTINT:%[^ ]*]] = ptrtoint i8* [[VT]] to i64
-  // DIAG-ABORT-NEXT: call void @__ubsan_handle_cfi_check_fail_abort(i8* getelementptr inbounds ({{.*}} @[[BADTYPESTATIC]], i32 0, i32 0), i64 [[VTINT]])
+  // DIAG-NEXT: [[VTVALID:%[^ ]*]] = zext i1 [[VTVALID0]] to i64
+  // DIAG-ABORT-NEXT: call void @__ubsan_handle_cfi_check_fail_abort(i8* getelementptr inbounds ({{.*}} @[[BADTYPESTATIC]], i32 0, i32 0), i64 [[VTINT]], i64 [[VTVALID]])
   // DIAG-ABORT-NEXT: unreachable
-  // DIAG-RECOVER-NEXT: call void @__ubsan_handle_cfi_check_fail(i8* getelementptr inbounds ({{.*}} @[[BADTYPESTATIC]], i32 0, i32 0), i64 [[VTINT]])
+  // DIAG-RECOVER-NEXT: call void @__ubsan_handle_cfi_check_fail(i8* getelementptr inbounds ({{.*}} @[[BADTYPESTATIC]], i32 0, i32 0), i64 [[VTINT]], i64 [[VTVALID]])
   // DIAG-RECOVER-NEXT: br label %[[CONTBB]]
 
   // CHECK: [[CONTBB]]
@@ -157,32 +159,45 @@
 
 }
 
-// Check for the expected number of elements (9 or 15 respectively).
-// MS: !llvm.bitsets = !{[[X:[^,]*(,[^,]*){8}]]}
-// ITANIUM: !llvm.bitsets = !{[[X:[^,]*(,[^,]*){14}]]}
+// Check for the expected number of elements (15 or 23 respectively).
+// MS: !llvm.bitsets = !{[[X:[^,]*(,[^,]*){15}]]}
+// ITANIUM: !llvm.bitsets = !{[[X:[^,]*(,[^,]*){23}]]}
 
 // ITANIUM-DAG: !{!"_ZTS1A", [3 x i8*]* @_ZTV1A, i64 16}
+// ITANIUM-DAG: !{!"all-vtables", [3 x i8*]* @_ZTV1A, i64 16}
 // ITANIUM-DAG: !{!"_ZTS1A", [7 x i8*]* @_ZTCN12_GLOBAL__N_11DE0_1B, i64 32}
+// ITANIUM-DAG: !{!"all-vtables", [7 x i8*]* @_ZTCN12_GLOBAL__N_11DE0_1B, i64 32}
 // ITANIUM-DAG: !{!"_ZTS1B", [7 x i8*]* @_ZTCN12_GLOBAL__N_11DE0_1B, i64 32}
 // ITANIUM-DAG: !{!"_ZTS1A", [9 x i8*]* @_ZTCN12_GLOBAL__N_11DE8_1C, i64 64}
+// ITANIUM-DAG: !{!"all-vtables", [9 x i8*]* @_ZTCN12_GLOBAL__N_11DE8_1C, i64 64}
 // ITANIUM-DAG: !{!"_ZTS1C", [9 x i8*]* @_ZTCN12_GLOBAL__N_11DE8_1C, i64 32}
 // ITANIUM-DAG: !{!"_ZTS1A", [12 x i8*]* @_ZTVN12_GLOBAL__N_11DE, i64 32}
+// ITANIUM-DAG: !{!"all-vtables", [12 x i8*]* @_ZTVN12_GLOBAL__N_11DE, i64 32}
 // ITANIUM-DAG: !{!"_ZTS1B", [12 x i8*]* @_ZTVN12_GLOBAL__N_11DE, i64 32}
 // ITANIUM-DAG: !{!"_ZTS1C", [12 x i8*]* @_ZTVN12_GLOBAL__N_11DE, i64 88}
+// ITANIUM-DAG: !{!"all-vtables", [12 x i8*]* @_ZTVN12_GLOBAL__N_11DE, i64 88}
 // ITANIUM-DAG: !{![[DTYPE]], [12 x i8*]* @_ZTVN12_GLOBAL__N_11DE, i64 32}
 // ITANIUM-DAG: !{!"_ZTS1A", [7 x i8*]* @_ZTV1B, i64 32}
+// ITANIUM-DAG: !{!"all-vtables", [7 x i8*]* @_ZTV1B, i64 32}
 // ITANIUM-DAG: !{!"_ZTS1B", [7 x i8*]* @_ZTV1B, i64 32}
 // ITANIUM-DAG: !{!"_ZTS1A", [5 x i8*]* @_ZTV1C, i64 32}
 // ITANIUM-DAG: !{!"_ZTS1C", [5 x i8*]* @_ZTV1C, i64 32}
 // ITANIUM-DAG: !{!"_ZTS1A", [3 x i8*]* @_ZTVZ3foovE2FA, i64 16}
 // ITANIUM-DAG: !{!{{[0-9]+}}, [3 x i8*]* @_ZTVZ3foovE2FA, i64 16}
 
 // MS-DAG: !{!"?AUA@@", [2 x i8*]* @[[VTA]], i64 8}
+// MS-DAG: !{!"all-vtables", [2 x i8*]* @[[VTA]], i64 8}
 // MS-DAG: !{!"?AUB@@", [3 x i8*]* @[[VTB]], i64 8}
+// MS-DAG: !{!"all-vtables", [3 x i8*]* @[[VTB]], i64 8}
 // MS-DAG: !{!"?AUA@@", [2 x i8*]* @[[VTAinB]], i64 8}
+// MS-DAG: !{!"all-vtables", [2 x i8*]* @[[VTAinB]], i64 8}
 // MS-DAG: !{!"?AUA@@", [2 x i8*]* @[[VTAinC]], i64 8}
+// MS-DAG: !{!"all-vtables", [2 x i8*]* @[[VTAinC]], i64 8}
 // MS-DAG: !{!"?AUB@@", [3 x i8*]* @[[VTBinD]], i64 8}
+// MS-DAG: !{!"all-vtables", [3 x i8*]* @[[VTBinD]], i64 8}
 // MS-DAG: !{![[DTYPE]], [3 x i8*]* @[[VTBinD]], i64 8}
 // MS-DAG: !{!"?AUA@@", [2 x i8*]* @[[VTAinBinD]], i64 8}
+// MS-DAG: !{!"all-vtables", [2 x i8*]* @[[VTAinBinD]], i64 8}
 // MS-DAG: !{!"?AUA@@", [2 x i8*]* @[[VTFA]], i64 8}
+// MS-DAG: !{!"all-vtables", [2 x i8*]* @[[VTFA]], i64 8}
 // MS-DAG: !{!{{[0-9]+}}, [2 x i8*]* @[[VTFA]], i64 8}
Index: test/CodeGenCXX/cfi-cast.cpp
===================================================================
--- test/CodeGenCXX/cfi-cast.cpp
+++ test/CodeGenCXX/cfi-cast.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-derived-cast -fsanitize-trap=cfi-derived-cast -emit-llvm -o - %s | FileCheck -check-prefix=CHECK-DCAST %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-unrelated-cast -fsanitize-trap=cfi-unrelated-cast -emit-llvm -o - %s | FileCheck -check-prefix=CHECK-UCAST %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-unrelated-cast,cfi-cast-strict -fsanitize-trap=cfi-unrelated-cast,cfi-cast-strict -emit-llvm -o - %s | FileCheck -check-prefix=CHECK-UCAST-STRICT %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -std=c++11 -fsanitize=cfi-derived-cast -fsanitize-trap=cfi-derived-cast -emit-llvm -o - %s | FileCheck -check-prefix=CHECK-DCAST %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -std=c++11 -fsanitize=cfi-unrelated-cast -fsanitize-trap=cfi-unrelated-cast -emit-llvm -o - %s | FileCheck -check-prefix=CHECK-UCAST %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -std=c++11 -fsanitize=cfi-unrelated-cast,cfi-cast-strict -fsanitize-trap=cfi-unrelated-cast,cfi-cast-strict -emit-llvm -o - %s | FileCheck -check-prefix=CHECK-UCAST-STRICT %s
 
 // In this test the main thing we are searching for is something like
 // 'metadata !"1B"' where "1B" is the mangled name of the class we are
@@ -28,7 +28,7 @@
 
   // CHECK-DCAST: [[CONTBB]]
   // CHECK-DCAST: ret
-  static_cast<B*>(a);
+  (void)static_cast<B*>(a);
 }
 
 // CHECK-DCAST-LABEL: define void @_Z3abrR1A
@@ -42,7 +42,7 @@
 
   // CHECK-DCAST: [[CONTBB]]
   // CHECK-DCAST: ret
-  static_cast<B&>(a);
+  (void)static_cast<B&>(a);
 }
 
 // CHECK-DCAST-LABEL: define void @_Z4abrrO1A
@@ -56,7 +56,7 @@
 
   // CHECK-DCAST: [[CONTBB]]
   // CHECK-DCAST: ret
-  static_cast<B&&>(a);
+  (void)static_cast<B&&>(a);
 }
 
 // CHECK-UCAST-LABEL: define void @_Z3vbpPv
@@ -70,7 +70,7 @@
 
   // CHECK-UCAST: [[CONTBB]]
   // CHECK-UCAST: ret
-  static_cast<B*>(p);
+  (void)static_cast<B*>(p);
 }
 
 // CHECK-UCAST-LABEL: define void @_Z3vbrRc
@@ -84,7 +84,7 @@
 
   // CHECK-UCAST: [[CONTBB]]
   // CHECK-UCAST: ret
-  reinterpret_cast<B&>(r);
+  (void)reinterpret_cast<B&>(r);
 }
 
 // CHECK-UCAST-LABEL: define void @_Z4vbrrOc
@@ -98,23 +98,23 @@
 
   // CHECK-UCAST: [[CONTBB]]
   // CHECK-UCAST: ret
-  reinterpret_cast<B&&>(r);
+  (void)reinterpret_cast<B&&>(r);
 }
 
 // CHECK-UCAST-LABEL: define void @_Z3vcpPv
 // CHECK-UCAST-STRICT-LABEL: define void @_Z3vcpPv
 void vcp(void *p) {
   // CHECK-UCAST: call i1 @llvm.bitset.test(i8* {{%[^ ]*}}, metadata !"_ZTS1A")
   // CHECK-UCAST-STRICT: call i1 @llvm.bitset.test(i8* {{%[^ ]*}}, metadata !"_ZTS1C")
-  static_cast<C*>(p);
+  (void)static_cast<C*>(p);
 }
 
 // CHECK-UCAST-LABEL: define void @_Z3bcpP1B
 // CHECK-UCAST-STRICT-LABEL: define void @_Z3bcpP1B
 void bcp(B *p) {
   // CHECK-UCAST: call i1 @llvm.bitset.test(i8* {{%[^ ]*}}, metadata !"_ZTS1A")
   // CHECK-UCAST-STRICT: call i1 @llvm.bitset.test(i8* {{%[^ ]*}}, metadata !"_ZTS1C")
-  (C *)p;
+  (void)(C *)p;
 }
 
 // CHECK-UCAST-LABEL: define void @_Z8bcp_callP1B
Index: test/CodeGen/cfi-check-fail.c
===================================================================
--- test/CodeGen/cfi-check-fail.c
+++ test/CodeGen/cfi-check-fail.c
@@ -26,9 +26,11 @@
 // CHECK:   br i1 %[[NOT_0]], label %[[CONT1:.*]], label %[[HANDLE0:.*]], !prof
 
 // CHECK: [[HANDLE0]]:
+// CHECK:   %[[VTVALID0:.*]] = call i1 @llvm.bitset.test(i8* %[[ADDR]], metadata !"all-vtables")
 // CHECK:   %[[DATA0:.*]] = ptrtoint i8* %[[DATA]] to i64,
 // CHECK:   %[[ADDR0:.*]] = ptrtoint i8* %[[ADDR]] to i64,
-// CHECK:   call void @__ubsan_handle_cfi_check_fail(i64 %[[DATA0]], i64 %[[ADDR0]])
+// CHECK:   %[[VTVALID0_ZEXT:.*]] = zext i1 %[[VTVALID0]] to i64
+// CHECK:   call void @__ubsan_handle_cfi_check_fail(i64 %[[DATA0]], i64 %[[ADDR0]], i64 %[[VTVALID0_ZEXT]])
 // CHECK:   br label %[[CONT1]]
 
 // CHECK: [[CONT1]]:
@@ -44,19 +46,23 @@
 // CHECK:   br i1 %[[NOT_2]], label %[[CONT3:.*]], label %[[HANDLE2:.*]], !prof
 
 // CHECK: [[HANDLE2]]:
+// CHECK:   %[[VTVALID2:.*]] = call i1 @llvm.bitset.test(i8* %[[ADDR]], metadata !"all-vtables")
 // CHECK:   %[[DATA2:.*]] = ptrtoint i8* %[[DATA]] to i64,
 // CHECK:   %[[ADDR2:.*]] = ptrtoint i8* %[[ADDR]] to i64,
-// CHECK:   call void @__ubsan_handle_cfi_check_fail_abort(i64 %[[DATA2]], i64 %[[ADDR2]])
+// CHECK:   %[[VTVALID2_ZEXT:.*]] = zext i1 %[[VTVALID2]] to i64
+// CHECK:   call void @__ubsan_handle_cfi_check_fail_abort(i64 %[[DATA2]], i64 %[[ADDR2]], i64 %[[VTVALID2_ZEXT]])
 // CHECK:   unreachable
 
 // CHECK: [[CONT3]]:
 // CHECK:   %[[NOT_3:.*]] = icmp ne i8 %[[KIND]], 3
 // CHECK:   br i1 %[[NOT_3]], label %[[CONT4:.*]], label %[[HANDLE3:.*]], !prof
 
 // CHECK: [[HANDLE3]]:
+// CHECK:   %[[VTVALID3:.*]] = call i1 @llvm.bitset.test(i8* %[[ADDR]], metadata !"all-vtables")
 // CHECK:   %[[DATA3:.*]] = ptrtoint i8* %[[DATA]] to i64,
 // CHECK:   %[[ADDR3:.*]] = ptrtoint i8* %[[ADDR]] to i64,
-// CHECK:   call void @__ubsan_handle_cfi_check_fail(i64 %[[DATA3]], i64 %[[ADDR3]])
+// CHECK:   %[[VTVALID3_ZEXT:.*]] = zext i1 %[[VTVALID3]] to i64
+// CHECK:   call void @__ubsan_handle_cfi_check_fail(i64 %[[DATA3]], i64 %[[ADDR3]], i64 %[[VTVALID3_ZEXT]])
 // CHECK:   br label %[[CONT4]]
 
 // CHECK: [[CONT4]]:
Index: lib/CodeGen/CodeGenModule.cpp
===================================================================
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -4049,6 +4049,20 @@
       BitsetsMD->addOperand(llvm::MDTuple::get(getLLVMContext(), BitsetOps2));
     }
   }
+
+  if (!CodeGenOpts.SanitizeTrap.has(SanitizerKind::CFIVCall) ||
+      !CodeGenOpts.SanitizeTrap.has(SanitizerKind::CFINVCall) ||
+      !CodeGenOpts.SanitizeTrap.has(SanitizerKind::CFIDerivedCast) ||
+      !CodeGenOpts.SanitizeTrap.has(SanitizerKind::CFIUnrelatedCast)) {
+    llvm::Metadata *MD = llvm::MDString::get(getLLVMContext(), "all-vtables");
+    llvm::Metadata *BitsetOps[] = {
+      MD, llvm::ConstantAsMetadata::get(VTable),
+      llvm::ConstantAsMetadata::get(
+          llvm::ConstantInt::get(Int64Ty, Offset.getQuantity()))};
+    // Avoid adding a node to BitsetsMD twice.
+    if (!llvm::MDTuple::getIfExists(getLLVMContext(), BitsetOps))
+      BitsetsMD->addOperand(llvm::MDTuple::get(getLLVMContext(), BitsetOps));
+  }
 }
 
 // Fills in the supplied string map with the set of target features for the
Index: lib/CodeGen/CodeGenFunction.h
===================================================================
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -3024,7 +3024,8 @@
   /// branch to it.
   void EmitCheck(ArrayRef<std::pair<llvm::Value *, SanitizerMask>> Checked,
                  StringRef CheckName, ArrayRef<llvm::Constant *> StaticArgs,
-                 ArrayRef<llvm::Value *> DynamicArgs);
+                 ArrayRef<llvm::Value *> DynamicArgs,
+                 llvm::Value *CheckAndAppendValidVtable = nullptr);
 
   /// \brief Emit a slow path cross-DSO CFI check which calls __cfi_slowpath
   /// if Cond if false.
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2434,7 +2434,8 @@
 void CodeGenFunction::EmitCheck(
     ArrayRef<std::pair<llvm::Value *, SanitizerMask>> Checked,
     StringRef CheckName, ArrayRef<llvm::Constant *> StaticArgs,
-    ArrayRef<llvm::Value *> DynamicArgs) {
+    ArrayRef<llvm::Value *> DynamicArgs,
+    llvm::Value *CheckAndAppendValidVtable) {
   assert(IsSanitizerScope);
   assert(Checked.size() > 0);
 
@@ -2489,13 +2490,23 @@
   Branch->setMetadata(llvm::LLVMContext::MD_prof, Node);
   EmitBlock(Handlers);
 
+  llvm::Value *ValidVtable = nullptr;
+  if (CheckAndAppendValidVtable) {
+    llvm::Value *AllVtables = llvm::MetadataAsValue::get(
+        CGM.getLLVMContext(),
+        llvm::MDString::get(CGM.getLLVMContext(), "all-vtables"));
+    ValidVtable =
+        Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::bitset_test),
+                           {CheckAndAppendValidVtable, AllVtables});
+  }
+
   // Handler functions take an i8* pointing to the (handler-specific) static
   // information block, followed by a sequence of intptr_t arguments
   // representing operand values.
   SmallVector<llvm::Value *, 4> Args;
   SmallVector<llvm::Type *, 4> ArgTypes;
-  Args.reserve(DynamicArgs.size() + 1);
-  ArgTypes.reserve(DynamicArgs.size() + 1);
+  Args.reserve(DynamicArgs.size() + 1 + !!CheckAndAppendValidVtable);
+  ArgTypes.reserve(DynamicArgs.size() + 1 + !!CheckAndAppendValidVtable);
 
   // Emit handler arguments and create handler function type.
   if (!StaticArgs.empty()) {
@@ -2514,6 +2525,11 @@
     ArgTypes.push_back(IntPtrTy);
   }
 
+  if (CheckAndAppendValidVtable) {
+    Args.push_back(EmitCheckValue(ValidVtable));
+    ArgTypes.push_back(IntPtrTy);
+  }
+
   llvm::FunctionType *FnType =
     llvm::FunctionType::get(CGM.VoidTy, ArgTypes, false);
 
@@ -2649,7 +2665,8 @@
     SanitizerMask Mask = CheckKindMaskPair.second;
     llvm::Value *Cond =
         Builder.CreateICmpNE(CheckKind, llvm::ConstantInt::get(Int8Ty, Kind));
-    EmitCheck(std::make_pair(Cond, Mask), "cfi_check_fail", {}, {Data, Addr});
+    EmitCheck(std::make_pair(Cond, Mask), "cfi_check_fail", {},
+              {Data, Addr}, Addr);
   }
 
   FinishFunction();
@@ -3970,7 +3987,8 @@
                            CastedCallee, StaticData);
     } else {
       EmitCheck(std::make_pair(BitSetTest, SanitizerKind::CFIICall),
-                "cfi_check_fail", StaticData, CastedCallee);
+                "cfi_check_fail", StaticData,
+                {CastedCallee, llvm::UndefValue::get(IntPtrTy)});
     }
   }
 
Index: lib/CodeGen/CGClass.cpp
===================================================================
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2609,7 +2609,7 @@
     EmitCfiSlowPathCheck(M, BitSetTest, TypeId, CastedVTable, StaticData);
   } else {
     EmitCheck(std::make_pair(BitSetTest, M), "cfi_check_fail", StaticData,
-              CastedVTable);
+              CastedVTable, CastedVTable);
   }
 }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to