From: James Y Knight Date: Wed, 6 Feb 2019 00:06:03 +0000 (+0000) Subject: Fix MSVC constructor call extension after b92d290e48e9 (r353181). X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=12c4033099e6c0cc99222965f7cfbdea9fb036c4;p=clang Fix MSVC constructor call extension after b92d290e48e9 (r353181). The assert added to EmitCall there was triggering in Windows Chromium builds, due to a mismatch of the return type. The MSVC constructor call extension (`this->Foo::Foo()`) was emitting the constructor call from 'EmitCXXMemberOrOperatorMemberCallExpr' via calling 'EmitCXXMemberOrOperatorCall', instead of 'EmitCXXConstructorCall'. On targets where HasThisReturn is true, that was failing to set the proper return type in the call info. Switching to calling EmitCXXConstructorCall also allowed removing some code e.g. the trivial copy/move support, which is already handled in EmitCXXConstructorCall. Ref: https://bugs.chromium.org/p/chromium/issues/detail?id=928861 Differential Revision: https://reviews.llvm.org/D57794 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@353246 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp index 13612d377c..e5cc22a119 100644 --- a/lib/CodeGen/CGExprCXX.cpp +++ b/lib/CodeGen/CGExprCXX.cpp @@ -249,13 +249,25 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr( This = EmitLValue(Base); } + if (const CXXConstructorDecl *Ctor = dyn_cast(MD)) { + // This is the MSVC p->Ctor::Ctor(...) extension. We assume that's + // constructing a new complete object of type Ctor. + assert(!RtlArgs); + assert(ReturnValue.isNull() && "Constructor shouldn't have return value"); + CallArgList Args; + commonEmitCXXMemberOrOperatorCall( + *this, Ctor, This.getPointer(), /*ImplicitParam=*/nullptr, + /*ImplicitParamTy=*/QualType(), CE, Args, nullptr); + + EmitCXXConstructorCall(Ctor, Ctor_Complete, /*ForVirtualBase=*/false, + /*Delegating=*/false, This.getAddress(), Args, + AggValueSlot::DoesNotOverlap, CE->getExprLoc(), + /*NewPointerIsChecked=*/false); + return RValue::get(nullptr); + } if (MD->isTrivial() || (MD->isDefaulted() && MD->getParent()->isUnion())) { if (isa(MD)) return RValue::get(nullptr); - if (isa(MD) && - cast(MD)->isDefaultConstructor()) - return RValue::get(nullptr); - if (!MD->getParent()->mayInsertExtraPadding()) { if (MD->isCopyAssignmentOperator() || MD->isMoveAssignmentOperator()) { // We don't like to generate the trivial copy/move assignment operator @@ -268,20 +280,6 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr( EmitAggregateAssign(This, RHS, CE->getType()); return RValue::get(This.getPointer()); } - - if (isa(MD) && - cast(MD)->isCopyOrMoveConstructor()) { - // Trivial move and copy ctor are the same. - assert(CE->getNumArgs() == 1 && "unexpected argcount for trivial ctor"); - const Expr *Arg = *CE->arg_begin(); - LValue RHS = EmitLValue(Arg); - LValue Dest = MakeAddrLValue(This.getAddress(), Arg->getType()); - // This is the MSVC p->Ctor::Ctor(...) extension. We assume that's - // constructing a new complete object of type Ctor. - EmitAggregateCopy(Dest, RHS, Arg->getType(), - AggValueSlot::DoesNotOverlap); - return RValue::get(This.getPointer()); - } llvm_unreachable("unknown trivial member function"); } } @@ -293,9 +291,6 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr( if (const auto *Dtor = dyn_cast(CalleeDecl)) FInfo = &CGM.getTypes().arrangeCXXStructorDeclaration( Dtor, StructorType::Complete); - else if (const auto *Ctor = dyn_cast(CalleeDecl)) - FInfo = &CGM.getTypes().arrangeCXXStructorDeclaration( - Ctor, StructorType::Complete); else FInfo = &CGM.getTypes().arrangeCXXMethodDeclaration(CalleeDecl); @@ -318,11 +313,9 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr( if (IsImplicitObjectCXXThis || isa(IOA)) SkippedChecks.set(SanitizerKind::Null, true); } - EmitTypeCheck( - isa(CalleeDecl) ? CodeGenFunction::TCK_ConstructorCall - : CodeGenFunction::TCK_MemberCall, - CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent()), - /*Alignment=*/CharUnits::Zero(), SkippedChecks); + EmitTypeCheck(CodeGenFunction::TCK_MemberCall, CallLoc, This.getPointer(), + C.getRecordType(CalleeDecl->getParent()), + /*Alignment=*/CharUnits::Zero(), SkippedChecks); // C++ [class.virtual]p12: // Explicit qualification with the scope operator (5.1) suppresses the @@ -366,11 +359,7 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr( // 'CalleeDecl' instead. CGCallee Callee; - if (const CXXConstructorDecl *Ctor = dyn_cast(MD)) { - Callee = CGCallee::forDirect( - CGM.GetAddrOfFunction(GlobalDecl(Ctor, Ctor_Complete), Ty), - GlobalDecl(Ctor, Ctor_Complete)); - } else if (UseVirtualCall) { + if (UseVirtualCall) { Callee = CGCallee::forVirtual(CE, MD, This.getAddress(), Ty); } else { if (SanOpts.has(SanitizerKind::CFINVCall) && diff --git a/test/CodeGenCXX/constructor-direct-call.cpp b/test/CodeGenCXX/constructor-direct-call.cpp index bcddc0fa7a..0ed9cd9027 100644 --- a/test/CodeGenCXX/constructor-direct-call.cpp +++ b/test/CodeGenCXX/constructor-direct-call.cpp @@ -1,4 +1,5 @@ -// RUN: %clang_cc1 -triple i686-pc-mingw32 -fms-extensions -Wmicrosoft %s -emit-llvm -o - | FileCheck %s +// RUN: %clang_cc1 -triple i686-pc-mingw32 -fms-extensions -Wmicrosoft %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK32 --check-prefix=CHECK +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc19.11.0 -fms-extensions -Wmicrosoft %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK64 --check-prefix=CHECK class Test1 { public: @@ -9,7 +10,8 @@ void f1() { Test1 var; var.Test1::Test1(); - // CHECK: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 4, i1 false) + // CHECK32: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 4, i1 false) + // CHECK64: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i64 4, i1 false) var.Test1::Test1(var); } @@ -22,13 +24,16 @@ public: void f2() { // CHECK: %var = alloca %class.Test2, align 4 - // CHECK-NEXT: call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var) + // CHECK32-NEXT: call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var) + // CHECK64-NEXT: %call = call %class.Test2* @"??0Test2@@QEAA@XZ"(%class.Test2* %var) Test2 var; - // CHECK-NEXT: call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var) + // CHECK32-NEXT: call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var) + // CHECK64-NEXT: %call1 = call %class.Test2* @"??0Test2@@QEAA@XZ"(%class.Test2* %var) var.Test2::Test2(); - // CHECK: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 8, i1 false) + // CHECK32: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 8, i1 false) + // CHECK64: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i64 8, i1 false) var.Test2::Test2(var); } @@ -45,15 +50,19 @@ public: }; void f3() { - // CHECK: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var) + // CHECK32: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var) + // CHECK64: %call = call %class.Test3* @"??0Test3@@QEAA@XZ"(%class.Test3* %var) Test3 var; - // CHECK-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var2) + // CHECK32-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var2) + // CHECK64-NEXT: %call1 = call %class.Test3* @"??0Test3@@QEAA@XZ"(%class.Test3* %var2) Test3 var2; - // CHECK-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var) + // CHECK32-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var) + // CHECK64-NEXT: %call2 = call %class.Test3* @"??0Test3@@QEAA@XZ"(%class.Test3* %var) var.Test3::Test3(); - // CHECK-NEXT: call x86_thiscallcc void @_ZN5Test3C1ERKS_(%class.Test3* %var, %class.Test3* dereferenceable({{[0-9]+}}) %var2) + // CHECK32-NEXT: call x86_thiscallcc void @_ZN5Test3C1ERKS_(%class.Test3* %var, %class.Test3* dereferenceable({{[0-9]+}}) %var2) + // CHECK64-NEXT: %call3 = call %class.Test3* @"??0Test3@@QEAA@AEBV0@@Z"(%class.Test3* %var, %class.Test3* dereferenceable({{[0-9]+}}) %var2) var.Test3::Test3(var2); }