[PATCH] D16823: [cfi] Safe handling of unaddressable vtable pointers (clang).

[Evgeniy Stepanov via cfe-commits]

eugenis added inline comments.

================
Comment at: lib/CodeGen/CGExpr.cpp:2494
@@ +2493,3 @@
+  llvm::Value *ValidVtable = nullptr;
+  if (CheckAndAppendValidVtable) {
+    llvm::Value *AllVtables = llvm::MetadataAsValue::get(
----------------
pcc wrote:
> eugenis wrote:
> > samsonov wrote:
> > > This is really ugly. Why are you not passing it down in DynamicArgs? Is it performance penalty you don't want to pay if the check will not succeed? How large will it be?
> > Yes, I want this code to be on the failing side of the check.
> > This would cost about the same as the check itself, so I suspect it could double the overhead.
> > 
> I would just emit the call unconditionally. We don't care too much about the performance in non-trapping mode, and if it becomes a problem in practice we can see if we can have the optimizer move the call into the conditional block (which I suspect it already knows how to do).
I care about performance in non-trapping mode.
Doing this change would not make the code any less ugly. For example, EmitCheck may not use the argument if the check has -fsanitize-trap behaviour, in which case we get an unused llvm.bitset.test call that affects some of the clang tests.



Repository:
  rL LLVM

http://reviews.llvm.org/D16823