Re: libbacktrace integration for _GLIBCXX_DEBUG mode

[François Dumont <frs dot dumont at gmail dot com>]

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 <jwakely@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.
+
+// 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*);
+
+struct backtrace_state;
+#endif
+
 #if __cpp_rtti
 # include <typeinfo>
 # define _GLIBCXX_TYPEID(_Type) &typeid(_Type)
@@ -156,6 +179,9 @@ namespace __gnu_debug
 
   class _Error_formatter
   {
+    typedef int (*_Bt_full_t) (backtrace_state*, int, backtrace_full_callback,
+			       backtrace_error_callback, void*);
+
     // Tags denoting the type of parameter for construction
     struct _Is_iterator { };
     struct _Is_iterator_value_type { };
@@ -558,11 +584,21 @@ namespace __gnu_debug
     _M_print_string(const char* __string) const _GLIBCXX_DEPRECATED;
 #endif
 
+    void
+    _M_print_backtrace(backtrace_full_callback __cb, void* __data) const
+    { _M_backtrace_full_func(_M_backtrace_state, 1, __cb, 0, __data); }
+
   private:
     _Error_formatter(const char* __file, unsigned int __line,
 		     const char* __function)
     : _M_file(__file), _M_line(__line), _M_num_parameters(0), _M_text(0)
     , _M_function(__function)
+#if BACKTRACE_SUPPORTED
+    , _M_backtrace_state(backtrace_create_state(NULL, 0, NULL, NULL))
+    , _M_backtrace_full_func(&backtrace_full)
+#else
+    , _M_backtrace_state()
+#endif
     { }
 
 #if !_GLIBCXX_INLINE_VERSION
@@ -578,6 +614,8 @@ namespace __gnu_debug
     unsigned int	_M_num_parameters;
     const char*		_M_text;
     const char*		_M_function;
+    backtrace_state*	_M_backtrace_state;
+    _Bt_full_t		_M_backtrace_full_func;
 
   public:
     static _Error_formatter&
diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc
index f5a49992efa..aa7c17248fe 100644
--- a/libstdc++-v3/src/c++11/debug.cc
+++ b/libstdc++-v3/src/c++11/debug.cc
@@ -34,16 +34,13 @@
 
 #include <cassert>
 #include <cstdio>
-#include <cctype> // for std::isspace
+#include <cctype>	// for std::isspace.
+#include <cstring>	// for std::strstr.
 
-#include <algorithm> // for std::min
+#include <algorithm>	// for std::min.
+#include <string>
 
-#include <cxxabi.h> // for __cxa_demangle
-
-// libstdc++/85768
-#if 0 // defined _GLIBCXX_HAVE_EXECINFO_H
-# include <execinfo.h> // for backtrace
-#endif
+#include <cxxabi.h>	// for __cxa_demangle.
 
 #include "mutex_pool.h"
 
@@ -570,7 +567,8 @@ namespace
   struct PrintContext
   {
     PrintContext()
-      : _M_max_length(78), _M_column(1), _M_first_line(true), _M_wordwrap(false)
+    : _M_max_length(78), _M_column(1), _M_first_line(true), _M_wordwrap(false)
+    , _M_demangle_name(false)
     { get_max_length(_M_max_length); }
 
     std::size_t	_M_max_length;
@@ -578,16 +576,18 @@ namespace
     std::size_t	_M_column;
     bool	_M_first_line;
     bool	_M_wordwrap;
+    bool	_M_demangle_name;
   };
 
+  typedef void (*_Print_func_t) (PrintContext&, const char*);
+
   template<size_t Length>
     void
     print_literal(PrintContext& ctx, const char(&word)[Length])
     { print_word(ctx, word, Length - 1); }
 
   void
-  print_word(PrintContext& ctx, const char* word,
-	     std::ptrdiff_t count = -1)
+  print_word(PrintContext& ctx, const char* word, std::ptrdiff_t count)
   {
     size_t length = count >= 0 ? count : __builtin_strlen(word);
     if (length == 0)
@@ -623,7 +623,7 @@ namespace
 	    ctx._M_column += ctx._M_indent;
 	  }
 
-	int written = fprintf(stderr, "%s", word);
+	int written = fprintf(stderr, word);
 
 	if (word[length - 1] == '\n')
 	  {
@@ -640,6 +640,133 @@ namespace
       }
   }
 
