[PATCH] D50372: Introduce the VTable interleaving scheme to the CFI design documentation

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

Please upload patches with context. `arc diff` will do this for you.



================
Comment at: clang/docs/ControlFlowIntegrityDesign.rst:277
 
+Forward-Edge CFI for Virtual Calls by Interleaving Virtual Tables
+=================================================================
----------------
I would add this as a subsection of "Forward-Edge CFI for Virtual Calls".


================
Comment at: clang/docs/ControlFlowIntegrityDesign.rst:286
+
+On the high level, the interleaving scheme consists of two steps: 1) order virtual tables by a pre-order 
+traversal of the class hierarchy and 2) interleave the virtual tables entry by entry.
----------------
On the high level -> At a high level


================
Comment at: clang/docs/ControlFlowIntegrityDesign.rst:322
+This step will arrange the virtual tables for A, B, C, and D in the order of *vtable-of-A, vtable-of-B, vtable-of-D, vtable-of-C*.
+In this order, for any class all the compatible virtual tables will appear consecutively.
+
----------------
I would move this sentence to the start of the subsection because it isn't specific to your example and clarify that although GlobalLayoutBuilder tries to place compatible vtables consecutively (but doesn't always succeed because the Itanium ABI glues vtables together), this algorithm requires them to appear consecutively.


================
Comment at: clang/docs/ControlFlowIntegrityDesign.rst:331
+works under this scheme because the interleaved virtual table has the property that for 
+each virtual funtion the distance between an entry of this function and the corresponding 
+address point is always the same. 
----------------
funtion -> function


================
Comment at: clang/docs/ControlFlowIntegrityDesign.rst:339
+  
+  A::offset-to-top, B::offset-to-top, D::offset-to-top, C::offset-to-top, &A::rtti, &B::rtti, &D::rtti, &C::rtti, &A::f1, &B::f1, &D::f1, &C::f1, &B::f2, &D::f2, &C::f3, &D::f4
+
----------------
This layout isn't necessarily going to work with traditional RTTI because the `__dynamic_cast` function is allowed to assume that the rtti and offset-to-top fields appear at the offsets behind the address point that the ABI says that they will appear at. Indeed, the libcxxabi implementation makes that assumption:

https://github.com/llvm-mirror/libcxxabi/blob/master/src/private_typeinfo.cpp#L627

It's probably more something to keep in mind for the implementation, but I think we at least need to mention the RTTI incompatibility here.


Repository:
  rC Clang

https://reviews.llvm.org/D50372