[PATCH] Implement Control Flow Integrity for virtual calls.

JF Bastien jfb at chromium.org
Thu Feb 5 09:48:09 PST 2015


================
Comment at: docs/ControlFlowIntegrity.rst:3
@@ +2,3 @@
+Control Flow Integrity
+======================
+
----------------
Could you add a section which contains links to publications on CFI, for the approaches that are implemented as well as ones that aren't but may be relevant.

================
Comment at: lib/AST/ItaniumMangle.cpp:3945
@@ +3944,3 @@
+    SourceManager &SM = getASTContext().getSourceManager();
+    Out << "[" << SM.getFileEntryForID(SM.getMainFileID())->getName() << "]";
+  }
----------------
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?

================
Comment at: lib/AST/MicrosoftMangle.cpp:2574
@@ +2573,3 @@
+                                                       raw_ostream &Out) {
+  llvm_unreachable("Cannot mangle bitsets yet");
+}
----------------
`report_fatal_error`

================
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()) {
----------------
`if (auto Arr = dyn_cast<llvm::ConstantArray>(GV->getInitializer())) {` is LLVM-idiomatic.

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

================
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}
----------------
What's in the `{{.*}}`?

http://reviews.llvm.org/D7424

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






More information about the cfe-commits mailing list