r321239 - Fix for PR32990

Hal Finkel via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 20 18:12:38 PST 2017


On 12/20/2017 08:07 PM, Erich Keane via cfe-commits wrote:
> Author: erichkeane
> Date: Wed Dec 20 18:07:46 2017
> New Revision: 321239
>
> URL: http://llvm.org/viewvc/llvm-project?rev=321239&view=rev
> Log:
> Fix for PR32990
>
> This fixes the bug in https://bugs.llvm.org/show_bug.cgi?id=32990.

Too late now, but "Fix for PR32990" is not a useful subject, and only 
the bug reference is not a useful commit message. Commits should 
independently describe the problem and the solution. "Fixes PR32990." 
should be only a part of the message.

Thanks again,
Hal

>
> Patch By: zahiraam
> Differential Revision: https://reviews.llvm.org/D39063
>
> Added:
>      cfe/trunk/test/CodeGenCXX/dllimport-virtual-base.cpp
>      cfe/trunk/test/CodeGenCXX/external-linkage.cpp
> Modified:
>      cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>      cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp
>      cfe/trunk/test/CodeGenCXX/dllimport-members.cpp
>      cfe/trunk/test/CodeGenCXX/dllimport.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=321239&r1=321238&r2=321239&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Wed Dec 20 18:07:46 2017
> @@ -855,14 +855,25 @@ CodeGenModule::getFunctionLinkage(Global
>     GVALinkage Linkage = getContext().GetGVALinkageForFunction(D);
>   
>     if (isa<CXXDestructorDecl>(D) &&
> -      getCXXABI().useThunkForDtorVariant(cast<CXXDestructorDecl>(D),
> -                                         GD.getDtorType())) {
> -    // Destructor variants in the Microsoft C++ ABI are always internal or
> -    // linkonce_odr thunks emitted on an as-needed basis.
> -    return Linkage == GVA_Internal ? llvm::GlobalValue::InternalLinkage
> -                                   : llvm::GlobalValue::LinkOnceODRLinkage;
> +      Context.getTargetInfo().getCXXABI().isMicrosoft()) {
> +    switch (GD.getDtorType()) {
> +    case CXXDtorType::Dtor_Base:
> +      break;
> +    case CXXDtorType::Dtor_Comdat:
> +    case CXXDtorType::Dtor_Complete:
> +      if (D->hasAttr<DLLImportAttr>() &&
> +	  (cast<CXXDestructorDecl>(D)->getParent()->getNumVBases() ||
> +	   (Linkage == GVA_AvailableExternally ||
> +	    Linkage == GVA_StrongExternal)))
> +	return llvm::Function::AvailableExternallyLinkage;
> +      else
> +        return Linkage == GVA_Internal ? llvm::GlobalValue::InternalLinkage
> +                                       : llvm::GlobalValue::LinkOnceODRLinkage;
> +    case CXXDtorType::Dtor_Deleting:
> +      return Linkage == GVA_Internal ? llvm::GlobalValue::InternalLinkage
> +                                     : llvm::GlobalValue::LinkOnceODRLinkage;
> +    }
>     }
> -
>     if (isa<CXXConstructorDecl>(D) &&
>         cast<CXXConstructorDecl>(D)->isInheritingConstructor() &&
>         Context.getTargetInfo().getCXXABI().isMicrosoft()) {
> @@ -878,12 +889,25 @@ CodeGenModule::getFunctionLinkage(Global
>   void CodeGenModule::setFunctionDLLStorageClass(GlobalDecl GD, llvm::Function *F) {
>     const auto *FD = cast<FunctionDecl>(GD.getDecl());
>   
> -  if (const auto *Dtor = dyn_cast_or_null<CXXDestructorDecl>(FD)) {
> -    if (getCXXABI().useThunkForDtorVariant(Dtor, GD.getDtorType())) {
> +  if (dyn_cast_or_null<CXXDestructorDecl>(FD)) {
> +    switch (GD.getDtorType()) {
> +    case CXXDtorType::Dtor_Comdat:
> +    case CXXDtorType::Dtor_Deleting: {
>         // Don't dllexport/import destructor thunks.
>         F->setDLLStorageClass(llvm::GlobalValue::DefaultStorageClass);
>         return;
>       }
> +    case CXXDtorType::Dtor_Complete:
> +      if (FD->hasAttr<DLLImportAttr>())
> +        F->setDLLStorageClass(llvm::GlobalVariable::DLLImportStorageClass);
> +      else if (FD->hasAttr<DLLExportAttr>())
> +        F->setDLLStorageClass(llvm::GlobalVariable::DLLExportStorageClass);
> +      else
> +        F->setDLLStorageClass(llvm::GlobalVariable::DefaultStorageClass);
> +      return;
> +    case CXXDtorType::Dtor_Base:
> +      break;
> +    }
>     }
>   
>     if (FD->hasAttr<DLLImportAttr>())
>
> Modified: cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp?rev=321239&r1=321238&r2=321239&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp Wed Dec 20 18:07:46 2017
> @@ -1,4 +1,5 @@
>   // RUN: %clang_cc1 -mconstructor-aliases %s -triple x86_64-windows-msvc -fms-extensions -emit-llvm -o - | FileCheck %s
> +// RUN: %clang_cc1 -mconstructor-aliases %s -triple x86_64-windows-msvc -fms-extensions -emit-llvm -O1 -disable-llvm-passes -o - | FileCheck --check-prefix=MO1 %s
>   
>   // FIXME: We should really consider removing -mconstructor-aliases for MS C++
>   // ABI. The risk of bugs introducing ABI incompatibility under
> @@ -23,9 +24,7 @@ struct __declspec(dllimport) ImportOverr
>     virtual ~ImportOverrideVDtor() {}
>   };
>   
> -// Virtually inherits from a non-dllimport base class. This time we need to call
> -// the complete destructor and emit it inline. It's not exported from the DLL,
> -// and it must be emitted.
> +// Virtually inherits from a non-dllimport base class. Emit the vbase destructor.
>   struct __declspec(dllimport) ImportVBaseOverrideVDtor
>       : public virtual BaseClass {
>     virtual ~ImportVBaseOverrideVDtor() {}
> @@ -41,9 +40,11 @@ extern "C" void testit() {
>   // needs the complete destructor (_D).
>   // CHECK-LABEL: define void @testit()
>   // CHECK:  call void @"\01??_DImportVBaseOverrideVDtor@@QEAAXXZ"(%struct.ImportVBaseOverrideVDtor* %{{.*}})
> -// CHECK:  call void @"\01??1ImportOverrideVDtor@@UEAA at XZ"(%struct.ImportOverrideVDtor* %{{.*}})
> -// CHECK:  call void @"\01??1ImportIntroVDtor@@UEAA at XZ"(%struct.ImportIntroVDtor* %{{.*}})
> +// CHECK:  call void @"\01??_DImportOverrideVDtor@@QEAAXXZ"(%struct.ImportOverrideVDtor* %{{.*}})
> +// CHECK:  call void @"\01??_DImportIntroVDtor@@QEAAXXZ"(%struct.ImportIntroVDtor* %{{.*}})
>   
> -// CHECK-LABEL: define linkonce_odr void @"\01??_DImportVBaseOverrideVDtor@@QEAAXXZ"
> -// CHECK-LABEL: declare dllimport void @"\01??1ImportOverrideVDtor@@UEAA at XZ"
> -// CHECK-LABEL: declare dllimport void @"\01??1ImportIntroVDtor@@UEAA at XZ"
> +// CHECK-LABEL: declare dllimport void @"\01??_DImportVBaseOverrideVDtor@@QEAAXXZ"(%struct.ImportVBaseOverrideVDtor*)
> +// CHECK-LABEL: declare dllimport void @"\01??_DImportOverrideVDtor@@QEAAXXZ"(%struct.ImportOverrideVDtor*)
> +// CHECK-LABEL: declare dllimport void @"\01??_DImportIntroVDtor@@QEAAXXZ"(%struct.ImportIntroVDtor*)
> +
> +// MO1-DAG: define available_externally dllimport void @"\01??_DImportIntroVDtor@@QEAAXXZ"
>
> Modified: cfe/trunk/test/CodeGenCXX/dllimport-members.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllimport-members.cpp?rev=321239&r1=321238&r2=321239&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/dllimport-members.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/dllimport-members.cpp Wed Dec 20 18:07:46 2017
> @@ -414,8 +414,7 @@ struct ImportSpecials {
>     // G64-DAG: declare dllimport                void                    @_ZN14ImportSpecialsC1Ev(%struct.ImportSpecials*)
>     __declspec(dllimport) ImportSpecials();
>   
> -  // M32-DAG: declare dllimport x86_thiscallcc void @"\01??1ImportSpecials@@QAE at XZ"(%struct.ImportSpecials*)
> -  // M64-DAG: declare dllimport                void @"\01??1ImportSpecials@@QEAA at XZ"(%struct.ImportSpecials*)
> +  // M32-DAG: declare dllimport x86_thiscallcc void @"\01??_DImportSpecials@@QAEXXZ"(%struct.ImportSpecials*)
>     // G32-DAG: declare dllimport x86_thiscallcc void                    @_ZN14ImportSpecialsD1Ev(%struct.ImportSpecials*)
>     // G64-DAG: declare dllimport                void                    @_ZN14ImportSpecialsD1Ev(%struct.ImportSpecials*)
>     __declspec(dllimport) ~ImportSpecials();
> @@ -457,8 +456,7 @@ struct ImportInlineSpecials {
>     // GO1-DAG: define linkonce_odr x86_thiscallcc void @_ZN20ImportInlineSpecialsC1Ev(
>     __declspec(dllimport) ImportInlineSpecials() {}
>   
> -  // M32-DAG: declare dllimport   x86_thiscallcc void @"\01??1ImportInlineSpecials@@QAE at XZ"(%struct.ImportInlineSpecials*)
> -  // M64-DAG: declare dllimport                  void @"\01??1ImportInlineSpecials@@QEAA at XZ"(%struct.ImportInlineSpecials*)
> +  // M32-DAG: declare dllimport x86_thiscallcc void @"\01??_DImportInlineSpecials@@QAEXXZ"(%struct.ImportInlineSpecials*)
>     // G32-DAG: define linkonce_odr x86_thiscallcc void @_ZN20ImportInlineSpecialsD1Ev(%struct.ImportInlineSpecials* %this)
>     // G64-DAG: define linkonce_odr                void @_ZN20ImportInlineSpecialsD1Ev(%struct.ImportInlineSpecials* %this)
>     // MO1-DAG: define available_externally dllimport x86_thiscallcc void @"\01??1ImportInlineSpecials@@QAE at XZ"(
> @@ -512,8 +510,7 @@ struct ImportDefaulted {
>     // GO1-DAG: define linkonce_odr x86_thiscallcc void @_ZN15ImportDefaultedC1Ev(%struct.ImportDefaulted* %this)
>     __declspec(dllimport) ImportDefaulted() = default;
>   
> -  // M32-DAG: declare dllimport   x86_thiscallcc void @"\01??1ImportDefaulted@@QAE at XZ"(%struct.ImportDefaulted*)
> -  // M64-DAG: declare dllimport                  void @"\01??1ImportDefaulted@@QEAA at XZ"(%struct.ImportDefaulted*)
> +  // M32-DAG: declare dllimport x86_thiscallcc void @"\01??_DImportDefaulted@@QAEXXZ"(%struct.ImportDefaulted*)
>     // G32-DAG: define linkonce_odr x86_thiscallcc void @_ZN15ImportDefaultedD1Ev(%struct.ImportDefaulted* %this)
>     // G64-DAG: define linkonce_odr                void @_ZN15ImportDefaultedD1Ev(%struct.ImportDefaulted* %this)
>     // MO1-DAG: define available_externally dllimport x86_thiscallcc void @"\01??1ImportDefaulted@@QAE at XZ"(%struct.ImportDefaulted* %this)
> @@ -578,8 +575,7 @@ __declspec(dllimport) ImportDefaultedDef
>   
>   #ifdef MSABI
>   // For MinGW, the function will not be dllimport, and we cannot add the attribute now.
> -// M32-DAG: declare dllimport x86_thiscallcc void @"\01??1ImportDefaultedDefs@@QAE at XZ"(%struct.ImportDefaultedDefs*)
> -// M64-DAG: declare dllimport                void @"\01??1ImportDefaultedDefs@@QEAA at XZ"(%struct.ImportDefaultedDefs*)
> +// M32-DAG: declare dllimport x86_thiscallcc void @"\01??_DImportDefaulted@@QAEXXZ"(%struct.ImportDefaulted*)
>   __declspec(dllimport) ImportDefaultedDefs::~ImportDefaultedDefs() = default;
>   #endif
>   
>
> Added: cfe/trunk/test/CodeGenCXX/dllimport-virtual-base.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllimport-virtual-base.cpp?rev=321239&view=auto
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/dllimport-virtual-base.cpp (added)
> +++ cfe/trunk/test/CodeGenCXX/dllimport-virtual-base.cpp Wed Dec 20 18:07:46 2017
> @@ -0,0 +1,73 @@
> +// RUN: %clang_cc1 -triple i386-pc-windows -emit-llvm -fms-compatibility %s -x c++ -o - | FileCheck %s
> +
> +namespace test1 {
> +struct BaseClass {
> +  ~BaseClass();
> +};
> +
> +struct __declspec(dllimport) Concrete : virtual BaseClass {
> +};
> +
> +Concrete c;
> +
> +// CHECK-LABEL: declare dllimport x86_thiscallcc %"struct.test1::Concrete"* @"\01??0Concrete at test1@@QAE at XZ"
> +// CHECK-LABEL: declare dllimport x86_thiscallcc void @"\01??_DConcrete at test1@@QAEXXZ"(%"struct.test1::Concrete"*) unnamed_addr
> +
> +} // namespace test1
> +
> +namespace test2 {
> +class BaseClass {
> +public:
> +  virtual ~BaseClass(){};
> +};
> +
> +class __declspec(dllimport) VirtualClass : public virtual BaseClass {
> +public:
> +  virtual ~VirtualClass(){};
> +};
> +
> +int main() {
> +  VirtualClass c;
> +  return 0;
> +}
> +
> +// CHECK-LABEL: declare dllimport x86_thiscallcc %"class.test2::VirtualClass"* @"\01??0VirtualClass at test2@@QAE at XZ"
> +// CHECK-LABEL: declare dllimport x86_thiscallcc void @"\01??_DVirtualClass at test2@@QAEXXZ"(%"class.test2::VirtualClass"*)
> +
> +} // namespace test2
> +
> +namespace test3 {
> +class IVirtualBase {
> +public:
> +  virtual ~IVirtualBase(){};
> +  virtual void speak() = 0;
> +};
> +
> +class VirtualClass : public virtual IVirtualBase {
> +public:
> +  virtual ~VirtualClass(){};
> +  virtual void eat() = 0;
> +};
> +
> +class __declspec(dllimport) ConcreteClass : public VirtualClass {
> +public:
> +  ConcreteClass(int nn);
> +  void speak();
> +  void eat();
> +  virtual ~ConcreteClass();
> +
> +private:
> +  int n;
> +};
> +
> +int main() {
> +  ConcreteClass c(10);
> +  c.speak();
> +  c.eat();
> +  return 0;
> +}
> +
> +// CHECK-LABEL: declare dllimport x86_thiscallcc %"class.test3::ConcreteClass"* @"\01??0ConcreteClass at test3@@QAE at H@Z"
> +// CHECK-LABEL: declare dllimport x86_thiscallcc void @"\01??_DConcreteClass at test3@@QAEXXZ"(%"class.test3::ConcreteClass"*)
> +
> +} // namespace test3
>
> Modified: cfe/trunk/test/CodeGenCXX/dllimport.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllimport.cpp?rev=321239&r1=321238&r2=321239&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/dllimport.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/dllimport.cpp Wed Dec 20 18:07:46 2017
> @@ -362,8 +362,8 @@ struct __declspec(dllimport) ClassWithNo
>   struct __declspec(dllimport) ClassWithNonDllImportBase : public ClassWithDtor { };
>   USECLASS(ClassWithNonDllImportField);
>   USECLASS(ClassWithNonDllImportBase);
> -// MO1-DAG: declare dllimport x86_thiscallcc void @"\01??1ClassWithNonDllImportBase@@QAE at XZ"(%struct.ClassWithNonDllImportBase*)
> -// MO1-DAG: declare dllimport x86_thiscallcc void @"\01??1ClassWithNonDllImportField@@QAE at XZ"(%struct.ClassWithNonDllImportField*)
> +// MO1-DAG: declare dllimport x86_thiscallcc void @"\01??_DClassWithNonDllImportBase@@QAEXXZ"(%struct.ClassWithNonDllImportBase*)
> +// MO1-DAG: declare dllimport x86_thiscallcc void @"\01??_DClassWithNonDllImportField@@QAEXXZ"(%struct.ClassWithNonDllImportField*)
>   struct ClassWithCtor { ClassWithCtor() {} };
>   struct __declspec(dllimport) ClassWithNonDllImportFieldWithCtor { ClassWithCtor t; };
>   USECLASS(ClassWithNonDllImportFieldWithCtor);
>
> Added: cfe/trunk/test/CodeGenCXX/external-linkage.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/external-linkage.cpp?rev=321239&view=auto
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/external-linkage.cpp (added)
> +++ cfe/trunk/test/CodeGenCXX/external-linkage.cpp Wed Dec 20 18:07:46 2017
> @@ -0,0 +1,17 @@
> +// RUN: %clang_cc1 -triple i686-windows-msvc   -fno-rtti -fno-threadsafe-statics -fms-extensions -fms-compatibility-version=18.00 -emit-llvm -std=c++1y -O1 -disable-llvm-passes -o - %s -DMSABI -w | FileCheck --check-prefix=MO1 --check-prefix=MO2 %s
> +
> +// RUN: %clang_cc1 -triple i686-windows-msvc -fno-rtti -fno-threadsafe-statics -fms-extensions -fms-compatibility-version=18.00 -emit-llvm -std=c++1y -o - %s -DMSABI -w | FileCheck --check-prefix=MO3 --check-prefix=MO4 %s
> +
> +// MO1-DAG:@"\01??_8B@@7B@" = available_externally dllimport unnamed_addr constant [2 x i32] [i32 0, i32 4]
> +// MO2-DAG: define available_externally dllimport x86_thiscallcc %struct.B* @"\01??0B@@QAE at XZ"
> +
> +struct __declspec(dllimport) A {
> +  virtual ~A();
> +};
> +struct __declspec(dllimport) B : virtual A {
> +  virtual ~B();
> +};
> +void f() { B b; }
> +
> +// MO3-DAG: declare dllimport x86_thiscallcc %struct.B* @"\01??0B@@QAE at XZ"
> +// MO4-DAG: declare dllimport x86_thiscallcc void @"\01??_DB@@QAEXXZ"
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory



More information about the cfe-commits mailing list