Inherit dll attributes to static locals

[hans]

Summary

This makes us handle static locals in exported/imported functions correctly.

[abdulras]

It would be nice to have similar tests for the Itanium ABI, but I suspect that you are less interested in that.

lib/CodeGen/MicrosoftCXXABI.cpp

1429

clsas -> class.

test/CodeGenCXX/dllexport.cpp

274

It really would be nice to split this file up at some point.

[nrieck]

lib/CodeGen/MicrosoftCXXABI.cpp

1429

"class"

lib/Sema/SemaDecl.cpp

9112

There's already a getDLLAttr() function somewhere, but I guess it's not available here. Maybe this helper could be moved to SemaInternal.h?

test/CodeGenCXX/dllexport.cpp

257

These tests should be at the end of the section for globals (like in SemaCXX/dllexport.cpp) IMO. Same goes for the dllimport tests.

262

These checks can be MSC-DAG.

267

Superfluous ";"

269

These checks can be MSC-DAG, and please add a comment that MinGW doesn't do this and is not just forgotten here (at least my tests with GCC show this to be true).

[hans]

Thanks for the review!

It would be nice to have similar tests for the Itanium ABI, but I suspect that you are less interested in that.

As Nico pointed out, they don't seem to export the static locals :/

lib/CodeGen/MicrosoftCXXABI.cpp

1429

Done.

lib/Sema/SemaDecl.cpp

9112

Sounds good to me.

test/CodeGenCXX/dllexport.cpp

257

Done.

262

Done.

267

Removed.

269

Done.

[hans]

Addressing review comments.

Please take another look!

[hans]

Nico: does the new version of the patch look ok to you?

[rnk]

lib/Sema/SemaDecl.cpp

9108

Should this go before checkAttributesAfterMerging? Consider this obnoxious test case:

int f();
inline __declspec(dllexport) int g() {
  static __declspec(dllimport) int x = f();
  return x;
}

[hans]

lib/Sema/SemaDecl.cpp

9108

Local variables cannot have explicit dll attributes. They can only get them by inheriting from the function they're part of, so calling checkAttributesAfterMerging to check up on the attribute we just added seems redundant.

[rnk]

lgtm

lib/Sema/SemaDecl.cpp

9108

OK. I assume this is covered by Nico's extensive test cases. :)

[nrieck]

In D4136#10, @hans wrote:

Nico: does the new version of the patch look ok to you?

LGTM

lib/Sema/SemaDecl.cpp

9108

Yup, this is handled by requiring external linkage.

[hans]

Closed by commit rL211173 (authored by @hans).

lib/CodeGen/CGDecl.cpp

new llvm::GlobalVariable(CGM.getModule(), LTy,
                              llvm::GlobalVariable::NotThreadLocal,
                              AddrSpace);
   GV->setAlignment(getContext().getDeclAlign(&D).getQuantity());
   CGM.setGlobalVisibility(GV, &D);
 
   if (D.getTLSKind())
     CGM.setTLSMode(GV, D);
 
+  if (D.isExternallyVisible()) {
+    if (D.hasAttr<DLLImportAttr>())
+      GV->setDLLStorageClass(llvm::GlobalVariable::DLLImportStorageClass);
+    else if (D.hasAttr<DLLExportAttr>())
+      GV->setDLLStorageClass(llvm::GlobalVariable::DLLExportStorageClass);
+  }
+
   // Make sure the result is of the correct type.
   unsigned ExpectedAddrSpace = CGM.getContext().getTargetAddressSpace(Ty);
   if (AddrSpace != ExpectedAddrSpace) {
     llvm::PointerType *PTy = llvm::PointerType::get(LTy, ExpectedAddrSpace);
     return llvm::ConstantExpr::getAddrSpaceCast(GV, PTy);
   }
 
   return GV;

lib/CodeGen/MicrosoftCXXABI.cpp

if (!GI->Guard) {
     // Mangle the name for the guard.
     SmallString<256> GuardName;
     {
       llvm::raw_svector_ostream Out(GuardName);
       getMangleContext().mangleStaticGuardVariable(&D, Out);
       Out.flush();
     }
 
-    // Create the guard variable with a zero-initializer.  Just absorb linkage
-    // and visibility from the guarded variable.
+    // Create the guard variable with a zero-initializer.  Just absorb linkage,
+    // visibility and dll storage clsas from the guarded variable.
     GI->Guard =
         new llvm::GlobalVariable(CGM.getModule(), GuardTy, false,
                                  GV->getLinkage(), Zero, GuardName.str());
     GI->Guard->setVisibility(GV->getVisibility());
+    GI->Guard->setDLLStorageClass(GV->getDLLStorageClass());
   } else {
     assert(GI->Guard->getLinkage() == GV->getLinkage() &&
            "static local from the same function had different linkage");
   }
 
   // Pseudo code for the test:
   // if (!(GuardVar & MyGuardBit)) {
   //   GuardVar |= MyGuardBit;

lib/Sema/SemaDecl.cpp

Sema::FinalizeDeclaration(Decl *ThisDecl) {
   ParsingInitForAutoVars.erase(ThisDecl);
 
   VarDecl *VD = dyn_cast_or_null<VarDecl>(ThisDecl);
   if (!VD)
     return;
 
   checkAttributesAfterMerging(*this, *VD);
 
+  // Static locals inherit dll attributes from their function.
+  if (VD->isStaticLocal()) {
+    if (FunctionDecl *FD =
+            dyn_cast<FunctionDecl>(VD->getParentFunctionOrMethod())) {
+      Attr *A = FD->getAttr<DLLImportAttr>();
+      if (!A)
+        A = FD->getAttr<DLLExportAttr>();
+      if (A) {
+        auto *NewAttr = cast<InheritableAttr>(A->clone(getASTContext()));
+        NewAttr->setInherited(true);
+        VD->addAttr(NewAttr);
+      }
+    }
+  }
+
   // Imported static data members cannot be defined out-of-line.
   if (const DLLImportAttr *IA = VD->getAttr<DLLImportAttr>()) {
     if (VD->isStaticDataMember() && VD->isOutOfLine() &&
         VD->isThisDeclarationADefinition()) {
       // We allow definitions of dllimport class template static data members
       // with a warning.
       CXXRecordDecl *Context =
         cast<CXXRecordDecl>(VD->getFirstDecl()->getDeclContext());

test/CodeGenCXX/dllexport.cpp

 // GNU-DAG: define dllexport noalias i8* @_Znw{{[yj]}}(
 void* alloc(__SIZE_TYPE__ n);
 __declspec(dllexport) void* operator new(__SIZE_TYPE__ n) { return alloc(n); }
 
 // MSC-DAG: define dllexport void @"\01?externalFunc@ns@@YAXXZ"()
 // GNU-DAG: define dllexport void @_ZN2ns12externalFuncEv()
 namespace ns { __declspec(dllexport) void externalFunc() {} }
 
+//===----------------------------------------------------------------------===//
+// Static locals
+//===----------------------------------------------------------------------===//
 
+int f();
+// M32-DAG: @"\01?x@?0??nonInlineStaticLocalsFunc@@YAHXZ@4HA" = internal {{(unnamed_addr )*}}global i32 0
+// M32-DAG: @"\01?$S1@?0??nonInlineStaticLocalsFunc@@YAHXZ@4IA" = internal {{(unnamed_addr )*}}global i32 0
+int __declspec(dllexport) nonInlineStaticLocalsFunc() {
+  static int x = f();
+  return x++;
+};
+
+// M32-DAG: @"\01?x@?1??inlineStaticLocalsFunc@@YAHXZ@4HA" = weak_odr dllexport global i32 0
+// M32-DAG: @"\01??_B?1??inlineStaticLocalsFunc@@YAHXZ@51" = weak_odr dllexport global i32 0
+inline int __declspec(dllexport) inlineStaticLocalsFunc() {
+  static int x = f();
+  return x++;
+};
 
 //===----------------------------------------------------------------------===//
 // Function templates
 //===----------------------------------------------------------------------===//
 
 // Export function template definition.
 // MSC-DAG: define weak_odr dllexport void @"\01??$funcTmplDef@UExplicitInst_Exported@@@@YAXXZ"()
 // GNU-DAG: define weak_odr dllexport void @_Z11funcTmplDefI21ExplicitInst_ExportedEvv()

test/CodeGenCXX/dllimport.cpp

 // MSC-DAG: declare dllimport void @"\01?externalFunc@ns@@YAXXZ"()
 // GNU-DAG: declare dllimport void @_ZN2ns12externalFuncEv()
 namespace ns { __declspec(dllimport) void externalFunc(); }
 USE(ns::externalFunc)
 
 
 
 //===----------------------------------------------------------------------===//
+// Static locals
+//===----------------------------------------------------------------------===//
+
+int f();
+// MO1-DAG: @"\01?x@?1??inlineStaticLocalsFunc@@YAHXZ@4HA" = available_externally dllimport global i32 0
+// MO1-DAG: @"\01??_B?1??inlineStaticLocalsFunc@@YAHXZ@51" = available_externally dllimport global i32 0
+inline int __declspec(dllimport) inlineStaticLocalsFunc() {
+  static int x = f();
+  return x++;
+};
+USE(inlineStaticLocalsFunc);
+
+
+//===----------------------------------------------------------------------===//
 // Function templates
 //===----------------------------------------------------------------------===//
 
 // Import function template declaration.
 // MSC-DAG: declare dllimport void @"\01??$funcTmplDecl@UImplicitInst_Imported@@@@YAXXZ"()
 // GNU-DAG: declare dllimport void @_Z12funcTmplDeclI21ImplicitInst_ImportedEvv()
 template<typename T> __declspec(dllimport) void funcTmplDecl();
 USE(funcTmplDecl<ImplicitInst_Imported>)