+  void
+  print_word(PrintContext& ctx, const char* word)
+  { print_word(ctx, word, -1); }
+
+  void
+  print_raw(PrintContext&, const char* word)
+  { fprintf(stderr, word); }
+
+  void
+  print_function(PrintContext& ctx, const char* function,
+		 _Print_func_t print_func)
+  {
+    const string cxx1998 = "__cxx1998::";
+    const string allocator = ", std::allocator<";
+    const string safe_iterator = "__gnu_debug::_Safe_iterator<";
+    string func(function);
+    string::size_type index = 0;
+    for (;;)
+      {
+	auto idx1 = func.find(cxx1998, index);
+	auto idx2 = func.find(allocator, index);
+	auto idx3 = ctx._M_demangle_name
+	  ? func.find(safe_iterator, index) : string::npos;
+	if (idx1 != string::npos || idx2 != string::npos ||
+	    idx3 != string::npos)
+	  {
+	    if (idx1 != string::npos
+		&& (idx2 == string::npos || idx1 < idx2)
+		&& (idx3 == string::npos || idx1 < idx3))
+	      {
+		func[idx1] = '\0';
+		print_func(ctx, func.c_str() + index);
+		index = idx1 + cxx1998.size();
+	      }
+	    else if (idx2 != string::npos
+		     && (idx3 == string::npos || idx2 < idx3))
+	      {
+		func[idx2] = '\0';
+		print_func(ctx, func.c_str() + index);
+
+		// We need to look for the closing '>'
+		idx2 += allocator.size();
+		int open_bracket = 0;
+		for (; idx2 != func.size(); ++idx2)
+		  {
+		    if (func[idx2] == '>')
+		      {
+			if (open_bracket)
+			  --open_bracket;
+			else
+			  break;
+		      }
+		    else if (func[idx2] == '<')
+		      ++open_bracket;
+		  }
+
+		index = idx2 + 1;
+		idx2 = func.find(" const&", index);
+		if (idx2 == index)
+		  // Allocator parameter qualification, skipped too.
+		  index += sizeof(" const&") - 1;
+	      }
+	    else if (idx3 != string::npos)
+	      {
+		// Just keep the 1st template parameter.
+		// Look for 1st comma outside any additional brackets.
+		idx3 += safe_iterator.size();
+		char c = func[idx3];
+		func[idx3] = '\0';
+		print_func(ctx, func.c_str() + index);
+		func[idx3] = c;
+		index = idx3;
+		int nb_open_bracket = 0;
+		for (; nb_open_bracket != -1;)
+		  {
+		    auto n = func.find_first_of("<,>", idx3);
+		    switch (func[n])
+		      {
+		      case '<':
+			++nb_open_bracket;
+			break;
+		      case '>':
+			--nb_open_bracket;
+			break;
+		      case ',':
+			if (nb_open_bracket == 0)
+			  {
+			    func[n] = '\0';
+			    print_function(ctx, func.c_str() + index, print_func);
+			    nb_open_bracket = -1;
+			  }
+
+			break;
+		      }
+
+		    idx3 = n + 1;
+		  }
+
+		// Now look for the closing '>'.
+		nb_open_bracket = 0;
+		for (; idx3 != func.size(); ++idx3)
+		  {
+		    if (func[idx3] == '>')
+		      {
+			if (nb_open_bracket)
+			  --nb_open_bracket;
+			else
+			  break;
+		      }
+		    else if (func[idx3] == '<')
+		      ++nb_open_bracket;
+		  }
+
+		index = idx3;
+	      }
+
+	    while (isspace(func[index]))
+	      ++index;
+	  }
+	else
+	  {
+	    print_func(ctx, func.c_str() + index);
+	    break;
+	  }
+      }
+  }
+
   template<size_t Length>
     void
     print_type(PrintContext& ctx,
@@ -653,7 +780,13 @@ namespace
 	  int status;
 	  char* demangled_name =
 	    __cxxabiv1::__cxa_demangle(info->name(), NULL, NULL, &status);
-	  print_word(ctx, status == 0 ? demangled_name : info->name());
+	  if (status == 0)
+	    {
+	      ctx._M_demangle_name = true;
+	      print_function(ctx, demangled_name, &print_word);
+	    }
+	  else
+	    print_word(ctx, info->name());
 	  free(demangled_name);
 	}
     }
