| Message ID | 20160517092323.GH28550@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
Hi Jakub, thanks for backporting this! Do you have any plans to apply this patch to GCC 5 and 6 branches? AFAIK people hit on this ASan + newer Glibc bug by using GCC 5.3.1 on Fedora 23. On 17/05/16 12:23, Jakub Jelinek wrote: > Hi! > > This is a cherry-pick of upstream fix, so that dlsym can call not just > calloc, but also malloc or realloc, even before asan is initialized. > > Tested on x86_64-linux, committed so far to trunk. > > IMHO even better would be to make sure that in the common case (recent > glibc) we don't have failed dlsym calls (still, this hack is useful just in > case) - the __isoc99_*printf* interceptors make no sense, glibc has never > exported those. Thus, if we bump ABI of libasan again for GCC 7, IMHO those > bogus interceptors should be ifdefed out for glibc or removed completely. > Or, if we don't want to break ABI, at least changed so that they actuall > dlsym the corresponding *printf* (not __isoc99_ prefixed) functions > instead of the bogus ones. We should definitely bump libasan version on next libsanitizer merge, because it would contain ABI breaking changes in ASan. Perhaps we could ifdef these __isoc99 interceptors as a local GCC patch then? > 2016-05-17 Jakub Jelinek <jakub@redhat.com> > > PR sanitizer/71160 > * asan/asan_malloc_linux.cc: Cherry pick upstream r254395 > and r269633. > > --- libsanitizer/asan/asan_malloc_linux.cc.jj 2014-09-24 11:08:01.772039066 +0200 > +++ libsanitizer/asan/asan_malloc_linux.cc 2016-05-17 11:02:37.859379380 +0200 > @@ -24,39 +24,62 @@ > // ---------------------- Replacement functions ---------------- {{{1 > using namespace __asan; // NOLINT > > +static uptr allocated_for_dlsym; > +static const uptr kDlsymAllocPoolSize = 1024; > +static uptr alloc_memory_for_dlsym[kDlsymAllocPoolSize]; > + > +static bool IsInDlsymAllocPool(const void *ptr) { > + uptr off = (uptr)ptr - (uptr)alloc_memory_for_dlsym; > + return off < sizeof(alloc_memory_for_dlsym); > +} > + > +static void *AllocateFromLocalPool(uptr size_in_bytes) { > + uptr size_in_words = RoundUpTo(size_in_bytes, kWordSize) / kWordSize; > + void *mem = (void*)&alloc_memory_for_dlsym[allocated_for_dlsym]; > + allocated_for_dlsym += size_in_words; > + CHECK_LT(allocated_for_dlsym, kDlsymAllocPoolSize); > + return mem; > +} > + > INTERCEPTOR(void, free, void *ptr) { > GET_STACK_TRACE_FREE; > + if (UNLIKELY(IsInDlsymAllocPool(ptr))) > + return; > asan_free(ptr, &stack, FROM_MALLOC); > } > > INTERCEPTOR(void, cfree, void *ptr) { > GET_STACK_TRACE_FREE; > + if (UNLIKELY(IsInDlsymAllocPool(ptr))) > + return; > asan_free(ptr, &stack, FROM_MALLOC); > } > > INTERCEPTOR(void*, malloc, uptr size) { > + if (UNLIKELY(!asan_inited)) > + // Hack: dlsym calls malloc before REAL(malloc) is retrieved from dlsym. > + return AllocateFromLocalPool(size); > GET_STACK_TRACE_MALLOC; > return asan_malloc(size, &stack); > } > > INTERCEPTOR(void*, calloc, uptr nmemb, uptr size) { > - if (UNLIKELY(!asan_inited)) { > + if (UNLIKELY(!asan_inited)) > // Hack: dlsym calls calloc before REAL(calloc) is retrieved from dlsym. > - const uptr kCallocPoolSize = 1024; > - static uptr calloc_memory_for_dlsym[kCallocPoolSize]; > - static uptr allocated; > - uptr size_in_words = ((nmemb * size) + kWordSize - 1) / kWordSize; > - void *mem = (void*)&calloc_memory_for_dlsym[allocated]; > - allocated += size_in_words; > - CHECK(allocated < kCallocPoolSize); > - return mem; > - } > + return AllocateFromLocalPool(nmemb * size); > GET_STACK_TRACE_MALLOC; > return asan_calloc(nmemb, size, &stack); > } > > INTERCEPTOR(void*, realloc, void *ptr, uptr size) { > GET_STACK_TRACE_MALLOC; > + if (UNLIKELY(IsInDlsymAllocPool(ptr))) { > + uptr offset = (uptr)ptr - (uptr)alloc_memory_for_dlsym; > + uptr copy_size = Min(size, kDlsymAllocPoolSize - offset); > + void *new_ptr = asan_malloc(size, &stack); > + internal_memcpy(new_ptr, ptr, copy_size); > + return new_ptr; > + } > return asan_realloc(ptr, size, &stack); > } > > > Jakub > >
On Tue, May 17, 2016 at 05:38:27PM +0300, Maxim Ostapenko wrote: > Hi Jakub, > > thanks for backporting this! Do you have any plans to apply this patch to > GCC 5 and 6 branches? AFAIK people hit on this ASan + newer Glibc bug by > using GCC 5.3.1 on Fedora 23. I don't have the newer glibc on my box, therefore I'm waiting until somebody confirms the trunk change fixed it before backporting. > >IMHO even better would be to make sure that in the common case (recent > >glibc) we don't have failed dlsym calls (still, this hack is useful just in > >case) - the __isoc99_*printf* interceptors make no sense, glibc has never > >exported those. Thus, if we bump ABI of libasan again for GCC 7, IMHO those > >bogus interceptors should be ifdefed out for glibc or removed completely. > >Or, if we don't want to break ABI, at least changed so that they actuall > >dlsym the corresponding *printf* (not __isoc99_ prefixed) functions > >instead of the bogus ones. > > We should definitely bump libasan version on next libsanitizer merge, > because it would contain ABI breaking changes in ASan. Perhaps we could > ifdef these __isoc99 interceptors as a local GCC patch then? Well, with the patch it is nothing urgent, but IMNSHO the bogus interceptors aren't needed upstream either. Jakub
On 05/17/2016 04:46 PM, Jakub Jelinek wrote: > On Tue, May 17, 2016 at 05:38:27PM +0300, Maxim Ostapenko wrote: >> Hi Jakub, >> >> thanks for backporting this! Do you have any plans to apply this patch to >> GCC 5 and 6 branches? AFAIK people hit on this ASan + newer Glibc bug by >> using GCC 5.3.1 on Fedora 23. > > I don't have the newer glibc on my box, therefore I'm waiting until somebody > confirms the trunk change fixed it before backporting. I compiled GCC trunk (r236371) and today's glibc master (around commit 0014680d6a5bdeb4fe17682450105ebed19f35da), and both work together, in the sense that this test program, when compiled with ASAN, reports a memory leak: #include <stdlib.h> int main () { malloc(35); return 0; } ================================================================= ==30982==ERROR: LeakSanitizer: detected memory leaks Direct leak of 35 byte(s) in 1 object(s) allocated from: #0 0x7fa00ea15928 in __interceptor_malloc ../../../../trunk/libsanitizer/asan/asan_malloc_linux.cc:62 #1 0x400763 (/home/fweimer/src/gnu/glibc/build/elf/ld.so+0x400763) #2 0x7fa00e5d625f in __libc_start_main ../csu/libc-start.c:289 SUMMARY: AddressSanitizer: 35 byte(s) leaked in 1 allocation(s). Before, when compiled with GCC 5.3.1 (in Fedora 22), it would report an internal error: ==30945==AddressSanitizer CHECK failed: ../../../../libsanitizer/asan/asan_rtl.cc:556 "((!asan_init_is_running && "ASan init calls itself!")) != (0)" (0x0, 0x0) <empty stack> Thanks, Florian
--- libsanitizer/asan/asan_malloc_linux.cc.jj 2014-09-24 11:08:01.772039066 +0200 +++ libsanitizer/asan/asan_malloc_linux.cc 2016-05-17 11:02:37.859379380 +0200 @@ -24,39 +24,62 @@ // ---------------------- Replacement functions ---------------- {{{1 using namespace __asan; // NOLINT +static uptr allocated_for_dlsym; +static const uptr kDlsymAllocPoolSize = 1024; +static uptr alloc_memory_for_dlsym[kDlsymAllocPoolSize]; + +static bool IsInDlsymAllocPool(const void *ptr) { + uptr off = (uptr)ptr - (uptr)alloc_memory_for_dlsym; + return off < sizeof(alloc_memory_for_dlsym); +} + +static void *AllocateFromLocalPool(uptr size_in_bytes) { + uptr size_in_words = RoundUpTo(size_in_bytes, kWordSize) / kWordSize; + void *mem = (void*)&alloc_memory_for_dlsym[allocated_for_dlsym]; + allocated_for_dlsym += size_in_words; + CHECK_LT(allocated_for_dlsym, kDlsymAllocPoolSize); + return mem; +} + INTERCEPTOR(void, free, void *ptr) { GET_STACK_TRACE_FREE; + if (UNLIKELY(IsInDlsymAllocPool(ptr))) + return; asan_free(ptr, &stack, FROM_MALLOC); } INTERCEPTOR(void, cfree, void *ptr) { GET_STACK_TRACE_FREE; + if (UNLIKELY(IsInDlsymAllocPool(ptr))) + return; asan_free(ptr, &stack, FROM_MALLOC); } INTERCEPTOR(void*, malloc, uptr size) { + if (UNLIKELY(!asan_inited)) + // Hack: dlsym calls malloc before REAL(malloc) is retrieved from dlsym. + return AllocateFromLocalPool(size); GET_STACK_TRACE_MALLOC; return asan_malloc(size, &stack); } INTERCEPTOR(void*, calloc, uptr nmemb, uptr size) { - if (UNLIKELY(!asan_inited)) { + if (UNLIKELY(!asan_inited)) // Hack: dlsym calls calloc before REAL(calloc) is retrieved from dlsym. - const uptr kCallocPoolSize = 1024; - static uptr calloc_memory_for_dlsym[kCallocPoolSize]; - static uptr allocated; - uptr size_in_words = ((nmemb * size) + kWordSize - 1) / kWordSize; - void *mem = (void*)&calloc_memory_for_dlsym[allocated]; - allocated += size_in_words; - CHECK(allocated < kCallocPoolSize); - return mem; - } + return AllocateFromLocalPool(nmemb * size); GET_STACK_TRACE_MALLOC; return asan_calloc(nmemb, size, &stack); } INTERCEPTOR(void*, realloc, void *ptr, uptr size) { GET_STACK_TRACE_MALLOC; + if (UNLIKELY(IsInDlsymAllocPool(ptr))) { + uptr offset = (uptr)ptr - (uptr)alloc_memory_for_dlsym; + uptr copy_size = Min(size, kDlsymAllocPoolSize - offset); + void *new_ptr = asan_malloc(size, &stack); + internal_memcpy(new_ptr, ptr, copy_size); + return new_ptr; + } return asan_realloc(ptr, size, &stack); }