[PATCH] [ms-cxxabi] Implement codegen for dereferencing member pointers

[John McCall]

On Apr 2, 2013, at 11:00 AM, Reid Kleckner <rnk at google.com> wrote:
> Handles all inheritance models for both data and function member
> pointers.  This does not require knowing the layout of the vbtable,
> because the member pointer itself carries the offset into the vbtable.
> 
> Clang still cannot emit multi-field member pointers because it would
> need to know the layout of the vbtable to generate the correct offsets.
> 
> Also implements isZeroInitializable() and refactors some of the null
> member pointer code.

+  llvm::tie(Ptrs, Ints) = MPT->getMSMemberPointerSlots();

I missed this particular method getting added.  I think this is probably
going a bit too far.  It's not really generally meaningful;  you have two
clients of it, and one of them has to do weird special-casing anyway
(i.e. filling in -1 in the right place).

Also, your methods aren't consistent about null pointers.  Your isZeroInitializable
says that all the bits are zero unless the class uses single inheritance and
isn't polymorphic, but then EmitNullMemberPointer tosses a -1 on the end
of certain wide null-pointers.

+  switch (Inheritance) {
+  default:
+    llvm_unreachable("invalid inheritance");

Kill the default case and put the unreachable after the switch.  Having a
default case prevents the compiler from warning you about missing cases.

+  case MSIM_Unspecified: {
+    CharUnits offs = getContext().getASTRecordLayout(RD).getVBPtrOffset();
+    llvm::Constant *VBTableOffset =
+      llvm::ConstantInt::get(CGM.IntTy, offs.getQuantity());
+    llvm::Constant *fields[] = {
+      FieldOffset,
+      VBTableOffset,
+      getZeroInt()    // FIXME: VirtualBaseAdjustmentOffset
+    };

The VirtualBaseAdjustmentOffset should be some special value to indicate
that this isn't a member of a virtual base;  you just need to leave that value
in there.

Also, are you sure you're supposed to leave a meaningful vb-table offset
in here even when the member *isn't* a member of a virtual base?

Also, recall that the class very well might not have a vb-table;  there might
be a signal value for that, too.

+  llvm::Type *Int8Ptr = Builder.getInt8Ty()->getPointerTo(0);

This is CGM.Int8PtrTy.

+llvm::Value *
+MicrosoftCXXABI::AdjustVirtualBase(CGBuilderTy &Builder,
+                                   const CXXRecordDecl *RD, llvm::Value *Base,
+                                   llvm::Value *VirtualBaseAdjustmentOffset,
+                                   llvm::Value *VBTableOffset) {

This code is assuming that it *has* to adjust by a virtual base.
That is not the case (unless MSVC does something interesting
in its vb-table layouts, like leave a zero offset at a particular offset
in the table...), and even then it's suspect in the MSIM_unspecified
case where it's completely legitimate that the record type might not
have a vb-table.

John.