Author: Mariya Podchishchaeva Date: 2026-03-05T10:19:39+01:00 New Revision: 8c206a26e408bcbf98b1cc6b0770b17c1e8d0320
URL: https://github.com/llvm/llvm-project/commit/8c206a26e408bcbf98b1cc6b0770b17c1e8d0320 DIFF: https://github.com/llvm/llvm-project/commit/8c206a26e408bcbf98b1cc6b0770b17c1e8d0320.diff LOG: [win][clang] Fix devirtualization of vector deleting destructor call (#183741) 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. No release note since this is a relatively recent regression. This aims to fix https://github.com/llvm/llvm-project/issues/183621 Added: clang/test/CodeGenCXX/ms-vdtors-devirtualization.cpp Modified: clang/lib/CodeGen/CGExprCXX.cpp Removed: ################################################################################ diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index c8e1fe69da9c7..6caef19d3be0f 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -1898,6 +1898,29 @@ static void EmitDestroyingObjectDelete(CodeGenFunction &CGF, ElementType); } +static CXXDestructorDecl *TryDevirtualizeDtorCall(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). + return MaybeDevirtualizedDtor; + } + // 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 nullptr; +} + /// Emit the code for deleting a single object. /// \return \c true if we started emitting UnconditionalDeleteBlock, \c false /// if not. @@ -1917,34 +1940,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) { + if (auto *DevirtualizedDtor = + TryDevirtualizeDtorCall(DE, Dtor, CGF.CGM.getLangOpts())) { + Dtor = DevirtualizedDtor; + } else { CGF.CGM.getCXXABI().emitVirtualObjectDelete(CGF, DE, Ptr, ElementType, Dtor); return false; @@ -2095,32 +2100,38 @@ 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)) + // Emit normal loop over the array elements if we can easily + // devirtualize destructor call. + // Emit virtual call to vector deleting destructor otherwise. + if (!TryDevirtualizeDtorCall(E, Dtor, CGM.getLangOpts())) { + 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); + CGM.getCXXABI().emitVirtualObjectDelete(*this, E, Ptr, DeleteTy, + Dtor); EmitBlock(DeleteEnd); - return; + 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 %{{.}}( _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
