Skip to content

Commit e11ede5

Browse files
zmodemzmodem
committed
Revert "[MS][clang] Add support for vector deleting destructors (llvm#126240)"
This caused link errors when building with sancov. See comment on the PR. > 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. > > Fixes llvm#19772 This reverts commit d6942d5.
1 parent 90a8322 commit e11ede5

34 files changed

+125
-510
lines changed

‎clang/docs/ReleaseNotes.rst

-2
Original file line numberDiff line numberDiff line change
@@ -357,8 +357,6 @@ Android Support
357357
Windows Support
358358
^^^^^^^^^^^^^^^
359359

360-
- Clang now supports MSVC vector deleting destructors (GH19772).
361-
362360
LoongArch Support
363361
^^^^^^^^^^^^^^^^^
364362

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

+2-4
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(bool HasVectorDeletingDtors) const {
153+
GlobalDecl getGlobalDecl() const {
154154
assert(isUsedFunctionPointerKind() &&
155155
"GlobalDecl can be created only from virtual function");
156156

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

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

+4-5
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,10 @@ 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
38-
Dtor_VectorDeleting ///< Vector deleting dtor
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
3938
};
4039

4140
} // end namespace clang

‎clang/lib/AST/ItaniumMangle.cpp

-2
Original file line numberDiff line numberDiff line change
@@ -6014,8 +6014,6 @@ void CXXNameMangler::mangleCXXDtorType(CXXDtorType T) {
60146014
case Dtor_Comdat:
60156015
Out << "D5";
60166016
break;
6017-
case Dtor_VectorDeleting:
6018-
llvm_unreachable("Itanium ABI does not use vector deleting dtors");
60196017
}
60206018
}
60216019

‎clang/lib/AST/MicrosoftMangle.cpp

