[PATCH] D44623: Fix asan on i?86-linux (32-bit) against glibc 2.27 and later

[Jakub Jelínek via Phabricator via llvm-commits]

jakubjelinek added inline comments.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cc:177
+#ifndef __GLIBC_PREREQ
+#define __GLIBC_PREREQ(x, y) 0
+#endif
----------------
vitalybuka wrote:
> Do we need to check for this?
> Maybe just always go for dlvsym(RTLD_NEXT, "glob", "GLIBC_2.27") ?
It is not strictly needed, just an optimization.  At least in GCC, if somebody configures the compiler against some version of binutils, glibc etc., we assume that at runtime that version or newer of it is available, so it is an optimization; if somebody configures against glibc 2.27+, it avoids the dlvsym calls on startup of every sanitized app.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cc:190
+  void *get_tls_static_info_ptr = dlsym(RTLD_NEXT, "_dl_get_tls_static_info");
+#if defined(__i386__) && !__GLIBC_PREREQ(2, 27)
+  /* On i?86, _dl_get_tls_static_info used to be internal_function, i.e.
----------------
vitalybuka wrote:
> kcc wrote:
> > nested ifdefs? I'd prefer to avoid them. Move this code into a separate function, maybe? 
> maybe?
> ```
> template<typename GetTlsType>
> void GetTlsImpl() {
>    void *get_tls_static_info_ptr = dlsym(RTLD_NEXT, "_dl_get_tls_static_info");
>    GetTlsType get_tls;
>     CHECK_EQ(sizeof(get_tls), sizeof(get_tls_static_info_ptr));
>     internal_memcpy(&get_tls, &get_tls_static_info_ptr,
>                     sizeof(get_tls_static_info_ptr));
>     CHECK_NE(get_tls, 0);
>     get_tls(&tls_size, &tls_align);
> }
> 
> static void GetTls() {
> #if defined(__i386__)
>    if (!dlvsym(RTLD_NEXT, "glob", "GLIBC_2.27"))
>      return GetTlsImpl<>;
> #endif
>  return GetTlsImpl<>
> }
> 
> ```
Attributes on function pointers passed to templates have historically been problematic in several compilers, so I think it is better to avoid that.
E.g. clang++ 6.0.0 r319306 rejects:

```
typedef void (*F) (int) __attribute__((regparm (3), stdcall));
typedef void (*G) (int);

template <typename T>
void foo (T x)
{
  x (0);
}

void
bar (F x, G y)
{
  foo (x);
  foo (y);
}
```
g++ before 5 emit 2 different instantiations with the same mangled name and so it fails to compile, g++ 5 ICEs on it, only 6+ handles it right and provide different mangled names for both.
So, if it would be a template, it would need to have 2 different classes that would contain say a typedef in them and that is what the template would call.


Repository:
  rL LLVM

https://reviews.llvm.org/D44623