[PATCH] Fix PR20444 -- wrong number of vftable slots created when return adjustment thunks are needed

Reid Kleckner rnk at google.com
Fri Aug 8 11:17:30 PDT 2014


lgtm

================
Comment at: lib/AST/VTableBuilder.cpp:2795
@@ -2794,3 @@
-/// methods.
-bool VFTableBuilder::NeedsReturnAdjustingThunk(const CXXMethodDecl *MD) {
-  OverriddenMethodsSetTy OverriddenMethods;
----------------
So this fix (I think I wrote it?) was incorrect.  We need to look back at the chain of method overrides in the current vftable, rather than all possible overrides.

================
Comment at: lib/AST/VTableBuilder.cpp:2885-2886
@@ +2884,4 @@
+      // convertible to the OverriddenMD's one.
+      // Interestingly, a new vftable slot is also created if OverriddenMD
+      // itself required an extra slot due to return adjustment.
+      ReturnAdjustingThunk = !ComputeReturnAdjustmentBaseOffset(
----------------
I would say something like:
  // Once a chain of method overrides adds a return adjusting vftable slot, all subsequent overrides will also use an extra method slot.

Would you agree with that statement?

http://reviews.llvm.org/D4822






More information about the cfe-commits mailing list