MS ABI: Always emit dllexported in-class initialized static data members (PR20140)

[hans]

Summary

This patch makes us emit dllexported in-class initialized static data members (which are treated as definitions in MSVC), even when they're not referenced.

It also makes their special linkage reflected in the GVA linkage instead of getting massaged in CodeGen.

Please take a look!

[majnemer]

lib/AST/ASTContext.cpp

4787 ↗

(On Diff #11582)

I believe this should be controlled by -fms-compatibility not that ABI. MSDN says that compiling with /Za would disable this behavior: http://msdn.microsoft.com/en-us/library/34h23df8.aspx

[hans]

I believe this should be controlled by -fms-compatibility not that ABI. MSDN says that compiling with /Za would disable this behavior: http://msdn.microsoft.com/en-us/library/34h23df8.aspx

Sounds reasonable. I've updated the patch.

[rnk]

lgtm

I guess I agree with David w.r.t. -fms-compatibility vs ABI.

[hans]

Closed by commit rL213304 (authored by @hans).

cfe/trunk/include/clang/AST/ASTContext.h

private:
   void getObjCEncodingForStructureImpl(RecordDecl *RD, std::string &S,
                                        const FieldDecl *Field,
                                        bool includeVBases = true) const;
 public:
   // Adds the encoding of a method parameter or return type.
   void getObjCEncodingForMethodParameter(Decl::ObjCDeclQualifier QT,
                                          QualType T, std::string& S,
                                          bool Extended) const;
-  
+  /// \brief Returns true if this is an inline-initialized static data member
+  /// which is treated as a definition for MSVC compatibility.
+  bool isMSStaticDataMemberInlineDefinition(const VarDecl *VD) const;
+  
 private:
   const ASTRecordLayout &
   getObjCLayout(const ObjCInterfaceDecl *D,
                 const ObjCImplementationDecl *Impl) const;
 
   /// \brief A set of deallocations that should be performed when the
   /// ASTContext is destroyed.
   typedef llvm::SmallDenseMap<void(*)(void*), llvm::SmallVector<void*, 16> >

cfe/trunk/lib/AST/ASTContext.cpp

CharUnits ASTContext::getObjCEncodingTypeSize(QualType type) const {
   if (sz.isPositive() && type->isIntegralOrEnumerationType())
     sz = std::max(sz, getTypeSizeInChars(IntTy));
   // Treat arrays as pointers, since that's how they're passed in.
   else if (type->isArrayType())
     sz = getTypeSizeInChars(VoidPtrTy);
   return sz;
 }
 
+bool ASTContext::isMSStaticDataMemberInlineDefinition(const VarDecl *VD) const {
+  return getLangOpts().MSVCCompat && VD->isStaticDataMember() &&
+         VD->getType()->isIntegralOrEnumerationType() &&
+         !VD->getFirstDecl()->isOutOfLine() && VD->getFirstDecl()->hasInit();
+}
+
 static inline 
 std::string charUnitsToString(const CharUnits &CU) {
   return llvm::itostr(CU.getQuantity());
 }
 
 /// getObjCEncodingForBlock - Return the encoded type for this block
 /// declaration.
 std::string ASTContext::getObjCEncodingForBlock(const BlockExpr *Expr) const {
if (VD->isStaticLocal()) {
 
     // GVA_StrongODR function linkage is stronger than what we need,
     // downgrade to GVA_DiscardableODR.
     // This allows us to discard the variable if we never end up needing it.
     return StaticLocalLinkage == GVA_StrongODR ? GVA_DiscardableODR
                                                : StaticLocalLinkage;
   }
 
+  // MSVC treats in-class initialized static data members as definitions.
+  // By giving them non-strong linkage, out-of-line definitions won't
+  // cause link errors.
+  if (Context.isMSStaticDataMemberInlineDefinition(VD))
+    return GVA_DiscardableODR;
+
   switch (VD->getTemplateSpecializationKind()) {
   case TSK_Undeclared:
   case TSK_ExplicitSpecialization:
     return GVA_StrongExternal;
 
   case TSK_ExplicitInstantiationDefinition:
     return GVA_StrongODR;
 
if (Linkage == GVA_Internal || Linkage == GVA_AvailableExternally ||
         Linkage == GVA_DiscardableODR)
       return false;
     return true;
   }
   
   const VarDecl *VD = cast<VarDecl>(D);
   assert(VD->isFileVarDecl() && "Expected file scoped var");
 
-  if (VD->isThisDeclarationADefinition() == VarDecl::DeclarationOnly)
+  if (VD->isThisDeclarationADefinition() == VarDecl::DeclarationOnly &&
+      !isMSStaticDataMemberInlineDefinition(VD))
     return false;
 
   // Variables that can be needed in other TUs are required.
   GVALinkage L = GetGVALinkageForVariable(VD);
   if (L != GVA_Internal && L != GVA_AvailableExternally &&
       L != GVA_DiscardableODR)
     return true;
 

cfe/trunk/lib/CodeGen/CGCXXABI.h

virtual void EmitReturnFromThunk(CodeGenFunction &CGF,
                                    RValue RV, QualType ResultType);
 
   /// Gets the pure virtual member call function.
   virtual StringRef GetPureVirtualCallName() = 0;
 
   /// Gets the deleted virtual member call name.
   virtual StringRef GetDeletedVirtualCallName() = 0;
 
-  /// \brief Returns true iff static data members that are initialized in the
-  /// class definition should have linkonce linkage.
-  virtual bool isInlineInitializedStaticDataMemberLinkOnce() { return false; }
-
   /**************************** Array cookies ******************************/
 
   /// Returns the extra size required in order to store the array
   /// cookie for the given new-expression.  May return 0 to indicate that no
   /// array cookie is required.
   ///
   /// Several cases are filtered out before this method is called:
   ///   - non-array allocations never need a cookie

cfe/trunk/lib/CodeGen/CodeGenModule.cpp

if (!FD->doesThisDeclarationHaveABody()) {
       GetOrCreateLLVMFunction(MangledName, Ty, GD, /*ForVTable=*/false,
                               /*DontDefer=*/false);
       return;
     }
   } else {
     const auto *VD = cast<VarDecl>(Global);
     assert(VD->isFileVarDecl() && "Cannot emit local var decl as global.");
 
-    if (VD->isThisDeclarationADefinition() != VarDecl::Definition)
+    if (VD->isThisDeclarationADefinition() != VarDecl::Definition &&
+        !Context.isMSStaticDataMemberInlineDefinition(VD))
       return;
   }
 
   // Defer code generation when possible if this is a static definition, inline
   // function etc.  These we only want to emit if they are used.
   if (!MayDeferGeneration(Global)) {
     // Emit the definition if it can't be deferred.
     EmitGlobalDefinition(GD);
if (const CXXRecordDecl *Record
           = Context.getBaseElementType(Ty)->getAsCXXRecordDecl())
       return ExcludeCtor && !Record->hasMutableFields() &&
              Record->hasTrivialDestructor();
   }
 
   return true;
 }
 
