[sanitizer_common] Support Solaris < 11.4 in GetStaticTlsBoundary

[ro]

Summary

This patch, on top of D120048, supports GetTls on Solaris 11.3 and Illumos that lack dlpi_tls_modid. It's the same method originally used in D91605, but integrated into GetStaticTlsBoundary.

Tested on amd64-pc-solaris2.11, sparcv9-sun-solaris2.11, and x86_64-pc-linux-gnu.

[MaskRay]

D120048 decreases the number of lines and looks good to me. But for this one, there is now more complexity... Can we just let old systems (Solaris<=11.3) use *addr = 0; *size = 0;?

[ro]

In D120059#3330337, @MaskRay wrote:

D120048 decreases the number of lines and looks good to me. But for this one, there is now more complexity... Can we just let old systems (Solaris<=11.3) use *addr = 0; *size = 0;?

I don't care much about Solaris 11.3. While I do have patches to make LLVM work, I almost certainly won't submit them. However, even current day Illumos is the same, and just using D120048 will break the build for them due to the use of dlpi_tls_modid. Besides, we still have the matter of distinguishing Solaris and Illumos at build time. The current patch avoids all this.

[MaskRay]

In D120059#3330493, @ro wrote:

In D120059#3330337, @MaskRay wrote:

D120048 decreases the number of lines and looks good to me. But for this one, there is now more complexity... Can we just let old systems (Solaris<=11.3) use *addr = 0; *size = 0;?

I don't care much about Solaris 11.3. While I do have patches to make LLVM work, I almost certainly won't submit them. However, even current day Illumos is the same, and just using D120048 will break the build for them due to the use of dlpi_tls_modid. Besides, we still have the matter of distinguishing Solaris and Illumos at build time. The current patch avoids all this.

Oh, thanks. Then perhaps adjust the code to work for Solaris 11.4 and let Illumos/Solaris 11.3 use the *addr = 0; *size = 0; fallback? That will have minimum code...

I hope we can get rid of main_tls_modid = 1; and rt_tlsmodid...

[ro]

In D120059#3330523, @MaskRay wrote:

Oh, thanks. Then perhaps adjust the code to work for Solaris 11.4 and let Illumos/Solaris 11.3 use the *addr = 0; *size = 0; fallback? That will have minimum code...

That would effectively mean using D120048 amended to use some SANITIZER_ILLUMOS to distinguish the two. Currently, we don't have that unfortunately, as discussed in D119695. We'd also need to disable the ld -z relax=transtls workaround in that patch, but haven't a way to distinguish Solaris and Illumos in cmake files either.

I have an idea how to get __illumos__ defined (using uname -o in llvm/CMakeLists.txt) to handle the former, but in the end the Illumos community will have to come up with a solution that works for both Solaris and Illumos.

I hope we can get rid of main_tls_modid = 1; and rt_tlsmodid...

Fully agreed that's ugly, but at least it works for everyone and has been proven so in GCC. I'm certainly not vested in it, though, given that I primarily care about Solaris 11.4 where the common code just works.

[MaskRay]

In D120059#3330715, @ro wrote:

In D120059#3330523, @MaskRay wrote:

Oh, thanks. Then perhaps adjust the code to work for Solaris 11.4 and let Illumos/Solaris 11.3 use the *addr = 0; *size = 0; fallback? That will have minimum code...

That would effectively mean using D120048 amended to use some SANITIZER_ILLUMOS to distinguish the two. Currently, we don't have that unfortunately, as discussed in D119695. We'd also need to disable the ld -z relax=transtls workaround in that patch, but haven't a way to distinguish Solaris and Illumos in cmake files either.

I have an idea how to get __illumos__ defined (using uname -o in llvm/CMakeLists.txt) to handle the former, but in the end the Illumos community will have to come up with a solution that works for both Solaris and Illumos.

I hope we can get rid of main_tls_modid = 1; and rt_tlsmodid...

Fully agreed that's ugly, but at least it works for everyone and has been proven so in GCC. I'm certainly not vested in it, though, given that I primarily care about Solaris 11.4 where the common code just works.

Glad to know what you only care about Solaris>=11.4:)
It seems that D84046 is the first compiler-rt commit with llumos in the description. So I think compiler-rt has never officially supported illumnos.
Perhaps we should ask them to fix things before adding workarounds to compiler-rt?
Perhaps we can set up a milestone: they need to at least support __illumos__ and dlpi_tls_modid before adding the port to compiler-rt...

[ro]

In D120059#3331363, @MaskRay wrote:

Glad to know what you only care about Solaris>=11.4:)

Sure: I'm keeping support in GCC since the 11.3/11.4 differences are useful in a couple of places and the effort is small. LLVM is different, though, given that Solaris support is only slowly coming.

