Re: 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
...