Fix linkage of static locals in available_externally functions to be DiscardableODR/linkonce_odr

[dblaikie]

Summary

As Mehdi put it, entities should either be
available_externally+weak_odr, or linkonce_odr+linkonce_odr. While some
functions are emitted a_e/weak, their local variables were emitted
a_e/linkonce_odr.

While it might be nice to emit them a_e/weak, the Itanium ABI (& best
guess at MSVC's behavior as well) requires the local to be
linkonce/linkonce.

[mehdi_amini]

LGTM.

lib/AST/ASTContext.cpp

8909

I assume you looked it up, do you have a ref? (Citation or pointer to right section/paragraph).

[dblaikie]

Improve comment/mention Itanium ABI

lib/AST/ASTContext.cpp

 }
 
 static GVALinkage basicGVALinkageForVariable(const ASTContext &Context,
                                              const VarDecl *VD) {
   if (!VD->isExternallyVisible())
     return GVA_Internal;
 
   if (VD->isStaticLocal()) {
-    GVALinkage StaticLocalLinkage = GVA_DiscardableODR;
     const DeclContext *LexicalContext = VD->getParentFunctionOrMethod();
     while (LexicalContext && !isa<FunctionDecl>(LexicalContext))
       LexicalContext = LexicalContext->getLexicalParent();
 
-    // Let the static local variable inherit its linkage from the nearest
-    // enclosing function.
-    if (LexicalContext)
-      StaticLocalLinkage =
-          Context.GetGVALinkageForFunction(cast<FunctionDecl>(LexicalContext));
+    // ObjC Blocks can create local variables that don't have a FunctionDecl
+    // LexicalContext.
+    if (!LexicalContext)
+      return GVA_DiscardableODR;
+
+    // Otherwise, let the static local variable inherit its linkage from the
+    // nearest enclosing function.
+    auto StaticLocalLinkage =
 
-    // 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;
+    // Itanium ABI 5.2.2: "Each COMDAT group [for a static local variable] must
+    // be emitted in any object with references to the symbol for the object it
+    // contains, whether inline or out-of-line."
+    // Similar behavior is observed with MSVC. An alternative ABI could use
+    // StrongODR/AvailableExternally to match the function, but none are
+    // known/supported currently.
+    if (StaticLocalLinkage == GVA_StrongODR ||
+        StaticLocalLinkage == GVA_AvailableExternally)
+      return GVA_DiscardableODR;
+    return 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;
 

test/CodeGenCXX/explicit-instantiation.cpp

 // RUN: %clang_cc1 -emit-llvm -triple i686-pc-linux-gnu -std=c++1y -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NO-OPT
 // RUN: %clang_cc1 -emit-llvm -triple i686-pc-linux-gnu -std=c++1y -O3 -disable-llvm-passes -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-OPT
 // RUN: %clang_cc1 -emit-llvm -triple i686-pc-win32 -std=c++1y -o - %s | FileCheck %s --check-prefix=CHECK-MS
 
 // This check logically is attached to 'template int S<int>::i;' below.
 // CHECK: @_ZN1SIiE1iE = weak_odr global i32
 
+// This check is logically attached to 'template int ExportedStaticLocal::f<int>()' below.
+// CHECK-OPT: @_ZZN19ExportedStaticLocal1fIiEEvvE1i = linkonce_odr global
+
 template<typename T, typename U, typename Result>
 struct plus {
   Result operator()(const T& t, const U& u) const;
 };
 
 template<typename T, typename U, typename Result>
 Result plus<T, U, Result>::operator()(const T& t, const U& u) const {
   return t + u;

 
 // CHECK-LABEL: define weak_odr void @_ZN1SIiE2S21hEv
 template void S<int>::S2::h();
 
 template <typename T> void S<T>::f() {}
 template <typename T> void S<T>::g() {}
 template <typename T> int S<T>::i;
 template <typename T> void S<T>::S2::h() {}
+
+namespace ExportedStaticLocal {
+void sink(int&);
+template <typename T>
+inline void f() {
+  static int i;
+  sink(i);
+}
+// See the check line at the top of the file.
+extern template void f<int>();
+void use() {
+  f<int>();
+}
+}