It seems that D84046 is the first compiler-rt commit with llumos in the description. So I think compiler-rt has never officially supported illumnos.

Seems so: even the clang 13 in their oi-userland repo, which has some local patches, doesn't build compiler-rt.

Perhaps we should ask them to fix things before adding workarounds to compiler-rt?

Seems like a plan: instead of me constantly worrying about breaking the build for them without a sensible way for a workaround, the ball is in their court. I certainly don't intend to cause them problems and will continue to point out potential issues in my patches as far as I'm aware of them.

Perhaps we can set up a milestone: they need to at least support __illumos__ and dlpi_tls_modid before adding the port to compiler-rt...

Even the former would be enough as a first step: in that case, we can use the *addr = 0; *size = 0; fallback (though some sort of isOSIllumos() would be needed in clang to avoid using ld -z relax=transtls which they lack).

As a first step, I've filed Issue #53919 so what needs to be done can be kept in one place rather than spread over several reviews.

[ro]

@MaskRay: With this patch effectively abandoned and the Illumos situation in their court, are you ok with D120048 going in now as is?

[MaskRay]

In D120059#3335287, @ro wrote:

@MaskRay: With this patch effectively abandoned and the Illumos situation in their court, are you ok with D120048 going in now as is?

Yes!

[ro]

Abandoned unless absolutely necessary for Illumos support. As discussed, a way better alternative would be for them to implement dlpi_tls_modid or at least provide a way to distuingish Illumos from Solaris as outlined in Issue #53919.

[ro]

In D120059#3338258, @ro wrote:

Abandoned unless absolutely necessary for Illumos support. As discussed, a way better alternative would be for them to implement dlpi_tls_modid or at least provide a way to distuingish Illumos from Solaris as outlined in Issue #53919.

Current compiler-rt was just integrated into GCC trunk, which broke the build on Solaris 11.3 for the lack of dlpi_tls_modid. S11.3 is still supported in GCC (and will remain so for quite some time), so I guess the best option forward is to reactivate this patch? @MaskRay?

[ro]

Minor formatting change.

[ro]

As mentioned before, I'd like to revive this patch (although there's no Action in Phabricator for this AFAICS): importing upstream sanitizers into GCC libsanitizer broke the Solaris 11.3 build, which is still supported in GCC. As a side effect, it would also unbreak (at least in this regard) compiler-rt on Illumos.

Retested on amd64-pc-solaris2.11, sparcv9-sun-solaris2.11 (Solaris 11.4), and x86_64-pc-linux-gnu; as well as the same targets inside the GCC tree (also for Solaris 11.3 there).

[MaskRay]

compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp

361

Instead of having two chunks of comments, just have one saying that the #if SANITIZER_SOLARIS branch is for Solaris < 11.4 compatibility?

[ro]

compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp

361

That's way clearer indeed.

[ro]

Merge comments.

compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp

 #include "sanitizer_file.h"
 #include "sanitizer_flags.h"
 #include "sanitizer_freebsd.h"
 #include "sanitizer_getauxval.h"
 #include "sanitizer_glibc_version.h"
 #include "sanitizer_linux.h"
 #include "sanitizer_placement_new.h"
 #include "sanitizer_procmaps.h"
+#include "sanitizer_solaris.h"
 
 #if SANITIZER_NETBSD
 #define _RTLD_SOURCE  // for __lwp_gettcb_fast() / __lwp_getprivate_fast()
 #endif
 
 #include <dlfcn.h>  // for dlsym()
 #include <link.h>
 #include <pthread.h>

 
 #if SANITIZER_NETBSD
 #include <sys/sysctl.h>
 #include <sys/tls.h>
 #include <lwp.h>
 #endif
 
 #if SANITIZER_SOLARIS
+#include <stddef.h>
 #include <stdlib.h>
 #include <thread.h>
 #endif
 
 #if SANITIZER_ANDROID
 #include <android/api-level.h>
 #if !defined(CPU_COUNT) && !defined(__aarch64__)
 #include <dirent.h>
asm("basr %%r14, %[__tls_get_offset]"
       : [__tls_get_offset] "r"(__tls_get_offset), "r"(r12)
       : "memory", "cc", "0", "1", "3", "4", "5", "14");
   return r2;
 }
 #else
 extern "C" void *__tls_get_addr(size_t *);
 #endif
 