+9-13
Original file line numberDiff line numberDiff line change
@@ -1484,9 +1484,8 @@ 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-
case Dtor_VectorDeleting:
1488-
Out << "?_E";
1489-
return;
1487+
// FIXME: Add a vector deleting dtor type. It goes in the vtable, so we need
1488+
// it.
14901489
case Dtor_Comdat:
14911490
llvm_unreachable("not expecting a COMDAT");
14921491
}
@@ -2887,12 +2886,9 @@ void MicrosoftCXXNameMangler::mangleFunctionType(const FunctionType *T,
28872886
// ::= @ # structors (they have no declared return type)
28882887
if (IsStructor) {
28892888
if (isa<CXXDestructorDecl>(D) && isStructorDecl(D)) {
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) {
2889+
// The scalar deleting destructor takes an extra int argument which is not
2890+
// reflected in the AST.
2891+
if (StructorType == Dtor_Deleting) {
28962892
Out << (PointersAre64Bit ? "PEAXI@Z" : "PAXI@Z");
28972893
return;
28982894
}
@@ -3865,10 +3861,10 @@ void MicrosoftMangleContextImpl::mangleCXXDtorThunk(const CXXDestructorDecl *DD,
38653861
const ThunkInfo &Thunk,
38663862
bool /*ElideOverrideInfo*/,
38673863
raw_ostream &Out) {
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);
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);
38723868
msvc_hashing_ostream MHO(Out);
38733869
MicrosoftCXXNameMangler Mangler(*this, MHO, DD, Type);
38743870
Mangler.getStream() << "??_E";

‎clang/lib/AST/VTableBuilder.cpp

+6-13
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,11 +2657,7 @@ class VFTableBuilder {
26572657
MethodVFTableLocation Loc(MI.VBTableIndex, WhichVFPtr.getVBaseWithVPtr(),
26582658
WhichVFPtr.NonVirtualOffset, MI.VFTableIndex);
26592659
if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD)) {
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;
2660+
MethodVFTableLocations[GlobalDecl(DD, Dtor_Deleting)] = Loc;
26652661
} else {
26662662
MethodVFTableLocations[MD] = Loc;
26672663
}
@@ -3291,10 +3287,7 @@ void VFTableBuilder::dumpLayout(raw_ostream &Out) {
32913287
const CXXDestructorDecl *DD = Component.getDestructorDecl();
32923288

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

32993292
if (DD->isPureVirtual())
33003293
Out << " [pure]";
@@ -3765,7 +3758,7 @@ void MicrosoftVTableContext::dumpMethodLocations(
37653758
PredefinedIdentKind::PrettyFunctionNoVirtual, MD);
37663759

37673760
if (isa<CXXDestructorDecl>(MD)) {
3768-
IndicesMap[I.second] = MethodName + " [vector deleting]";
3761+
IndicesMap[I.second] = MethodName + " [scalar deleting]";
37693762
} else {
37703763
IndicesMap[I.second] = MethodName;
37713764
}
@@ -3882,7 +3875,7 @@ MicrosoftVTableContext::getMethodVFTableLocation(GlobalDecl GD) {
38823875
assert(hasVtableSlot(cast<CXXMethodDecl>(GD.getDecl())) &&
38833876
"Only use this method for virtual methods or dtors");
38843877
if (isa<CXXDestructorDecl>(GD.getDecl()))
3885-
assert(GD.getDtorType() == Dtor_VectorDeleting);
3878+
assert(GD.getDtorType() == Dtor_Deleting);
38863879

38873880
GD = GD.getCanonicalDecl();
38883881

‎clang/lib/CodeGen/CGCXX.cpp

+1-36
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ bool CodeGenModule::TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D) {
175175
// requires explicit comdat support in the IL.
176176
if (llvm::GlobalValue::isWeakForLinker(TargetLinkage))
177177
return true;
178+
178179
// Create the alias with no name.
179180
auto *Alias = llvm::GlobalAlias::create(AliasValueType, 0, Linkage, "",
180181
Aliasee, &getModule());
@@ -200,42 +201,6 @@ bool CodeGenModule::TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D) {
200201
return false;
201202
}
202203

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-
239204
llvm::Function *CodeGenModule::codegenCXXStructor(GlobalDecl GD) {
240205
const CGFunctionInfo &FnInfo = getTypes().arrangeCXXStructorDeclaration(GD);
241206
auto *Fn = cast<llvm::Function>(

‎clang/lib/CodeGen/CGCXXABI.cpp

-14
Original file line numberDiff line numberDiff line change
@@ -273,20 +273,6 @@ 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-
290276
llvm::Value *CGCXXABI::readArrayCookieImpl(CodeGenFunction &CGF,
291277
Address ptr,
292278
CharUnits cookieSize) {

‎clang/lib/CodeGen/CGCXXABI.h

-7
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,6 @@ class CGCXXABI {
275275
virtual CatchTypeInfo getCatchAllTypeInfo();
276276

277277
virtual bool shouldTypeidBeNullChecked(QualType SrcRecordTy) = 0;
278-
virtual bool hasVectorDeletingDtors() = 0;
279278
virtual void EmitBadTypeidCall(CodeGenFunction &CGF) = 0;
280279
virtual llvm::Value *EmitTypeid(CodeGenFunction &CGF, QualType SrcRecordTy,
281280
Address ThisPtr,
@@ -576,12 +575,6 @@ class CGCXXABI {
576575
QualType ElementType, llvm::Value *&NumElements,
577576
llvm::Value *&AllocPtr, CharUnits &CookieSize);
578577

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-
585578
/// Return whether the given global decl needs a VTT parameter.
586579
virtual bool NeedsVTTParameter(GlobalDecl GD);
587580

‎clang/lib/CodeGen/CGClass.cpp

+4-73
Original file line numberDiff line numberDiff line change
@@ -1433,70 +1433,6 @@ static bool CanSkipVTablePointerInitialization(CodeGenFunction &CGF,
14331433
return true;
14341434
}
14351435

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

15631495
case Dtor_Complete:
15641496
assert((Body || getTarget().getCXXABI().isMicrosoft()) &&
@@ -1641,6 +1573,7 @@ namespace {
16411573
return CGF.EmitScalarExpr(ThisArg);
16421574
return CGF.LoadCXXThis();
16431575
}
1576+
16441577
/// Call the operator delete associated with the current destructor.
16451578
struct CallDtorDelete final : EHScopeStack::Cleanup {
16461579
CallDtorDelete() {}
@@ -1659,10 +1592,8 @@ namespace {
16591592
bool ReturnAfterDelete) {
16601593
llvm::BasicBlock *callDeleteBB = CGF.createBasicBlock("dtor.call_delete");
16611594
llvm::BasicBlock *continueBB = CGF.createBasicBlock("dtor.continue");
1662-
auto *CondTy = cast<llvm::IntegerType>(ShouldDeleteCondition->getType());
1663-
llvm::Value *CheckTheBit = CGF.Builder.CreateAnd(
1664-
ShouldDeleteCondition, llvm::ConstantInt::get(CondTy, 1));
1665-
llvm::Value *ShouldCallDelete = CGF.Builder.CreateIsNull(CheckTheBit);
1595+
llvm::Value *ShouldCallDelete
1596+
= CGF.Builder.CreateIsNull(ShouldDeleteCondition);
16661597
CGF.Builder.CreateCondBr(ShouldCallDelete, continueBB, callDeleteBB);
16671598

16681599
CGF.EmitBlock(callDeleteBB);

‎clang/lib/CodeGen/CGDebugInfo.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -2119,8 +2119,7 @@ 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 =
2123-
DD ? GlobalDecl(DD, Dtor_VectorDeleting) : GlobalDecl(Method);
2122+
GlobalDecl GD = DD ? GlobalDecl(DD, Dtor_Deleting) : GlobalDecl(Method);
21242123
MethodVFTableLocation ML =
21252124
CGM.getMicrosoftVTableContext().getMethodVFTableLocation(GD);
21262125
VIndex = ML.Index;

0 commit comments

Comments
 (0)