On Wed, Dec 4, 2013 at 5:02 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> On Wed, Dec 04, 2013 at 04:49:22PM +0400, Konstantin Serebryany wrote:
>> I would start from kernel version and glibc version, this should cover
>> the majority of use cases.
>
> 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.
> contain (where needed) wrappers for kernel headers or their replacements
> to make sure things compile, if you don't care about it in the compiler-rt
> tree. But for the ppc32 stuff, we can't avoid modifying sanitizer_common
> (the first patch I've posted recently, btw, I wonder if it works on sparc*,
> we'll need to wait for somebody to test it), or e.g. for the .cfi_* stuff
> (if you just apply the the .cfi_* related part of the patch I've posted
> with say the macros __GCC_HAVE_* replaced by SANITIZER_USE_CFI_DIRECTIVES
> or similar, I guess we can provide the right definition for that outside of
> the compiler-rt maintained files.
.cfi is used only in tsan sources now, and tsan is not supported
anywhere but x86_64
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.
> 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.
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)?
Index: sanitizer_linux_libcdep.cc
===================================================================
--- 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.
#endif
uptr ThreadDescriptorSize() {
@@ -255,7 +255,7 @@
*stk_addr = stack_bottom;
*stk_size = stack_top - stack_bottom;
- if (!main) {
+ if (!main && kThreadDescriptorSize) {
// If stack and tls intersect, make them non-intersecting.
if (*tls_addr > *stk_addr && *tls_addr < *stk_addr + *stk_size) {
CHECK_GT(*tls_addr + *tls_size, *stk_addr);
Index: tests/sanitizer_linux_test.cc
===================================================================
--- tests/sanitizer_linux_test.cc (revision 196375)
+++ tests/sanitizer_linux_test.cc (working copy)
@@ -224,6 +224,7 @@
TEST(SanitizerLinux, ThreadDescriptorSize) {
pthread_t tid;
+ if (!ThreadDescriptorSize()) return;
void *result;
ASSERT_EQ(0, pthread_create(&tid, 0, thread_descriptor_size_test_func, 0));
ASSERT_EQ(0, pthread_join(tid, &result));
<grumbling>
If I had a buildbot with "old" Fedora, I would simply submit the
change and see if it broke/fixed it.
</grumbling>
--kcc
>
> Jakub