@@ -921,10 +1054,10 @@ namespace
   }
 
   void
-  print_string(PrintContext& ctx, const char* string,
+  print_string(PrintContext& ctx, const char* str,
 	       const _Parameter* parameters, std::size_t num_parameters)
   {
-    const char* start = string;
+    const char* start = str;
     const int bufsize = 128;
     char buf[bufsize];
     int bufindex = 0;
@@ -1013,6 +1146,63 @@ namespace
 	print_word(ctx, buf, bufindex);
       }
   }
+
+  void
+  print_string(PrintContext& ctx, const char* str)
+  { print_string(ctx, str, nullptr, 0); }
+
+  int
+  print_backtrace(void* data, uintptr_t pc, const char* filename,
+		  int lineno, const char* function)
+  {
+    const int bufsize = 64;
+    char buf[bufsize];
+
+    PrintContext& ctx = *static_cast<PrintContext*>(data);
+
+    int written = __builtin_sprintf(buf, "%p ", (void*)pc);
+    print_word(ctx, buf, written);
+
+    int ret = 0;
+    if (function)
+      {
+	int status;
+	char* demangled_name =
+	  __cxxabiv1::__cxa_demangle(function, NULL, NULL, &status);
+	if (status == 0)
+	  {
+	    ctx._M_demangle_name = true;
+	    print_function(ctx, demangled_name, &print_raw);
+	  }
+	else
+	  print_word(ctx, function);
+
+	free(demangled_name);
+	ret = strstr(function, "main") ? 1 : 0;
+      }
+
+    print_literal(ctx, "\n");
+
+    if (filename)
+      {
+	bool wordwrap = false;
+	swap(wordwrap, ctx._M_wordwrap);
+	print_word(ctx, filename);
+
+	if (lineno)
+	  {
+	    written = __builtin_sprintf(buf, ":%u\n", lineno);
+	    print_word(ctx, buf, written);
+	  }
+	else
+	  print_literal(ctx, "\n");
+	swap(wordwrap, ctx._M_wordwrap);
+      }
+    else
+      print_literal(ctx, "???:0\n");
+
+    return ret;
+  }
 }
 
 namespace __gnu_debug
@@ -1054,35 +1244,19 @@ namespace __gnu_debug
     if (_M_function)
       {
 	print_literal(ctx, "In function:\n");
-	print_string(ctx, _M_function, nullptr, 0);
+	print_function(ctx, _M_function, &print_string);
 	print_literal(ctx, "\n");
 	ctx._M_first_line = true;
 	print_literal(ctx, "\n");
       }
 
-// libstdc++/85768
-#if 0 //defined _GLIBCXX_HAVE_EXECINFO_H
-    {
-      void* stack[32];
-      int nb = backtrace(stack, 32);
-
-      // Note that we skip current method symbol.
-      if (nb > 1)
-	{
-	  print_literal(ctx, "Backtrace:\n");
-	  auto symbols = backtrace_symbols(stack, nb);
-	  for (int i = 1; i < nb; ++i)
-	    {
-	      print_word(ctx, symbols[i]);
-	      print_literal(ctx, "\n");
-	    }
-
-	  free(symbols);
-	  ctx._M_first_line = true;
-	  print_literal(ctx, "\n");
-	}
-    }
-#endif
+    if (_M_backtrace_state)
+      {
+	print_literal(ctx, "Backtrace:\n");
+	_M_print_backtrace(print_backtrace, &ctx);
+	ctx._M_first_line = true;
+	print_literal(ctx, "\n");
+      }
 
     print_literal(ctx, "Error: ");