https://github.com/Fznamznon updated https://github.com/llvm/llvm-project/pull/183741
>From e79308a124664ff0cb4d10b7bd61c4f71e65197d Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Fri, 27 Feb 2026 06:04:27 -0800 Subject: [PATCH 1/3] [win][clang] Do not attempt to devirtualize vector deleting destructor call Since vector deleting destructor performs a loop over array elements and calls delete[], simply devirtualizing call to it produces wrong code with memory leaks. Before emitting virtual call to vector deleting destructor, check if it can be devirtualized, if yes, emit normal loop over array elements instead of a virtual call. This aims to fix https://github.com/llvm/llvm-project/issues/183621 --- clang/lib/CodeGen/CGExprCXX.cpp | 67 +++++++---- .../CodeGenCXX/ms-vdtors-devirtualization.cpp | 111 ++++++++++++++++++ 2 files changed, 152 insertions(+), 26 deletions(-) create mode 100644 clang/test/CodeGenCXX/ms-vdtors-devirtualization.cpp diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index c8e1fe69da9c7..199a50cbb650c 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -2095,32 +2095,47 @@ void CodeGenFunction::EmitCXXDeleteExpr(const CXXDeleteExpr *E) { if (auto *RD = DeleteTy->getAsCXXRecordDecl()) { auto *Dtor = RD->getDestructor(); if (Dtor && Dtor->isVirtual()) { - llvm::Value *NumElements = nullptr; - llvm::Value *AllocatedPtr = nullptr; - CharUnits CookieSize; - llvm::BasicBlock *BodyBB = createBasicBlock("vdtor.call"); - llvm::BasicBlock *DoneBB = createBasicBlock("vdtor.nocall"); - // Check array cookie to see if the array has length 0. Don't call - // the destructor in that case. - CGM.getCXXABI().ReadArrayCookie(*this, Ptr, E, DeleteTy, NumElements, - AllocatedPtr, CookieSize); - - auto *CondTy = cast<llvm::IntegerType>(NumElements->getType()); - llvm::Value *IsEmpty = Builder.CreateICmpEQ( - NumElements, llvm::ConstantInt::get(CondTy, 0)); - Builder.CreateCondBr(IsEmpty, DoneBB, BodyBB); - - // Delete cookie for empty array. - const FunctionDecl *OperatorDelete = E->getOperatorDelete(); - EmitBlock(DoneBB); - EmitDeleteCall(OperatorDelete, AllocatedPtr, DeleteTy, NumElements, - CookieSize); - EmitBranch(DeleteEnd); - - EmitBlock(BodyBB); - if (!EmitObjectDelete(*this, E, Ptr, DeleteTy, DeleteEnd)) - EmitBlock(DeleteEnd); - return; + bool CanDevirtualizeCall = false; + const Expr *DBase = E->getArgument(); + // Emit normal loop over the array elements if we can easily + // devirtualize destructor call. + if (auto *DevirtualizedDtor = dyn_cast_or_null<const CXXDestructorDecl>( + Dtor->getDevirtualizedMethod( + DBase, CGM.getLangOpts().AppleKext))) { + const CXXRecordDecl *DevirtualizedClass = + DevirtualizedDtor->getParent(); + if (declaresSameEntity(getCXXRecord(DBase), DevirtualizedClass)) + CanDevirtualizeCall = true; + } + // Emit virtual call to vector deleting destructor otherwise. + if (!CanDevirtualizeCall) { + llvm::Value *NumElements = nullptr; + llvm::Value *AllocatedPtr = nullptr; + CharUnits CookieSize; + llvm::BasicBlock *BodyBB = createBasicBlock("vdtor.call"); + llvm::BasicBlock *DoneBB = createBasicBlock("vdtor.nocall"); + // Check array cookie to see if the array has length 0. Don't call + // the destructor in that case. + CGM.getCXXABI().ReadArrayCookie(*this, Ptr, E, DeleteTy, NumElements, + AllocatedPtr, CookieSize); + + auto *CondTy = cast<llvm::IntegerType>(NumElements->getType()); + llvm::Value *IsEmpty = Builder.CreateICmpEQ( + NumElements, llvm::ConstantInt::get(CondTy, 0)); + Builder.CreateCondBr(IsEmpty, DoneBB, BodyBB); + + // Delete cookie for empty array. + const FunctionDecl *OperatorDelete = E->getOperatorDelete(); + EmitBlock(DoneBB); + EmitDeleteCall(OperatorDelete, AllocatedPtr, DeleteTy, NumElements, + CookieSize); + EmitBranch(DeleteEnd); + + EmitBlock(BodyBB); + if (!EmitObjectDelete(*this, E, Ptr, DeleteTy, DeleteEnd)) + EmitBlock(DeleteEnd); + return; + } } } } diff --git a/clang/test/CodeGenCXX/ms-vdtors-devirtualization.cpp b/clang/test/CodeGenCXX/ms-vdtors-devirtualization.cpp new file mode 100644 index 0000000000000..1cd09780b722d --- /dev/null +++ b/clang/test/CodeGenCXX/ms-vdtors-devirtualization.cpp @@ -0,0 +1,111 @@ +// RUN: %clang_cc1 -emit-llvm -fms-extensions %s -triple=x86_64-pc-windows-msvc -o - | FileCheck --check-prefixes=CHECK,X64 %s +// RUN: %clang_cc1 -emit-llvm -fms-extensions %s -triple=i386-pc-windows-msvc -o - | FileCheck --check-prefixes=CHECK,X86 %s + +struct Base { + virtual ~Base() {} +}; + +struct A final : Base { + virtual ~A(); +}; + +struct B : Base { virtual ~B() final {} }; + +struct D { virtual ~D() final = 0; }; + +void case1(A *arg) { + delete[] arg; +} +// X64-LABEL: define {{.*}} void @"?case1@@YAXPEAUA@@@Z" +// X64-SAME: (ptr noundef %[[ARG:.*]]) +// X86-LABEL: define {{.*}} void @"?case1@@YAXPAUA@@@Z" +// X86-SAME: (ptr noundef %[[ARG:.*]]) +// CHECK: entry: +// CHECK-NEXT: %[[ARGADDR:.*]] = alloca ptr +// CHECK-NEXT: store ptr %[[ARG]], ptr %[[ARGADDR]], +// CHECK-NEXT: %[[ARR:.*]] = load ptr, ptr %[[ARGADDR]] +// CHECK-NEXT: %[[ISNULL:.*]] = icmp eq ptr %[[ARR]], null +// CHECK-NEXT: br i1 %[[ISNULL]], label %delete.end2, label %delete.notnull +// CHECK: delete.notnull: +// X64-NEXT: %[[COOKIEADDR:.*]] = getelementptr inbounds i8, ptr %[[ARR]], i64 -8 +// X86-NEXT: %[[COOKIEADDR:.*]] = getelementptr inbounds i8, ptr %0, i32 -4 +// X64-NEXT: %[[COOKIE:.*]] = load i64, ptr %[[COOKIEADDR]] +// X86-NEXT: %[[COOKIE:.*]] = load i32, ptr %[[COOKIEADDR]] +// X64-NEXT: %[[END:.*]] = getelementptr inbounds %struct.A, ptr %[[ARR]], i64 %[[COOKIE]] +// X86-NEXT: %[[END:.*]] = getelementptr inbounds %struct.A, ptr %[[ARR]], i32 %[[COOKIE]] +// CHECK-NEXT: %[[ISEMPTY:.*]] = icmp eq ptr %[[ARR]], %[[END]] +// CHECK-NEXT: br i1 %arraydestroy.isempty, label %arraydestroy.done1, label %arraydestroy.body +// CHECK: arraydestroy.body: +// CHECK-NEXT: %arraydestroy.elementPast = phi ptr [ %delete.end, %delete.notnull ], [ %arraydestroy.element, %arraydestroy.body ] +// X64-NEXT: %arraydestroy.element = getelementptr inbounds %struct.A, ptr %arraydestroy.elementPast, i64 -1 +// X86-NEXT: %arraydestroy.element = getelementptr inbounds %struct.A, ptr %arraydestroy.elementPast, i32 -1 +// X64-NEXT: call void @"??1A@@UEAA@XZ"(ptr noundef nonnull align 8 dereferenceable(8) %arraydestroy.element) +// X86-NEXT: call x86_thiscallcc void @"??1A@@UAE@XZ"(ptr noundef nonnull align 4 dereferenceable(4) %arraydestroy.element) +// CHECK-NEXT: %arraydestroy.done = icmp eq ptr %arraydestroy.element, %0 +// CHECK-NEXT: br i1 %arraydestroy.done, label %arraydestroy.done1, label %arraydestroy.body +// CHECK: arraydestroy.done1: +// X64-NEXT: %[[HOWMANYELEMS:.*]] = mul i64 8, %[[COOKIE]] +// X86-NEXT: %[[HOWMANYELEMS:.*]] = mul i32 4, %[[COOKIE]] +// X64-NEXT: %[[SIZEPLUSCOOKIE:.*]] = add i64 %[[HOWMANYELEMS]], 8 +// X86-NEXT: %[[SIZEPLUSCOOKIE:.*]] = add i32 %[[HOWMANYELEMS]], 4 +// X64-NEXT: call void @"??_V@YAXPEAX_K@Z"(ptr noundef %[[COOKIEADDR]], i64 noundef %[[SIZEPLUSCOOKIE]]) +// X86-NEXT: call void @"??_V@YAXPAXI@Z"(ptr noundef %[[COOKIEADDR]], i32 noundef %[[SIZEPLUSCOOKIE]]) +// CHECK-NEXT: br label %delete.end2 + +void case2(B *arg) { + delete[] arg; +} + +// X64-LABEL: define {{.*}} void @"?case2@@YAXPEAUB@@@Z" +// X64-SAME: (ptr noundef %[[ARG:.*]]) +// X86-LABEL: define {{.*}} void @"?case2@@YAXPAUB@@@Z" +// X86-SAME: (ptr noundef %[[ARG:.*]]) +// CHECK: entry: +// CHECK-NEXT: %[[ARGADDR:.*]] = alloca ptr +// CHECK-NEXT: store ptr %[[ARG]], ptr %[[ARGADDR]], +// CHECK-NEXT: %[[ARR:.*]] = load ptr, ptr %[[ARGADDR]] +// CHECK-NEXT: %[[ISNULL:.*]] = icmp eq ptr %[[ARR]], null +// CHECK-NEXT: br i1 %[[ISNULL]], label %delete.end2, label %delete.notnull +// CHECK: delete.notnull: +// X64-NEXT: %[[COOKIEADDR:.*]] = getelementptr inbounds i8, ptr %[[ARR]], i64 -8 +// X86-NEXT: %[[COOKIEADDR:.*]] = getelementptr inbounds i8, ptr %0, i32 -4 +// X64-NEXT: %[[COOKIE:.*]] = load i64, ptr %[[COOKIEADDR]] +// X86-NEXT: %[[COOKIE:.*]] = load i32, ptr %[[COOKIEADDR]] +// X64-NEXT: %[[END:.*]] = getelementptr inbounds %struct.B, ptr %[[ARR]], i64 %[[COOKIE]] +// X86-NEXT: %[[END:.*]] = getelementptr inbounds %struct.B, ptr %[[ARR]], i32 %[[COOKIE]] +// CHECK-NEXT: %[[ISEMPTY:.*]] = icmp eq ptr %[[ARR]], %[[END]] +// CHECK-NEXT: br i1 %arraydestroy.isempty, label %arraydestroy.done1, label %arraydestroy.body +// CHECK: arraydestroy.body: +// CHECK-NEXT: %arraydestroy.elementPast = phi ptr [ %delete.end, %delete.notnull ], [ %arraydestroy.element, %arraydestroy.body ] +// X64-NEXT: %arraydestroy.element = getelementptr inbounds %struct.B, ptr %arraydestroy.elementPast, i64 -1 +// X86-NEXT: %arraydestroy.element = getelementptr inbounds %struct.B, ptr %arraydestroy.elementPast, i32 -1 +// X64-NEXT: call void @"??1B@@UEAA@XZ"(ptr noundef nonnull align 8 dereferenceable(8) %arraydestroy.element) +// X86-NEXT: call x86_thiscallcc void @"??1B@@UAE@XZ"(ptr noundef nonnull align 4 dereferenceable(4) %arraydestroy.element) +// CHECK-NEXT: %arraydestroy.done = icmp eq ptr %arraydestroy.element, %0 +// CHECK-NEXT: br i1 %arraydestroy.done, label %arraydestroy.done1, label %arraydestroy.body +// CHECK: arraydestroy.done1: +// X64-NEXT: %[[HOWMANYELEMS:.*]] = mul i64 8, %[[COOKIE]] +// X86-NEXT: %[[HOWMANYELEMS:.*]] = mul i32 4, %[[COOKIE]] +// X64-NEXT: %[[SIZEPLUSCOOKIE:.*]] = add i64 %[[HOWMANYELEMS]], 8 +// X86-NEXT: %[[SIZEPLUSCOOKIE:.*]] = add i32 %[[HOWMANYELEMS]], 4 +// X64-NEXT: call void @"??_V@YAXPEAX_K@Z"(ptr noundef %[[COOKIEADDR]], i64 noundef %[[SIZEPLUSCOOKIE]]) +// X86-NEXT: call void @"??_V@YAXPAXI@Z"(ptr noundef %[[COOKIEADDR]], i32 noundef %[[SIZEPLUSCOOKIE]]) +// CHECK-NEXT: br label %delete.end2 + + +void case3(D *arg) { + delete[] arg; +} + +// CHECK-LABEL: case3 +// X64: call noundef ptr %{{.}}( +// X86: call x86_thiscallcc noundef ptr %{{.}}( + +void case4(D **arg) { + delete[] arg[0]; + delete[] arg[1]; +} + +// CHECK-LABEL: case4 +// X64: call noundef ptr %{{.}}( +// X86: call x86_thiscallcc noundef ptr %{{.}}( >From 223f27d6b4b5294883317c6c91e25638a51f0362 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Fri, 27 Feb 2026 06:16:03 -0800 Subject: [PATCH 2/3] Make format happy --- clang/lib/CodeGen/CGExprCXX.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index 199a50cbb650c..d2788c26c8efd 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -2100,8 +2100,8 @@ void CodeGenFunction::EmitCXXDeleteExpr(const CXXDeleteExpr *E) { // Emit normal loop over the array elements if we can easily // devirtualize destructor call. if (auto *DevirtualizedDtor = dyn_cast_or_null<const CXXDestructorDecl>( - Dtor->getDevirtualizedMethod( - DBase, CGM.getLangOpts().AppleKext))) { + Dtor->getDevirtualizedMethod(DBase, + CGM.getLangOpts().AppleKext))) { const CXXRecordDecl *DevirtualizedClass = DevirtualizedDtor->getParent(); if (declaresSameEntity(getCXXRecord(DBase), DevirtualizedClass)) >From 5c5120040ea289ca5f2577c5e3c8550755de818e Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Mon, 2 Mar 2026 03:57:51 -0800 Subject: [PATCH 3/3] Reduce duplication and fragility by moving code to a function --- clang/lib/CodeGen/CGExprCXX.cpp | 70 +++++++++++++++------------------ 1 file changed, 31 insertions(+), 39 deletions(-) diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index d2788c26c8efd..8afeb6c1e6373 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -1898,6 +1898,30 @@ static void EmitDestroyingObjectDelete(CodeGenFunction &CGF, ElementType); } +static bool CanDevirtualizeDtorCall(const CXXDeleteExpr *E, + CXXDestructorDecl *&Dtor, + const LangOptions &LO) { + assert(Dtor && Dtor->isVirtual() && "virtual dtor is expected"); + const Expr *DBase = E->getArgument(); + if (auto *MaybeDevirtualizedDtor = dyn_cast_or_null<CXXDestructorDecl>( + Dtor->getDevirtualizedMethod(DBase, LO.AppleKext))) { + const CXXRecordDecl *DevirtualizedClass = + MaybeDevirtualizedDtor->getParent(); + if (declaresSameEntity(getCXXRecord(DBase), DevirtualizedClass)) { + // Devirtualized to the class of the base type (the type of the + // whole expression). + Dtor = MaybeDevirtualizedDtor; + return true; + } + // Devirtualized to some other type. Would need to cast the this + // pointer to that type but we don't have support for that yet, so + // do a virtual call. FIXME: handle the case where it is + // devirtualized to the derived type (the type of the inner + // expression) as in EmitCXXMemberOrOperatorMemberCallExpr. + } + return false; +} + /// Emit the code for deleting a single object. /// \return \c true if we started emitting UnconditionalDeleteBlock, \c false /// if not. @@ -1917,38 +1941,16 @@ static bool EmitObjectDelete(CodeGenFunction &CGF, const CXXDeleteExpr *DE, // Find the destructor for the type, if applicable. If the // destructor is virtual, we'll just emit the vcall and return. - const CXXDestructorDecl *Dtor = nullptr; + CXXDestructorDecl *Dtor = nullptr; if (const auto *RD = ElementType->getAsCXXRecordDecl()) { if (RD->hasDefinition() && !RD->hasTrivialDestructor()) { Dtor = RD->getDestructor(); - if (Dtor->isVirtual()) { - bool UseVirtualCall = true; - const Expr *Base = DE->getArgument(); - if (auto *DevirtualizedDtor = dyn_cast_or_null<const CXXDestructorDecl>( - Dtor->getDevirtualizedMethod( - Base, CGF.CGM.getLangOpts().AppleKext))) { - UseVirtualCall = false; - const CXXRecordDecl *DevirtualizedClass = - DevirtualizedDtor->getParent(); - if (declaresSameEntity(getCXXRecord(Base), DevirtualizedClass)) { - // Devirtualized to the class of the base type (the type of the - // whole expression). - Dtor = DevirtualizedDtor; - } else { - // Devirtualized to some other type. Would need to cast the this - // pointer to that type but we don't have support for that yet, so - // do a virtual call. FIXME: handle the case where it is - // devirtualized to the derived type (the type of the inner - // expression) as in EmitCXXMemberOrOperatorMemberCallExpr. - UseVirtualCall = true; - } - } - if (UseVirtualCall) { - CGF.CGM.getCXXABI().emitVirtualObjectDelete(CGF, DE, Ptr, ElementType, - Dtor); - return false; - } + if (Dtor->isVirtual() && + !CanDevirtualizeDtorCall(DE, Dtor, CGF.CGM.getLangOpts())) { + CGF.CGM.getCXXABI().emitVirtualObjectDelete(CGF, DE, Ptr, ElementType, + Dtor); + return false; } } } @@ -2095,20 +2097,10 @@ void CodeGenFunction::EmitCXXDeleteExpr(const CXXDeleteExpr *E) { if (auto *RD = DeleteTy->getAsCXXRecordDecl()) { auto *Dtor = RD->getDestructor(); if (Dtor && Dtor->isVirtual()) { - bool CanDevirtualizeCall = false; - const Expr *DBase = E->getArgument(); // Emit normal loop over the array elements if we can easily // devirtualize destructor call. - if (auto *DevirtualizedDtor = dyn_cast_or_null<const CXXDestructorDecl>( - Dtor->getDevirtualizedMethod(DBase, - CGM.getLangOpts().AppleKext))) { - const CXXRecordDecl *DevirtualizedClass = - DevirtualizedDtor->getParent(); - if (declaresSameEntity(getCXXRecord(DBase), DevirtualizedClass)) - CanDevirtualizeCall = true; - } // Emit virtual call to vector deleting destructor otherwise. - if (!CanDevirtualizeCall) { + if (!CanDevirtualizeDtorCall(E, Dtor, CGM.getLangOpts())) { llvm::Value *NumElements = nullptr; llvm::Value *AllocatedPtr = nullptr; CharUnits CookieSize; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
