jacobly created this revision. Herald added a project: All. jacobly requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.
With the Microsoft ABI, some destructors need to offset a parameter to get the derived this pointer, in which case the type of that parameter should not be a pointer to the derived type. Fixes #60465 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D143233 Files: clang/lib/CodeGen/CGCXXABI.h clang/lib/CodeGen/CGCall.cpp clang/lib/CodeGen/CGExprCXX.cpp clang/lib/CodeGen/MicrosoftCXXABI.cpp clang/test/CodeGenCXX/constructor-destructor-return-this.cpp clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp clang/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp clang/test/CodeGenCXX/microsoft-abi-structors.cpp clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
Index: clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp =================================================================== --- clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp +++ clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp @@ -53,6 +53,7 @@ B::~B() { // CHECK-LABEL: define dso_local x86_thiscallcc void @"??1B@@UAE@XZ" // Store initial this: + // CHECK: %[[THIS:.*]] = alloca %struct.B* // CHECK: %[[THIS_ADDR:.*]] = alloca %struct.B* // CHECK: store %struct.B* %{{.*}}, %struct.B** %[[THIS_ADDR]], align 4 // Reload and adjust the this parameter: @@ -90,8 +91,7 @@ // CHECK2: %[[THIS:.*]] = load %struct.B*, %struct.B** {{.*}} // CHECK2: %[[THIS_i8:.*]] = bitcast %struct.B* %[[THIS]] to i8* // CHECK2: %[[B_i8:.*]] = getelementptr i8, i8* %[[THIS_i8]], i32 8 - // CHECK2: %[[B:.*]] = bitcast i8* %[[B_i8]] to %struct.B* - // CHECK2: call x86_thiscallcc void @"??1B@@UAE@XZ"(%struct.B* {{[^,]*}} %[[B]]) + // CHECK2: call x86_thiscallcc void @"??1B@@UAE@XZ"(i8*{{[^,]*}} %[[B_i8]]) // CHECK2: %[[THIS_i8:.*]] = bitcast %struct.B* %[[THIS]] to i8* // CHECK2: %[[VBASE_i8:.*]] = getelementptr inbounds i8, i8* %[[THIS_i8]], i32 8 // CHECK2: %[[VBASE:.*]] = bitcast i8* %[[VBASE_i8]] to %struct.VBase* @@ -99,6 +99,7 @@ // CHECK2: ret // CHECK2-LABEL: define linkonce_odr dso_local x86_thiscallcc noundef i8* @"??_GB@@UAEPAXI@Z" + // CHECK2: store %struct.B* %{{.*}}, %struct.B** %[[THIS:.*]], align 4 // CHECK2: store %struct.B* %{{.*}}, %struct.B** %[[THIS_ADDR:.*]], align 4 // CHECK2: %[[THIS:.*]] = load %struct.B*, %struct.B** %[[THIS_ADDR]] // CHECK2: %[[THIS_PARAM_i8:.*]] = bitcast %struct.B* %[[THIS]] to i8* @@ -196,7 +197,6 @@ // CHECK: %[[VBOFFSET32:.*]] = load i32, i32* %[[VBENTRY]] // CHECK: %[[VBOFFSET:.*]] = add nsw i32 0, %[[VBOFFSET32]] // CHECK: %[[VBASE_i8:.*]] = getelementptr inbounds i8, i8* %[[OBJ_i8]], i32 %[[VBOFFSET]] -// CHECK: %[[VBASE:.*]] = bitcast i8* %[[VBASE_i8]] to %struct.B* // // CHECK: %[[OBJ_i8:.*]] = bitcast %struct.B* %[[OBJ]] to i8* // CHECK: %[[VBPTR:.*]] = getelementptr inbounds i8, i8* %[[OBJ_i8]], i32 0 @@ -206,12 +206,12 @@ // CHECK: %[[VBOFFSET32:.*]] = load i32, i32* %[[VBENTRY]] // CHECK: %[[VBOFFSET:.*]] = add nsw i32 0, %[[VBOFFSET32]] // CHECK: %[[VBASE_i8:.*]] = getelementptr inbounds i8, i8* %[[OBJ_i8]], i32 %[[VBOFFSET]] -// CHECK: %[[VFPTR:.*]] = bitcast i8* %[[VBASE_i8]] to i8* (%struct.B*, i32)*** -// CHECK: %[[VFTABLE:.*]] = load i8* (%struct.B*, i32)**, i8* (%struct.B*, i32)*** %[[VFPTR]] -// CHECK: %[[VFUN:.*]] = getelementptr inbounds i8* (%struct.B*, i32)*, i8* (%struct.B*, i32)** %[[VFTABLE]], i64 0 -// CHECK: %[[VFUN_VALUE:.*]] = load i8* (%struct.B*, i32)*, i8* (%struct.B*, i32)** %[[VFUN]] +// CHECK: %[[VFPTR:.*]] = bitcast i8* %[[VBASE_i8]] to i8* (i8*, i32)*** +// CHECK: %[[VFTABLE:.*]] = load i8* (i8*, i32)**, i8* (i8*, i32)*** %[[VFPTR]] +// CHECK: %[[VFUN:.*]] = getelementptr inbounds i8* (i8*, i32)*, i8* (i8*, i32)** %[[VFTABLE]], i64 0 +// CHECK: %[[VFUN_VALUE:.*]] = load i8* (i8*, i32)*, i8* (i8*, i32)** %[[VFUN]] // -// CHECK: call x86_thiscallcc noundef i8* %[[VFUN_VALUE]](%struct.B* {{[^,]*}} %[[VBASE]], i32 noundef 1) +// CHECK: call x86_thiscallcc noundef i8* %[[VFUN_VALUE]](i8* {{[^,]*}} %[[VBASE]], i32 noundef 1) // CHECK: ret void } @@ -295,8 +295,9 @@ } d; D::~D() { - // CHECK-LABEL: define dso_local x86_thiscallcc void @"??1D@diamond@@UAE@XZ"(%"struct.diamond::D"*{{.*}}) + // CHECK-LABEL: define dso_local x86_thiscallcc void @"??1D@diamond@@UAE@XZ"(i8*{{.*}}) // Store initial this: + // CHECK: %[[THIS:.*]] = alloca %"struct.diamond::D"* // CHECK: %[[THIS_ADDR:.*]] = alloca %"struct.diamond::D"* // CHECK: store %"struct.diamond::D"* %{{.*}}, %"struct.diamond::D"** %[[THIS_ADDR]], align 4 // @@ -310,16 +311,13 @@ // CHECK: %[[C_i8:.*]] = getelementptr inbounds i8, i8* %[[D_i8]], i32 4 // CHECK: %[[C:.*]] = bitcast i8* %[[C_i8]] to %"struct.diamond::C"* // CHECK: %[[C_i8:.*]] = bitcast %"struct.diamond::C"* %[[C]] to i8* - // CHECK: %[[ARG_i8:.*]] = getelementptr i8, i8* %{{.*}}, i32 16 - // FIXME: We might consider changing the dtor this parameter type to i8*. - // CHECK: %[[ARG:.*]] = bitcast i8* %[[ARG_i8]] to %"struct.diamond::C"* - // CHECK: call x86_thiscallcc void @"??1C@diamond@@UAE@XZ"(%"struct.diamond::C"* {{[^,]*}} %[[ARG]]) + // CHECK: %[[ARG:.*]] = getelementptr i8, i8* %{{.*}}, i32 16 + // CHECK: call x86_thiscallcc void @"??1C@diamond@@UAE@XZ"(i8*{{[^,]*}} %[[ARG]]) // CHECK: %[[B:.*]] = bitcast %"struct.diamond::D"* %[[THIS]] to %"struct.diamond::B"* // CHECK: %[[B_i8:.*]] = bitcast %"struct.diamond::B"* %[[B]] to i8* - // CHECK: %[[ARG_i8:.*]] = getelementptr i8, i8* %[[B_i8]], i32 4 - // CHECK: %[[ARG:.*]] = bitcast i8* %[[ARG_i8]] to %"struct.diamond::B"* - // CHECK: call x86_thiscallcc void @"??1B@diamond@@UAE@XZ"(%"struct.diamond::B"* {{[^,]*}} %[[ARG]]) + // CHECK: %[[ARG:.*]] = getelementptr i8, i8* %[[B_i8]], i32 4 + // CHECK: call x86_thiscallcc void @"??1B@diamond@@UAE@XZ"(i8*{{[^,]*}} %[[ARG]]) // CHECK: ret void } @@ -443,9 +441,10 @@ }; E::~E() { - // CHECK-LABEL: define dso_local x86_thiscallcc void @"??1E@test4@@UAE@XZ"(%"struct.test4::E"* {{[^,]*}} %this) + // CHECK-LABEL: define dso_local x86_thiscallcc void @"??1E@test4@@UAE@XZ"(i8*{{[^,]*}} %this.coerce) // In this case "this" points to the most derived class, so no GEPs needed. + // CHECK: bitcast i8* %this.coerce // CHECK-NOT: getelementptr // CHECK-NOT: bitcast // CHECK: %[[VFPTR_i8:.*]] = bitcast %"struct.test4::E"* %{{.*}} to i32 (...)*** @@ -458,16 +457,14 @@ // CHECK-NOT: getelementptr // CHECK: %[[OBJ_i8:.*]] = bitcast %"struct.test4::E"* %[[OBJ]] to i8* - // CHECK: %[[B_i8:.*]] = getelementptr inbounds i8, i8* %[[OBJ_i8]], i32 4 - // FIXME: in fact, the call should take i8* and the bitcast is redundant. - // CHECK: %[[B_as_E:.*]] = bitcast i8* %[[B_i8]] to %"struct.test4::E"* + // CHECK: %[[THIS_i8:.*]] = getelementptr inbounds i8, i8* %[[OBJ_i8]], i32 4 // CHECK: %[[OBJ_i8:.*]] = bitcast %"struct.test4::E"* %[[OBJ:.*]] to i8* // CHECK: %[[B_i8:.*]] = getelementptr inbounds i8, i8* %[[OBJ_i8]], i32 4 - // CHECK: %[[VPTR:.*]] = bitcast i8* %[[B_i8]] to i8* (%"struct.test4::E"*, i32)*** - // CHECK: %[[VFTABLE:.*]] = load i8* (%"struct.test4::E"*, i32)**, i8* (%"struct.test4::E"*, i32)*** %[[VPTR]] - // CHECK: %[[VFTENTRY:.*]] = getelementptr inbounds i8* (%"struct.test4::E"*, i32)*, i8* (%"struct.test4::E"*, i32)** %[[VFTABLE]], i64 0 - // CHECK: %[[VFUN:.*]] = load i8* (%"struct.test4::E"*, i32)*, i8* (%"struct.test4::E"*, i32)** %[[VFTENTRY]] - // CHECK: call x86_thiscallcc noundef i8* %[[VFUN]](%"struct.test4::E"* {{[^,]*}} %[[B_as_E]], i32 noundef 1) + // CHECK: %[[VPTR:.*]] = bitcast i8* %[[B_i8]] to i8* (i8*, i32)*** + // CHECK: %[[VFTABLE:.*]] = load i8* (i8*, i32)**, i8* (i8*, i32)*** %[[VPTR]] + // CHECK: %[[VFTENTRY:.*]] = getelementptr inbounds i8* (i8*, i32)*, i8* (i8*, i32)** %[[VFTABLE]], i64 0 + // CHECK: %[[VFUN:.*]] = load i8* (i8*, i32)*, i8* (i8*, i32)** %[[VFTENTRY]] + // CHECK: call x86_thiscallcc noundef i8* %[[VFUN]](i8*{{[^,]*}} %[[THIS_i8]], i32 noundef 1) delete obj; } Index: clang/test/CodeGenCXX/microsoft-abi-structors.cpp =================================================================== --- clang/test/CodeGenCXX/microsoft-abi-structors.cpp +++ clang/test/CodeGenCXX/microsoft-abi-structors.cpp @@ -155,8 +155,7 @@ }; C::~C() { -// CHECK-LABEL: define dso_local x86_thiscallcc void @"??1C@dtor_in_second_nvbase@@UAE@XZ" -// CHECK: (%"struct.dtor_in_second_nvbase::C"* {{[^,]*}} %this) +// CHECK-LABEL: define dso_local x86_thiscallcc void @"??1C@dtor_in_second_nvbase@@UAE@XZ"(i8*{{[^,]*}} %this.coerce) // No this adjustment! // CHECK-NOT: getelementptr // CHECK: load %"struct.dtor_in_second_nvbase::C"*, %"struct.dtor_in_second_nvbase::C"** %{{.*}} @@ -164,19 +163,16 @@ // CHECK: bitcast %"struct.dtor_in_second_nvbase::C"* %{{.*}} to i8* // CHECK: getelementptr inbounds i8, i8* %{{.*}}, i32 4 // CHECK: bitcast i8* %{{.*}} to %"struct.dtor_in_second_nvbase::B"* -// CHECK: call x86_thiscallcc void @"??1B@dtor_in_second_nvbase@@UAE@XZ" -// CHECK: (%"struct.dtor_in_second_nvbase::B"* {{[^,]*}} %{{.*}}) +// CHECK: call x86_thiscallcc void @"??1B@dtor_in_second_nvbase@@UAE@XZ"(%"struct.dtor_in_second_nvbase::B"*{{[^,]*}} %{{.*}}) // CHECK: ret void } void foo() { C c; } -// DTORS2-LABEL: define linkonce_odr dso_local x86_thiscallcc i8* @"??_EC@dtor_in_second_nvbase@@W3AEPAXI@Z" -// DTORS2: (%"struct.dtor_in_second_nvbase::C"* %this, i32 %should_call_delete) +// DTORS2-LABEL: define linkonce_odr dso_local x86_thiscallcc i8* @"??_EC@dtor_in_second_nvbase@@W3AEPAXI@Z"(i8* %this.coerce, i32 %should_call_delete) // Do an adjustment from B* to C*. // DTORS2: getelementptr i8, i8* %{{.*}}, i32 -4 -// DTORS2: bitcast i8* %{{.*}} to %"struct.dtor_in_second_nvbase::C"* // DTORS2: %[[CALL:.*]] = tail call x86_thiscallcc i8* @"??_GC@dtor_in_second_nvbase@@UAEPAXI@Z" // DTORS2: ret i8* %[[CALL]] } @@ -195,7 +191,7 @@ struct F : D, E { ~F(); int f; }; F::~F() { -// CHECK-LABEL: define dso_local x86_thiscallcc void @"??1F@test2@@UAE@XZ"(%"struct.test2::F"*{{[^,]*}}) +// CHECK-LABEL: define dso_local x86_thiscallcc void @"??1F@test2@@UAE@XZ"(i8*{{[^,]*}}) // Do an adjustment from C vbase subobject to F as though F was the // complete type. // CHECK: getelementptr inbounds i8, i8* %{{.*}}, i32 -20 @@ -209,7 +205,6 @@ // DTORS3-LABEL: define linkonce_odr dso_local x86_thiscallcc void @"??_DF@test2@@QAEXXZ"({{.*}} {{.*}} comdat // Do an adjustment from C* to F*. // DTORS3: getelementptr i8, i8* %{{.*}}, i32 20 -// DTORS3: bitcast i8* %{{.*}} to %"struct.test2::F"* // DTORS3: call x86_thiscallcc void @"??1F@test2@@UAE@XZ" // DTORS3: ret void Index: clang/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp =================================================================== --- clang/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp +++ clang/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp @@ -216,7 +216,6 @@ // WIN32-NOT: load // WIN32: getelementptr i8, i8* %{{.*}}, i32 4 // WIN32-NOT: load -// WIN32: bitcast i8* %{{.*}} to %"struct.crash_on_partial_destroy::B"* // WIN32: call x86_thiscallcc void @"??1B@crash_on_partial_destroy@@UAE@XZ" // // WIN32-NOT: load Index: clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp =================================================================== --- clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp +++ clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp @@ -90,14 +90,13 @@ // For MS, we don't add a new vtable slot to the primary vtable for the virtual // destructor. Instead we cast to the VDel base class. // CHECK-MSABI: bitcast {{.*}} %[[d]] -// CHECK-MSABI64-NEXT: getelementptr {{.*}}, i64 8 -// CHECK-MSABI32-NEXT: getelementptr {{.*}}, i32 4 -// CHECK-MSABI-NEXT: %[[d:.*]] = bitcast i8* +// CHECK-MSABI64-NEXT: %[[d:.*]] = getelementptr {{.*}}, i64 8 +// CHECK-MSABI32-NEXT: %[[d:.*]] = getelementptr {{.*}}, i32 4 // // CHECK: %[[VTABLE:.*]] = load // CHECK: %[[DTOR:.*]] = load // -// CHECK: call {{void|noundef i8\*|x86_thiscallcc noundef i8\*}} %[[DTOR]](%{{.*}}* {{[^,]*}} %[[d]] +// CHECK: call {{void|noundef i8\*|x86_thiscallcc noundef i8\*}} %[[DTOR]]({{.*}}* {{[^,]*}} %[[d]] // CHECK-MSABI-SAME: , i32 noundef 1) // CHECK-NOT: call // CHECK: } Index: clang/test/CodeGenCXX/constructor-destructor-return-this.cpp =================================================================== --- clang/test/CodeGenCXX/constructor-destructor-return-this.cpp +++ clang/test/CodeGenCXX/constructor-destructor-return-this.cpp @@ -153,7 +153,7 @@ // CHECKFUCHSIA-LABEL: define{{.*}} %class.D* @_ZN1DD1Ev(%class.D* {{[^,]*}} returned{{[^,]*}} %this) // CHECKMS-LABEL: define dso_local x86_thiscallcc noundef %class.D* @"??0D@@QAE@XZ"(%class.D* {{[^,]*}} returned{{[^,]*}} %this, i32 noundef %is_most_derived) -// CHECKMS-LABEL: define dso_local x86_thiscallcc void @"??1D@@UAE@XZ"(%class.D* {{[^,]*}} %this) +// CHECKMS-LABEL: define dso_local x86_thiscallcc void @"??1D@@UAE@XZ"(i8* {{[^,]*}} %this.coerce) class E { public: Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp =================================================================== --- clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -235,11 +235,24 @@ void EmitCXXDestructors(const CXXDestructorDecl *D) override; - const CXXRecordDecl * - getThisArgumentTypeForMethod(const CXXMethodDecl *MD) override { - if (MD->isVirtual() && !isa<CXXDestructorDecl>(MD)) { + const CXXRecordDecl *getThisArgumentTypeForMethod(GlobalDecl GD) override { + const CXXMethodDecl *MD = cast<CXXMethodDecl>(GD.getDecl()); + + if (MD->isVirtual()) { + GlobalDecl LookupGD = GD; + if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(MD)) { + // Complete dtors take a pointer to the complete object, + // thus don't need adjustment. + if (GD.getDtorType() == Dtor_Complete) + return MD->getParent(); + + // There's only Dtor_Deleting in vftable but it shares the this + // adjustment with the base one, so look up the deleting one instead. + LookupGD = GlobalDecl(DD, Dtor_Deleting); + } MethodVFTableLocation ML = - CGM.getMicrosoftVTableContext().getMethodVFTableLocation(MD); + CGM.getMicrosoftVTableContext().getMethodVFTableLocation(LookupGD); + // The vbases might be ordered differently in the final overrider object // and the complete object, so the "this" argument may sometimes point to // memory that has no particular type (e.g. past the complete object). Index: clang/lib/CodeGen/CGExprCXX.cpp =================================================================== --- clang/lib/CodeGen/CGExprCXX.cpp +++ clang/lib/CodeGen/CGExprCXX.cpp @@ -33,10 +33,12 @@ } static MemberCallInfo -commonEmitCXXMemberOrOperatorCall(CodeGenFunction &CGF, const CXXMethodDecl *MD, +commonEmitCXXMemberOrOperatorCall(CodeGenFunction &CGF, GlobalDecl GD, llvm::Value *This, llvm::Value *ImplicitParam, QualType ImplicitParamTy, const CallExpr *CE, CallArgList &Args, CallArgList *RtlArgs) { + auto *MD = cast<CXXMethodDecl>(GD.getDecl()); + assert(CE == nullptr || isa<CXXMemberCallExpr>(CE) || isa<CXXOperatorCallExpr>(CE)); assert(MD->isInstance() && @@ -44,7 +46,7 @@ // Push the this ptr. const CXXRecordDecl *RD = - CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD); + CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(GD); Args.add(RValue::get(This), CGF.getTypes().DeriveThisType(RD, MD)); // If there is an implicit parameter (e.g. VTT), emit it. @@ -110,7 +112,7 @@ } CallArgList Args; - commonEmitCXXMemberOrOperatorCall(*this, DtorDecl, This, ImplicitParam, + commonEmitCXXMemberOrOperatorCall(*this, Dtor, This, ImplicitParam, ImplicitParamTy, CE, Args, nullptr); return EmitCall(CGM.getTypes().arrangeCXXStructorDeclaration(Dtor), Callee, ReturnValueSlot(), Args, nullptr, CE && CE == MustTailCall, @@ -285,7 +287,8 @@ assert(ReturnValue.isNull() && "Constructor shouldn't have return value"); CallArgList Args; commonEmitCXXMemberOrOperatorCall( - *this, Ctor, This.getPointer(*this), /*ImplicitParam=*/nullptr, + *this, {Ctor, Ctor_Complete}, This.getPointer(*this), + /*ImplicitParam=*/nullptr, /*ImplicitParamTy=*/QualType(), CE, Args, nullptr); EmitCXXConstructorCall(Ctor, Ctor_Complete, /*ForVirtualBase=*/false, Index: clang/lib/CodeGen/CGCall.cpp =================================================================== --- clang/lib/CodeGen/CGCall.cpp +++ clang/lib/CodeGen/CGCall.cpp @@ -321,7 +321,9 @@ SmallVector<CanQualType, 16> argTypes; SmallVector<FunctionProtoType::ExtParameterInfo, 16> paramInfos; - argTypes.push_back(DeriveThisType(MD->getParent(), MD)); + + const CXXRecordDecl *ThisType = TheCXXABI.getThisArgumentTypeForMethod(GD); + argTypes.push_back(DeriveThisType(ThisType, MD)); bool PassParams = true; Index: clang/lib/CodeGen/CGCXXABI.h =================================================================== --- clang/lib/CodeGen/CGCXXABI.h +++ clang/lib/CodeGen/CGCXXABI.h @@ -379,9 +379,8 @@ /// zero if no specific type is applicable, e.g. if the ABI expects the "this" /// parameter to point to some artificial offset in a complete object due to /// vbases being reordered. - virtual const CXXRecordDecl * - getThisArgumentTypeForMethod(const CXXMethodDecl *MD) { - return MD->getParent(); + virtual const CXXRecordDecl *getThisArgumentTypeForMethod(GlobalDecl GD) { + return cast<CXXMethodDecl>(GD.getDecl())->getParent(); } /// Perform ABI-specific "this" argument adjustment required prior to
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits