[PATCH] Implement Control Flow Integrity for virtual calls.

[Peter Collingbourne]

================
Comment at: lib/AST/ItaniumMangle.cpp:3945
@@ +3944,3 @@
+    SourceManager &SM = getASTContext().getSourceManager();
+    Out << "[" << SM.getFileEntryForID(SM.getMainFileID())->getName() << "]";
+  }
----------------
jfb wrote:
> Is that just the basename? Does it take capitalization into account? I want to make sure it'll mangle the same way on different hosts, so it's possible to generate the same binary when cross-compiling.
> 
> IIUC the caveat is that two files with the same name and classses in anonymous namespaces would mangle to the same bitset? I would point out this cornercase in a comment.
> 
> Also, what happens if a user (foolishly) defines a class in a header file within an anonymous namespace, and then uses it or subclasses it in .cc files? I guess that should just work (with or without CFI) but would be inefficient because of the duplication?
> Is that just the basename? Does it take capitalization into account? I want to make sure it'll mangle the same way on different hosts, so it's possible to generate the same binary when cross-compiling.

It is the full source file name as supplied on the command line. These names won't make it into the final object file, so that seems like it would mostly do as far as hermeticity goes.

> IIUC the caveat is that two files with the same name and classses in anonymous namespaces would mangle to the same bitset? I would point out this cornercase in a comment.

I think you would either need the two object files to be compiled from the same relative path from different directories, or somehow build two translation units from the same source file.

> Also, what happens if a user (foolishly) defines a class in a header file within an anonymous namespace, and then uses it or subclasses it in .cc files? I guess that should just work (with or without CFI) but would be inefficient because of the duplication?

I don't think it would work under CFI. The two classes would have different identities, and the source file names mangled into the bitset names would be different. So the program would type check, but would fail at runtime under CFI if one translation unit tries to vcall a class from another translation unit.

================
Comment at: lib/AST/MicrosoftMangle.cpp:2574
@@ +2573,3 @@
+                                                       raw_ostream &Out) {
+  llvm_unreachable("Cannot mangle bitsets yet");
+}
----------------
jfb wrote:
> `report_fatal_error`
Makes sense. I suppose you can contrive to get here with the Microsoft ABI.

================
Comment at: lib/CodeGen/CGVTables.cpp:867
@@ +866,3 @@
+      auto Arr = dyn_cast<llvm::ConstantArray>(GV->getInitializer());
+      if (Arr) {
+        for (llvm::Value *Op : Arr->operands()) {
----------------
jfb wrote:
> `if (auto Arr = dyn_cast<llvm::ConstantArray>(GV->getInitializer())) {` is LLVM-idiomatic.
More to the point, this entire block (lines 862-872) is obsolete (it belonged to an earlier version of the code). I'll remove it.

================
Comment at: lib/CodeGen/CGVTables.cpp:903
@@ +902,3 @@
+                           cast<llvm::ConstantAsMetadata>(T2->getOperand(2))
+                               ->getValue())->getZExtValue();
+    return Offset1 < Offset2;
----------------
jfb wrote:
> `assert(Offset1 != Offset2);`
Will do.

================
Comment at: test/CodeGenCXX/cfi-vptr.cpp:70
@@ +69,3 @@
+// CHECK-DAG: !{!"1C", [10 x i8*]* @_ZTVN12_GLOBAL__N_11DE, i64 72}
+// CHECK-DAG: !{!"[{{.*}}cfi-vptr.cpp]N12_GLOBAL__N_11DE", [10 x i8*]* @_ZTVN12_GLOBAL__N_11DE, i64 32}
+// CHECK-DAG: !{!"1A", [5 x i8*]* @_ZTV1B, i64 32}
----------------
jfb wrote:
> What's in the `{{.*}}`?
That would be the absolute path to the file.

http://reviews.llvm.org/D7424

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/