Re: libsanitizer merge from upstream r196090

[Jakub Jelinek <jakub at redhat dot com>]

On Wed, Dec 04, 2013 at 05:28:40PM +0400, Konstantin Serebryany wrote:
> > Well, for the kernel headers what we perhaps can do is just add
> > libsanitizer/include/linux/ tree that will be maintained by GCC and will
> 
> if that works for you, no objections.

I haven't tried to do that yet, so don't know how much work it will be,
but at least from the second patch posted recently it it might work fine, at
least for now.

> .cfi is used only in tsan sources now, and tsan is not supported
> anywhere but x86_64

But the .cfi_* issue is platform independent.  Whether the compiler
decides to emit them or not depends on how it was configured, on assembler
and on compiler flags.
I don't see how it can be a maintainance problem to just guard the few
(right now two) .cfi_* occurrences in the C++ sources, or using CFI_* macros
instead of .cfi_* directives directly in the assembly file.
Other projects (e.g. glibc) manage to do that for years without any trouble.

> ppc32 never worked (last time I tried there were several different
> issues so we disabled 32-bit build)
> -- we should just disable it in GCC too. There is not value in
> building code that does not run.

That doesn't mean it can't be made to work, and the patch I've posted is
at least an (IMHO correct) step towards that.  Note, I had just much bigger
problems on ppc64 with the addr2line symbolization because of the ppc64
opd/plt stuff, though supposedly that might go away once I patch
libsanitizer to use libbacktrace for symbolization.
There is no inherent reason why ppc32 wouldn't work and ppc64 would, after
all ppc64 with its weirdo function descriptor stuff is much harder to
support.

> > Regarding the TLS size, can you e.g. just only do it for glibc 2.13 and
> > later, rather than having an (even for glibc 2.11/2.12 incorrect) values for
> > older glibcs?
> 
> That would work for me, although it may bring some surprises later.
> If we incorrectly compute the tls boundaries, lsan my produce false
> positives or false negatives.

But is that solely for lsan and nothing else?  Because, the assertion
was failing in asan tests, without any asan options to request leak
checking.  And for non-i?86/x86_64 you ignore the tls boundaries too.

> Having kThreadDescriptorSize=0 means that we include the stack
> descriptor in the lsan's root set and thus
> may miss a leak (with rather low probability). I can live with this.
> 
> Like this (tested only on my box)?

> --- sanitizer_linux_libcdep.cc  (revision 196375)
> +++ sanitizer_linux_libcdep.cc  (working copy)
> @@ -207,12 +207,12 @@
> 
>  #if defined(__x86_64__) || defined(__i386__)
>  // sizeof(struct thread) from glibc.
> -// There has been a report of this being different on glibc 2.11 and 2.13. We
> -// don't know when this change happened, so 2.14 is a conservative estimate.
> -#if __GLIBC_PREREQ(2, 14)
> +// This may change between glibc versions, we only support the versions we know
> +// avout (>= 2.13). For others we set kThreadDescriptorSize to 0.
> +#if __GLIBC_PREREQ(2, 13)
>  const uptr kThreadDescriptorSize = FIRST_32_SECOND_64(1216, 2304);
>  #else
> -const uptr kThreadDescriptorSize = FIRST_32_SECOND_64(1168, 2304);
> +const uptr kThreadDescriptorSize = 0;  // Unknown.

Depends on (as I've asked earlier) on if you need the exact precise
value or if say conservatively smaller value is fine.  Then you could
say for glibc >= 2.5 pick the minimum of the values I've gathered.

	Jakub