From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 70105 invoked by alias); 29 May 2019 23:05:03 -0000 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 Received: (qmail 70091 invoked by uid 89); 29 May 2019 23:05:03 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-15.7 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,SPF_HELO_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 29 May 2019 23:05:00 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D64763091754; Wed, 29 May 2019 23:04:45 +0000 (UTC) Received: from localhost (unknown [10.33.36.135]) by smtp.corp.redhat.com (Postfix) with ESMTP id 51FFB60852; Wed, 29 May 2019 23:04:44 +0000 (UTC) Date: Thu, 30 May 2019 00:09:00 -0000 From: Jonathan Wakely To: =?iso-8859-1?Q?Fran=E7ois?= Dumont Cc: Ville Voutilainen , "libstdc++@gcc.gnu.org" , gcc-patches Subject: Re: libbacktrace integration for _GLIBCXX_DEBUG mode Message-ID: <20190529230443.GM2599@redhat.com> References: <20181221203504.GG3077@redhat.com> <20181221210345.GI3077@redhat.com> <04943096-5365-4146-af4b-dff031e0402c@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <04943096-5365-4146-af4b-dff031e0402c@gmail.com> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.11.3 (2019-02-01) X-SW-Source: 2019-05/txt/msg01981.txt.bz2 On 23/05/19 07:39 +0200, François Dumont wrote: >Hi > >    So here what I come up with. > >    _GLIBCXX_DEBUG_BACKTRACE controls the feature. If the user define >it and there is a detectable issue with libbacktrace then I generate a >compilation error. I want to avoid users defining it but having no >backtrace in the end in the debug assertion. > >    With this new setup I manage to run testsuite with it like that: > >export LD_LIBRARY_PATH=/home/fdt/dev/gcc/install/lib/ >make CXXFLAGS='-D_GLIBCXX_DEBUG_BACKTRACE >-I/home/fdt/dev/gcc/install/include -lbacktrace' check-debug > >    An example of result: > >/home/fdt/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/vector:606: >In function: >    std::__debug::vector<_Tp, _Allocator>::iterator >    std::__debug::vector<_Tp, >_Allocator>::insert(std::__debug::vector<_Tp, >    _Allocator>::const_iterator, _InputIterator, _InputIterator) [with >    _InputIterator = int*; = void; _Tp = int; >    _Allocator = std::allocator; std::__debug::vector<_Tp, >    _Allocator>::iterator = >__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator    vector >, std::__debug::vector, >    std::random_access_iterator_tag>; typename >std::iterator_traits    std::vector<_Tp, _Alloc>::iterator>::iterator_category = >    std::random_access_iterator_tag; typename std::vector<_Tp, >    _Alloc>::iterator = __gnu_cxx::__normal_iteratorstd::vector >    >; std::__debug::vector<_Tp, _Allocator>::const_iterator = >__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator    std::vector >, std::__debug::vector, >    std::random_access_iterator_tag>; typename >std::iterator_traits    std::vector<_Tp, _Alloc>::const_iterator>::iterator_category = >    std::random_access_iterator_tag; typename std::vector<_Tp, >    _Alloc>::const_iterator = __gnu_cxx::__normal_iteratorstd:: >    vector >] > >Backtrace: >    0x402718 >__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iteratorstd::vector >> std::__debug::vector::insertvoid>(__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iteratorconst*, std::vector >>, int*, int*) >/home/fdt/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/vector:606 >    0x402718 test01() >/home/fdt/dev/gcc/git/libstdc++-v3/testsuite/23_containers/vector/debug/57779_neg.cc:29 >    0x401428 main >/home/fdt/dev/gcc/git/libstdc++-v3/testsuite/23_containers/vector/debug/57779_neg.cc:34 > >Error: attempt to insert with an iterator range [__first, __last) from this >container. > >Objects involved in the operation: >    iterator "__first" @ 0x0x7fff730b96b0 { >      type = int* (mutable iterator); >    } >    iterator "__last" @ 0x0x7fff730b96b8 { >      type = int* (mutable iterator); >    } >    sequence "this" @ 0x0x7fff730b9720 { >      type = std::__debug::vector; >    } >XFAIL: 23_containers/vector/debug/57779_neg.cc execution test > > >    * include/debug/formatter.h [_GLIBCXX_DEBUG_BACKTRACE]: Include >    and . >    [!_GLIBCXX_DEBUG_BACKTRACE]: Include . >    [!_GLIBCXX_DEBUG_BACKTRACE](backtrace_error_callback): New. >    [!_GLIBCXX_DEBUG_BACKTRACE](backtrace_full_callback): New. >    [!_GLIBCXX_DEBUG_BACKTRACE](struct backtrace_state): New declaration. >    (_Error_formatter::_Bt_full_t): New function pointer type. >    (_Error_formatter::_M_print_backtrace): New. >    (_Error_formatter::_M_backtrace_state): New. >    (_Error_formatter::_M_backtrace_full_func): New. >    * src/c++11/debug.cc: Include and . >    (PrintContext::_M_demangle_name): New. >    (_Print_func_t): New. >    (print_word(PrintContext&, const char*)): New. >    (print_raw(PrintContext&, const char*)): New. >    (print_function(PrintContext&, const char*, _Print_func_t)): New. >    (print_type): Use latter. >    (print_string(PrintContext&, const char*)): New. >    (print_backtrace(void*, uintptr_t, const char*, int, const char*)): >    New. >    (_Error_formatter::_M_error()): Adapt. >    * doc/xml/manual/debug_mode.xml: Document _GLIBCXX_DEBUG_BACKTRACE. > >Tested under Linux x86_64. > >Ok to commit ? > >François > > >On 12/21/18 10:03 PM, Jonathan Wakely wrote: >>On 21/12/18 22:47 +0200, Ville Voutilainen wrote: >>>On Fri, 21 Dec 2018 at 22:35, Jonathan Wakely >>>wrote: >>>>>    I also explcitely define BACKTRACE_SUPPORTED to 0 to make sure >>>>>libstdc++ has no libbacktrace dependency after usual build. >>> >>>>I'm concerned about the requirement to link to libbacktrace >>>>explicitly (which will break existing makefiles and build systems that >>>>currently use debug mode in testing). >>> >>>But see what Francois wrote, "I also explcitely define >>>BACKTRACE_SUPPORTED to 0 to make sure >>>libstdc++ has no libbacktrace dependency after usual build." >> >>Yes, but if you happen to install libbacktrace headers, the behaviour >>for users building their own code changes. I agree that if you install >>those headers, it's probably for a reason, but it might be a different >>reason to "so that libstdc++ prints better backtraces". >> >>>>Also, some of the glibc team pointed out to me that running *any* >>>>extra code after undefined behaviour has been detected is a potential >>>>risk. The less that you do between detecting UB and calling abort(), >>>>the better. Giving the users more information is helpful, but comes >>>>with some additional risk. >>> >>>Ditto. Having said those things, I think we need to figure out a good >>>way to provide this sensibly >>>as an opt-in. The backtrace support is bloody useful, and dovetails >>>into a possible Contracts-aware >>>implementation of our library, but I think we need to do some more >>>thought-work on this, thus I agree >>>that it's not stage3 material. I do think it's something that we need >>>to keep in mind, thanks >>>for working on it, Francois! >> >>Yes, I agree that making it available via a more explicit opt-in would >>be good. Maybe require users to define _GLIBCXX_DEBUG_BACKTRACE as well >>as _GLIBCXX_DEBUG, or something like that. >> >> >> > >diff --git a/libstdc++-v3/doc/xml/manual/debug_mode.xml b/libstdc++-v3/doc/xml/manual/debug_mode.xml >index 570c17ba28a..27873151dae 100644 >--- a/libstdc++-v3/doc/xml/manual/debug_mode.xml >+++ b/libstdc++-v3/doc/xml/manual/debug_mode.xml >@@ -104,9 +104,11 @@ > The following library components provide extra debugging > capabilities in debug mode: > >+ std::array (no safe iterators) > std::basic_string (no safe iterators and see note below) > std::bitset > std::deque >+ std::forward_list > std::list > std::map > std::multimap >@@ -160,6 +162,13 @@ which always works correctly. > GLIBCXX_DEBUG_MESSAGE_LENGTH can be used to request a > different length. > >+Starting with GCC 10 libstdc++ has integrated >+ + xlink:href="https://github.com/ianlancetaylor/libbacktrace">libbacktrace >+ to produce backtrace on error. Use -D_GLIBCXX_DEBUG_BACKTRACE to >+ activate it. Note that if not properly installed or if libbacktrace is not >+ supported compilation will fail. You'll also have to use the >+ -lbacktrace to build your application. > > >
Using a Specific Debug Container >diff --git a/libstdc++-v3/include/debug/formatter.h b/libstdc++-v3/include/debug/formatter.h >index 220379994c0..690750f42be 100644 >--- a/libstdc++-v3/include/debug/formatter.h >+++ b/libstdc++-v3/include/debug/formatter.h >@@ -31,6 +31,29 @@ > > #include > >+#if defined(_GLIBCXX_DEBUG_BACKTRACE) >+# if !defined(BACKTRACE_SUPPORTED) >+# if defined(__has_include) && !__has_include() >+# error No libbacktrace backtrace-supported.h file found. >+# endif >+# include >+# endif >+# if !BACKTRACE_SUPPORTED >+# error libbacktrace not supported. >+# endif >+# include >+#else >+# include // For uintptr_t. Please use and std::uintptr_t. >+// Extracted from libbacktrace. >+typedef void (*backtrace_error_callback) (void*, const char*, int); >+ >+typedef int (*backtrace_full_callback) (void*, uintptr_t, const char*, int, >+ const char*); These typedefs should use __reserved_names. >+struct backtrace_state; Although this one can't use a reserved name, unless we're going to create opaque wrappers around the libbacktrace type. Introducing t his non-reserved name means that defining _GLIBCXX_DEBUG makes the library non-conforming. It would be possible to avoid declaring this struct, by making _M_backtrace_state a void* and creating a wrapper function for backtrace_create_state, and a weak symbol in the library. I'll have to think about this more.