[PATCH] D36013: Fix logic for generating llvm.type.test()s

[Vlad Tsyrklevich via Phabricator via cfe-commits]

vlad.tsyrklevich created this revision.

`CodeGenFunction::EmitTypeMetadataCodeForVCall()` could output an 
`llvm.assume(llvm.type.test())`when CFI was enabled, optimizing out the vcall 
check. This case was only reached when: 1) CFI-vcall was enabled, 2) 
-fwhole-program-tables was specified, and 3) -fno-sanitize-trap=cfi-vcall was 
specified.

This change is blocked on https://reviews.llvm.org/D36012 as it uncovers that 
bug.


https://reviews.llvm.org/D36013

Files:
  lib/CodeGen/CGClass.cpp
  test/CodeGenCXX/cfi-vcall-no-trap.cpp


Index: test/CodeGenCXX/cfi-vcall-no-trap.cpp
===================================================================
--- /dev/null
+++ test/CodeGenCXX/cfi-vcall-no-trap.cpp
@@ -0,0 +1,14 @@
+// Only output llvm.assume(llvm.type.test()) if cfi-vcall is disabled and 
whole-program-vtables is enabled
+// RUN: %clang_cc1 -cc1 -fvisibility hidden -fsanitize=cfi-vcall 
-fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=CHECK 
--check-prefix=CFI %s
+// RUN: %clang_cc1 -cc1 -fvisibility hidden -fwhole-program-vtables -emit-llvm 
-o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOCFI %s
+
+struct S1 {
+  virtual void f();
+};
+
+// CHECK: define{{.*}}s1f
+// CFI-NOT: llvm.assume
+// NOCFI: llvm.assume
+void s1f(S1 *s1) {
+  s1->f();
+}
Index: lib/CodeGen/CGClass.cpp
===================================================================
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2523,8 +2523,10 @@
 void CodeGenFunction::EmitTypeMetadataCodeForVCall(const CXXRecordDecl *RD,
                                                    llvm::Value *VTable,
                                                    SourceLocation Loc) {
-  if (CGM.getCodeGenOpts().WholeProgramVTables &&
-      CGM.HasHiddenLTOVisibility(RD)) {
+  if (SanOpts.has(SanitizerKind::CFIVCall))
+    EmitVTablePtrCheckForCall(RD, VTable, CodeGenFunction::CFITCK_VCall, Loc);
+  else if (CGM.getCodeGenOpts().WholeProgramVTables &&
+           CGM.HasHiddenLTOVisibility(RD)) {
     llvm::Metadata *MD =
         CGM.CreateMetadataIdentifierForType(QualType(RD->getTypeForDecl(), 0));
     llvm::Value *TypeId =
@@ -2536,9 +2538,6 @@
                            {CastedVTable, TypeId});
     Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::assume), TypeTest);
   }
-
-  if (SanOpts.has(SanitizerKind::CFIVCall))
-    EmitVTablePtrCheckForCall(RD, VTable, CodeGenFunction::CFITCK_VCall, Loc);
 }
 
 void CodeGenFunction::EmitVTablePtrCheckForCall(const CXXRecordDecl *RD,



Index: test/CodeGenCXX/cfi-vcall-no-trap.cpp
===================================================================
--- /dev/null
+++ test/CodeGenCXX/cfi-vcall-no-trap.cpp
@@ -0,0 +1,14 @@
+// Only output llvm.assume(llvm.type.test()) if cfi-vcall is disabled and whole-program-vtables is enabled
+// RUN: %clang_cc1 -cc1 -fvisibility hidden -fsanitize=cfi-vcall -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=CFI %s
+// RUN: %clang_cc1 -cc1 -fvisibility hidden -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NOCFI %s
+
+struct S1 {
+  virtual void f();
+};
+
+// CHECK: define{{.*}}s1f
+// CFI-NOT: llvm.assume
+// NOCFI: llvm.assume
+void s1f(S1 *s1) {
+  s1->f();
+}
Index: lib/CodeGen/CGClass.cpp
===================================================================
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2523,8 +2523,10 @@
 void CodeGenFunction::EmitTypeMetadataCodeForVCall(const CXXRecordDecl *RD,
                                                    llvm::Value *VTable,
                                                    SourceLocation Loc) {
-  if (CGM.getCodeGenOpts().WholeProgramVTables &&
-      CGM.HasHiddenLTOVisibility(RD)) {
+  if (SanOpts.has(SanitizerKind::CFIVCall))
+    EmitVTablePtrCheckForCall(RD, VTable, CodeGenFunction::CFITCK_VCall, Loc);
+  else if (CGM.getCodeGenOpts().WholeProgramVTables &&
+           CGM.HasHiddenLTOVisibility(RD)) {
     llvm::Metadata *MD =
         CGM.CreateMetadataIdentifierForType(QualType(RD->getTypeForDecl(), 0));
     llvm::Value *TypeId =
@@ -2536,9 +2538,6 @@
                            {CastedVTable, TypeId});
     Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::assume), TypeTest);
   }
-
-  if (SanOpts.has(SanitizerKind::CFIVCall))
-    EmitVTablePtrCheckForCall(RD, VTable, CodeGenFunction::CFITCK_VCall, Loc);
 }
 
 void CodeGenFunction::EmitVTablePtrCheckForCall(const CXXRecordDecl *RD,

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits