Skip to content

Commit 842b57b

Browse files
FznamznonFznamznon
authored
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 #131929 . The second commit of this PR also contains a fix for a runtime failure in chromium reported in #126240 (comment) . Fixes #19772
1 parent 809f857 commit 842b57b

34 files changed

+534
-125
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,7 @@ Windows Support
434434
- Clang now can process the `i128` and `ui128` integeral suffixes when MSVC
435435
extensions are enabled. This allows for properly processing ``intsafe.h`` in
436436
the Windows SDK.
437+
- Clang now supports MSVC vector deleting destructors (GH19772).
437438

438439
LoongArch Support
439440
^^^^^^^^^^^^^^^^^

‎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
@@ -6004,6 +6004,8 @@ void CXXNameMangler::mangleCXXDtorType(CXXDtorType T) {
60046004
case Dtor_Comdat:
60056005
Out << "D5";
60066006
break;
6007+
case Dtor_VectorDeleting:
6008+
llvm_unreachable("Itanium ABI does not use vector deleting dtors");
60076009
}
60086010
}
60096011

‎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
}
@@ -2886,9 +2887,12 @@ void MicrosoftCXXNameMangler::mangleFunctionType(const FunctionType *T,
28862887
// ::= @ # structors (they have no declared return type)
28872888
if (IsStructor) {
28882889
if (isa<CXXDestructorDecl>(D) && isStructorDecl(D)) {
2889-
// The scalar deleting destructor takes an extra int argument which is not
2890-
// reflected in the AST.
2891-
if (StructorType == Dtor_Deleting) {
2890+
// The deleting destructors take an extra argument of type int that
2891+
// indicates whether the storage for the object should be deleted and
2892+
// whether a single object or an array of objects is being destroyed. This
2893+
// extra argument is not reflected in the AST.
2894+
if (StructorType == Dtor_Deleting ||
2895+
StructorType == Dtor_VectorDeleting) {
28922896
Out << (PointersAre64Bit ? "PEAXI@Z" : "PAXI@Z");
28932897
return;
28942898
}
@@ -3861,10 +3865,10 @@ void MicrosoftMangleContextImpl::mangleCXXDtorThunk(const CXXDestructorDecl *DD,
38613865
const ThunkInfo &Thunk,
38623866
bool /*ElideOverrideInfo*/,
38633867
raw_ostream &Out) {
3864-
// FIXME: Actually, the dtor thunk should be emitted for vector deleting
3865-
// dtors rather than scalar deleting dtors. Just use the vector deleting dtor
3866-
// mangling manually until we support both deleting dtor types.
3867-
assert(Type == Dtor_Deleting);
3868+
// The dtor thunk should use vector deleting dtor mangling, however as an
3869+
// optimization we may end up emitting only scalar deleting dtor body, so just
3870+
// use the vector deleting dtor mangling manually.
3871+
assert(Type == Dtor_Deleting || Type == Dtor_VectorDeleting);
38683872
msvc_hashing_ostream MHO(Out);
38693873
MicrosoftCXXNameMangler Mangler(*this, MHO, DD, Type);
38703874
Mangler.getStream() << "??_E";

‎clang/lib/AST/VTableBuilder.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1735,8 +1735,8 @@ void ItaniumVTableBuilder::LayoutPrimaryAndSecondaryVTables(
17351735
const CXXMethodDecl *MD = I.first;
17361736
const MethodInfo &MI = I.second;
17371737
if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD)) {
1738-
MethodVTableIndices[GlobalDecl(DD, Dtor_Complete)]
1739-
= MI.VTableIndex - AddressPoint;
1738+
MethodVTableIndices[GlobalDecl(DD, Dtor_Complete)] =
1739+
MI.VTableIndex - AddressPoint;
17401740
MethodVTableIndices[GlobalDecl(DD, Dtor_Deleting)]
17411741
= MI.VTableIndex + 1 - AddressPoint;
17421742
} else {
@@ -2657,7 +2657,11 @@ class VFTableBuilder {
26572657
MethodVFTableLocation Loc(MI.VBTableIndex, WhichVFPtr.getVBaseWithVPtr(),
26582658
WhichVFPtr.NonVirtualOffset, MI.VFTableIndex);
26592659
if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD)) {
2660-
MethodVFTableLocations[GlobalDecl(DD, Dtor_Deleting)] = Loc;
2660+
// In Microsoft ABI vftable always references vector deleting dtor.
2661+
CXXDtorType DtorTy = Context.getTargetInfo().getCXXABI().isMicrosoft()
2662+
? Dtor_VectorDeleting
2663+
: Dtor_Deleting;
2664+
MethodVFTableLocations[GlobalDecl(DD, DtorTy)] = Loc;
26612665
} else {
26622666
MethodVFTableLocations[MD] = Loc;
26632667
}
@@ -3287,7 +3291,10 @@ void VFTableBuilder::dumpLayout(raw_ostream &Out) {
32873291
const CXXDestructorDecl *DD = Component.getDestructorDecl();
32883292

32893293
DD->printQualifiedName(Out);
3290-
Out << "() [scalar deleting]";
3294+
if (Context.getTargetInfo().getCXXABI().isMicrosoft())
3295+
Out << "() [vector deleting]";
3296+
else
3297+
Out << "() [scalar deleting]";
32913298

32923299
if (DD->isPureVirtual())
32933300
Out << " [pure]";
@@ -3757,7 +3764,7 @@ void MicrosoftVTableContext::dumpMethodLocations(
37573764
PredefinedIdentKind::PrettyFunctionNoVirtual, MD);
37583765

37593766
if (isa<CXXDestructorDecl>(MD)) {
3760-
IndicesMap[I.second] = MethodName + " [scalar deleting]";
3767+
IndicesMap[I.second] = MethodName + " [vector deleting]";
37613768
} else {
37623769
IndicesMap[I.second] = MethodName;
37633770
}
@@ -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
@@ -175,7 +175,6 @@ bool CodeGenModule::TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D) {
175175
// requires explicit comdat support in the IL.
176176
if (llvm::GlobalValue::isWeakForLinker(TargetLinkage))
177177
return true;
178-
179178
// Create the alias with no name.
180179
auto *Alias = llvm::GlobalAlias::create(AliasValueType, 0, Linkage, "",
181180
Aliasee, &getModule());
@@ -201,6 +200,42 @@ bool CodeGenModule::TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D) {
201200
return false;
202201
}
203202

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

‎clang/lib/CodeGen/CGCXXABI.cpp

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

275+
void CGCXXABI::ReadArrayCookie(CodeGenFunction &CGF, Address ptr,
276+
QualType eltTy, llvm::Value *&numElements,
277+
llvm::Value *&allocPtr, CharUnits &cookieSize) {
278+
assert(eltTy.isDestructedType());
279+
280+
// Derive a char* in the same address space as the pointer.
281+
ptr = ptr.withElementType(CGF.Int8Ty);
282+
283+
cookieSize = getArrayCookieSizeImpl(eltTy);
284+
Address allocAddr = CGF.Builder.CreateConstInBoundsByteGEP(ptr, -cookieSize);
285+
allocPtr = allocAddr.emitRawPointer(CGF);
286+
numElements = readArrayCookieImpl(CGF, allocAddr, cookieSize);
287+
}
288+
275289
llvm::Value *CGCXXABI::readArrayCookieImpl(CodeGenFunction &CGF,
276290
Address ptr,
277291
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
@@ -1432,6 +1432,70 @@ static bool CanSkipVTablePointerInitialization(CodeGenFunction &CGF,
14321432
return true;
14331433
}
14341434

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

14941562
case Dtor_Complete:
14951563
assert((Body || getTarget().getCXXABI().isMicrosoft()) &&
@@ -1572,7 +1640,6 @@ namespace {
15721640
return CGF.EmitScalarExpr(ThisArg);
15731641
return CGF.LoadCXXThis();
15741642
}
1575-
15761643
/// Call the operator delete associated with the current destructor.
15771644
struct CallDtorDelete final : EHScopeStack::Cleanup {
15781645
CallDtorDelete() {}
@@ -1591,8 +1658,10 @@ namespace {
15911658
bool ReturnAfterDelete) {
15921659
llvm::BasicBlock *callDeleteBB = CGF.createBasicBlock("dtor.call_delete");
15931660
llvm::BasicBlock *continueBB = CGF.createBasicBlock("dtor.continue");
1594-
llvm::Value *ShouldCallDelete
1595-
= CGF.Builder.CreateIsNull(ShouldDeleteCondition);
1661+
auto *CondTy = cast<llvm::IntegerType>(ShouldDeleteCondition->getType());
1662+
llvm::Value *CheckTheBit = CGF.Builder.CreateAnd(
1663+
ShouldDeleteCondition, llvm::ConstantInt::get(CondTy, 1));
1664+
llvm::Value *ShouldCallDelete = CGF.Builder.CreateIsNull(CheckTheBit);
15961665
CGF.Builder.CreateCondBr(ShouldCallDelete, continueBB, callDeleteBB);
15971666

15981667
CGF.EmitBlock(callDeleteBB);

‎clang/lib/CodeGen/CGDebugInfo.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2119,7 +2119,8 @@ llvm::DISubprogram *CGDebugInfo::CreateCXXMemberFunction(
21192119
// Emit MS ABI vftable information. There is only one entry for the
21202120
// deleting dtor.
21212121
const auto *DD = dyn_cast<CXXDestructorDecl>(Method);
2122-
GlobalDecl GD = DD ? GlobalDecl(DD, Dtor_Deleting) : GlobalDecl(Method);
2122+
GlobalDecl GD =
2123+
DD ? GlobalDecl(DD, Dtor_VectorDeleting) : GlobalDecl(Method);
21232124
MethodVFTableLocation ML =
21242125
CGM.getMicrosoftVTableContext().getMethodVFTableLocation(GD);
21252126
VIndex = ML.Index;

0 commit comments

Comments
 (0)