-static bool isVarDeclInlineInitializedStaticDataMember(const VarDecl *VD) {
-  if (!VD->isStaticDataMember())
-    return false;
-  const VarDecl *InitDecl;
-  const Expr *InitExpr = VD->getAnyInitializer(InitDecl);
-  if (!InitExpr)
-    return false;
-  if (InitDecl->isThisDeclarationADefinition())
-    return false;
-  return true;
-}
-
 /// GetOrCreateLLVMGlobal - If the specified mangled name is not in the module,
 /// create and return an llvm GlobalVariable with the specified type.  If there
 /// is something in the module with the specified name, return it potentially
 /// bitcasted to the right type.
 ///
 /// If D is non-null, it specifies a decl that correspond to this.  This is used
 /// to set the attributes on the global when it is first created.
 llvm::Constant *
if (D) {
     if (D->getTLSKind()) {
       if (D->getTLSKind() == VarDecl::TLS_Dynamic)
         CXXThreadLocals.push_back(std::make_pair(D, GV));
       setTLSMode(GV, *D);
     }
 
     // If required by the ABI, treat declarations of static data members with
     // inline initializers as definitions.
-    if (getCXXABI().isInlineInitializedStaticDataMemberLinkOnce() &&
-        isVarDeclInlineInitializedStaticDataMember(D))
+    if (getContext().isMSStaticDataMemberInlineDefinition(D)) {
       EmitGlobalVarDefinition(D);
+    }
 
     // Handle XCore specific ABI requirements.
     if (getTarget().getTriple().getArch() == llvm::Triple::xcore &&
         D->getLanguageLinkage() == CLanguageLinkage &&
         D->getType().isConstant(Context) &&
         isExternallyVisible(D->getLinkageAndVisibility().getLinkage()))
       GV->setSection(".cp.rodata");
   }
