libbacktrace integration for _GLIBCXX_DEBUG mode

[Jonathan Wakely]

On 29/05/19 19:45 +0200, François Dumont wrote:
>On 5/29/19 12:06 AM, Jonathan Wakely wrote:
>>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
>>
>>Thanks for making this opt-in.
>>
>>>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.
>>
>>Why do you want to avoid that?
>>
>>This means users can't just define the macro in their makefiles
>>unconditionally, they have to check if libbacktrace is installed and
>>supported. That might mean having platform-specific conditionals in
>>the makefile to only enable it sometimes.
>>
>>What harm does it do to just ignore the _GLIBCXX_DEBUG_BACKTRACE macro
>>if backtraces can't be enabled? Or just use #warning instead of
>>#error?
>>
>What I want to avoid is PRÂ  saying that despite 
>_GLIBCXX_DEBUG_BACKTRACE being defined there isn't any backtrace 
>displayed in the assertion message.

I think optimizing for "we don't want to get PRs" instead of what's
useful to users is the wrong goal.

>Maybe I can still fail to compile if I can't include 
>backtrace-supported.h to make clear that the -I... option is missing 
>but ignore if !BACKTRACE_SUPPORTED. Would it be fine ?

How about just warnings?

#if defined(_GLIBCXX_DEBUG_BACKTRACE)
# if !defined(BACKTRACE_SUPPORTED)
#  if defined(__has_include) && !__has_include(<backtrace-supported.h>)
#   warning "_GLIBCXX_DEBUG_BACKTRACE is defined but <backtrace-supported.h> header from libbacktrace was not found"
#  endif
#  include <backtrace-supported.h>
# endif
# if !BACKTRACE_SUPPORTED
#   warning "_GLIBCXX_DEBUG_BACKTRACE is defined but libbacktrace is not supported"
# endif
# include <backtrace.h>
#else

It might be even better to give a way to suppress those warnings:

#if defined(_GLIBCXX_DEBUG_BACKTRACE)
# if !defined(BACKTRACE_SUPPORTED)
#  if defined(__has_include) && !__has_include(<backtrace-supported.h>) \
      && !defined _GLIBCXX_DEBUG_BACKTRACE_FAIL_QUIETLY
#   warning "_GLIBCXX_DEBUG_BACKTRACE is defined but <backtrace-supported.h> header from libbacktrace was not found"
#  endif
#  include <backtrace-supported.h>
# endif
# if !BACKTRACE_SUPPORTED && !defined _GLIBCXX_DEBUG_BACKTRACE_FAIL_QUIETLY
#   warning "_GLIBCXX_DEBUG_BACKTRACE is defined but libbacktrace is not supported"
# endif
# include <backtrace.h>
#else



>>
>>>Â Â Â  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*; <template-parameter-2-2> = void; _Tp = int;
>>>Â Â Â  _Allocator = std::allocator<int>; std::__debug::vector<_Tp,
>>>Â Â Â  _Allocator>::iterator =
>>>__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::
>>>Â Â Â  vector<int> >, std::__debug::vector<int>,
>>>Â Â Â  std::random_access_iterator_tag>; typename 
>>>std::iterator_traits<typename
>>>Â Â Â  std::vector<_Tp, _Alloc>::iterator>::iterator_category =
>>>Â Â Â  std::random_access_iterator_tag; typename std::vector<_Tp,
>>>Â Â Â  _Alloc>::iterator = __gnu_cxx::__normal_iterator<int*, 
>>>std::vector<int>
>>>Â Â Â  >; std::__debug::vector<_Tp, _Allocator>::const_iterator =
>>>__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<const int*,
>>>Â Â Â  std::vector<int> >, std::__debug::vector<int>,
>>>Â Â Â  std::random_access_iterator_tag>; typename 
>>>std::iterator_traits<typename
>>>Â Â Â  std::vector<_Tp, _Alloc>::const_iterator>::iterator_category =
>>>Â Â Â  std::random_access_iterator_tag; typename std::vector<_Tp,
>>>Â Â Â  _Alloc>::const_iterator = __gnu_cxx::__normal_iterator<const 
>>>int*, std::
>>>Â Â Â  vector<int> >]
>>>
>>>Backtrace:
>>>Â Â Â  0x402718 
>>>__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, 
>>>std::vector<int> >> std::__debug::vector<int>::insert<int*, 
>>>void>(__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int 
>>>const*, std::vector<int> >>, 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<int>;
>>>Â Â Â  }
>>
>>This is nice.
>
>Yes, I forgot to say that I made an effort to clean a little the 
>demangle names but I think you saw it already.

Yes, all that code concerns me. I'll reply to the patch email again
...