[PATCH] D15699: [cfi] Cross-DSO CFI diagnostic mode (clang part)

[Evgeniy Stepanov via cfe-commits]

eugenis added inline comments.

================
Comment at: lib/CodeGen/CGExpr.cpp:2654-2678
@@ +2653,27 @@
+  for (auto CheckKindMaskPair : CheckKinds) {
+    int Kind = CheckKindMaskPair.first;
+    SanitizerMask Mask = CheckKindMaskPair.second;
+    // All CFI checks are recoverable.
+    assert(getRecoverableKind(Mask) == CheckRecoverableKind::Recoverable);
+    if (CGM.getCodeGenOpts().SanitizeTrap.has(Mask)) {
+      SI->addCase(llvm::ConstantInt::get(Int8Ty, Kind), TrapBB);
+    } else if (CGM.getCodeGenOpts().SanitizeRecover.has(Mask)) {
+      if (!RecoverBB) {
+        RecoverBB = createBasicBlock("non_fatal");
+        EmitBlock(RecoverBB);
+        emitCheckHandlerCall(*this, F->getFunctionType(), {Data, Addr},
+                             "cfi_check_fail",
+                             CheckRecoverableKind::Recoverable, false, ExitBB);
+      }
+      SI->addCase(llvm::ConstantInt::get(Int8Ty, Kind), RecoverBB);
+    } else {
+      if (!FatalBB) {
+        FatalBB = createBasicBlock("fatal");
+        EmitBlock(FatalBB);
+        emitCheckHandlerCall(*this, F->getFunctionType(), {Data, Addr},
+                             "cfi_check_fail",
+                             CheckRecoverableKind::Recoverable, true, ExitBB);
+      }
+      SI->addCase(llvm::ConstantInt::get(Int8Ty, Kind), FatalBB);
+    }
+  }
----------------
pcc wrote:
> Can't you replace all this code with a call to `EmitCheck` passing `icmp`s for each of the check kinds? Sure, it wouldn't be a switch, but we don't care about the performance of this code anyway.
Done. The code became a bit simpler, but the test looks awful now.

================
Comment at: test/CodeGen/cfi-icall-cross-dso.c:28
@@ -22,3 +27,3 @@
 // ITANIUM: call i1 @llvm.bitset.test(i8* %{{.*}}, metadata !"_ZTSFvE"), !nosanitize
-// ITANIUM: call void @__cfi_slowpath(i64 6588678392271548388, i8* %{{.*}}) {{.*}}, !nosanitize
+// ITANIUM: call void @__cfi_slowpath_diag(i64 6588678392271548388, i8* %{{.*}}, {{.*}}@[[DATA]]{{.*}}, !nosanitize
 
----------------
pcc wrote:
> Are you still testing the `__cfi_slowpath` code path?
added a test for the non-diag path


Repository:
  rL LLVM

http://reviews.llvm.org/D15699