-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[CIR] Add support for initializing classes with multiple vtables #155275
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clangir Author: Andy Kaylor (andykaylor) ChangesThis adds support for initializing the vptr members in a class that requires multiple vtables because of multiple inheritence. This still does not handle virtual bases. Full diff: https://github.com/llvm/llvm-project/pull/155275.diff 3 Files Affected:
diff --git a/clang/lib/CIR/CodeGen/CIRGenClass.cpp b/clang/lib/CIR/CodeGen/CIRGenClass.cpp
index 3e5dc22426d8e..26d6447480750 100644
--- a/clang/lib/CIR/CodeGen/CIRGenClass.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenClass.cpp
@@ -14,6 +14,7 @@
#include "CIRGenFunction.h"
#include "CIRGenValue.h"
+#include "clang/AST/EvaluatedExprVisitor.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/RecordLayout.h"
#include "clang/AST/Type.h"
@@ -126,6 +127,32 @@ static bool isInitializerOfDynamicClass(const CXXCtorInitializer *baseInit) {
return baseClassDecl->isDynamicClass();
}
+namespace {
+/// A visitor which checks whether an initializer uses 'this' in a
+/// way which requires the vtable to be properly set.
+struct DynamicThisUseChecker
+ : ConstEvaluatedExprVisitor<DynamicThisUseChecker> {
+ using super = ConstEvaluatedExprVisitor<DynamicThisUseChecker>;
+
+ bool usesThis;
+
+ DynamicThisUseChecker(const ASTContext &c) : super(c), usesThis(false) {}
+
+ // Black-list all explicit and implicit references to 'this'.
+ //
+ // Do we need to worry about external references to 'this' derived
+ // from arbitrary code? If so, then anything which runs arbitrary
+ // external code might potentially access the vtable.
+ void VisitCXXThisExpr(const CXXThisExpr *e) { usesThis = true; }
+};
+} // end anonymous namespace
+
+static bool baseInitializerUsesThis(ASTContext &c, const Expr *init) {
+ DynamicThisUseChecker checker(c);
+ checker.Visit(init);
+ return checker.usesThis;
+}
+
/// Gets the address of a direct base class within a complete object.
/// This should only be used for (1) non-virtual bases or (2) virtual bases
/// when the type is known to be complete (e.g. in complete destructors).
@@ -170,10 +197,8 @@ void CIRGenFunction::emitBaseInitializer(mlir::Location loc,
// If the initializer for the base (other than the constructor
// itself) accesses 'this' in any way, we need to initialize the
// vtables.
- if (classDecl->isDynamicClass()) {
- cgm.errorNYI(loc, "emitBaseInitializer: dynamic class");
- return;
- }
+ if (baseInitializerUsesThis(getContext(), baseInit->getInit()))
+ initializeVTablePointers(loc, classDecl);
// We can pretend to be a complete class because it only matters for
// virtual bases, and we only do virtual bases for complete ctors.
@@ -264,6 +289,37 @@ void CIRGenFunction::emitCtorPrologue(const CXXConstructorDecl *cd,
}
}
+static Address applyNonVirtualAndVirtualOffset(
+ mlir::Location loc, CIRGenFunction &cgf, Address addr,
+ CharUnits nonVirtualOffset, mlir::Value virtualOffset,
+ const CXXRecordDecl *derivedClass, const CXXRecordDecl *nearestVBase,
+ mlir::Type baseValueTy = {}, bool assumeNotNull = true) {
+ // Assert that we have something to do.
+ assert(!nonVirtualOffset.isZero() || virtualOffset != nullptr);
+
+ // Compute the offset from the static and dynamic components.
+ mlir::Value baseOffset;
+ if (!nonVirtualOffset.isZero()) {
+ if (virtualOffset) {
+ cgf.cgm.errorNYI(
+ loc,
+ "applyNonVirtualAndVirtualOffset: virtual and non-virtual offset");
+ return Address::invalid();
+ } else {
+ assert(baseValueTy && "expected base type");
+ // If no virtualOffset is present this is the final stop.
+ return cgf.getBuilder().createBaseClassAddr(
+ loc, addr, baseValueTy, nonVirtualOffset.getQuantity(),
+ assumeNotNull);
+ }
+ } else {
+ baseOffset = virtualOffset;
+ }
+
+ cgf.cgm.errorNYI(loc, "applyNonVirtualAndVirtualOffset: virtual offset");
+ return Address::invalid();
+}
+
void CIRGenFunction::initializeVTablePointer(mlir::Location loc,
const VPtr &vptr) {
// Compute the address point.
@@ -291,8 +347,9 @@ void CIRGenFunction::initializeVTablePointer(mlir::Location loc,
// Apply the offsets.
Address classAddr = loadCXXThisAddress();
if (!nonVirtualOffset.isZero() || virtualOffset) {
- cgm.errorNYI(loc,
- "initializeVTablePointer: non-virtual and virtual offset");
+ classAddr = applyNonVirtualAndVirtualOffset(
+ loc, *this, classAddr, nonVirtualOffset, virtualOffset,
+ vptr.vtableClass, vptr.nearestVBase, baseValueTy);
}
// Finally, store the address point. Use the same CIR types as the field.
@@ -326,10 +383,11 @@ void CIRGenFunction::initializeVTablePointers(mlir::Location loc,
CIRGenFunction::VPtrsVector
CIRGenFunction::getVTablePointers(const CXXRecordDecl *vtableClass) {
CIRGenFunction::VPtrsVector vptrsResult;
+ VisitedVirtualBasesSetTy vbases;
getVTablePointers(BaseSubobject(vtableClass, CharUnits::Zero()),
/*NearestVBase=*/nullptr,
/*OffsetFromNearestVBase=*/CharUnits::Zero(),
- /*BaseIsNonVirtualPrimaryBase=*/false, vtableClass,
+ /*BaseIsNonVirtualPrimaryBase=*/false, vtableClass, vbases,
vptrsResult);
return vptrsResult;
}
@@ -339,6 +397,7 @@ void CIRGenFunction::getVTablePointers(BaseSubobject base,
CharUnits offsetFromNearestVBase,
bool baseIsNonVirtualPrimaryBase,
const CXXRecordDecl *vtableClass,
+ VisitedVirtualBasesSetTy &vbases,
VPtrsVector &vptrs) {
// If this base is a non-virtual primary base the address point has already
// been set.
@@ -350,8 +409,39 @@ void CIRGenFunction::getVTablePointers(BaseSubobject base,
const CXXRecordDecl *rd = base.getBase();
- if (rd->getNumBases())
- cgm.errorNYI(rd->getSourceRange(), "getVTablePointers: traverse bases");
+ for (const auto &nextBase : rd->bases()) {
+ const auto *baseDecl =
+ cast<CXXRecordDecl>(
+ nextBase.getType()->castAs<RecordType>()->getOriginalDecl())
+ ->getDefinitionOrSelf();
+
+ // Ignore classes without a vtable.
+ if (!baseDecl->isDynamicClass())
+ continue;
+
+ CharUnits baseOffset;
+ CharUnits baseOffsetFromNearestVBase;
+ bool baseDeclIsNonVirtualPrimaryBase;
+ const CXXRecordDecl *nextBaseDecl;
+
+ if (nextBase.isVirtual()) {
+ cgm.errorNYI(rd->getSourceRange(), "getVTablePointers: virtual base");
+ return;
+ } else {
+ const ASTRecordLayout &layout = getContext().getASTRecordLayout(rd);
+
+ nextBaseDecl = baseDecl;
+ baseOffset = base.getBaseOffset() + layout.getBaseClassOffset(baseDecl);
+ baseOffsetFromNearestVBase =
+ offsetFromNearestVBase + layout.getBaseClassOffset(baseDecl);
+ baseDeclIsNonVirtualPrimaryBase = layout.getPrimaryBase() == baseDecl;
+ }
+
+ getVTablePointers(BaseSubobject(baseDecl, baseOffset), nextBaseDecl,
+ baseOffsetFromNearestVBase,
+ baseDeclIsNonVirtualPrimaryBase, vtableClass, vbases,
+ vptrs);
+ }
}
Address CIRGenFunction::loadCXXThisAddress() {
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index 064b3c15a310b..c799ecdc27538 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -574,6 +574,9 @@ class CIRGenFunction : public CIRGenTypeCache {
const clang::CXXRecordDecl *vtableClass;
};
+ using VisitedVirtualBasesSetTy =
+ llvm::SmallPtrSet<const clang::CXXRecordDecl *, 4>;
+
using VPtrsVector = llvm::SmallVector<VPtr, 4>;
VPtrsVector getVTablePointers(const clang::CXXRecordDecl *vtableClass);
void getVTablePointers(clang::BaseSubobject base,
@@ -581,7 +584,7 @@ class CIRGenFunction : public CIRGenTypeCache {
clang::CharUnits offsetFromNearestVBase,
bool baseIsNonVirtualPrimaryBase,
const clang::CXXRecordDecl *vtableClass,
- VPtrsVector &vptrs);
+ VisitedVirtualBasesSetTy &vbases, VPtrsVector &vptrs);
/// Return the Value of the vtable pointer member pointed to by thisAddr.
mlir::Value getVTablePtr(mlir::Location loc, Address thisAddr,
const clang::CXXRecordDecl *vtableClass);
diff --git a/clang/test/CIR/CodeGen/multi-vtable.cpp b/clang/test/CIR/CodeGen/multi-vtable.cpp
index c127285800844..b42f8a6518095 100644
--- a/clang/test/CIR/CodeGen/multi-vtable.cpp
+++ b/clang/test/CIR/CodeGen/multi-vtable.cpp
@@ -22,6 +22,7 @@ class Father {
class Child : public Mother, public Father {
public:
+ Child();
void MotherKey() override;
};
@@ -36,6 +37,50 @@ void Child::MotherKey() {}
// CIR-DAG: !rec_Mother = !cir.record<class "Mother" {!cir.vptr}
// CIR-DAG: !rec_Child = !cir.record<class "Child" {!rec_Mother, !rec_Father}
+// Child vtable
+
+// CIR: cir.global "private" external @_ZTV5Child = #cir.vtable<{
+// CIR-SAME: #cir.const_array<[
+// CIR-SAME: #cir.ptr<null> : !cir.ptr<!u8i>,
+// CIR-SAME: #cir.ptr<null> : !cir.ptr<!u8i>,
+// CIR-SAME: #cir.global_view<@_ZN5Child9MotherKeyEv> : !cir.ptr<!u8i>,
+// CIR-SAME: #cir.global_view<@_ZN6Mother12MotherNonKeyEv> : !cir.ptr<!u8i>
+// CIR-SAME: ]> : !cir.array<!cir.ptr<!u8i> x 4>,
+// CIR-SAME: #cir.const_array<[
+// CIR-SAME: #cir.ptr<-8 : i64> : !cir.ptr<!u8i>,
+// CIR-SAME: #cir.ptr<null> : !cir.ptr<!u8i>,
+// CIR-SAME: #cir.global_view<@_ZN6Father9FatherKeyEv> : !cir.ptr<!u8i>
+// CIR-SAME: ]> : !cir.array<!cir.ptr<!u8i> x 3>
+// CIR-SAME: }> : [[CHILD_VTABLE_TYPE]]
+
+// LLVM: @_ZTV5Child = global { [4 x ptr], [3 x ptr] } {
+// LLVM-SAME: [4 x ptr] [
+// LLVM-SAME: ptr null,
+// LLVM-SAME: ptr null,
+// LLVM-SAME: ptr @_ZN5Child9MotherKeyEv,
+// LLVM-SAME: ptr @_ZN6Mother12MotherNonKeyEv
+// LLVM-SAME: ],
+// LLVM-SAME: [3 x ptr] [
+// LLVM-SAME: ptr inttoptr (i64 -8 to ptr),
+// LLVM-SAME: ptr null,
+// LLVM-SAME: ptr @_ZN6Father9FatherKeyEv
+// LLVM-SAME: ]
+// LLVM-SAME: }
+
+// OGCG: @_ZTV5Child = unnamed_addr constant { [4 x ptr], [3 x ptr] } {
+// OGCG-SAME: [4 x ptr] [
+// OGCG-SAME: ptr null,
+// OGCG-SAME: ptr null,
+// OGCG-SAME: ptr @_ZN5Child9MotherKeyEv,
+// OGCG-SAME: ptr @_ZN6Mother12MotherNonKeyEv
+// OGCG-SAME: ],
+// OGCG-SAME: [3 x ptr] [
+// OGCG-SAME: ptr inttoptr (i64 -8 to ptr),
+// OGCG-SAME: ptr null,
+// OGCG-SAME: ptr @_ZN6Father9FatherKeyEv
+// OGCG-SAME: ]
+// OGCG-SAME: }
+
// Mother vtable
// CIR: cir.global "private" external @_ZTV6Mother = #cir.vtable<{
@@ -91,46 +136,48 @@ void Child::MotherKey() {}
// OGCG-SAME: ]
// OGCG-SAME: }
-// Child vtable
-
-// CIR: cir.global "private" external @_ZTV5Child = #cir.vtable<{
-// CIR-SAME: #cir.const_array<[
-// CIR-SAME: #cir.ptr<null> : !cir.ptr<!u8i>,
-// CIR-SAME: #cir.ptr<null> : !cir.ptr<!u8i>,
-// CIR-SAME: #cir.global_view<@_ZN5Child9MotherKeyEv> : !cir.ptr<!u8i>,
-// CIR-SAME: #cir.global_view<@_ZN6Mother12MotherNonKeyEv> : !cir.ptr<!u8i>
-// CIR-SAME: ]> : !cir.array<!cir.ptr<!u8i> x 4>,
-// CIR-SAME: #cir.const_array<[
-// CIR-SAME: #cir.ptr<-8 : i64> : !cir.ptr<!u8i>,
-// CIR-SAME: #cir.ptr<null> : !cir.ptr<!u8i>,
-// CIR-SAME: #cir.global_view<@_ZN6Father9FatherKeyEv> : !cir.ptr<!u8i>
-// CIR-SAME: ]> : !cir.array<!cir.ptr<!u8i> x 3>
-// CIR-SAME: }> : [[CHILD_VTABLE_TYPE]]
-// LLVM: @_ZTV5Child = global { [4 x ptr], [3 x ptr] } {
-// LLVM-SAME: [4 x ptr] [
-// LLVM-SAME: ptr null,
-// LLVM-SAME: ptr null,
-// LLVM-SAME: ptr @_ZN5Child9MotherKeyEv,
-// LLVM-SAME: ptr @_ZN6Mother12MotherNonKeyEv
-// LLVM-SAME: ],
-// LLVM-SAME: [3 x ptr] [
-// LLVM-SAME: ptr inttoptr (i64 -8 to ptr),
-// LLVM-SAME: ptr null,
-// LLVM-SAME: ptr @_ZN6Father9FatherKeyEv
-// LLVM-SAME: ]
-// LLVM-SAME: }
-
-// OGCG: @_ZTV5Child = unnamed_addr constant { [4 x ptr], [3 x ptr] } {
-// OGCG-SAME: [4 x ptr] [
-// OGCG-SAME: ptr null,
-// OGCG-SAME: ptr null,
-// OGCG-SAME: ptr @_ZN5Child9MotherKeyEv,
-// OGCG-SAME: ptr @_ZN6Mother12MotherNonKeyEv
-// OGCG-SAME: ],
-// OGCG-SAME: [3 x ptr] [
-// OGCG-SAME: ptr inttoptr (i64 -8 to ptr),
-// OGCG-SAME: ptr null,
-// OGCG-SAME: ptr @_ZN6Father9FatherKeyEv
-// OGCG-SAME: ]
-// OGCG-SAME: }
+Child::Child() {}
+
+// CIR: cir.func {{.*}} @_ZN5ChildC2Ev(%[[THIS_ARG:.*]]: !cir.ptr<!rec_Child>
+// CIR: %[[THIS_ADDR:.*]] = cir.alloca {{.*}} ["this", init]
+// CIR: cir.store %[[THIS_ARG]], %[[THIS_ADDR]]
+// CIR: %[[THIS:.*]] = cir.load %[[THIS_ADDR]]
+// CIR: %[[MOTHER_BASE:.*]] = cir.base_class_addr %[[THIS]] : !cir.ptr<!rec_Child> nonnull [0] -> !cir.ptr<!rec_Mother>
+// CIR: cir.call @_ZN6MotherC2Ev(%[[MOTHER_BASE]]) nothrow : (!cir.ptr<!rec_Mother>) -> ()
+// CIR: %[[FATHER_BASE:.*]] = cir.base_class_addr %[[THIS]] : !cir.ptr<!rec_Child> nonnull [8] -> !cir.ptr<!rec_Father>
+// CIR: cir.call @_ZN6FatherC2Ev(%[[FATHER_BASE]]) nothrow : (!cir.ptr<!rec_Father>) -> ()
+// CIR: %[[CHILD_VPTR:.*]] = cir.vtable.address_point(@_ZTV5Child, address_point = <index = 0, offset = 2>) : !cir.vptr
+// CIR: %[[CHILD_VPTR_ADDR:.*]] = cir.vtable.get_vptr %[[THIS]] : !cir.ptr<!rec_Child> -> !cir.ptr<!cir.vptr>
+// CIR: cir.store{{.*}} %[[CHILD_VPTR]], %[[CHILD_VPTR_ADDR]] : !cir.vptr, !cir.ptr<!cir.vptr>
+// CIR: %[[FATHER_IN_CHILD_VPTR:.*]] = cir.vtable.address_point(@_ZTV5Child, address_point = <index = 1, offset = 2>) : !cir.vptr
+// CIR: %[[FATHER_BASE:.*]] = cir.base_class_addr %[[THIS]] : !cir.ptr<!rec_Child> nonnull [8] -> !cir.ptr<!rec_Father>
+// CIR: %[[FATHER_IN_CHILD_VPTR_ADDR:.*]] = cir.vtable.get_vptr %[[FATHER_BASE]] : !cir.ptr<!rec_Father> -> !cir.ptr<!cir.vptr>
+// CIR: cir.store{{.*}} %[[FATHER_IN_CHILD_VPTR]], %[[FATHER_IN_CHILD_VPTR_ADDR]] : !cir.vptr, !cir.ptr<!cir.vptr>
+// CIR: cir.return
+
+// The GEP instructions are different between LLVM and OGCG, but they calculate the same addresses.
+
+// LLVM: define{{.*}} void @_ZN5ChildC2Ev(ptr{{.*}} %[[THIS_ARG:.*]])
+// LLVM: %[[THIS_ADDR:.*]] = alloca ptr
+// LLVM: store ptr %[[THIS_ARG]], ptr %[[THIS_ADDR]]
+// LLVM: %[[THIS:.*]] = load ptr, ptr %[[THIS_ADDR]]
+// LLVM: call void @_ZN6MotherC2Ev(ptr{{.*}} %[[THIS]])
+// LLVM: %[[FATHER_BASE:.*]] = getelementptr{{.*}} i8, ptr %[[THIS]], i32 8
+// LLVM: call void @_ZN6FatherC2Ev(ptr{{.*}} %[[FATHER_BASE]])
+// LLVM: store ptr getelementptr inbounds nuw (i8, ptr @_ZTV5Child, i64 16), ptr %[[THIS]]
+// LLVM: %[[FATHER_BASE:.*]] = getelementptr{{.*}} i8, ptr %[[THIS]], i32 8
+// LLVM: store ptr getelementptr inbounds nuw (i8, ptr @_ZTV5Child, i64 48), ptr %[[FATHER_BASE]]
+// LLVM: ret void
+
+// OGCG: define{{.*}} void @_ZN5ChildC2Ev(ptr{{.*}} %[[THIS_ARG:.*]])
+// OGCG: %[[THIS_ADDR:.*]] = alloca ptr
+// OGCG: store ptr %[[THIS_ARG]], ptr %[[THIS_ADDR]]
+// OGCG: %[[THIS:.*]] = load ptr, ptr %[[THIS_ADDR]]
+// OGCG: call void @_ZN6MotherC2Ev(ptr {{.*}} %[[THIS]])
+// OGCG: %[[FATHER_BASE:.*]] = getelementptr{{.*}} i8, ptr %[[THIS]], i64 8
+// OGCG: call void @_ZN6FatherC2Ev(ptr{{.*}} %[[FATHER_BASE]])
+// OGCG: store ptr getelementptr inbounds inrange(-16, 16) ({ [4 x ptr], [3 x ptr] }, ptr @_ZTV5Child, i32 0, i32 0, i32 2), ptr %[[THIS]]
+// OGCG: %[[FATHER_BASE:.*]] = getelementptr{{.*}} i8, ptr %[[THIS]], i64 8
+// OGCG: store ptr getelementptr inbounds inrange(-16, 8) ({ [4 x ptr], [3 x ptr] }, ptr @_ZTV5Child, i32 0, i32 1, i32 2), ptr %[[FATHER_BASE]]
+// OGCG: ret void
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 nit, else lgtm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGMT, just nits
assert(!nonVirtualOffset.isZero() || virtualOffset != nullptr); | ||
|
||
// Compute the offset from the static and dynamic components. | ||
mlir::Value baseOffset; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think baseOffset
is unused in this PR; the value is updated, but it is not used. Should it be removed from this PR?
// Black-list all explicit and implicit references to 'this'. | ||
// | ||
// Do we need to worry about external references to 'this' derived | ||
// from arbitrary code? If so, then anything which runs arbitrary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// from arbitrary code? If so, then anything which runs arbitrary | |
// from arbitrary code? If so, then anything which runs arbitrary |
This adds support for initializing the vptr members in a class that requires multiple vtables because of multiple inheritence. This still does not handle virtual bases.
201375a
to
0d2904e
Compare
This adds support for initializing the vptr members in a class that requires multiple vtables because of multiple inheritence. This still does not handle virtual bases.