Skip to content

Commit 88d0b08

Browse files
FznamznonFznamznon
authored
[MS][clang] Revert vector deleting destructors support (#135611)
Finding operator delete[] is still problematic, without it the extension is a security hazard, so reverting until the problem with operator delete[] is figured out. This reverts the following PRs: Reland [MS][clang] Add support for vector deleting destructors (#133451) [MS][clang] Make sure vector deleting dtor calls correct operator delete (#133950) [MS][clang] Fix crash on deletion of array of pointers (#134088) [clang] Do not diagnose unused deleted operator delete[] (#134357) [MS][clang] Error about ambiguous operator delete[] only when required (#135041)
1 parent fe54d1a commit 88d0b08

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+131
-717
lines changed

‎clang/docs/ReleaseNotes.rst

-1
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,6 @@ Windows Support
530530
- Clang now can process the `i128` and `ui128` integeral suffixes when MSVC
531531
extensions are enabled. This allows for properly processing ``intsafe.h`` in
532532
the Windows SDK.
533-
- Clang now supports MSVC vector deleting destructors (GH19772).
534533

535534
LoongArch Support
536535
^^^^^^^^^^^^^^^^^

‎clang/include/clang/AST/DeclCXX.h

-6
Original file line numberDiff line numberDiff line change
@@ -2852,7 +2852,6 @@ class CXXDestructorDecl : public CXXMethodDecl {
28522852
// FIXME: Don't allocate storage for these except in the first declaration
28532853
// of a virtual destructor.
28542854
FunctionDecl *OperatorDelete = nullptr;
2855-
FunctionDecl *OperatorArrayDelete = nullptr;
28562855
Expr *OperatorDeleteThisArg = nullptr;
28572856

28582857
CXXDestructorDecl(ASTContext &C, CXXRecordDecl *RD, SourceLocation StartLoc,
@@ -2878,16 +2877,11 @@ class CXXDestructorDecl : public CXXMethodDecl {
28782877
static CXXDestructorDecl *CreateDeserialized(ASTContext &C, GlobalDeclID ID);
28792878

28802879
void setOperatorDelete(FunctionDecl *OD, Expr *ThisArg);
2881-
void setOperatorArrayDelete(FunctionDecl *OD);
28822880

28832881
const FunctionDecl *getOperatorDelete() const {
28842882
return getCanonicalDecl()->OperatorDelete;
28852883
}
28862884

2887-
const FunctionDecl *getArrayOperatorDelete() const {
2888-
return getCanonicalDecl()->OperatorArrayDelete;
2889-
}
2890-
28912885
Expr *getOperatorDeleteThisArg() const {
28922886
return getCanonicalDecl()->OperatorDeleteThisArg;
28932887
}

‎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/include/clang/Sema/Sema.h

-1
Original file line numberDiff line numberDiff line change
@@ -8366,7 +8366,6 @@ class Sema final : public SemaBase {
83668366
DeclarationName Name);
83678367
FunctionDecl *FindDeallocationFunctionForDestructor(SourceLocation StartLoc,
83688368
CXXRecordDecl *RD,
8369-
DeclarationName Name,
83708369
bool Diagnose = true);
83718370

83728371
/// ActOnCXXDelete - Parsed a C++ 'delete' expression (C++ 5.3.5), as in:

‎clang/lib/AST/DeclCXX.cpp

-6
Original file line numberDiff line numberDiff line change
@@ -3064,12 +3064,6 @@ void CXXDestructorDecl::setOperatorDelete(FunctionDecl *OD, Expr *ThisArg) {
30643064
}
30653065
}
30663066

3067-
void CXXDestructorDecl::setOperatorArrayDelete(FunctionDecl *OD) {
3068-
auto *First = cast<CXXDestructorDecl>(getFirstDecl());
3069-
if (OD && !First->OperatorArrayDelete)
3070-
First->OperatorArrayDelete = OD;
3071-
}
3072-
30733067
bool CXXDestructorDecl::isCalledByDelete(const FunctionDecl *OpDel) const {
30743068
// C++20 [expr.delete]p6: If the value of the operand of the delete-
30753069
// expression is not a null pointer value and the selected deallocation

‎clang/lib/AST/Expr.cpp

-3
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,6 @@ const CXXRecordDecl *Expr::getBestDynamicClassType() const {
7171
if (const PointerType *PTy = DerivedType->getAs<PointerType>())
7272
DerivedType = PTy->getPointeeType();
7373

74-
while (const ArrayType *ATy = DerivedType->getAsArrayTypeUnsafe())
75-
DerivedType = ATy->getElementType();
76-
7774
if (DerivedType->isDependentType())
7875
return nullptr;
7976

‎clang/lib/AST/ItaniumMangle.cpp

-2
Original file line numberDiff line numberDiff line change
@@ -6047,8 +6047,6 @@ void CXXNameMangler::mangleCXXDtorType(CXXDtorType T) {
60476047
case Dtor_Comdat:
60486048
Out << "D5";
60496049
break;
6050-
case Dtor_VectorDeleting:
6051-
llvm_unreachable("Itanium ABI does not use vector deleting dtors");
60526050
}
60536051
}
60546052

‎clang/lib/AST/MicrosoftMangle.cpp

+9-13
Original file line numberDiff line numberDiff line change
@@ -1490,9 +1490,8 @@ void MicrosoftCXXNameMangler::mangleCXXDtorType(CXXDtorType T) {
14901490
// <operator-name> ::= ?_G # scalar deleting destructor
14911491
case Dtor_Deleting: Out << "?_G"; return;
14921492
// <operator-name> ::= ?_E # vector deleting destructor
1493-
case Dtor_VectorDeleting:
1494-
Out << "?_E";
1495-
return;
1493+
// FIXME: Add a vector deleting dtor type. It goes in the vtable, so we need
1494+
// it.
14961495
case Dtor_Comdat:
14971496
llvm_unreachable("not expecting a COMDAT");
14981497
}
@@ -2893,12 +2892,9 @@ void MicrosoftCXXNameMangler::mangleFunctionType(const FunctionType *T,
28932892
// ::= @ # structors (they have no declared return type)
28942893
if (IsStructor) {
28952894
if (isa<CXXDestructorDecl>(D) && isStructorDecl(D)) {
2896-
// The deleting destructors take an extra argument of type int that
2897-
// indicates whether the storage for the object should be deleted and
2898-
// whether a single object or an array of objects is being destroyed. This
2899-
// extra argument is not reflected in the AST.
2900-
if (StructorType == Dtor_Deleting ||
2901-
StructorType == Dtor_VectorDeleting) {
2895+
// The scalar deleting destructor takes an extra int argument which is not
2896+
// reflected in the AST.
2897+
if (StructorType == Dtor_Deleting) {
29022898
Out << (PointersAre64Bit ? "PEAXI@Z" : "PAXI@Z");
29032899
return;
29042900
}
@@ -3871,10 +3867,10 @@ void MicrosoftMangleContextImpl::mangleCXXDtorThunk(const CXXDestructorDecl *DD,
38713867
const ThunkInfo &Thunk,
38723868
bool /*ElideOverrideInfo*/,
38733869
raw_ostream &Out) {
3874-
// The dtor thunk should use vector deleting dtor mangling, however as an
3875-
// optimization we may end up emitting only scalar deleting dtor body, so just
3876-
// use the vector deleting dtor mangling manually.
3877-
assert(Type == Dtor_Deleting || Type == Dtor_VectorDeleting);
3870+
// FIXME: Actually, the dtor thunk should be emitted for vector deleting
3871+
// dtors rather than scalar deleting dtors. Just use the vector deleting dtor
3872+
// mangling manually until we support both deleting dtor types.
3873+
assert(Type == Dtor_Deleting);
38783874
msvc_hashing_ostream MHO(Out);
38793875
MicrosoftCXXNameMangler Mangler(*this, MHO, DD, Type);
38803876
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]";
@@ -3764,7 +3757,7 @@ void MicrosoftVTableContext::dumpMethodLocations(
37643757
PredefinedIdentKind::PrettyFunctionNoVirtual, MD);
37653758

37663759
if (isa<CXXDestructorDecl>(MD)) {
3767-
IndicesMap[I.second] = MethodName + " [vector deleting]";
3760+
IndicesMap[I.second] = MethodName + " [scalar deleting]";
37683761
} else {
37693762
IndicesMap[I.second] = MethodName;
37703763
}
@@ -3880,7 +3873,7 @@ MicrosoftVTableContext::getMethodVFTableLocation(GlobalDecl GD) {
38803873
assert(hasVtableSlot(cast<CXXMethodDecl>(GD.getDecl())) &&
38813874
"Only use this method for virtual methods or dtors");
38823875
if (isa<CXXDestructorDecl>(GD.getDecl()))
3883-
assert(GD.getDtorType() == Dtor_VectorDeleting);
3876+
assert(GD.getDtorType() == Dtor_Deleting);
38843877

38853878
GD = GD.getCanonicalDecl();
38863879

‎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
@@ -271,20 +271,6 @@ void CGCXXABI::ReadArrayCookie(CodeGenFunction &CGF, Address ptr,
271271
numElements = readArrayCookieImpl(CGF, allocAddr, cookieSize);
272272
}
273273

274-
void CGCXXABI::ReadArrayCookie(CodeGenFunction &CGF, Address ptr,
275-
QualType eltTy, llvm::Value *&numElements,
276-
llvm::Value *&allocPtr, CharUnits &cookieSize) {
277-
assert(eltTy.isDestructedType());
278-
279-
// Derive a char* in the same address space as the pointer.
280-
ptr = ptr.withElementType(CGF.Int8Ty);
281-
282-
cookieSize = getArrayCookieSizeImpl(eltTy);
283-
Address allocAddr = CGF.Builder.CreateConstInBoundsByteGEP(ptr, -cookieSize);
284-
allocPtr = allocAddr.emitRawPointer(CGF);
285-
numElements = readArrayCookieImpl(CGF, allocAddr, cookieSize);
286-
}
287-
288274
llvm::Value *CGCXXABI::readArrayCookieImpl(CodeGenFunction &CGF,
289275
Address ptr,
290276
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

0 commit comments

Comments
 (0)