PR48030: Fix COMDAT-related linking problem with C++ thread_local static data members.

[rsmith]

Summary

Previously when emitting a C++ guarded initializer, we tried to work out what
the enclosing function would be used for and added it to the COMDAT containing
the variable if we thought that doing so would be correct. But this was done
from a context in which we didn't -- and realistically couldn't -- correctly
infer how the enclosing function would be used.

Instead, add the initialization function to a COMDAT from the code that
creates it, in the case where it makes sense to do so: when we know that
the one and only reference to the initialization function is in
@llvm.global.ctors and that reference is in the same COMDAT.

[rsmith]

Note that there's a potential ABI impact here: because we accidentally put the __cxx_global_init function into a COMDAT with the variable, that meant that the _ZTH symbol (which is an alias to __cxx_global_init) also pointed into that same COMDAT. I'm not sure how much that matters given that the old behavior was broken.

[rjmccall]

LGTM

clang/lib/CodeGen/CGDeclCXX.cpp

if (D->getTLSKind()) {
 
     AddGlobalCtor(Fn, 65535, COMDATKey);
     if (COMDATKey && (getTriple().isOSBinFormatELF() ||
                       getTarget().getCXXABI().isMicrosoft())) {
       // When COMDAT is used on ELF or in the MS C++ ABI, the key must be in
       // llvm.used to prevent linker GC.
       addUsedGlobal(COMDATKey);
     }
+
+    // If we used a COMDAT key for the global ctor, the init function can be
+    // discarded if the global ctor entry is discarded.
+    // FIXME: Do we need to restrict this to ELF and Wasm?
+    llvm::Comdat *C = Addr->getComdat();
+    if (COMDATKey && C &&
+        (getTarget().getTriple().isOSBinFormatELF() ||
+         getTarget().getTriple().isOSBinFormatWasm())) {
+      Fn->setComdat(C);
+    }
   } else {
     I = DelayedCXXInitPosition.find(D); // Re-do lookup in case of re-hash.
     if (I == DelayedCXXInitPosition.end()) {
       CXXGlobalInits.push_back(Fn);
     } else if (I->second != ~0U) {
       assert(I->second < CXXGlobalInits.size() &&
              CXXGlobalInits[I->second] == nullptr);
       CXXGlobalInits[I->second] = Fn;

clang/lib/CodeGen/ItaniumCXXABI.cpp

if (!guard) {
     // The ABI says: "It is suggested that it be emitted in the same COMDAT
     // group as the associated data object." In practice, this doesn't work for
     // non-ELF and non-Wasm object formats, so only do it for ELF and Wasm.
     llvm::Comdat *C = var->getComdat();
     if (!D.isLocalVarDecl() && C &&
         (CGM.getTarget().getTriple().isOSBinFormatELF() ||
          CGM.getTarget().getTriple().isOSBinFormatWasm())) {
       guard->setComdat(C);
-      // An inline variable's guard function is run from the per-TU
-      // initialization function, not via a dedicated global ctor function, so
-      // we can't put it in a comdat.
-      if (!NonTemplateInline)
-        CGF.CurFn->setComdat(C);
     } else if (CGM.supportsCOMDAT() && guard->isWeakForLinker()) {
       guard->setComdat(CGM.getModule().getOrInsertComdat(guard->getName()));
     }
 
     CGM.setStaticLocalDeclGuardAddress(&D, guard);
   }
 
   Address guardAddr = Address(guard, guardAlignment);

clang/test/CodeGenCXX/cxx11-thread-local-instantiated.cpp

+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s
+
+// PR48030
+
+template<typename T> struct TLS { static thread_local T *mData; };
+inline decltype(nullptr) non_constant_initializer() { return nullptr; }
+template<typename T> thread_local T *TLS<T>::mData = non_constant_initializer();
+struct S {};
+S *current() { return TLS<S>::mData; };
+
+// CHECK-DAG: @_ZN3TLSI1SE5mDataE = linkonce_odr thread_local global {{.*}}, comdat,
+// CHECK-DAG: @_ZGVN3TLSI1SE5mDataE = linkonce_odr thread_local global {{.*}}, comdat($_ZN3TLSI1SE5mDataE),
+// CHECK-DAG: @_ZTHN3TLSI1SE5mDataE = linkonce_odr alias {{.*}} @__cxx_global_var_init
+
+// CHECK-LABEL: define {{.*}} @_Z7currentv()
+// CHECK: call {{.*}} @_ZTWN3TLSI1SE5mDataE()
+
+// CHECK-LABEL: define weak_odr hidden {{.*}} @_ZTWN3TLSI1SE5mDataE() {{.*}} comdat {
+// CHECK: call void @_ZTHN3TLSI1SE5mDataE()
+// CHECK: ret {{.*}} @_ZN3TLSI1SE5mDataE
+
+// Unlike for a global, the global initialization function must not be in a
+// COMDAT with the variable, because it is referenced from the _ZTH function
+// which is outside that COMDAT.
+//
+// CHECK-NOT: define {{.*}} @__cxx_global_var_init{{.*}}comdat

clang/test/CodeGenCXX/cxx11-thread-local.cpp

 // LINUX_AIX-LABEL: define weak_odr hidden {{.*}}* @_ZTWN1XIiE1mE()
 // DARWIN-LABEL: define weak_odr hidden cxx_fast_tlscc {{.*}}* @_ZTWN1XIiE1mE()
 // LINUX_AIX: call void @_ZTHN1XIiE1mE()
 // DARWIN: call cxx_fast_tlscc void @_ZTHN1XIiE1mE()
 // CHECK: ret {{.*}}* @_ZN1XIiE1mE
 
 // LINUX_AIX: define internal void @[[VF_M_INIT]]()
 // DARWIN: define internal cxx_fast_tlscc void @[[VF_M_INIT]]()
-// LINUX-SAME: comdat($_ZN1VIfE1mE)
-// DARWIN-NOT: comdat
+// CHECK-NOT: comdat
 // CHECK: load i8, i8* bitcast (i64* @_ZGVN1VIfE1mE to i8*)
 // CHECK: %[[VF_M_INITIALIZED:.*]] = icmp eq i8 %{{.*}}, 0
 // CHECK: br i1 %[[VF_M_INITIALIZED]],
 // need init:
 // CHECK: call{{.*}} i32 @_Z1gv()
 // CHECK: store i32 %{{.*}}, i32* @_ZN1VIfE1mE, align 4
 // CHECK: store i8 1, i8* bitcast (i64* @_ZGVN1VIfE1mE to i8*)
 // CHECK: br label
 
 // LINUX_AIX: define internal void @[[XF_M_INIT]]()
 // DARWIN: define internal cxx_fast_tlscc void @[[XF_M_INIT]]()
-// LINUX-SAME: comdat($_ZN1XIfE1mE)
-// DARWIN-NOT: comdat
+// CHECK-NOT: comdat
 // CHECK: load i8, i8* bitcast (i64* @_ZGVN1XIfE1mE to i8*)
 // CHECK: %[[XF_M_INITIALIZED:.*]] = icmp eq i8 %{{.*}}, 0
 // CHECK: br i1 %[[XF_M_INITIALIZED]],
 // need init:
 // AIX-NOT: br
 // LINUX: call {{.*}}__cxa_thread_atexit
 // AIX: call {{.*}}__pt_atexit_np
 // DARWIN: call {{.*}}_tlv_atexit

 void set_anon_i() {
   anon_i = 2;
 }
 // LINUX_AIX-LABEL: define internal i32* @_ZTWN12_GLOBAL__N_16anon_iE()
 // DARWIN-LABEL: define internal cxx_fast_tlscc i32* @_ZTWN12_GLOBAL__N_16anon_iE()
 
 // LINUX_AIX: define internal void @[[V_M_INIT]]()
 // DARWIN: define internal cxx_fast_tlscc void @[[V_M_INIT]]()
-// LINUX-SAME: comdat($_ZN1VIiE1mE)
-// DARWIN-NOT: comdat
+// CHECK-NOT: comdat
 // CHECK: load i8, i8* bitcast (i64* @_ZGVN1VIiE1mE to i8*)
 // CHECK: %[[V_M_INITIALIZED:.*]] = icmp eq i8 %{{.*}}, 0
 // CHECK: br i1 %[[V_M_INITIALIZED]],
 // need init:
 // CHECK: call{{.*}} i32 @_Z1gv()
 // CHECK: store i32 %{{.*}}, i32* @_ZN1VIiE1mE, align 4
 // CHECK: store i8 1, i8* bitcast (i64* @_ZGVN1VIiE1mE to i8*)
 // CHECK: br label
 
 // LINUX_AIX: define internal void @[[X_M_INIT]]()
 // DARWIN: define internal cxx_fast_tlscc void @[[X_M_INIT]]()
-// LINUX-SAME: comdat($_ZN1XIiE1mE)
-// DARWIN-NOT: comdat
+// CHECK-NOT: comdat
 // CHECK: load i8, i8* bitcast (i64* @_ZGVN1XIiE1mE to i8*)
 // CHECK: %[[X_M_INITIALIZED:.*]] = icmp eq i8 %{{.*}}, 0
 // CHECK: br i1 %[[X_M_INITIALIZED]],
 // need init:
 // LINUX: call {{.*}}__cxa_thread_atexit
 // AIX: call {{.*}}__pt_atexit_np
 // DARWIN: call {{.*}}_tlv_atexit
 // CHECK: store i8 1, i8* bitcast (i64* @_ZGVN1XIiE1mE to i8*)

clang/test/CodeGenCXX/cxx1z-inline-variables.cpp

 constexpr int Y<int>::a;
 // CHECK: @_ZN1YIiE1bE = linkonce_odr constant i32 456
 const int &yib = Y<int>::b;
 // CHECK-NOT: @_ZN1YIiE1cE
 
 // CHECK-LABEL: define {{.*}}global_var_init
 // CHECK: call i32 @_Z1fv
 
-// CHECK-LABEL: define {{.*}}global_var_init
-// CHECK-NOT: comdat
-// CHECK-SAME: {{$}}
+// CHECK-LABEL: define {{.*}}global_var_init{{.*}} comdat($b)
 // CHECK: load atomic {{.*}} acquire, align
 // CHECK: br
 // CHECK: __cxa_guard_acquire(i64* @_ZGV1b)
 // CHECK: br
 // CHECK: call i32 @_Z1fv
 // CHECK: __cxa_guard_release(i64* @_ZGV1b)
 
 // CHECK-LABEL: define {{.*}}global_var_init

clang/test/OpenMP/threadprivate_codegen.cpp

 // CHECK-TLS1-NEXT:    [[TMP19:%.*]] = load i32, i32* [[RES]], align 4
 // CHECK-TLS1-NEXT:    [[ADD11:%.*]] = add nsw i32 [[TMP19]], [[TMP18]]
 // CHECK-TLS1-NEXT:    store i32 [[ADD11]], i32* [[RES]], align 4
 // CHECK-TLS1-NEXT:    [[TMP20:%.*]] = load i32, i32* [[RES]], align 4
 // CHECK-TLS1-NEXT:    ret i32 [[TMP20]]
 //
 //
 // CHECK-TLS1-LABEL: define {{[^@]+}}@__cxx_global_var_init.3
-// CHECK-TLS1-SAME: () #[[ATTR0]] comdat($_ZN2STI2S4E2stE) {
+// CHECK-TLS1-SAME: () #[[ATTR0]] {
 // CHECK-TLS1-NEXT:  entry:
 // CHECK-TLS1-NEXT:    [[TMP0:%.*]] = load i8, i8* bitcast (i64* @_ZGVN2STI2S4E2stE to i8*), align 8
 // CHECK-TLS1-NEXT:    [[GUARD_UNINITIALIZED:%.*]] = icmp eq i8 [[TMP0]], 0
 // CHECK-TLS1-NEXT:    br i1 [[GUARD_UNINITIALIZED]], label [[INIT_CHECK:%.*]], label [[INIT_END:%.*]]
 // CHECK-TLS1:       init.check:
 // CHECK-TLS1-NEXT:    call void @_ZN2S4C1Ei(%struct.S4* nonnull align 4 dereferenceable(8) @_ZN2STI2S4E2stE, i32 23)
 // CHECK-TLS1-NEXT:    [[TMP1:%.*]] = call i32 @__cxa_thread_atexit(void (i8*)* bitcast (void (%struct.S4*)* @_ZN2S4D1Ev to void (i8*)*), i8* bitcast (%struct.S4* @_ZN2STI2S4E2stE to i8*), i8* @__dso_handle) #[[ATTR3]]
 // CHECK-TLS1-NEXT:    store i8 1, i8* bitcast (i64* @_ZGVN2STI2S4E2stE to i8*), align 8

 // CHECK-TLS2-NEXT:    store %struct.Smain* [[THIS]], %struct.Smain** [[THIS_ADDR]], align 8
 // CHECK-TLS2-NEXT:    [[THIS1:%.*]] = load %struct.Smain*, %struct.Smain** [[THIS_ADDR]], align 8
 // CHECK-TLS2-NEXT:    [[A:%.*]] = getelementptr inbounds [[STRUCT_SMAIN:%.*]], %struct.Smain* [[THIS1]], i32 0, i32 0
 // CHECK-TLS2-NEXT:    store i32 0, i32* [[A]], align 8
 // CHECK-TLS2-NEXT:    ret void
 //
 //
 // CHECK-TLS2-LABEL: define {{[^@]+}}@__cxx_global_var_init.3
-// CHECK-TLS2-SAME: () #[[ATTR6]] comdat($_ZN2STI2S4E2stE) {
+// CHECK-TLS2-SAME: () #[[ATTR6]] {
 // CHECK-TLS2-NEXT:  entry:
 // CHECK-TLS2-NEXT:    [[TMP0:%.*]] = load i8, i8* bitcast (i64* @_ZGVN2STI2S4E2stE to i8*), align 8
 // CHECK-TLS2-NEXT:    [[GUARD_UNINITIALIZED:%.*]] = icmp eq i8 [[TMP0]], 0
 // CHECK-TLS2-NEXT:    br i1 [[GUARD_UNINITIALIZED]], label [[INIT_CHECK:%.*]], label [[INIT_END:%.*]]
 // CHECK-TLS2:       init.check:
 // CHECK-TLS2-NEXT:    call void @_ZN2S4C1Ei(%struct.S4* nonnull align 4 dereferenceable(8) @_ZN2STI2S4E2stE, i32 23)
 // CHECK-TLS2-NEXT:    [[TMP1:%.*]] = call i32 @__cxa_thread_atexit(void (i8*)* bitcast (void (%struct.S4*)* @_ZN2S4D1Ev to void (i8*)*), i8* bitcast (%struct.S4* @_ZN2STI2S4E2stE to i8*), i8* @__dso_handle) #[[ATTR4]]
 // CHECK-TLS2-NEXT:    store i8 1, i8* bitcast (i64* @_ZGVN2STI2S4E2stE to i8*), align 8

 // CHECK-TLS3-NEXT:    [[TMP19:%.*]] = load i32, i32* [[RES]], align 4, !dbg [[DBG286:![0-9]+]]
 // CHECK-TLS3-NEXT:    [[ADD11:%.*]] = add nsw i32 [[TMP19]], [[TMP18]], !dbg [[DBG286]]
 // CHECK-TLS3-NEXT:    store i32 [[ADD11]], i32* [[RES]], align 4, !dbg [[DBG286]]
 // CHECK-TLS3-NEXT:    [[TMP20:%.*]] = load i32, i32* [[RES]], align 4, !dbg [[DBG287:![0-9]+]]
 // CHECK-TLS3-NEXT:    ret i32 [[TMP20]], !dbg [[DBG288:![0-9]+]]
 //
 //
 // CHECK-TLS3-LABEL: define {{[^@]+}}@__cxx_global_var_init.3
-// CHECK-TLS3-SAME: () #[[ATTR0]] comdat($_ZN2STI2S4E2stE) !dbg [[DBG289:![0-9]+]] {
+// CHECK-TLS3-SAME: () #[[ATTR0]] !dbg [[DBG289:![0-9]+]] {
 // CHECK-TLS3-NEXT:  entry:
 // CHECK-TLS3-NEXT:    [[TMP0:%.*]] = load i8, i8* bitcast (i64* @_ZGVN2STI2S4E2stE to i8*), align 8, !dbg [[DBG290:![0-9]+]]
 // CHECK-TLS3-NEXT:    [[GUARD_UNINITIALIZED:%.*]] = icmp eq i8 [[TMP0]], 0, !dbg [[DBG290]]
 // CHECK-TLS3-NEXT:    br i1 [[GUARD_UNINITIALIZED]], label [[INIT_CHECK:%.*]], label [[INIT_END:%.*]], !dbg [[DBG290]]
 // CHECK-TLS3:       init.check:
 // CHECK-TLS3-NEXT:    call void @_ZN2S4C1Ei(%struct.S4* nonnull align 4 dereferenceable(8) @_ZN2STI2S4E2stE, i32 23), !dbg [[DBG291:![0-9]+]]
 // CHECK-TLS3-NEXT:    [[TMP1:%.*]] = call i32 @__cxa_thread_atexit(void (i8*)* bitcast (void (%struct.S4*)* @_ZN2S4D1Ev to void (i8*)*), i8* bitcast (%struct.S4* @_ZN2STI2S4E2stE to i8*), i8* @__dso_handle) #[[ATTR3]], !dbg [[DBG290]]
 // CHECK-TLS3-NEXT:    store i8 1, i8* bitcast (i64* @_ZGVN2STI2S4E2stE to i8*), align 8, !dbg [[DBG290]]

 // CHECK-TLS4-NEXT:    call void @llvm.dbg.declare(metadata %struct.Smain** [[THIS_ADDR]], metadata [[META283:![0-9]+]], metadata !DIExpression()), !dbg [[DBG284:![0-9]+]]
 // CHECK-TLS4-NEXT:    [[THIS1:%.*]] = load %struct.Smain*, %struct.Smain** [[THIS_ADDR]], align 8
 // CHECK-TLS4-NEXT:    [[A:%.*]] = getelementptr inbounds [[STRUCT_SMAIN:%.*]], %struct.Smain* [[THIS1]], i32 0, i32 0, !dbg [[DBG285:![0-9]+]]
 // CHECK-TLS4-NEXT:    store i32 0, i32* [[A]], align 8, !dbg [[DBG287:![0-9]+]]
 // CHECK-TLS4-NEXT:    ret void, !dbg [[DBG288:![0-9]+]]
 //
 //
 // CHECK-TLS4-LABEL: define {{[^@]+}}@__cxx_global_var_init.3
-// CHECK-TLS4-SAME: () #[[ATTR7]] comdat($_ZN2STI2S4E2stE) !dbg [[DBG289:![0-9]+]] {
+// CHECK-TLS4-SAME: () #[[ATTR7]] !dbg [[DBG289:![0-9]+]] {
 // CHECK-TLS4-NEXT:  entry:
 // CHECK-TLS4-NEXT:    [[TMP0:%.*]] = load i8, i8* bitcast (i64* @_ZGVN2STI2S4E2stE to i8*), align 8, !dbg [[DBG290:![0-9]+]]
 // CHECK-TLS4-NEXT:    [[GUARD_UNINITIALIZED:%.*]] = icmp eq i8 [[TMP0]], 0, !dbg [[DBG290]]
 // CHECK-TLS4-NEXT:    br i1 [[GUARD_UNINITIALIZED]], label [[INIT_CHECK:%.*]], label [[INIT_END:%.*]], !dbg [[DBG290]]
 // CHECK-TLS4:       init.check:
 // CHECK-TLS4-NEXT:    call void @_ZN2S4C1Ei(%struct.S4* nonnull align 4 dereferenceable(8) @_ZN2STI2S4E2stE, i32 23), !dbg [[DBG291:![0-9]+]]
 // CHECK-TLS4-NEXT:    [[TMP1:%.*]] = call i32 @__cxa_thread_atexit(void (i8*)* bitcast (void (%struct.S4*)* @_ZN2S4D1Ev to void (i8*)*), i8* bitcast (%struct.S4* @_ZN2STI2S4E2stE to i8*), i8* @__dso_handle) #[[ATTR5]], !dbg [[DBG290]]
 // CHECK-TLS4-NEXT:    store i8 1, i8* bitcast (i64* @_ZGVN2STI2S4E2stE to i8*), align 8, !dbg [[DBG290]]