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*; <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>;
    }
XFAIL: 23_containers/vector/debug/57779_neg.cc execution test


    * include/debug/formatter.h [_GLIBCXX_DEBUG_BACKTRACE]: Include
    <backtrace-supported.h> and <backtrace.h>.
    [!_GLIBCXX_DEBUG_BACKTRACE]: Include <stdint.h>.
    [!_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 <cstring> and <string>.
    (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 <jwak...@redhat.com> 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 @@
<para>The following library components provide extra debugging
  capabilities in debug mode:</para>
<itemizedlist>
+  <listitem><para><code>std::array</code> (no safe iterators)</para></listitem>
  <listitem><para><code>std::basic_string</code> (no safe iterators and see note 
below)</para></listitem>
  <listitem><para><code>std::bitset</code></para></listitem>
  <listitem><para><code>std::deque</code></para></listitem>
+  <listitem><para><code>std::forward_list</code></para></listitem>
  <listitem><para><code>std::list</code></para></listitem>
  <listitem><para><code>std::map</code></para></listitem>
  <listitem><para><code>std::multimap</code></para></listitem>
@@ -160,6 +162,13 @@ which always works correctly.
  <code>GLIBCXX_DEBUG_MESSAGE_LENGTH</code> can be used to request a
  different length.</para>

+<para>Starting with GCC 10 libstdc++ has integrated
+  <link xmlns:xlink="http://www.w3.org/1999/xlink";
+  
xlink:href="https://github.com/ianlancetaylor/libbacktrace";>libbacktrace</link>
+  to produce backtrace on error. Use <code>-D_GLIBCXX_DEBUG_BACKTRACE</code> 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
+  <code>-lbacktrace</code> to build your application.</para>
</section>

<section xml:id="debug_mode.using.specific" xreflabel="Using Specific"><info><title>Using a 
Specific Debug Container</title></info>
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 <bits/c++config.h>

+#if defined(_GLIBCXX_DEBUG_BACKTRACE)
+# if !defined(BACKTRACE_SUPPORTED)
+#  if defined(__has_include) && !__has_include(<backtrace-supported.h>)
+#   error No libbacktrace backtrace-supported.h file found.
+#  endif
+#  include <backtrace-supported.h>
+# endif
+# if !BACKTRACE_SUPPORTED
+#  error libbacktrace not supported.
+# endif
+# include <backtrace.h>
+#else
+# include <stdint.h> // For uintptr_t.

Please use <cstdint> 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.


Reply via email to