[MS] Skip vbase construction in abstract class ctors
As background, when constructing a complete object, virtual bases are
constructed first. If an exception is thrown later in the ctor, those
virtual bases are destroyed, so sema marks the relevant constructors and
destructors of virtual bases as referenced. If necessary, they are
emitted.
However, an abstract class can never be used to construct a complete
object. In the Itanium C++ ABI, this works out nicely, because we never
end up emitting the "complete" constructor variant, only the "base"
constructor variant, which can be called by constructors of derived
classes. Clang's Sema::MarkBaseAndMemberDestructorsReferenced is aware
of this optimization, and it does not mark ctors and dtors of virtual
bases referenced when the constructor of an abstract class is emitted.
In the Microsoft ABI, there are no complete/base variants, so before
this change, the constructor of an abstract class could reference ctors
and dtors of a virtual base without marking them referenced. This could
lead to unresolved symbol errors at link time, as reported in PR41065.
The fix is to implement the same optimization as Sema: If the class is
abstract, don't bother initializing its virtual bases. The "is this
class the most derived class" check in the constructor will never pass,
and the virtual base constructor calls are always dead. Skip them.
I think Richard noticed this missed optimization back in 2016 when he
was implementing inheriting constructors. I wasn't able to find any bugs
or email about it, though.
Fixes PR41065
llvm-svn: 356425
diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index 597d0e9..429c334 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -526,8 +526,7 @@
static void EmitBaseInitializer(CodeGenFunction &CGF,
const CXXRecordDecl *ClassDecl,
- CXXCtorInitializer *BaseInit,
- CXXCtorType CtorType) {
+ CXXCtorInitializer *BaseInit) {
assert(BaseInit->isBaseInitializer() &&
"Must have base initializer!");
@@ -539,10 +538,6 @@
bool isBaseVirtual = BaseInit->isBaseVirtual();
- // The base constructor doesn't construct virtual bases.
- if (CtorType == Ctor_Base && isBaseVirtual)
- return;
-
// If the initializer for the base (other than the constructor
// itself) accesses 'this' in any way, we need to initialize the
// vtables.
@@ -1264,24 +1259,37 @@
CXXConstructorDecl::init_const_iterator B = CD->init_begin(),
E = CD->init_end();
+ // Virtual base initializers first, if any. They aren't needed if:
+ // - This is a base ctor variant
+ // - There are no vbases
+ // - The class is abstract, so a complete object of it cannot be constructed
+ //
+ // The check for an abstract class is necessary because sema may not have
+ // marked virtual base destructors referenced.
+ bool ConstructVBases = CtorType != Ctor_Base &&
+ ClassDecl->getNumVBases() != 0 &&
+ !ClassDecl->isAbstract();
+
+ // In the Microsoft C++ ABI, there are no constructor variants. Instead, the
+ // constructor of a class with virtual bases takes an additional parameter to
+ // conditionally construct the virtual bases. Emit that check here.
llvm::BasicBlock *BaseCtorContinueBB = nullptr;
- if (ClassDecl->getNumVBases() &&
+ if (ConstructVBases &&
!CGM.getTarget().getCXXABI().hasConstructorVariants()) {
- // The ABIs that don't have constructor variants need to put a branch
- // before the virtual base initialization code.
BaseCtorContinueBB =
- CGM.getCXXABI().EmitCtorCompleteObjectHandler(*this, ClassDecl);
+ CGM.getCXXABI().EmitCtorCompleteObjectHandler(*this, ClassDecl);
assert(BaseCtorContinueBB);
}
llvm::Value *const OldThis = CXXThisValue;
- // Virtual base initializers first.
for (; B != E && (*B)->isBaseInitializer() && (*B)->isBaseVirtual(); B++) {
+ if (!ConstructVBases)
+ continue;
if (CGM.getCodeGenOpts().StrictVTablePointers &&
CGM.getCodeGenOpts().OptimizationLevel > 0 &&
isInitializerOfDynamicClass(*B))
CXXThisValue = Builder.CreateLaunderInvariantGroup(LoadCXXThis());
- EmitBaseInitializer(*this, ClassDecl, *B, CtorType);
+ EmitBaseInitializer(*this, ClassDecl, *B);
}
if (BaseCtorContinueBB) {
@@ -1298,7 +1306,7 @@
CGM.getCodeGenOpts().OptimizationLevel > 0 &&
isInitializerOfDynamicClass(*B))
CXXThisValue = Builder.CreateLaunderInvariantGroup(LoadCXXThis());
- EmitBaseInitializer(*this, ClassDecl, *B, CtorType);
+ EmitBaseInitializer(*this, ClassDecl, *B);
}
CXXThisValue = OldThis;