Skip to content

Commit a799b27

Browse files
FznamznonKornevNikita
Fznamznon
authored and
KornevNikita
committed
Reland [MS][clang] Add support for vector deleting destructors (#133451)
Whereas it is UB in terms of the standard to delete an array of objects via pointer whose static type doesn't match its dynamic type, MSVC supports an extension allowing to do it. Aside from array deletion not working correctly in the mentioned case, currently not having this extension implemented causes clang to generate code that is not compatible with the code generated by MSVC, because clang always puts scalar deleting destructor to the vftable. This PR aims to resolve these problems. It was reverted due to link time errors in chromium with sanitizer coverage enabled, which is fixed by llvm/llvm-project#131929 . The second commit of this PR also contains a fix for a runtime failure in chromium reported in llvm/llvm-project#126240 (comment) . Fixes llvm/llvm-project#19772
1 parent 404f1c3 commit a799b27

34 files changed

+535
-125
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,8 @@ Android Support
168168
Windows Support
169169
^^^^^^^^^^^^^^^
170170

171+
- Clang now supports MSVC vector deleting destructors (GH19772).
172+
171173
LoongArch Support
172174
^^^^^^^^^^^^^^^^^
173175

‎clang/include/clang/AST/VTableBuilder.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ class VTableComponent {
150150

151151
bool isRTTIKind() const { return isRTTIKind(getKind()); }
152152

153-
GlobalDecl getGlobalDecl() const {
153+
GlobalDecl getGlobalDecl(bool HasVectorDeletingDtors) const {
154154
assert(isUsedFunctionPointerKind() &&
155155
"GlobalDecl can be created only from virtual function");
156156

@@ -161,7 +161,9 @@ class VTableComponent {
161161
case CK_CompleteDtorPointer:
162162
return GlobalDecl(DtorDecl, CXXDtorType::Dtor_Complete);
163163
case CK_DeletingDtorPointer:
164-
return GlobalDecl(DtorDecl, CXXDtorType::Dtor_Deleting);
164+
return GlobalDecl(DtorDecl, (HasVectorDeletingDtors)
165+
? CXXDtorType::Dtor_VectorDeleting
166+
: CXXDtorType::Dtor_Deleting);
165167
case CK_VCallOffset:
166168
case CK_VBaseOffset:
167169
case CK_OffsetToTop:

‎clang/include/clang/Basic/ABI.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,11 @@ enum CXXCtorType {
3131

3232
/// C++ destructor types.
3333
enum CXXDtorType {
34-
Dtor_Deleting, ///< Deleting dtor
35-
Dtor_Complete, ///< Complete object dtor
36-
Dtor_Base, ///< Base object dtor
37-
Dtor_Comdat ///< The COMDAT used for dtors
34+
Dtor_Deleting, ///< Deleting dtor
35+
Dtor_Complete, ///< Complete object dtor
36+
Dtor_Base, ///< Base object dtor
37+
Dtor_Comdat, ///< The COMDAT used for dtors
38+
Dtor_VectorDeleting ///< Vector deleting dtor
3839
};
3940

4041
} // end namespace clang

‎clang/lib/AST/ItaniumMangle.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6022,6 +6022,8 @@ void CXXNameMangler::mangleCXXDtorType(CXXDtorType T) {
60226022
case Dtor_Comdat:
60236023
Out << "D5";
60246024
break;
6025+
case Dtor_VectorDeleting:
6026+
llvm_unreachable("Itanium ABI does not use vector deleting dtors");
60256027
}
60266028
}
60276029

‎clang/lib/AST/MicrosoftMangle.cpp

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1484,8 +1484,9 @@ void MicrosoftCXXNameMangler::mangleCXXDtorType(CXXDtorType T) {
14841484
// <operator-name> ::= ?_G # scalar deleting destructor
14851485
case Dtor_Deleting: Out << "?_G"; return;
14861486
// <operator-name> ::= ?_E # vector deleting destructor
1487-
// FIXME: Add a vector deleting dtor type. It goes in the vtable, so we need
1488-
// it.
1487+
case Dtor_VectorDeleting:
1488+
Out << "?_E";
1489+
return;
14891490
case Dtor_Comdat:
14901491
llvm_unreachable("not expecting a COMDAT");
14911492
}
@@ -2944,9 +2945,12 @@ void MicrosoftCXXNameMangler::mangleFunctionType(const FunctionType *T,
29442945
// ::= @ # structors (they have no declared return type)
29452946
if (IsStructor) {
29462947
if (isa<CXXDestructorDecl>(D) && isStructorDecl(D)) {
2947-
// The scalar deleting destructor takes an extra int argument which is not
2948-
// reflected in the AST.
2949-
if (StructorType == Dtor_Deleting) {
2948+
// The deleting destructors take an extra argument of type int that
2949+
// indicates whether the storage for the object should be deleted and
2950+
// whether a single object or an array of objects is being destroyed. This
2951+
// extra argument is not reflected in the AST.
2952+
if (StructorType == Dtor_Deleting ||
2953+
StructorType == Dtor_VectorDeleting) {
29502954
Out << (PointersAre64Bit ? "PEAXI@Z" : "PAXI@Z");
29512955
return;
29522956
}
@@ -3930,10 +3934,10 @@ void MicrosoftMangleContextImpl::mangleCXXDtorThunk(const CXXDestructorDecl *DD,
39303934
const ThunkInfo &Thunk,
39313935
bool /*ElideOverrideInfo*/,
39323936
raw_ostream &Out) {
3933-
// FIXME: Actually, the dtor thunk should be emitted for vector deleting
3934-
// dtors rather than scalar deleting dtors. Just use the vector deleting dtor
3935-
// mangling manually until we support both deleting dtor types.
3936-
assert(Type == Dtor_Deleting);
3937+
// The dtor thunk should use vector deleting dtor mangling, however as an
3938+
// optimization we may end up emitting only scalar deleting dtor body, so just
3939+
// use the vector deleting dtor mangling manually.
3940+
assert(Type == Dtor_Deleting || Type == Dtor_VectorDeleting);
39373941
msvc_hashing_ostream MHO(Out);
39383942
MicrosoftCXXNameMangler Mangler(*this, MHO, DD, Type);
39393943
Mangler.getStream() << "??_E";

‎clang/lib/AST/VTableBuilder.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1733,8 +1733,8 @@ void ItaniumVTableBuilder::LayoutPrimaryAndSecondaryVTables(
17331733
const CXXMethodDecl *MD = I.first;
17341734
const MethodInfo &MI = I.second;
17351735
if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD)) {
1736-
MethodVTableIndices[GlobalDecl(DD, Dtor_Complete)]
1737-
= MI.VTableIndex - AddressPoint;
1736+
MethodVTableIndices[GlobalDecl(DD, Dtor_Complete)] =
1737+
MI.VTableIndex - AddressPoint;
17381738
MethodVTableIndices[GlobalDecl(DD, Dtor_Deleting)]
17391739
= MI.VTableIndex + 1 - AddressPoint;
17401740
} else {
@@ -2655,7 +2655,11 @@ class VFTableBuilder {
26552655
MethodVFTableLocation Loc(MI.VBTableIndex, WhichVFPtr.getVBaseWithVPtr(),
26562656
WhichVFPtr.NonVirtualOffset, MI.VFTableIndex);
26572657
if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD)) {
2658-
MethodVFTableLocations[GlobalDecl(DD, Dtor_Deleting)] = Loc;
2658+
// In Microsoft ABI vftable always references vector deleting dtor.
2659+
CXXDtorType DtorTy = Context.getTargetInfo().getCXXABI().isMicrosoft()
2660+
? Dtor_VectorDeleting
2661+
: Dtor_Deleting;
2662+
MethodVFTableLocations[GlobalDecl(DD, DtorTy)] = Loc;
26592663
} else {
26602664
MethodVFTableLocations[MD] = Loc;
26612665
}
@@ -3285,7 +3289,10 @@ void VFTableBuilder::dumpLayout(raw_ostream &Out) {
32853289
const CXXDestructorDecl *DD = Component.getDestructorDecl();
32863290

32873291
DD->printQualifiedName(Out);
3288-
Out << "() [scalar deleting]";
3292+
if (Context.getTargetInfo().getCXXABI().isMicrosoft())
3293+
Out << "() [vector deleting]";
3294+
else
3295+
Out << "() [scalar deleting]";
32893296

32903297
if (DD->isPureVirtual())
32913298
Out << " [pure]";
@@ -3756,7 +3763,7 @@ void MicrosoftVTableContext::dumpMethodLocations(
37563763
PredefinedIdentKind::PrettyFunctionNoVirtual, MD);
37573764

37583765
if (isa<CXXDestructorDecl>(MD)) {
3759-
IndicesMap[I.second] = MethodName + " [scalar deleting]";
3766+
IndicesMap[I.second] = MethodName + " [vector deleting]";
37603767
} else {
37613768
IndicesMap[I.second] = MethodName;
37623769
}
@@ -3873,7 +3880,7 @@ MicrosoftVTableContext::getMethodVFTableLocation(GlobalDecl GD) {
38733880
assert(hasVtableSlot(cast<CXXMethodDecl>(GD.getDecl())) &&
38743881
"Only use this method for virtual methods or dtors");
38753882
if (isa<CXXDestructorDecl>(GD.getDecl()))
3876-
assert(GD.getDtorType() == Dtor_Deleting);
3883+
assert(GD.getDtorType() == Dtor_VectorDeleting);
38773884

38783885
GD = GD.getCanonicalDecl();
38793886

‎clang/lib/CodeGen/CGCXX.cpp

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,6 @@ bool CodeGenModule::TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D) {
182182
// requires explicit comdat support in the IL.
183183
if (llvm::GlobalValue::isWeakForLinker(TargetLinkage))
184184
return true;
185-
186185
// Create the alias with no name.
187186
auto *Alias = llvm::GlobalAlias::create(AliasValueType, 0, Linkage, "",
188187
Aliasee, &getModule());
@@ -207,6 +206,42 @@ bool CodeGenModule::TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D) {
207206
return false;
208207
}
209208

209+
/// Emit a definition as a global alias for another definition, unconditionally.
210+
void CodeGenModule::EmitDefinitionAsAlias(GlobalDecl AliasDecl,
211+
GlobalDecl TargetDecl) {
212+
213+
llvm::Type *AliasValueType = getTypes().GetFunctionType(AliasDecl);
214+
215+
StringRef MangledName = getMangledName(AliasDecl);
216+
llvm::GlobalValue *Entry = GetGlobalValue(MangledName);
217+
if (Entry && !Entry->isDeclaration())
218+
return;
219+
auto *Aliasee = cast<llvm::GlobalValue>(GetAddrOfGlobal(TargetDecl));
220+
221+
// Determine the linkage type for the alias.
222+
llvm::GlobalValue::LinkageTypes Linkage = getFunctionLinkage(AliasDecl);
223+
224+
// Create the alias with no name.
225+
auto *Alias = llvm::GlobalAlias::create(AliasValueType, 0, Linkage, "",
226+
Aliasee, &getModule());
227+
// Destructors are always unnamed_addr.
228+
Alias->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
229+
230+
if (Entry) {
231+
assert(Entry->getValueType() == AliasValueType &&
232+
Entry->getAddressSpace() == Alias->getAddressSpace() &&
233+
"declaration exists with different type");
234+
Alias->takeName(Entry);
235+
Entry->replaceAllUsesWith(Alias);
236+
Entry->eraseFromParent();
237+
} else {
238+
Alias->setName(MangledName);
239+
}
240+
241+
// Set any additional necessary attributes for the alias.
242+
SetCommonAttributes(AliasDecl, Alias);
243+
}
244+
210245
llvm::Function *CodeGenModule::codegenCXXStructor(GlobalDecl GD) {
211246
const CGFunctionInfo &FnInfo = getTypes().arrangeCXXStructorDeclaration(GD);
212247
auto *Fn = cast<llvm::Function>(

‎clang/lib/CodeGen/CGCXXABI.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,20 @@ void CGCXXABI::ReadArrayCookie(CodeGenFunction &CGF, Address ptr,
273273
numElements = readArrayCookieImpl(CGF, allocAddr, cookieSize);
274274
}
275275

276+
void CGCXXABI::ReadArrayCookie(CodeGenFunction &CGF, Address ptr,
277+
QualType eltTy, llvm::Value *&numElements,
278+
llvm::Value *&allocPtr, CharUnits &cookieSize) {
279+
assert(eltTy.isDestructedType());
280+
281+
// Derive a char* in the same address space as the pointer.
282+
ptr = ptr.withElementType(CGF.Int8Ty);
283+
284+
cookieSize = getArrayCookieSizeImpl(eltTy);
285+
Address allocAddr = CGF.Builder.CreateConstInBoundsByteGEP(ptr, -cookieSize);
286+
allocPtr = allocAddr.emitRawPointer(CGF);
287+
numElements = readArrayCookieImpl(CGF, allocAddr, cookieSize);
288+
}
289+
276290
llvm::Value *CGCXXABI::readArrayCookieImpl(CodeGenFunction &CGF,
277291
Address ptr,
278292
CharUnits cookieSize) {

‎clang/lib/CodeGen/CGCXXABI.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,7 @@ class CGCXXABI {
275275
virtual CatchTypeInfo getCatchAllTypeInfo();
276276

277277
virtual bool shouldTypeidBeNullChecked(QualType SrcRecordTy) = 0;
278+
virtual bool hasVectorDeletingDtors() = 0;
278279
virtual void EmitBadTypeidCall(CodeGenFunction &CGF) = 0;
279280
virtual llvm::Value *EmitTypeid(CodeGenFunction &CGF, QualType SrcRecordTy,
280281
Address ThisPtr,
@@ -575,6 +576,12 @@ class CGCXXABI {
575576
QualType ElementType, llvm::Value *&NumElements,
576577
llvm::Value *&AllocPtr, CharUnits &CookieSize);
577578

579+
/// Reads the array cookie associated with the given pointer,
580+
/// that should have one.
581+
void ReadArrayCookie(CodeGenFunction &CGF, Address Ptr, QualType ElementType,
582+
llvm::Value *&NumElements, llvm::Value *&AllocPtr,
583+
CharUnits &CookieSize);
584+
578585
/// Return whether the given global decl needs a VTT parameter.
579586
virtual bool NeedsVTTParameter(GlobalDecl GD);
580587

‎clang/lib/CodeGen/CGClass.cpp

Lines changed: 73 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1434,6 +1434,70 @@ static bool CanSkipVTablePointerInitialization(CodeGenFunction &CGF,
14341434
return true;
14351435
}
14361436

1437+
static void EmitConditionalArrayDtorCall(const CXXDestructorDecl *DD,
1438+
CodeGenFunction &CGF,
1439+
llvm::Value *ShouldDeleteCondition) {
1440+
Address ThisPtr = CGF.LoadCXXThisAddress();
1441+
llvm::BasicBlock *ScalarBB = CGF.createBasicBlock("dtor.scalar");
1442+
llvm::BasicBlock *callDeleteBB =
1443+
CGF.createBasicBlock("dtor.call_delete_after_array_destroy");
1444+
llvm::BasicBlock *VectorBB = CGF.createBasicBlock("dtor.vector");
1445+
auto *CondTy = cast<llvm::IntegerType>(ShouldDeleteCondition->getType());
1446+
llvm::Value *CheckTheBitForArrayDestroy = CGF.Builder.CreateAnd(
1447+
ShouldDeleteCondition, llvm::ConstantInt::get(CondTy, 2));
1448+
llvm::Value *ShouldDestroyArray =
1449+
CGF.Builder.CreateIsNull(CheckTheBitForArrayDestroy);
1450+
CGF.Builder.CreateCondBr(ShouldDestroyArray, ScalarBB, VectorBB);
1451+
1452+
CGF.EmitBlock(VectorBB);
1453+
1454+
llvm::Value *numElements = nullptr;
1455+
llvm::Value *allocatedPtr = nullptr;
1456+
CharUnits cookieSize;
1457+
QualType EltTy = DD->getThisType()->getPointeeType();
1458+
CGF.CGM.getCXXABI().ReadArrayCookie(CGF, ThisPtr, EltTy, numElements,
1459+
allocatedPtr, cookieSize);
1460+
1461+
// Destroy the elements.
1462+
QualType::DestructionKind dtorKind = EltTy.isDestructedType();
1463+
1464+
assert(dtorKind);
1465+
assert(numElements && "no element count for a type with a destructor!");
1466+
1467+
CharUnits elementSize = CGF.getContext().getTypeSizeInChars(EltTy);
1468+
CharUnits elementAlign =
1469+
ThisPtr.getAlignment().alignmentOfArrayElement(elementSize);
1470+
1471+
llvm::Value *arrayBegin = ThisPtr.emitRawPointer(CGF);
1472+
llvm::Value *arrayEnd = CGF.Builder.CreateInBoundsGEP(
1473+
ThisPtr.getElementType(), arrayBegin, numElements, "delete.end");
1474+
1475+
// We already checked that the array is not 0-length before entering vector
1476+
// deleting dtor.
1477+
CGF.emitArrayDestroy(arrayBegin, arrayEnd, EltTy, elementAlign,
1478+
CGF.getDestroyer(dtorKind),
1479+
/*checkZeroLength*/ false, CGF.needsEHCleanup(dtorKind));
1480+
1481+
llvm::BasicBlock *VectorBBCont = CGF.createBasicBlock("dtor.vector.cont");
1482+
CGF.EmitBlock(VectorBBCont);
1483+
1484+
llvm::Value *CheckTheBitForDeleteCall = CGF.Builder.CreateAnd(
1485+
ShouldDeleteCondition, llvm::ConstantInt::get(CondTy, 1));
1486+
1487+
llvm::Value *ShouldCallDelete =
1488+
CGF.Builder.CreateIsNull(CheckTheBitForDeleteCall);
1489+
CGF.Builder.CreateCondBr(ShouldCallDelete, CGF.ReturnBlock.getBlock(),
1490+
callDeleteBB);
1491+
CGF.EmitBlock(callDeleteBB);
1492+
const CXXDestructorDecl *Dtor = cast<CXXDestructorDecl>(CGF.CurCodeDecl);
1493+
const CXXRecordDecl *ClassDecl = Dtor->getParent();
1494+
CGF.EmitDeleteCall(Dtor->getOperatorDelete(), allocatedPtr,
1495+
CGF.getContext().getTagDeclType(ClassDecl));
1496+
1497+
CGF.EmitBranchThroughCleanup(CGF.ReturnBlock);
1498+
CGF.EmitBlock(ScalarBB);
1499+
}
1500+
14371501
/// EmitDestructorBody - Emits the body of the current destructor.
14381502
void CodeGenFunction::EmitDestructorBody(FunctionArgList &Args) {
14391503
const CXXDestructorDecl *Dtor = cast<CXXDestructorDecl>(CurGD.getDecl());
@@ -1463,7 +1527,9 @@ void CodeGenFunction::EmitDestructorBody(FunctionArgList &Args) {
14631527
// outside of the function-try-block, which means it's always
14641528
// possible to delegate the destructor body to the complete
14651529
// destructor. Do so.
1466-
if (DtorType == Dtor_Deleting) {
1530+
if (DtorType == Dtor_Deleting || DtorType == Dtor_VectorDeleting) {
1531+
if (CXXStructorImplicitParamValue && DtorType == Dtor_VectorDeleting)
1532+
EmitConditionalArrayDtorCall(Dtor, *this, CXXStructorImplicitParamValue);
14671533
RunCleanupsScope DtorEpilogue(*this);
14681534
EnterDtorCleanups(Dtor, Dtor_Deleting);
14691535
if (HaveInsertPoint()) {
@@ -1492,6 +1558,8 @@ void CodeGenFunction::EmitDestructorBody(FunctionArgList &Args) {
14921558
switch (DtorType) {
14931559
case Dtor_Comdat: llvm_unreachable("not expecting a COMDAT");
14941560
case Dtor_Deleting: llvm_unreachable("already handled deleting case");
1561+
case Dtor_VectorDeleting:
1562+
llvm_unreachable("already handled vector deleting case");
14951563

14961564
case Dtor_Complete:
14971565
assert((Body || getTarget().getCXXABI().isMicrosoft()) &&
@@ -1574,7 +1642,6 @@ namespace {
15741642
return CGF.EmitScalarExpr(ThisArg);
15751643
return CGF.LoadCXXThis();
15761644
}
1577-
15781645
/// Call the operator delete associated with the current destructor.
15791646
struct CallDtorDelete final : EHScopeStack::Cleanup {
15801647
CallDtorDelete() {}
@@ -1593,8 +1660,10 @@ namespace {
15931660
bool ReturnAfterDelete) {
15941661
llvm::BasicBlock *callDeleteBB = CGF.createBasicBlock("dtor.call_delete");
15951662
llvm::BasicBlock *continueBB = CGF.createBasicBlock("dtor.continue");
1596-
llvm::Value *ShouldCallDelete
1597-
= CGF.Builder.CreateIsNull(ShouldDeleteCondition);
1663+
auto *CondTy = cast<llvm::IntegerType>(ShouldDeleteCondition->getType());
1664+
llvm::Value *CheckTheBit = CGF.Builder.CreateAnd(
1665+
ShouldDeleteCondition, llvm::ConstantInt::get(CondTy, 1));
1666+
llvm::Value *ShouldCallDelete = CGF.Builder.CreateIsNull(CheckTheBit);
15981667
CGF.Builder.CreateCondBr(ShouldCallDelete, continueBB, callDeleteBB);
15991668

16001669
CGF.EmitBlock(callDeleteBB);

‎clang/lib/CodeGen/CGDebugInfo.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2114,7 +2114,8 @@ llvm::DISubprogram *CGDebugInfo::CreateCXXMemberFunction(
21142114
// Emit MS ABI vftable information. There is only one entry for the
21152115
// deleting dtor.
21162116
const auto *DD = dyn_cast<CXXDestructorDecl>(Method);
2117-
GlobalDecl GD = DD ? GlobalDecl(DD, Dtor_Deleting) : GlobalDecl(Method);
2117+
GlobalDecl GD =
2118+
DD ? GlobalDecl(DD, Dtor_VectorDeleting) : GlobalDecl(Method);
21182119
MethodVFTableLocation ML =
21192120
CGM.getMicrosoftVTableContext().getMethodVFTableLocation(GD);
21202121
VIndex = ML.Index;

0 commit comments

Comments
 (0)