[PATCH] D98300: [TLS] Initialization functions of dynamic TLS variables cannot be in a comdat (fix for PR48030)

[Wolfgang Pieb via Phabricator via cfe-commits]

wolfgangp created this revision.
wolfgangp added reviewers: cfe-commits, rsmith.
wolfgangp requested review of this revision.

This is an attempt to fix PR48030.

The initialization function of dynamic TLS variables are currently placed into comdats (on ELF at least). The wrapper functions that are used to access these variables call the initializers via an alias, but in TUs where the variables are not used, no wrapper function (and no alias) is generated. At link time it is possible that an initializer function from a TU without a wrapper function is selected, while the wrapper is selected from a different TU. In that case, the alias is undefined.

A possible fix is to not place the initializer function into a comdat. This will lead to some duplication in the final executable, but this can be mitigated by the linker via --gc-sections or the like.

The fix is based on a suggestion by Andrew Ng.


https://reviews.llvm.org/D98300

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/cxx11-thread-local.cpp


Index: clang/test/CodeGenCXX/cxx11-thread-local.cpp
===================================================================
--- clang/test/CodeGenCXX/cxx11-thread-local.cpp
+++ clang/test/CodeGenCXX/cxx11-thread-local.cpp
@@ -176,8 +176,7 @@
 
 // LINUX: 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]],
@@ -189,8 +188,7 @@
 
 // LINUX: 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]],
@@ -277,8 +275,7 @@
 
 // LINUX: 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]],
@@ -290,8 +287,7 @@
 
 // LINUX: 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]],
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2385,7 +2385,10 @@
       // 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)
+      // TLS access wrappers call the initialization function via an alias.
+      // In order to guarantee that the alias is defined, the initialization
+      // function must not be in a comdat.
+      if (!NonTemplateInline && D.getTLSKind() != VarDecl::TLS_Dynamic)
         CGF.CurFn->setComdat(C);
     } else if (CGM.supportsCOMDAT() && guard->isWeakForLinker()) {
       guard->setComdat(CGM.getModule().getOrInsertComdat(guard->getName()));


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D98300.329474.patch
Type: text/x-patch
Size: 2611 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210309/a46fa86e/attachment.bin>