Reland [MS][clang] Add support for vector deleting destructors

clang/include/clang/AST/ASTContext.h

@@ -382,6 +382,9 @@ class ASTContext : public RefCountedBase<ASTContext> {
   mutable llvm::DenseMap<const CXXDestructorDecl *, FunctionDecl *>
       GlobalArrayOperatorDeletesForVirtualDtor;
 
+  /// To remember which types did require a vector deleting dtor.
+  llvm::DenseSet<const CXXRecordDecl *> RequireVectorDeletingDtor;
+
   /// The next string literal "version" to allocate during constant evaluation.
   /// This is used to distinguish between repeated evaluations of the same
   /// string literal.
@@ -3494,6 +3497,8 @@ class ASTContext : public RefCountedBase<ASTContext> {
                                           OperatorDeleteKind K) const;
   bool dtorHasOperatorDelete(const CXXDestructorDecl *Dtor,
                              OperatorDeleteKind K) const;
+  void setClassNeedsVectorDeletingDestructor(const CXXRecordDecl *RD);
+  bool classNeedsVectorDeletingDestructor(const CXXRecordDecl *RD);
 
   /// Retrieve the context for computing mangling numbers in the given
   /// DeclContext.

clang/lib/AST/ASTContext.cpp

@@ -13402,6 +13402,26 @@ ASTContext::getOperatorDeleteForVDtor(const CXXDestructorDecl *Dtor,
   return nullptr;
 }
 
+bool ASTContext::classNeedsVectorDeletingDestructor(const CXXRecordDecl *RD) {
+  if (!getTargetInfo().emitVectorDeletingDtors(getLangOpts()))
+    return false;
+  CXXDestructorDecl *Dtor = RD->getDestructor();
+  // The compiler can't know if new[]/delete[] will be used outside of the DLL,
+  // so just force vector deleting destructor emission if dllexport is present.
+  // This matches MSVC behavior.
+  if (Dtor && Dtor->isVirtual() && Dtor->hasAttr<DLLExportAttr>())
+    return true;
+
+  return RequireVectorDeletingDtor.count(RD);
+}
+
+void ASTContext::setClassNeedsVectorDeletingDestructor(
+    const CXXRecordDecl *RD) {
+  if (!getTargetInfo().emitVectorDeletingDtors(getLangOpts()))
+    return;
+  RequireVectorDeletingDtor.insert(RD);
+}
+
 MangleNumberingContext &
 ASTContext::getManglingNumberContext(const DeclContext *DC) {
   assert(LangOpts.CPlusPlus);  // We don't need mangling numbers for plain C.

clang/lib/CodeGen/CGExprCXX.cpp

@@ -1206,16 +1206,6 @@ void CodeGenFunction::EmitNewArrayInitializer(
     EmitCXXAggrConstructorCall(Ctor, NumElements, CurPtr, CCE,
                                /*NewPointerIsChecked*/true,
                                CCE->requiresZeroInitialization());
-
-    // For MSVC vector deleting destructors support we record that for the class
-    // new[] was called. We try to optimize the code size and only emit vector
-    // deleting destructors when they are required. Vector deleting destructors
-    // are required for delete[] call but MSVC triggers emission of them
-    // whenever new[] is called for an object of the class and we do the same
-    // for compatibility.
-    if (CGM.getContext().getTargetInfo().emitVectorDeletingDtors(
-            CGM.getContext().getLangOpts()))
-      CGM.requireVectorDestructorDefinition(Ctor->getParent());
     return;
   }
 

clang/lib/CodeGen/CodeGenModule.cpp

@@ -8323,53 +8323,3 @@ void CodeGenModule::moveLazyEmissionStates(CodeGenModule *NewBuilder) {
 
   NewBuilder->ABI->MangleCtx = std::move(ABI->MangleCtx);
 }
-
-bool CodeGenModule::classNeedsVectorDestructor(const CXXRecordDecl *RD) {
-  if (!Context.getTargetInfo().emitVectorDeletingDtors(Context.getLangOpts()))
-    return false;
-  CXXDestructorDecl *Dtor = RD->getDestructor();
-  // The compiler can't know if new[]/delete[] will be used outside of the DLL,
-  // so just force vector deleting destructor emission if dllexport is present.
-  // This matches MSVC behavior.
-  if (Dtor && Dtor->isVirtual() && Dtor->isDefined() &&
-      Dtor->hasAttr<DLLExportAttr>())
-    return true;
-
-  return RequireVectorDeletingDtor.count(RD);
-}
-
-void CodeGenModule::requireVectorDestructorDefinition(const CXXRecordDecl *RD) {
-  if (!Context.getTargetInfo().emitVectorDeletingDtors(Context.getLangOpts()))
-    return;
-  RequireVectorDeletingDtor.insert(RD);
-
-  // To reduce code size in general case we lazily emit scalar deleting
-  // destructor definition and an alias from vector deleting destructor to
-  // scalar deleting destructor. It may happen that we first emitted the scalar
-  // deleting destructor definition and the alias and then discovered that the
-  // definition of the vector deleting destructor is required. Then we need to
-  // remove the alias and the scalar deleting destructor and queue vector
-  // deleting destructor body for emission. Check if that is the case.
-  CXXDestructorDecl *DtorD = RD->getDestructor();
-  GlobalDecl ScalarDtorGD(DtorD, Dtor_Deleting);
-  StringRef MangledName = getMangledName(ScalarDtorGD);
-  llvm::GlobalValue *Entry = GetGlobalValue(MangledName);
-  if (Entry && !Entry->isDeclaration()) {
-    GlobalDecl VectorDtorGD(DtorD, Dtor_VectorDeleting);
-    StringRef VDName = getMangledName(VectorDtorGD);
-    llvm::GlobalValue *VDEntry = GetGlobalValue(VDName);
-    // It exists and it should be an alias.
-    assert(VDEntry && isa<llvm::GlobalAlias>(VDEntry));
-    auto *NewFn = llvm::Function::Create(
-        cast<llvm::FunctionType>(VDEntry->getValueType()),
-        llvm::Function::ExternalLinkage, VDName, &getModule());
-    SetFunctionAttributes(VectorDtorGD, NewFn, /*IsIncompleteFunction*/ false,
-                          /*IsThunk*/ false);
-    NewFn->takeName(VDEntry);
-    VDEntry->replaceAllUsesWith(NewFn);
-    VDEntry->eraseFromParent();
-    Entry->replaceAllUsesWith(NewFn);
-    Entry->eraseFromParent();
-    addDeferredDeclToEmit(VectorDtorGD);
-  }
-}

clang/lib/CodeGen/CodeGenModule.h

@@ -529,9 +529,6 @@ class CodeGenModule : public CodeGenTypeCache {
   /// that we don't re-emit the initializer.
   llvm::DenseMap<const Decl*, unsigned> DelayedCXXInitPosition;
 
-  /// To remember which types did require a vector deleting dtor.
-  llvm::SmallPtrSet<const CXXRecordDecl *, 16> RequireVectorDeletingDtor;
-
   typedef std::pair<OrderGlobalInitsOrStermFinalizers, llvm::Function *>
       GlobalInitData;
 
@@ -1828,8 +1825,6 @@ class CodeGenModule : public CodeGenTypeCache {
     // behavior. So projects like the Linux kernel can rely on it.
     return !getLangOpts().CPlusPlus;
   }
-  void requireVectorDestructorDefinition(const CXXRecordDecl *RD);
-  bool classNeedsVectorDestructor(const CXXRecordDecl *RD);
 
   // Helper to get the alignment for a variable.
   unsigned getVtableGlobalVarAlignment(const VarDecl *D = nullptr) {

clang/lib/CodeGen/MicrosoftCXXABI.cpp

@@ -4105,7 +4105,7 @@ void MicrosoftCXXABI::emitCXXStructor(GlobalDecl GD) {
     return;
 
   if (GD.getDtorType() == Dtor_VectorDeleting &&
-      !CGM.classNeedsVectorDestructor(dtor->getParent())) {
+      !getContext().classNeedsVectorDeletingDestructor(dtor->getParent())) {
     // Create GlobalDecl object with the correct type for the scalar
     // deleting destructor.
     GlobalDecl ScalarDtorGD(dtor, Dtor_Deleting);

clang/lib/Sema/SemaDeclCXX.cpp

@@ -11193,6 +11193,7 @@ bool Sema::CheckDestructor(CXXDestructorDecl *Destructor) {
             FindDeallocationFunctionForDestructor(Loc, RD, /*Diagnose*/ false,
                                                   /*LookForGlobal*/ true, Name);
         Destructor->setOperatorGlobalDelete(GlobalOperatorDelete);
+        MarkFunctionReferenced(Loc, GlobalOperatorDelete);
       }
 
       if (Context.getTargetInfo().emitVectorDeletingDtors(
@@ -11209,6 +11210,8 @@ bool Sema::CheckDestructor(CXXDestructorDecl *Destructor) {
                                                     /*LookForGlobal*/ true,
                                                     VDeleteName);
           Destructor->setGlobalOperatorArrayDelete(GlobalArrOperatorDelete);
+          if (Context.classNeedsVectorDeletingDestructor(RD))
+            MarkFunctionReferenced(Loc, GlobalArrOperatorDelete);
         } else if (!ArrOperatorDelete) {
           ArrOperatorDelete = FindDeallocationFunctionForDestructor(
               Loc, RD, /*Diagnose*/ false,
@@ -11217,6 +11220,8 @@ bool Sema::CheckDestructor(CXXDestructorDecl *Destructor) {
         assert(ArrOperatorDelete &&
                "Should've found at least global array delete");
         Destructor->setOperatorArrayDelete(ArrOperatorDelete);
+        if (Context.classNeedsVectorDeletingDestructor(RD))
+          MarkFunctionReferenced(Loc, ArrOperatorDelete);
       }
     }
   }

clang/lib/Sema/SemaExprCXX.cpp

@@ -2631,6 +2631,19 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
     MarkFunctionReferenced(StartLoc, OperatorDelete);
   }
 
+  // For MSVC vector deleting destructors support we record that for the class
+  // new[] was called. We try to optimize the code size and only emit vector
+  // deleting destructors when they are required. Vector deleting destructors
+  // are required for delete[] call but MSVC triggers emission of them
+  // whenever new[] is called for an object of the class and we do the same
+  // for compatibility.
+  if (const CXXConstructExpr *CCE =
+          dyn_cast_or_null<CXXConstructExpr>(Initializer);
+      CCE && ArraySize) {
+    Context.setClassNeedsVectorDeletingDestructor(
+        CCE->getConstructor()->getParent());
+  }
+
   return CXXNewExpr::Create(Context, UseGlobal, OperatorNew, OperatorDelete,
                             IAP, UsualArrayDeleteWantsSize, PlacementArgs,
                             TypeIdParens, ArraySize, InitStyle, Initializer,

clang/test/CodeGenCXX/microsoft-vector-deleting-dtors2.cpp

@@ -0,0 +1,56 @@
+// RUN: %clang_cc1 -emit-llvm -fms-extensions -fms-compatibility -fno-dllexport-inlines %s -triple=x86_64-pc-windows-msvc -o - | FileCheck --check-prefixes=X64,CHECK %s
+// RUN: %clang_cc1 -emit-llvm -fms-extensions -fms-compatibility -fno-dllexport-inlines %s -triple=i386-pc-windows-msvc -o - | FileCheck --check-prefixes=X86,CHECK %s
+
+// Check that vector deleting destructor does not reference undefined symbols.
+
+void operator delete(void*, size_t) {}
+void operator delete[](void*, size_t) {}
+
+template <typename T> struct RefCounted {
+  void operator delete[](void *p) { }
+};
+
+struct __declspec(dllexport) DrawingBuffer : public RefCounted<DrawingBuffer> {
+  DrawingBuffer();
+  virtual ~DrawingBuffer();
+};
+
+DrawingBuffer::DrawingBuffer() {}
+DrawingBuffer::~DrawingBuffer() {}
+
+struct NoExport : public RefCounted<NoExport> {
+  NoExport();
+  virtual ~NoExport();
+};
+
+NoExport::NoExport() {}
+NoExport::~NoExport() {}
+
+// X64: define dso_local void @"??3@YAXPEAX_K@Z"(ptr noundef %0, i64 noundef %1)
+// X64: define dso_local void @"??_V@YAXPEAX_K@Z"(ptr noundef %0, i64 noundef %1)
+// X64: define dso_local dllexport void @"??1DrawingBuffer@@UEAA@XZ"(ptr noundef nonnull align 8 dereferenceable(8) %this)
+
+// X64: define weak dso_local noundef ptr @"??_EDrawingBuffer@@UEAAPEAXI@Z"
+// X64: call void @"??1DrawingBuffer@@UEAA@XZ"(ptr noundef nonnull align 8 dereferenceable(8) %arraydestroy.element)
+// X64: call void @"??_V?$RefCounted@UDrawingBuffer@@@@SAXPEAX@Z"(ptr noundef %2) #2
+// X64: call void @"??_V@YAXPEAX_K@Z"(ptr noundef %2, i64 noundef 8) #3
+// X64: call void @"??1DrawingBuffer@@UEAA@XZ"(ptr noundef nonnull align 8 dereferenceable(8) %this1)
+// X64: call void @"??3@YAXPEAX_K@Z"(ptr noundef %this1, i64 noundef 8) #3
+
+// X64: define linkonce_odr dso_local void @"??_V?$RefCounted@UDrawingBuffer@@@@SAXPEAX@Z"(ptr noundef %p)
+
+// X86: define dso_local void @"??3@YAXPAXI@Z"(ptr noundef %0, i32 noundef %1)
+// X86: define dso_local void @"??_V@YAXPAXI@Z"(ptr noundef %0, i32 noundef %1)
+
+// X86: define weak dso_local x86_thiscallcc noundef ptr @"??_EDrawingBuffer@@UAEPAXI@Z"
+// X86: call x86_thiscallcc void @"??1DrawingBuffer@@UAE@XZ"(ptr noundef nonnull align 4 dereferenceable(4) %arraydestroy.element)
+// X86: call void @"??_V?$RefCounted@UDrawingBuffer@@@@SAXPAX@Z"(ptr noundef %2)
+// X86: call void @"??_V@YAXPAXI@Z"(ptr noundef %2, i32 noundef 4)
+// X86  call x86_thiscallcc void @"??1DrawingBuffer@@UAE@XZ"(ptr noundef nonnull align 4 dereferenceable(4) %this1)
+// X86: call void @"??3@YAXPAXI@Z"(ptr noundef %this1, i32 noundef 4)
+
+// X86: define linkonce_odr dso_local void @"??_V?$RefCounted@UDrawingBuffer@@@@SAXPAX@Z"(ptr noundef %p)
+
+// X86: define linkonce_odr dso_local x86_thiscallcc noundef ptr @"??_GNoExport@@UAEPAXI@Z"(ptr noundef nonnull align 4 dereferenceable(4) %this, i32 noundef %should_call_delete)
+// X64: define linkonce_odr dso_local noundef ptr @"??_GNoExport@@UEAAPEAXI@Z"(ptr noundef nonnull align 8 dereferenceable(8) %this, i32 noundef %should_call_delete)
+// CHECK-NOT: define {{.*}}_V{{.*}}NoExport

clang/test/DebugInfo/CXX/ms-dtor-thunks.cpp

@@ -5,11 +5,11 @@ struct __declspec(dllexport) S { virtual ~S(); };
 struct __declspec(dllexport) T { virtual ~T(); };
 struct __declspec(dllexport) U : S, T { virtual ~U(); };
 
-// CHECK-LABEL: define {{.*}} @"??_GS@@UAEPAXI@Z"
+// CHECK-LABEL: define {{.*}} @"??_ES@@UAEPAXI@Z"
 // CHECK: call x86_thiscallcc void @"??1S@@UAE@XZ"(ptr {{[^,]*}} %this1){{.*}}!dbg !{{[0-9]+}}
 
-// CHECK-LABEL: define {{.*}} @"??_GT@@UAEPAXI@Z"
+// CHECK-LABEL: define {{.*}} @"??_ET@@UAEPAXI@Z"
 // CHECK: call x86_thiscallcc void @"??1T@@UAE@XZ"(ptr {{[^,]*}} %this1){{.*}}!dbg !{{[0-9]+}}
 
-// CHECK-LABEL: define {{.*}} @"??_GU@@UAEPAXI@Z"
+// CHECK-LABEL: define {{.*}} @"??_EU@@UAEPAXI@Z"
 // CHECK: call x86_thiscallcc void @"??1U@@UAE@XZ"(ptr {{[^,]*}} %this1){{.*}}!dbg !{{[0-9]+}}