Fix vtbl field addr space

[yaxunl]

Summary

Storing the vtable field of an object should use the same address space as
the this pointer. Currently it is assumed to be addr space 0 but this may not
be true.

This assumption (added in 054cc3b1b469de4b0cb25d1dc3af43c679c5dc44) caused
issues for the out-of-tree CHERI targets.

[rjmccall]

The following line is always just making a bitcast, then.

[arichardson]

Thanks, I can confirm that this fixes the assertions we were seeing after merging D103835. A few minor suggestions below:

There's a typo in the commit message, I'd maybe change it to:
Storing the vtable field of an object should use the same address space as the this pointer.
And maybe change This caused issue for some out of tree project. to something like This assumption (added in 054cc3b1b469de4b0cb25d1dc3af43c679c5dc44) caused issues for the out-of-tree CHERI targets.

clang/lib/CodeGen/CGClass.cpp

2520–2525

As noted by @rjmccall, changing this line still allows all tests to pass (including our newly added out-of-tree CHERI ones).

2521–2522

Maybe this is clearer?

[yaxunl]

In D109841#3002404, @arichardson wrote:

Thanks, I can confirm that this fixes the assertions we were seeing after merging D103835. A few minor suggestions below:

There's a typo in the commit message, I'd maybe change it to:
Storing the vtable field of an object should use the same address space as the this pointer.
And maybe change This caused issue for some out of tree project. to something like This assumption (added in 054cc3b1b469de4b0cb25d1dc3af43c679c5dc44) caused issues for the out-of-tree CHERI targets.

will do

clang/lib/CodeGen/CGClass.cpp

2520–2525

will do. I think we can also make changes so that

VTableField = Builder.CreateBitCast(
      VTableField, VTablePtrTy->getPointerTo(ThisAddrSpace));

2521–2522

will do

[yaxunl]

fix comments and casts

[rjmccall]

LGTM

clang/lib/CodeGen/CGClass.cpp

if (CGM.getCXXABI().isVirtualOffsetNeededForVTableField(*this, Vptr)) {
     NonVirtualOffset = Vptr.OffsetFromNearestVBase;
   } else {
     // We can just use the base offset in the complete class.
     NonVirtualOffset = Vptr.Base.getBaseOffset();
   }
 
   // Apply the offsets.
   Address VTableField = LoadCXXThisAddress();
+  unsigned ThisAddrSpace =
+      VTableField.getPointer()->getType()->getPointerAddressSpace();
 
   if (!NonVirtualOffset.isZero() || VirtualOffset)
     VTableField = ApplyNonVirtualAndVirtualOffset(
         *this, VTableField, NonVirtualOffset, VirtualOffset, Vptr.VTableClass,
         Vptr.NearestVBase);
 
   // Finally, store the address point. Use the same LLVM types as the field to
   // support optimization.
   unsigned GlobalsAS = CGM.getDataLayout().getDefaultGlobalsAddressSpace();
   unsigned ProgAS = CGM.getDataLayout().getProgramAddressSpace();
   llvm::Type *VTablePtrTy =
       llvm::FunctionType::get(CGM.Int32Ty, /*isVarArg=*/true)
           ->getPointerTo(ProgAS)
           ->getPointerTo(GlobalsAS);
-  // vtable field is is derived from `this` pointer, therefore it should be in
-  // default address space.
-  VTableField = Builder.CreatePointerBitCastOrAddrSpaceCast(
-      VTableField, VTablePtrTy->getPointerTo());
-  VTableAddressPoint = Builder.CreatePointerBitCastOrAddrSpaceCast(
-      VTableAddressPoint, VTablePtrTy);
+  // vtable field is is derived from `this` pointer, therefore they should be in
+  // the same addr space. Note that this might not be LLVM address space 0.
+  VTableField = Builder.CreateBitCast(VTableField,
+                                      VTablePtrTy->getPointerTo(ThisAddrSpace));
+  VTableAddressPoint = Builder.CreateBitCast(VTableAddressPoint, VTablePtrTy);
 
   llvm::StoreInst *Store = Builder.CreateStore(VTableAddressPoint, VTableField);
   TBAAAccessInfo TBAAInfo = CGM.getTBAAVTablePtrAccessInfo(VTablePtrTy);
   CGM.DecorateInstructionWithTBAA(Store, TBAAInfo);
   if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
       CGM.getCodeGenOpts().StrictVTablePointers)
     CGM.DecorateInstructionWithInvariantGroup(Store, Vptr.VTableClass);
 }