From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28415 invoked by alias); 13 Dec 2012 10:23:19 -0000 Received: (qmail 28403 invoked by uid 22791); 13 Dec 2012 10:23:17 -0000 X-SWARE-Spam-Status: No, hits=-5.4 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,TW_KC,TW_LV X-Spam-Check-By: sourceware.org Received: from mail-vb0-f47.google.com (HELO mail-vb0-f47.google.com) (209.85.212.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 13 Dec 2012 10:23:13 +0000 Received: by mail-vb0-f47.google.com with SMTP id e21so1962843vbm.20 for ; Thu, 13 Dec 2012 02:23:13 -0800 (PST) Received: by 10.58.207.196 with SMTP id ly4mr2357363vec.6.1355394192940; Thu, 13 Dec 2012 02:23:12 -0800 (PST) MIME-Version: 1.0 Received: by 10.220.33.72 with HTTP; Thu, 13 Dec 2012 02:22:52 -0800 (PST) In-Reply-To: <20121213083653.GL2315@tucnak.redhat.com> References: <20121128101420.GG2315@tucnak.redhat.com> <20121203110018.GR2315@tucnak.redhat.com> <20121205122906.GO2315@tucnak.redhat.com> <87ehivq8ce.fsf@redhat.com> <20121212213013.GH2315@tucnak.redhat.com> <20121213083653.GL2315@tucnak.redhat.com> From: Konstantin Serebryany Date: Thu, 13 Dec 2012 10:23:00 -0000 Message-ID: Subject: Re: [PATCH] asan unit tests from llvm lit-test incremental changes To: Jakub Jelinek Cc: Dodji Seketeli , Wei Mi , Mike Stump , GCC Patches , David Li , Diego Novillo , Kostya Serebryany , Dodji Seketeli , Alexander Potapenko , Evgeniy Stepanov , Alexey Samsonov , Dmitry Vyukov Content-Type: text/plain; charset=ISO-8859-1 X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2012-12/txt/msg00900.txt.bz2 I've added two flags, both on by default for now. // Use fast (frame-pointer-based) unwinder on fatal errors (if available). bool fast_unwind_on_fatal; // Use fast (frame-pointer-based) unwinder on malloc/free (if available). bool fast_unwind_on_malloc; % clang -fsanitize=address -g ~/llvm/projects/compiler-rt/lib/asan/lit_tests/overflow-in-qsort.cc % ASAN_OPTIONS=fast_unwind_on_fatal=0 ./a.out 2>&1 | asan_symbolize.py / | grep '#' #0 0x419731 in QsortCallback overflow-in-qsort.cc:18 #1 0x7fa7e137b61f in msort_with_tmp msort.c:143 #2 0x7fa7e137baba in msort_with_tmp msort.c:46 #3 0x419a58 in MyQsort overflow-in-qsort.cc:25 #4 0x419c94 in main overflow-in-qsort.cc:33 #5 0x7fa7e136276c in __libc_start_main libc-start.c:226 #6 0x4193ac in _start ??:0 % ASAN_OPTIONS=fast_unwind_on_fatal=1 ./a.out 2>&1 | asan_symbolize.py / | grep '#' #0 0x419731 in QsortCallback overflow-in-qsort.cc:18 #1 0x7f816783f61f in msort_with_tmp msort.c:143 % This feature still needs some love. I'll set fast_unwind_on_fatal to 0 after some testing and then merge to gcc (unless there is a rush). On Thu, Dec 13, 2012 at 12:36 PM, Jakub Jelinek wrote: > On Thu, Dec 13, 2012 at 11:44:12AM +0400, Konstantin Serebryany wrote: >> We are discussing it from time to time. >> Sometimes, if e.g. an error happens inside a qsort callback, >> the fp-based unwinder fails to unwind through libc, while _Unwind would work. >> >> I was opposed to this sometime ago because _Unwind often produced >> buggy stack traces on Ubuntu Lucid (the version we cared about). > > Weird, must be some distro modifications, we've been using _Unwind based > backtraces everywhere for many years successfully, glibc backtrace uses it > too, pthread_cancel as well. > >> >> and perhaps for >> >> > these malloc wrappers like ::operator new, ::operator new[] and their >> >> > const std::nothrow_t& variants libasan could intercept them, call >> >> > malloc and if that returns NULL, call the original corresponding function >> >> > so that it deals with exceptions, new handler etc. >> >> Hmm.. Why's that? >> Calling libc's malloc or libstdc++'s operator new in asan run-time is >> really a bad idea. > > I didn't mean calling libc malloc, I meant calling libstdc++'s operator new, > which then calls malloc (== any, thus asan version), but does some > additional bookkeeping for failures. Got it. Probably, a bad idea as well -- what if the standard C++ library is not libstdc++, but something else? Also, when compiling .c programs we don't link libstdc++ at all, so there will be some linking issues. If we want to support new handler and allow asan's malloc to return 0, it is much easier and cleaner to implement it from scratch. However, no one complained so far (after 1.5 years) so I'd prefer to do this lazily (i.e. only after first motivated complaint). And it'll be nowhere near the top of my TODO list :( > > The thing is that libstdc++'s operator new: > _GLIBCXX_WEAK_DEFINITION void * > operator new (std::size_t sz) _GLIBCXX_THROW (std::bad_alloc) > { > void *p; > > /* malloc (0) is unpredictable; avoid it. */ > if (sz == 0) > sz = 1; > p = (void *) malloc (sz); > while (p == 0) > { > new_handler handler = __new_handler; > if (! handler) > _GLIBCXX_THROW_OR_ABORT(bad_alloc()); > handler (); > p = (void *) malloc (sz); > } > > return p; > } > > _GLIBCXX_WEAK_DEFINITION void* > operator new[] (std::size_t sz) _GLIBCXX_THROW (std::bad_alloc) > { > return ::operator new(sz); > } > etc. aren't built with frame pointers, therefore ebp/rbp may be used for > anything, therefore non-unwind based backtrace will stop on that or get > confused. What I meant was have > void * > operator new (std::size_t sz) throw (std::bad_alloc) > { > void *p = malloc (sz); > if (__builtin_expect (p == NULL, 0)) > call_original_operator_new (sz); > return p; > } > and similarly for operator new[] etc. in libasan, forcefully built with > -fno-omit-frame-pointer, so that in the likely case that malloc doesn't > return NULL the non-_Unwind based backtrace in malloc would unwind well > through operator new as well as operator new[]. Or if libasan malloc really > never returns NULL and you don't plan to ever change that (why?), you could > just make operator new/operator new[] etc. in libasan aliases to malloc. I am planing to implement malloc/delete, new/free and new/delete[] mismatch detection, so using aliases is not a choice. --kcc > >> asan's allocator should never return 0 anyway, it should simply crash. >> I don't think we want to support new handler at all. > > Does it? Then it will abort perfectly valid programs. > > Jakub