[patch] Make _thread_db_sizeof_pthread public for Sanitizers

[patch] Make _thread_db_sizeof_pthread public for Sanitizers

[Jan Kratochvil]

Sanitizers currently contain ugly list of glibc versions and their
sizeof(struct pthread).
	https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp#L276

This list is not much maintained causing SEGVs of Sanitizers:
	$ echo 'int main(){}'|clang -g -fsanitize=leak -x c++ -;./a.out
	Tracer caught signal 11: addr=0x7f1087f51f40 pc=0x4222c8 sp=0x7f1086effd40
	==234624==LeakSanitizer has encountered a fatal error.
	==234624==HINT: For debugging, try setting environment variable LSAN_OPTIONS=verbosity=1:log_threads=1
	==234624==HINT: LeakSanitizer does not work under ptrace (strace, gdb, etc)

I would find better if just glibc made the value public, Sanitizers can then
read it by dlsym():
	http://people.redhat.com/jkratoch/lsan-pthread.patch

diff --git a/nptl/Versions b/nptl/Versions
index aed118e717..4144acbac7 100644
--- a/nptl/Versions
+++ b/nptl/Versions
@@ -297,6 +297,10 @@ libpthread {
     pthread_clockjoin_np;
   }
 
+  GLIBC_2.33 {
+    _thread_db_sizeof_pthread;
+  }
+
   GLIBC_PRIVATE {
     __pthread_initialize_minimal;
     __pthread_clock_gettime; __pthread_clock_settime;

Re: [patch] Make _thread_db_sizeof_pthread public for Sanitizers

[Florian Weimer]

* Jan Kratochvil via Libc-alpha:

> Sanitizers currently contain ugly list of glibc versions and their
> sizeof(struct pthread).
> 	https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp#L276
>
> This list is not much maintained causing SEGVs of Sanitizers:
> 	$ echo 'int main(){}'|clang -g -fsanitize=leak -x c++ -;./a.out
> 	Tracer caught signal 11: addr=0x7f1087f51f40 pc=0x4222c8 sp=0x7f1086effd40
> 	==234624==LeakSanitizer has encountered a fatal error.
> 	==234624==HINT: For debugging, try setting environment variable LSAN_OPTIONS=verbosity=1:log_threads=1
> 	==234624==HINT: LeakSanitizer does not work under ptrace (strace, gdb, etc)
>
> I would find better if just glibc made the value public, Sanitizers can then
> read it by dlsym():

Do you know why the GetTLS function needs to know the size of the
thread descriptor?  And why it adds it to the start address of the TLS
area, without subtracting it from the area size?  I think this
identifies the wrong memory region as TLS.

Re: [patch] Make _thread_db_sizeof_pthread public for Sanitizers

[Carlos O'Donell]

On 1/1/21 7:42 AM, Florian Weimer wrote:
> * Jan Kratochvil via Libc-alpha:
> 
>> Sanitizers currently contain ugly list of glibc versions and their
>> sizeof(struct pthread).
>> 	https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp#L276
>>
>> This list is not much maintained causing SEGVs of Sanitizers:
>> 	$ echo 'int main(){}'|clang -g -fsanitize=leak -x c++ -;./a.out
>> 	Tracer caught signal 11: addr=0x7f1087f51f40 pc=0x4222c8 sp=0x7f1086effd40
>> 	==234624==LeakSanitizer has encountered a fatal error.
>> 	==234624==HINT: For debugging, try setting environment variable LSAN_OPTIONS=verbosity=1:log_threads=1
>> 	==234624==HINT: LeakSanitizer does not work under ptrace (strace, gdb, etc)
>>
>> I would find better if just glibc made the value public, Sanitizers can then
>> read it by dlsym():
> 
> Do you know why the GetTLS function needs to know the size of the
> thread descriptor?  And why it adds it to the start address of the TLS
> area, without subtracting it from the area size?  I think this
> identifies the wrong memory region as TLS.
 
This also seems like a use case for GLIBC_DEBUG (available via dlsym,
with no copy relocs). However, like you, I'd like to know why the size
of the descriptor is needed (XY problem).

-- 
Cheers,
Carlos.

Re: [patch] Make _thread_db_sizeof_pthread public for Sanitizers

[Jan Kratochvil]

On Fri, 01 Jan 2021 13:42:43 +0100, Florian Weimer wrote:
> Do you know why the GetTLS function needs to know the size of the
> thread descriptor?  And why it adds it to the start address of the TLS
> area, without subtracting it from the area size?  I think this
> identifies the wrong memory region as TLS.

I do not know the memory layout of glibc TLSes (all of their kinds there are).

I just find my proposed fix a better one than to play the catch-up with each
glibc version. If you can find some better fix I sure welcome it.

https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp#L468
https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp#L237
=
static void GetTls(uptr *addr, uptr *size) {
#if defined(__x86_64__) || defined(__i386__) || defined(__s390__)
  *addr = ThreadSelf();
  *size = GetTlsSize(); // get_tls_static_info_ptr()->dl_tls_static_size
  *addr -= *size;
  *addr += ThreadDescriptorSize();


Jan

Re: [patch] Make _thread_db_sizeof_pthread public for Sanitizers

[Florian Weimer]

I've opned a sanitizer issue about this:

  Sanitizer requirements related to glibc thread descriptor/control block size
  <https://github.com/google/sanitizers/issues/1382>

Thanks,
Florian