[PATCH] D48255: [sanitizer] Fix LSAN for 32-bit glibc before 2.27

[Peter Wu via Phabricator via llvm-commits]

Lekensteyn added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_linux_libcdep.cc:189
+# define CHECK_GET_TLS_STATIC_INFO_VERSION 1
 #else
 # define CHECK_GET_TLS_STATIC_INFO_VERSION 0
----------------
jakubjelinek wrote:
> Why this hunk?  This will unnecessary forever pessimize sanitizers built against glibc 2.27+.
Only 32-bit builds are affected and the overhead is neglible as InitTlsSize is called only *once* per process. The advantage of this dynamic approach is that 32-bit programs can still be copied to a different OS with older glibc and execute without *unconditionally* crashing.

The previous code used `dlvsym` to perform the version check, but as I now call `gnu_get_libc_version` directly for GNU libc in D48254, its overhead is pretty small.

Given these circumstances, I think it is reasonable to unconditionally check the version.

If you are interested in some numbers,
line 220 required 2858 instructions (the dlsym call) while
line 226 took 1722 instructions (retrieval of the version and parsing it).
Line 230 took 1806 instructions (as measured with `record full` and `info record` in `gdb` and stepping with `next`).


https://reviews.llvm.org/D48255