sanitizer_common: Use plain thread_local for __sancov_lowest_stack definition.

[pcc]

Summary

We can't use initial-exec for the definition of __sancov_lowest_stack
because it gets linked into shared libraries such as the UBSan and
HWASan runtimes on Android. I think we can expect plain thread_local
to work unconditionally in sanitizer_common as several other sanitizers
are already using it, so drop the platform-dependent macro and just use
thread_local in the definition.

[hctim]

Any ideas why this was IE in the first place? Don't see any notes in the patch that added it (https://reviews.llvm.org/D37156)?

[vitalybuka]

I guess Android was that main why we didn't use it before. Probably older API levels. Probably our buildbot is still on them. Wide use of thread_local may simplify sanitizers.

As I see thread_local is used in msan, which is not on Android, fuzzers, and asan fuchsia and netbsd.
However it's used in scudo, which is on Android

+Apple folks for any concerns.

[yln]

Hi, thanks Vitaly!

AFAIR, thread_local (and TLS in general) require macOS 10.12 / iOS 10 / watchOS 3 or greater.

Therefore, can we hold off on this a bit?

@thetruestblue is looking into bumping the minimal sanitizer deployment targets for Apple platforms. Blue, when you do that, can you circle back here and apply this patch? This change is a good example for a sanity check: it doesn't work right now but should work afterwards.

[pcc]

In D146351#4207181, @yln wrote:

Hi, thanks Vitaly!

AFAIR, thread_local (and TLS in general) require macOS 10.12 / iOS 10 / watchOS 3 or greater.

Therefore, can we hold off on this a bit?

@thetruestblue is looking into bumping the minimal sanitizer deployment targets for Apple platforms. Blue, when you do that, can you circle back here and apply this patch? This change is a good example for a sanity check: it doesn't work right now but should work afterwards.

__sancov_lowest_stack is only used by the sancov instrumentation pass when -fsanitize-coverage=stack-depth is passed, which is not the default. This option can't possibly work if SANITIZER_TLS_INITIAL_EXEC_ATTRIBUTE was defined to an empty string, because the instrumentation pass will unconditionally define the variable with TLS: https://github.com/llvm/llvm-project/blob/f67b481098cc30567d0f50a2b21f8f57b92052bd/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp#L489

So I think this is broken on the platforms where SANITIZER_TLS_INITIAL_EXEC_ATTRIBUTE is an empty string. That means we can wrap the definition of __sancov_lowest_stack in an #if !SANITIZER_APPLE and the code will work on Apple platforms as well as it did before (i.e. the runtime will build, but the compiler flag won't work). I'll update the patch to do that.

[pcc]

Add #if !SANITIZER_APPLE

compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cpp

 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_pcs_init, const uptr* beg,
                              const uptr* end) {
   __sancov::SingletonCounterCoverage::CovPcsInit(beg, end);
 }
 }  // extern "C"
 // Weak definition for code instrumented with -fsanitize-coverage=stack-depth
 // and later linked with code containing a strong definition.
 // E.g., -fsanitize=fuzzer-no-link
+// FIXME: Update Apple deployment target so that thread_local is always
+// supported, and remove the #if.
+#if !SANITIZER_APPLE
 SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE
-SANITIZER_TLS_INITIAL_EXEC_ATTRIBUTE uptr __sancov_lowest_stack;
+thread_local uptr __sancov_lowest_stack;
+#endif
 
 #endif  // !SANITIZER_FUCHSIA

compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h

 #elif SANITIZER_GO
 # define SANITIZER_INTERFACE_ATTRIBUTE
 # define SANITIZER_WEAK_ATTRIBUTE
 #else
 # define SANITIZER_INTERFACE_ATTRIBUTE __attribute__((visibility("default")))
 # define SANITIZER_WEAK_ATTRIBUTE  __attribute__((weak))
 #endif
 
-// TLS is handled differently on different platforms
-#if SANITIZER_LINUX || SANITIZER_NETBSD || \
-  SANITIZER_FREEBSD
-# define SANITIZER_TLS_INITIAL_EXEC_ATTRIBUTE \
-    __attribute__((tls_model("initial-exec"))) thread_local
-#else
-# define SANITIZER_TLS_INITIAL_EXEC_ATTRIBUTE
-#endif
-
 //--------------------------- WEAK FUNCTIONS ---------------------------------//
 // When working with weak functions, to simplify the code and make it more
 // portable, when possible define a default implementation using this macro:
 //
 // SANITIZER_INTERFACE_WEAK_DEF(<return_type>, <name>, <parameter list>)
 //
 // For example:
 //   SANITIZER_INTERFACE_WEAK_DEF(bool, compare, int a, int b) { return a > b; }