Revert "[MS][clang] Add support for vector deleting destructors (#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 #19772

This reverts commit d6942d5.

Author: zmodem

clang/docs/ReleaseNotes.rst

@@ -357,8 +357,6 @@ Android Support
 Windows Support
 ^^^^^^^^^^^^^^^
 
-- Clang now supports MSVC vector deleting destructors (GH19772).
-
 LoongArch Support
 ^^^^^^^^^^^^^^^^^
 

clang/include/clang/AST/VTableBuilder.h

@@ -150,7 +150,7 @@ class VTableComponent {
 
   bool isRTTIKind() const { return isRTTIKind(getKind()); }
 
-  GlobalDecl getGlobalDecl(bool HasVectorDeletingDtors) const {
+  GlobalDecl getGlobalDecl() const {
     assert(isUsedFunctionPointerKind() &&
            "GlobalDecl can be created only from virtual function");
 
@@ -161,9 +161,7 @@ class VTableComponent {
     case CK_CompleteDtorPointer:
       return GlobalDecl(DtorDecl, CXXDtorType::Dtor_Complete);
     case CK_DeletingDtorPointer:
-      return GlobalDecl(DtorDecl, (HasVectorDeletingDtors)
-                                      ? CXXDtorType::Dtor_VectorDeleting
-                                      : CXXDtorType::Dtor_Deleting);
+      return GlobalDecl(DtorDecl, CXXDtorType::Dtor_Deleting);
     case CK_VCallOffset:
     case CK_VBaseOffset:
     case CK_OffsetToTop:

clang/include/clang/Basic/ABI.h

@@ -31,11 +31,10 @@ enum CXXCtorType {
 
 /// C++ destructor types.
 enum CXXDtorType {
-  Dtor_Deleting,      ///< Deleting dtor
-  Dtor_Complete,      ///< Complete object dtor
-  Dtor_Base,          ///< Base object dtor
-  Dtor_Comdat,        ///< The COMDAT used for dtors
-  Dtor_VectorDeleting ///< Vector deleting dtor
+    Dtor_Deleting, ///< Deleting dtor
+    Dtor_Complete, ///< Complete object dtor
+    Dtor_Base,     ///< Base object dtor
+    Dtor_Comdat    ///< The COMDAT used for dtors
 };
 
 } // end namespace clang

clang/lib/AST/ItaniumMangle.cpp

@@ -6014,8 +6014,6 @@ void CXXNameMangler::mangleCXXDtorType(CXXDtorType T) {
   case Dtor_Comdat:
     Out << "D5";
     break;
-  case Dtor_VectorDeleting:
-    llvm_unreachable("Itanium ABI does not use vector deleting dtors");
   }
 }
 

clang/lib/AST/MicrosoftMangle.cpp

@@ -1484,9 +1484,8 @@ void MicrosoftCXXNameMangler::mangleCXXDtorType(CXXDtorType T) {
   // <operator-name> ::= ?_G # scalar deleting destructor
   case Dtor_Deleting: Out << "?_G"; return;
   // <operator-name> ::= ?_E # vector deleting destructor
-  case Dtor_VectorDeleting:
-    Out << "?_E";
-    return;
+  // FIXME: Add a vector deleting dtor type.  It goes in the vtable, so we need
+  // it.
   case Dtor_Comdat:
     llvm_unreachable("not expecting a COMDAT");
   }
@@ -2887,12 +2886,9 @@ void MicrosoftCXXNameMangler::mangleFunctionType(const FunctionType *T,
   //               ::= @ # structors (they have no declared return type)
   if (IsStructor) {
     if (isa<CXXDestructorDecl>(D) && isStructorDecl(D)) {
-      // The deleting destructors take an extra argument of type int that
-      // indicates whether the storage for the object should be deleted and
-      // whether a single object or an array of objects is being destroyed. This
-      // extra argument is not reflected in the AST.
-      if (StructorType == Dtor_Deleting ||
-          StructorType == Dtor_VectorDeleting) {
+      // The scalar deleting destructor takes an extra int argument which is not
+      // reflected in the AST.
+      if (StructorType == Dtor_Deleting) {
         Out << (PointersAre64Bit ? "PEAXI@Z" : "PAXI@Z");
         return;
       }
@@ -3865,10 +3861,10 @@ void MicrosoftMangleContextImpl::mangleCXXDtorThunk(const CXXDestructorDecl *DD,
                                                     const ThunkInfo &Thunk,
                                                     bool /*ElideOverrideInfo*/,
                                                     raw_ostream &Out) {
-  // The dtor thunk should use vector deleting dtor mangling, however as an
-  // optimization we may end up emitting only scalar deleting dtor body, so just
-  // use the vector deleting dtor mangling manually.
-  assert(Type == Dtor_Deleting || Type == Dtor_VectorDeleting);
+  // FIXME: Actually, the dtor thunk should be emitted for vector deleting
+  // dtors rather than scalar deleting dtors. Just use the vector deleting dtor
+  // mangling manually until we support both deleting dtor types.
+  assert(Type == Dtor_Deleting);
   msvc_hashing_ostream MHO(Out);
   MicrosoftCXXNameMangler Mangler(*this, MHO, DD, Type);
   Mangler.getStream() << "??_E";

clang/lib/AST/VTableBuilder.cpp

@@ -1735,8 +1735,8 @@ void ItaniumVTableBuilder::LayoutPrimaryAndSecondaryVTables(
       const CXXMethodDecl *MD = I.first;
       const MethodInfo &MI = I.second;
       if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD)) {
-        MethodVTableIndices[GlobalDecl(DD, Dtor_Complete)] =
-            MI.VTableIndex - AddressPoint;
+        MethodVTableIndices[GlobalDecl(DD, Dtor_Complete)]
+            = MI.VTableIndex - AddressPoint;
         MethodVTableIndices[GlobalDecl(DD, Dtor_Deleting)]
             = MI.VTableIndex + 1 - AddressPoint;
       } else {
@@ -2657,11 +2657,7 @@ class VFTableBuilder {
       MethodVFTableLocation Loc(MI.VBTableIndex, WhichVFPtr.getVBaseWithVPtr(),
                                 WhichVFPtr.NonVirtualOffset, MI.VFTableIndex);
       if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD)) {
-        // In Microsoft ABI vftable always references vector deleting dtor.
-        CXXDtorType DtorTy = Context.getTargetInfo().getCXXABI().isMicrosoft()
-                                 ? Dtor_VectorDeleting
-                                 : Dtor_Deleting;
-        MethodVFTableLocations[GlobalDecl(DD, DtorTy)] = Loc;
+        MethodVFTableLocations[GlobalDecl(DD, Dtor_Deleting)] = Loc;
       } else {
         MethodVFTableLocations[MD] = Loc;
       }
@@ -3291,10 +3287,7 @@ void VFTableBuilder::dumpLayout(raw_ostream &Out) {
       const CXXDestructorDecl *DD = Component.getDestructorDecl();
 
       DD->printQualifiedName(Out);
-      if (Context.getTargetInfo().getCXXABI().isMicrosoft())
-        Out << "() [vector deleting]";
-      else
-        Out << "() [scalar deleting]";
+      Out << "() [scalar deleting]";
 
       if (DD->isPureVirtual())
         Out << " [pure]";
@@ -3765,7 +3758,7 @@ void MicrosoftVTableContext::dumpMethodLocations(
         PredefinedIdentKind::PrettyFunctionNoVirtual, MD);
 
     if (isa<CXXDestructorDecl>(MD)) {
-      IndicesMap[I.second] = MethodName + " [vector deleting]";
+      IndicesMap[I.second] = MethodName + " [scalar deleting]";
     } else {
       IndicesMap[I.second] = MethodName;
     }
@@ -3882,7 +3875,7 @@ MicrosoftVTableContext::getMethodVFTableLocation(GlobalDecl GD) {
   assert(hasVtableSlot(cast<CXXMethodDecl>(GD.getDecl())) &&
          "Only use this method for virtual methods or dtors");
   if (isa<CXXDestructorDecl>(GD.getDecl()))
-    assert(GD.getDtorType() == Dtor_VectorDeleting);
+    assert(GD.getDtorType() == Dtor_Deleting);
 
   GD = GD.getCanonicalDecl();
 

clang/lib/CodeGen/CGCXX.cpp

@@ -175,6 +175,7 @@ bool CodeGenModule::TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D) {
   // requires explicit comdat support in the IL.
   if (llvm::GlobalValue::isWeakForLinker(TargetLinkage))
     return true;
+
   // Create the alias with no name.
   auto *Alias = llvm::GlobalAlias::create(AliasValueType, 0, Linkage, "",
                                           Aliasee, &getModule());
@@ -200,42 +201,6 @@ bool CodeGenModule::TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D) {
   return false;
 }
 
-/// Emit a definition as a global alias for another definition, unconditionally.
-void CodeGenModule::EmitDefinitionAsAlias(GlobalDecl AliasDecl,
-                                          GlobalDecl TargetDecl) {
-
-  llvm::Type *AliasValueType = getTypes().GetFunctionType(AliasDecl);
-
-  StringRef MangledName = getMangledName(AliasDecl);
-  llvm::GlobalValue *Entry = GetGlobalValue(MangledName);
-  if (Entry && !Entry->isDeclaration())
-    return;
-  auto *Aliasee = cast<llvm::GlobalValue>(GetAddrOfGlobal(TargetDecl));
-
-  // Determine the linkage type for the alias.
-  llvm::GlobalValue::LinkageTypes Linkage = getFunctionLinkage(AliasDecl);
-
-  // Create the alias with no name.
-  auto *Alias = llvm::GlobalAlias::create(AliasValueType, 0, Linkage, "",
-                                          Aliasee, &getModule());
-  // Destructors are always unnamed_addr.
-  Alias->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
-
-  if (Entry) {
-    assert(Entry->getValueType() == AliasValueType &&
-           Entry->getAddressSpace() == Alias->getAddressSpace() &&
-           "declaration exists with different type");
-    Alias->takeName(Entry);
-    Entry->replaceAllUsesWith(Alias);
-    Entry->eraseFromParent();
-  } else {
-    Alias->setName(MangledName);
-  }
-
-  // Set any additional necessary attributes for the alias.
-  SetCommonAttributes(AliasDecl, Alias);
-}
-
 llvm::Function *CodeGenModule::codegenCXXStructor(GlobalDecl GD) {
   const CGFunctionInfo &FnInfo = getTypes().arrangeCXXStructorDeclaration(GD);
   auto *Fn = cast<llvm::Function>(

clang/lib/CodeGen/CGCXXABI.cpp

@@ -273,20 +273,6 @@ void CGCXXABI::ReadArrayCookie(CodeGenFunction &CGF, Address ptr,
   numElements = readArrayCookieImpl(CGF, allocAddr, cookieSize);
 }
 
-void CGCXXABI::ReadArrayCookie(CodeGenFunction &CGF, Address ptr,
-                               QualType eltTy, llvm::Value *&numElements,
-                               llvm::Value *&allocPtr, CharUnits &cookieSize) {
-  assert(eltTy.isDestructedType());
-
-  // Derive a char* in the same address space as the pointer.
-  ptr = ptr.withElementType(CGF.Int8Ty);
-
-  cookieSize = getArrayCookieSizeImpl(eltTy);
-  Address allocAddr = CGF.Builder.CreateConstInBoundsByteGEP(ptr, -cookieSize);
-  allocPtr = allocAddr.emitRawPointer(CGF);
-  numElements = readArrayCookieImpl(CGF, allocAddr, cookieSize);
-}
-
 llvm::Value *CGCXXABI::readArrayCookieImpl(CodeGenFunction &CGF,
                                            Address ptr,
                                            CharUnits cookieSize) {

clang/lib/CodeGen/CGCXXABI.h

@@ -275,7 +275,6 @@ class CGCXXABI {
   virtual CatchTypeInfo getCatchAllTypeInfo();
 
   virtual bool shouldTypeidBeNullChecked(QualType SrcRecordTy) = 0;
-  virtual bool hasVectorDeletingDtors() = 0;
   virtual void EmitBadTypeidCall(CodeGenFunction &CGF) = 0;
   virtual llvm::Value *EmitTypeid(CodeGenFunction &CGF, QualType SrcRecordTy,
                                   Address ThisPtr,
@@ -576,12 +575,6 @@ class CGCXXABI {
                                QualType ElementType, llvm::Value *&NumElements,
                                llvm::Value *&AllocPtr, CharUnits &CookieSize);
 
-  /// Reads the array cookie associated with the given pointer,
-  /// that should have one.
-  void ReadArrayCookie(CodeGenFunction &CGF, Address Ptr, QualType ElementType,
-                       llvm::Value *&NumElements, llvm::Value *&AllocPtr,
-                       CharUnits &CookieSize);
-
   /// Return whether the given global decl needs a VTT parameter.
   virtual bool NeedsVTTParameter(GlobalDecl GD);
 

clang/lib/CodeGen/CGClass.cpp

@@ -1433,70 +1433,6 @@ static bool CanSkipVTablePointerInitialization(CodeGenFunction &CGF,
   return true;
 }
 
-static void EmitConditionalArrayDtorCall(const CXXDestructorDecl *DD,
-                                         CodeGenFunction &CGF,
-                                         llvm::Value *ShouldDeleteCondition) {
-  Address ThisPtr = CGF.LoadCXXThisAddress();
-  llvm::BasicBlock *ScalarBB = CGF.createBasicBlock("dtor.scalar");
-  llvm::BasicBlock *callDeleteBB =
-      CGF.createBasicBlock("dtor.call_delete_after_array_destroy");
-  llvm::BasicBlock *VectorBB = CGF.createBasicBlock("dtor.vector");
-  auto *CondTy = cast<llvm::IntegerType>(ShouldDeleteCondition->getType());
-  llvm::Value *CheckTheBitForArrayDestroy = CGF.Builder.CreateAnd(
-      ShouldDeleteCondition, llvm::ConstantInt::get(CondTy, 2));
-  llvm::Value *ShouldDestroyArray =
-      CGF.Builder.CreateIsNull(CheckTheBitForArrayDestroy);
-  CGF.Builder.CreateCondBr(ShouldDestroyArray, ScalarBB, VectorBB);
-
-  CGF.EmitBlock(VectorBB);
-
-  llvm::Value *numElements = nullptr;
-  llvm::Value *allocatedPtr = nullptr;
-  CharUnits cookieSize;
-  QualType EltTy = DD->getThisType()->getPointeeType();
-  CGF.CGM.getCXXABI().ReadArrayCookie(CGF, ThisPtr, EltTy, numElements,
-                                      allocatedPtr, cookieSize);
-
-  // Destroy the elements.
-  QualType::DestructionKind dtorKind = EltTy.isDestructedType();
-
-  assert(dtorKind);
-  assert(numElements && "no element count for a type with a destructor!");
-
-  CharUnits elementSize = CGF.getContext().getTypeSizeInChars(EltTy);
-  CharUnits elementAlign =
-      ThisPtr.getAlignment().alignmentOfArrayElement(elementSize);
-
-  llvm::Value *arrayBegin = ThisPtr.emitRawPointer(CGF);
-  llvm::Value *arrayEnd = CGF.Builder.CreateInBoundsGEP(
-      ThisPtr.getElementType(), arrayBegin, numElements, "delete.end");
-
-  // We already checked that the array is not 0-length before entering vector
-  // deleting dtor.
-  CGF.emitArrayDestroy(arrayBegin, arrayEnd, EltTy, elementAlign,
-                       CGF.getDestroyer(dtorKind),
-                       /*checkZeroLength*/ false, CGF.needsEHCleanup(dtorKind));
-
-  llvm::BasicBlock *VectorBBCont = CGF.createBasicBlock("dtor.vector.cont");
-  CGF.EmitBlock(VectorBBCont);
-
-  llvm::Value *CheckTheBitForDeleteCall = CGF.Builder.CreateAnd(
-      ShouldDeleteCondition, llvm::ConstantInt::get(CondTy, 1));
-
-  llvm::Value *ShouldCallDelete =
-      CGF.Builder.CreateIsNull(CheckTheBitForDeleteCall);
-  CGF.Builder.CreateCondBr(ShouldCallDelete, CGF.ReturnBlock.getBlock(),
-                           callDeleteBB);
-  CGF.EmitBlock(callDeleteBB);
-  const CXXDestructorDecl *Dtor = cast<CXXDestructorDecl>(CGF.CurCodeDecl);
-  const CXXRecordDecl *ClassDecl = Dtor->getParent();
-  CGF.EmitDeleteCall(Dtor->getOperatorDelete(), allocatedPtr,
-                     CGF.getContext().getTagDeclType(ClassDecl));
-
-  CGF.EmitBranchThroughCleanup(CGF.ReturnBlock);
-  CGF.EmitBlock(ScalarBB);
-}
-
 /// EmitDestructorBody - Emits the body of the current destructor.
 void CodeGenFunction::EmitDestructorBody(FunctionArgList &Args) {
   const CXXDestructorDecl *Dtor = cast<CXXDestructorDecl>(CurGD.getDecl());
@@ -1526,9 +1462,7 @@ void CodeGenFunction::EmitDestructorBody(FunctionArgList &Args) {
   // outside of the function-try-block, which means it's always
   // possible to delegate the destructor body to the complete
   // destructor.  Do so.
-  if (DtorType == Dtor_Deleting || DtorType == Dtor_VectorDeleting) {
-    if (CXXStructorImplicitParamValue && DtorType == Dtor_VectorDeleting)
-      EmitConditionalArrayDtorCall(Dtor, *this, CXXStructorImplicitParamValue);
+  if (DtorType == Dtor_Deleting) {
     RunCleanupsScope DtorEpilogue(*this);
     EnterDtorCleanups(Dtor, Dtor_Deleting);
     if (HaveInsertPoint()) {
@@ -1557,8 +1491,6 @@ void CodeGenFunction::EmitDestructorBody(FunctionArgList &Args) {
   switch (DtorType) {
   case Dtor_Comdat: llvm_unreachable("not expecting a COMDAT");
   case Dtor_Deleting: llvm_unreachable("already handled deleting case");
-  case Dtor_VectorDeleting:
-    llvm_unreachable("already handled vector deleting case");
 
   case Dtor_Complete:
     assert((Body || getTarget().getCXXABI().isMicrosoft()) &&
@@ -1641,6 +1573,7 @@ namespace {
       return CGF.EmitScalarExpr(ThisArg);
     return CGF.LoadCXXThis();
   }
+
   /// Call the operator delete associated with the current destructor.
   struct CallDtorDelete final : EHScopeStack::Cleanup {
     CallDtorDelete() {}
@@ -1659,10 +1592,8 @@ namespace {
                                      bool ReturnAfterDelete) {
     llvm::BasicBlock *callDeleteBB = CGF.createBasicBlock("dtor.call_delete");
     llvm::BasicBlock *continueBB = CGF.createBasicBlock("dtor.continue");
-    auto *CondTy = cast<llvm::IntegerType>(ShouldDeleteCondition->getType());
-    llvm::Value *CheckTheBit = CGF.Builder.CreateAnd(
-        ShouldDeleteCondition, llvm::ConstantInt::get(CondTy, 1));
-    llvm::Value *ShouldCallDelete = CGF.Builder.CreateIsNull(CheckTheBit);
+    llvm::Value *ShouldCallDelete
+      = CGF.Builder.CreateIsNull(ShouldDeleteCondition);
     CGF.Builder.CreateCondBr(ShouldCallDelete, continueBB, callDeleteBB);
 
     CGF.EmitBlock(callDeleteBB);