+static size_t main_tls_modid;
+
 static int CollectStaticTlsBlocks(struct dl_phdr_info *info, size_t size,
                                   void *data) {
-  if (!info->dlpi_tls_modid)
+  size_t tls_modid;
+#if SANITIZER_SOLARIS
+  // With the introduction of dlpi_tls_modid, the tlsmodid of the executable
+  // was changed to 1 to match other implementations.
+  if (size >= offsetof(dl_phdr_info_test, dlpi_tls_modid))
+    main_tls_modid = 1;
+  else
+    main_tls_modid = 0;
+  // dlpi_tls_modid is only available since Solaris 11.4 SRU 10.  Use
+  // dlinfo(RTLD_DI_LINKMAP) instead which works on all of Solaris 11.3,
+  // 11.4, and Illumos.
+  Rt_map *map;
+  dlinfo(RTLD_SELF, RTLD_DI_LINKMAP, &map);
+  tls_modid = map->rt_tlsmodid;
+#else
+  main_tls_modid = 1;
+  tls_modid = info->dlpi_tls_modid;
+#endif
+
+  if (tls_modid < main_tls_modid)
     return 0;
   uptr begin = (uptr)info->dlpi_tls_data;
   if (!g_use_dlpi_tls_data) {
     // Call __tls_get_addr as a fallback. This forces TLS allocation on glibc
     // and FreeBSD.
 #ifdef __s390__
     begin = (uptr)__builtin_thread_pointer() +
-            TlsGetOffset(info->dlpi_tls_modid, 0);
+            TlsGetOffset(tls_modid, 0);
 #else
-    size_t mod_and_off[2] = {info->dlpi_tls_modid, 0};
+    size_t mod_and_off[2] = {tls_modid, 0};
     begin = (uptr)__tls_get_addr(mod_and_off);
 #endif
   }
   for (unsigned i = 0; i != info->dlpi_phnum; ++i)
     if (info->dlpi_phdr[i].p_type == PT_TLS) {
       static_cast<InternalMmapVector<TlsBlock> *>(data)->push_back(
           TlsBlock{begin, begin + info->dlpi_phdr[i].p_memsz,
-                   info->dlpi_phdr[i].p_align, info->dlpi_tls_modid});
+                   info->dlpi_phdr[i].p_align, tls_modid});
       break;
     }
   return 0;
 }
 
 __attribute__((unused)) static void GetStaticTlsBoundary(uptr *addr, uptr *size,
                                                          uptr *align) {
   InternalMmapVector<TlsBlock> ranges;
   dl_iterate_phdr(CollectStaticTlsBlocks, &ranges);
   uptr len = ranges.size();
   Sort(ranges.begin(), len);
-  // Find the range with tls_modid=1. For glibc, because libc.so uses PT_TLS,
-  // this module is guaranteed to exist and is one of the initially loaded
-  // modules.
+  // Find the range with tls_modid == main_tls_modid. For glibc, because
+  // libc.so uses PT_TLS, this module is guaranteed to exist and is one of
+  // the initially loaded modules.
   uptr one = 0;
-  while (one != len && ranges[one].tls_modid != 1) ++one;
+  while (one != len && ranges[one].tls_modid != main_tls_modid) ++one;
   if (one == len) {
     // This may happen with musl if no module uses PT_TLS.
     *addr = 0;
     *size = 0;
     *align = 1;
     return;
   }
   // Find the maximum consecutive ranges. We consider two modules consecutive if

compiler-rt/lib/sanitizer_common/sanitizer_solaris.h

+//===-- sanitizer_solaris.h -------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file is a part of Sanitizer runtime. It contains Solaris-specific
+// definitions.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef SANITIZER_SOLARIS_H
+#define SANITIZER_SOLARIS_H
+
+#include "sanitizer_internal_defs.h"
+
+#if SANITIZER_SOLARIS
+
+#include <link.h>
+
+namespace __sanitizer {
+
+// Beginning of declaration from OpenSolaris/Illumos
+// $SRC/cmd/sgs/include/rtld.h.
+struct Rt_map {
+  Link_map rt_public;
+  const char *rt_pathname;
+  ulong_t rt_padstart;
+  ulong_t rt_padimlen;
+  ulong_t rt_msize;
+  uint_t rt_flags;
+  uint_t rt_flags1;
+  ulong_t rt_tlsmodid;
+};
+
+// Structure matching the Solaris 11.4 struct dl_phdr_info used to determine
+// presence of dlpi_tls_modid field at runtime.  Cf. Solaris 11.4
+// dl_iterate_phdr(3C), Example 2.
+struct dl_phdr_info_test {
+  ElfW(Addr) dlpi_addr;
+  const char *dlpi_name;
+  const ElfW(Phdr) * dlpi_phdr;
+  ElfW(Half) dlpi_phnum;
+  u_longlong_t dlpi_adds;
+  u_longlong_t dlpi_subs;
+  size_t dlpi_tls_modid;
+  void *dlpi_tls_data;
+};
+
+}  // namespace __sanitizer
+
+#endif  // SANITIZER_SOLARIS
+
+#endif  // SANITIZER_SOLARIS_H