On Wed, Dec 4, 2013 at 5:44 PM, Jakub Jelinek <ja...@redhat.com> wrote: > 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.
replied separately. > >> 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. Sure it can. But all my previous grumbling about maintenance cost and our inability to test changes, etc applies here. > 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? Mmm. I *think* yes, today this is lsan-only. > 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. My patch above should remove the assertion on < 2.13 > >> 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. precise is better, otherwise we may lose leaks. > > Jakub