llvm::GlobalValue::LinkageTypes CodeGenModule::getLLVMLinkageForDeclarator(
   // An explicit instantiation of a template has weak linkage, since
   // explicit instantiations can occur in multiple translation units
   // and must all be equivalent. However, we are not allowed to
   // throw away these explicit instantiations.
   if (Linkage == GVA_StrongODR)
     return !Context.getLangOpts().AppleKext ? llvm::Function::WeakODRLinkage
                                             : llvm::Function::ExternalLinkage;
 
-  // If required by the ABI, give definitions of static data members with inline
-  // initializers at least linkonce_odr linkage.
-  auto const VD = dyn_cast<VarDecl>(D);
-  if (getCXXABI().isInlineInitializedStaticDataMemberLinkOnce() &&
-      VD && isVarDeclInlineInitializedStaticDataMember(VD)) {
-    if (VD->hasAttr<DLLImportAttr>())
-      return llvm::GlobalValue::AvailableExternallyLinkage;
-    if (VD->hasAttr<DLLExportAttr>())
-      return llvm::GlobalValue::WeakODRLinkage;
-    return llvm::GlobalValue::LinkOnceODRLinkage;
-  }
-
   // C++ doesn't have tentative definitions and thus cannot have common
   // linkage.
   if (!getLangOpts().CPlusPlus && isa<VarDecl>(D) &&
       !isVarDeclStrongDefinition(cast<VarDecl>(D), CodeGenOpts.NoCommon))
     return llvm::GlobalVariable::CommonLinkage;
 
   // selectany symbols are externally visible, so use weak instead of
   // linkonce.  MSVC optimizes away references to const selectany globals, so

cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp

public:
 
   bool isSRetParameterAfterThis() const override { return true; }
 
   StringRef GetPureVirtualCallName() override { return "_purecall"; }
   // No known support for deleted functions in MSVC yet, so this choice is
   // arbitrary.
   StringRef GetDeletedVirtualCallName() override { return "_purecall"; }
 
-  bool isInlineInitializedStaticDataMemberLinkOnce() override { return true; }
-
   llvm::Value *adjustToCompleteObject(CodeGenFunction &CGF,
                                       llvm::Value *ptr,
                                       QualType type) override;
 
   llvm::GlobalVariable *getMSCompleteObjectLocator(const CXXRecordDecl *RD,
                                                    const VPtrInfo *Info);
 
   llvm::Constant *getAddrOfRTTIDescriptor(QualType Ty) override;

cfe/trunk/lib/CodeGen/ModuleBuilder.cpp

public:
     /// to (e.g. struct, union, enum, class) is completed. This allows the
     /// client hack on the type, which can occur at any point in the file
     /// (because these can be defined in declspecs).
     void HandleTagDeclDefinition(TagDecl *D) override {
       if (Diags.hasErrorOccurred())
         return;
 
       Builder->UpdateCompletedType(D);
+
+      // For MSVC compatibility, treat declarations of static data members with
+      // inline initializers as definitions.
+      if (Ctx->getLangOpts().MSVCCompat) {
+        for (Decl *Member : D->decls()) {
+          if (VarDecl *VD = dyn_cast<VarDecl>(Member)) {
+            if (Ctx->isMSStaticDataMemberInlineDefinition(VD) &&
+                Ctx->DeclMustBeEmitted(VD)) {
+              Builder->EmitGlobal(VD);
+            }
+          }
+        }
+      }
     }
 
     void HandleTagDeclRequiredDefinition(const TagDecl *D) override {
       if (Diags.hasErrorOccurred())
         return;
 
       if (CodeGen::CGDebugInfo *DI = Builder->getModuleDebugInfo())
         if (const RecordDecl *RD = dyn_cast<RecordDecl>(D))

cfe/trunk/test/CodeGenCXX/dllexport-members.cpp

-// RUN: %clang_cc1 -triple i686-windows-msvc   -emit-llvm -std=c++1y -O0 -o - %s | FileCheck --check-prefix=MSC --check-prefix=M32 %s
-// RUN: %clang_cc1 -triple x86_64-windows-msvc -emit-llvm -std=c++1y -O0 -o - %s | FileCheck --check-prefix=MSC --check-prefix=M64 %s
-// RUN: %clang_cc1 -triple i686-windows-gnu    -emit-llvm -std=c++1y -O0 -o - %s | FileCheck --check-prefix=GNU --check-prefix=G32 %s
-// RUN: %clang_cc1 -triple x86_64-windows-gnu  -emit-llvm -std=c++1y -O0 -o - %s | FileCheck --check-prefix=GNU --check-prefix=G64 %s
+// RUN: %clang_cc1 -triple i686-windows-msvc -fms-compatibility   -emit-llvm -std=c++1y -O0 -o - %s | FileCheck --check-prefix=MSC --check-prefix=M32 %s
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-compatibility -emit-llvm -std=c++1y -O0 -o - %s | FileCheck --check-prefix=MSC --check-prefix=M64 %s
+// RUN: %clang_cc1 -triple i686-windows-gnu                       -emit-llvm -std=c++1y -O0 -o - %s | FileCheck --check-prefix=GNU --check-prefix=G32 %s
+// RUN: %clang_cc1 -triple x86_64-windows-gnu                     -emit-llvm -std=c++1y -O0 -o - %s | FileCheck --check-prefix=GNU --check-prefix=G64 %s
 
 // Helper structs to make templates more expressive.
 struct ImplicitInst_Exported {};
 struct ExplicitDecl_Exported {};
 struct ExplicitInst_Exported {};
 struct ExplicitSpec_Exported {};
 struct ExplicitSpec_Def_Exported {};
 struct ExplicitSpec_InlineDef_Exported {};

cfe/trunk/test/CodeGenCXX/dllimport-members.cpp

-// RUN: %clang_cc1 -triple i686-windows-msvc   -emit-llvm -std=c++1y -O0 -o - %s -DMSABI | FileCheck --check-prefix=MSC --check-prefix=M32 %s
-// RUN: %clang_cc1 -triple x86_64-windows-msvc -emit-llvm -std=c++1y -O0 -o - %s -DMSABI | FileCheck --check-prefix=MSC --check-prefix=M64 %s
-// RUN: %clang_cc1 -triple i686-windows-gnu    -emit-llvm -std=c++1y -O0 -o - %s         | FileCheck --check-prefix=GNU --check-prefix=G32 %s
-// RUN: %clang_cc1 -triple x86_64-windows-gnu  -emit-llvm -std=c++1y -O0 -o - %s         | FileCheck --check-prefix=GNU --check-prefix=G64 %s
-// RUN: %clang_cc1 -triple i686-windows-msvc   -emit-llvm -std=c++1y -O1 -o - %s -DMSABI | FileCheck --check-prefix=MO1 %s
-// RUN: %clang_cc1 -triple i686-windows-gnu    -emit-llvm -std=c++1y -O1 -o - %s         | FileCheck --check-prefix=GO1 %s
+// RUN: %clang_cc1 -triple i686-windows-msvc -fms-compatibility   -emit-llvm -std=c++1y -O0 -o - %s -DMSABI | FileCheck --check-prefix=MSC --check-prefix=M32 %s
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-compatibility -emit-llvm -std=c++1y -O0 -o - %s -DMSABI | FileCheck --check-prefix=MSC --check-prefix=M64 %s
+// RUN: %clang_cc1 -triple i686-windows-gnu                       -emit-llvm -std=c++1y -O0 -o - %s         | FileCheck --check-prefix=GNU --check-prefix=G32 %s
+// RUN: %clang_cc1 -triple x86_64-windows-gnu                     -emit-llvm -std=c++1y -O0 -o - %s         | FileCheck --check-prefix=GNU --check-prefix=G64 %s
+// RUN: %clang_cc1 -triple i686-windows-msvc -fms-compatibility   -emit-llvm -std=c++1y -O1 -o - %s -DMSABI | FileCheck --check-prefix=MO1 %s
+// RUN: %clang_cc1 -triple i686-windows-gnu                       -emit-llvm -std=c++1y -O1 -o - %s         | FileCheck --check-prefix=GO1 %s
 
 // Helper structs to make templates more expressive.
 struct ImplicitInst_Imported {};
 struct ExplicitDecl_Imported {};
 struct ExplicitInst_Imported {};
 struct ExplicitSpec_Imported {};
 struct ExplicitSpec_Def_Imported {};
 struct ExplicitSpec_InlineDef_Imported {};

cfe/trunk/test/CodeGenCXX/ms-integer-static-data-members-exported.cpp

+// RUN: %clang_cc1 -emit-llvm -triple=i386-pc-win32 -fms-compatibility %s -o - | FileCheck %s
+
+enum Enum { zero, one, two };
+
+struct __declspec(dllexport) S {
+  // In MS compatibility mode, this counts as a definition.
+  // Since it is exported, it must be emitted even if it's unreferenced.
+  static const short x = 42;
+
+  // This works for enums too.
+  static const Enum y = two;
+
+  struct NonExported {
+    // dllexport is not inherited by this nested class.
+    // Since z is not referenced, it should not be emitted.
+    static const int z = 42;
+  };
+};
+
+// CHECK: @"\01?x@S@@2FB" = weak_odr dllexport constant i16 42, align 2
+// CHECK: @"\01?y@S@@2W4Enum@@B" = weak_odr dllexport constant i32 2, align 4
+// CHECK-NOT: NonExported

cfe/trunk/test/CodeGenCXX/ms-integer-static-data-members.cpp

-// RUN: %clang_cc1 -emit-llvm -triple=i386-pc-win32 %s -o - | FileCheck %s
-// RUN: %clang_cc1 -DINLINE_INIT -emit-llvm -triple=i386-pc-win32 %s -o - | FileCheck %s --check-prefix=CHECK-INLINE
-// RUN: %clang_cc1 -DREAL_DEFINITION -emit-llvm -triple=i386-pc-win32 %s -o - | FileCheck %s --check-prefix=CHECK-OUTOFLINE
-// RUN: %clang_cc1 -DINLINE_INIT -DREAL_DEFINITION -emit-llvm -triple=i386-pc-win32 %s -o - | FileCheck %s --check-prefix=CHECK-INLINE
+// RUN: %clang_cc1 -emit-llvm -triple=i386-pc-win32 -fms-compatibility %s -o - | FileCheck %s
+// RUN: %clang_cc1 -DINLINE_INIT -emit-llvm -triple=i386-pc-win32 -fms-compatibility %s -o - | FileCheck %s --check-prefix=CHECK-INLINE
+// RUN: %clang_cc1 -DREAL_DEFINITION -emit-llvm -triple=i386-pc-win32 -fms-compatibility %s -o - | FileCheck %s --check-prefix=CHECK-OUTOFLINE
+// RUN: %clang_cc1 -DINLINE_INIT -DREAL_DEFINITION -emit-llvm -triple=i386-pc-win32 -fms-compatibility %s -o - | FileCheck %s --check-prefix=CHECK-INLINE
 
 struct S {
   // For MS ABI, we emit a linkonce_odr definition here, even though it's really just a declaration.
 #ifdef INLINE_INIT
   static const int x = 5;
 #else
   static const int x;
 #endif