[PATCH] D16821: Add whole-program vtable optimization feature to Clang.

[Richard Smith via cfe-commits]

rsmith added inline comments.

================
Comment at: lib/CodeGen/CGVTables.cpp:904-919
@@ -900,5 +903,18 @@
+
+bool CodeGenModule::IsBitSetBlacklistedRecord(const CXXRecordDecl *RD) {
+  std::string TypeName = RD->getQualifiedNameAsString();
+  auto isInBlacklist = [&](const SanitizerBlacklist &BL) {
+    if (RD->hasAttr<UuidAttr>() && BL.isBlacklistedType("attr:uuid"))
+      return true;
+
+    return BL.isBlacklistedType(TypeName);
+  };
 
-  return getContext().getSanitizerBlacklist().isBlacklistedType(
-      RD->getQualifiedNameAsString());
+  return isInBlacklist(WholeProgramVTablesBlacklist) ||
+         ((LangOpts.Sanitize.has(SanitizerKind::CFIVCall) ||
+           LangOpts.Sanitize.has(SanitizerKind::CFINVCall) ||
+           LangOpts.Sanitize.has(SanitizerKind::CFIDerivedCast) ||
+           LangOpts.Sanitize.has(SanitizerKind::CFIUnrelatedCast)) &&
+          isInBlacklist(getContext().getSanitizerBlacklist()));
 }
 
----------------
It looks like putting a class in a sanitizer blacklist turns off the vptr optimizations for the class and putting it in the vptr blacklist turns off CFI checks for it. Can we avoid that, perhaps by using separate bitsets for the vptr checks and CFI?

================
Comment at: lib/CodeGen/CodeGenModule.h:492
@@ -491,1 +491,3 @@
 
+  SanitizerBlacklist WholeProgramVTablesBlacklist;
+
----------------
Now might be a good time to rename the `SanitizerBlacklist` class to something more general (but not as part of this commit).

================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:1605
@@ -1604,5 +1604,3 @@
 
-  if (CGF.SanOpts.has(SanitizerKind::CFIVCall))
-    CGF.EmitVTablePtrCheckForCall(MethodDecl, VTable,
-                                  CodeGenFunction::CFITCK_VCall, Loc);
+  CGF.EmitBitSetCodeForVCall(MethodDecl->getParent(), VTable, Loc);
 
----------------
You can be a lot more aggressive than this -- you can make an assumption about the value of the vptr from within `EmitTypeCheck` in every case where the vptr sanitizer would emit a dynamic type check. I'm not sure that doing so will allow you to deduce a lot more vptrs, but it seems like it could help in some cases.


http://reviews.llvm.org/D16821