[PATCH] Fix PR17738 - add support for vtordisp thunks when using -cxx-abi microsoft

Timur Iskhodzhanov timurrrr at google.com
Tue Nov 5 10:17:26 PST 2013


  Please take another look


================
Comment at: include/clang/Basic/ABI.h:111
@@ +110,3 @@
+  /// adjustment, if needed.
+  union VirtualAdjustment {
+    // Itanium ABI
----------------
Reid Kleckner wrote:
> I'm still not really totally comfortable with the unionification of this stuff.  We should ask John what he thinks the right design is for this stuff.  This is consistent with what's already been committed, but we should probably figure out the right way to do this going forward.
During offline discussion, we've agreed to use unions for now* to significantly improve the support of virtual calls in this ABI and explicitly ask John for a post-commit review of the overall ABI.h change.

* - Mostly since we're highly unlikely to change anything for the Itanium ABI soon and just unlikely to need any changes for the Microsoft ABI.

================
Comment at: lib/AST/MicrosoftMangle.cpp:1849
@@ -1848,2 +1848,3 @@
                                       MicrosoftCXXNameMangler &Mangler,
                                       raw_ostream &Out) {
+  if (!Adjustment.Virtual.isEmpty()) {
----------------
Reid Kleckner wrote:
> This is complicated enough that it deserves it's own BNF comment with portions of the mangleFunctionClass() BNF diagram.  The G, O, W and A, I, Q access codes clearly correspond to it.
Agreed. I'm not a BNF master, so formatting/content suggestions are welcome!

It has actually reminded me we might sometimes have far thunks... I'm not sure when are they needed (hey, why would you need more than +-2^31 to adjust this inside a record type?) so I've added a FIXME. Anyways, the mangleFunctionClass didn't ever handle "far" functions either.


http://llvm-reviews.chandlerc.com/D2079



More information about the cfe-commits mailing list