[PATCH][_GLIBCXX_DEBUG] libbacktrace integration

Jonathan Wakely jwakely@redhat.com
Wed May 5 11:33:37 GMT 2021


On 24/04/21 15:46 +0200, François Dumont via Libstdc++ wrote:
>Hi
>
>    Here is the patch to add backtrace generation on _GLIBCXX_DEBUG 
>assertions thanks to libbacktrace.
>
>    In addition to this integration I am also improving the generation 
>of the assertion message thanks to the "%.*s" printf format, it avoids 
>an intermediate buffer most of the time. I am also removing the "__" 
>used for uglification to get a nicer output. I can propose it in a 
>dedicated patch if you prefer.
>
>    I am adding GLIBCXX_3.4.30 abi version to properly export the 2 
>new weak symbols. Let me know if it isn't necessary.
>
>    libstdc++: [_GLIBCXX_DEBUG] Add backtrace generation thanks to 
>libbacktrace
>
>      Add _GLIBCXX_DEBUG_BACKTRACE macro to activate backtrace 
>generation on
>    _GLIBCXX_DEBUG assertions using libbacktrace.
>
>            * config/abi/pre/gnu.ver: Add GLIBCXX_3.4.30 version and 
>new exports.
>            * include/debug/formatter.h [_GLIBCXX_DEBUG_BACKTRACE]:
>            Include <backtrace-supported.h>.
>            [_GLIBCXX_DEBUG_BACKTRACE && BACKTRACE_SUPPORTED]:
>            Include <backtrace.h>.
>            [(!_GLIBCXX_DEBUG_BACKTRACE || !BACKTRACE_SUPPORTED) &&
>            _GLIBCXX_USE_C99_STDINT_TR1]: Include <stdint.h>.
>            [_GLIBCXX_DEBUG_USE_LIBBACKTRACE]
>            (__gnu_debug::__create_backtrace_state): New.
>            [_GLIBCXX_DEBUG_USE_LIBBACKTRACE]
>            (__gnu_debug::__render_backtrace): New.
>[_GLIBCXX_DEBUG_USE_LIBBACKTRACE](_Error_formatter::_M_print_backtrace):
>            New.
>[_GLIBCXX_DEBUG_USE_LIBBACKTRACE](_Error_formatter::_M_backtrace_state):
>            New.
>            (_Error_formatter::_Error_formatter): Outline definition.
>            * src/c++11/debug.cc: Include <cstring>.
>            (_Print_func_t): New.
>            (print_word): Use '%.*s' format in fprintf to render only 
>expected
>            number of chars.
>            (print_raw(PrintContext&, const char*, ptrdiff_t)): New.
>            (print_function(PrintContext&, const char*, 
>_Print_func_t)): New.
>            (print_type): Use latter.
>            (print_string(PrintContext&, const char*, const 
>_Parameter*, size_t)):
>            Change signature to...
>            (print_string(PrintContext&, const char*, ptrdiff_t, const 
>_Parameter*,
>            size_t)): ...this and adapt. Remove intermediate buffer to 
>render input
>            string.
>            (print_string(PrintContext&, const char*, ptrdiff_t)): New.
>            [_GLIBCXX_DEBUG_USE_LIBBACKTRACE]
>            (print_backtrace(void*, uintptr_t, const char*, int, const 
>char*)): New.
>            (_Error_formatter::_M_error()): Adapt.
>            [_GLIBCXX_DEBUG_USE_LIBBACKTRACE]
>            (__gnu_debug::__create_backtrace_state): New, weak symbol.
>            [_GLIBCXX_DEBUG_USE_LIBBACKTRACE]
>            (__gnu_debug::__render_backtrace): New, weak symbol.
>            * testsuite/util/testsuite_abi.cc: Add new symbol version.
>            * doc/xml/manual/debug_mode.xml: Document 
>_GLIBCXX_DEBUG_BACKTRACE.
>            * doc/xml/manual/using.xml: Likewise.
>
>Tested under Linux x86_64.
>
>Ok to commit ?
>
>François
>

>diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
>index 5323c7f0604..2606d67d8a9 100644
>--- a/libstdc++-v3/config/abi/pre/gnu.ver
>+++ b/libstdc++-v3/config/abi/pre/gnu.ver
>@@ -2397,6 +2397,15 @@ GLIBCXX_3.4.29 {
> 
> } GLIBCXX_3.4.28;
> 
>+GLIBCXX_3.4.30 {
>+
>+    # __gnu_debug::__create_backtrace
>+    _ZN11__gnu_debug24__create_backtrace_stateEv;
>+    _ZN11__gnu_debug18__render_backtraceEPvPFiS0_mPKciS2_ES0_;
>+
>+} GLIBCXX_3.4.29;
>+
>+
> # Symbols in the support library (libsupc++) have their own tag.
> CXXABI_1.3 {
> 
>diff --git a/libstdc++-v3/doc/xml/manual/debug_mode.xml b/libstdc++-v3/doc/xml/manual/debug_mode.xml
>index 883e8cb4f03..931b09710f3 100644
>--- a/libstdc++-v3/doc/xml/manual/debug_mode.xml
>+++ b/libstdc++-v3/doc/xml/manual/debug_mode.xml
>@@ -160,6 +160,12 @@ which always works correctly.
>   <code>GLIBCXX_DEBUG_MESSAGE_LENGTH</code> can be used to request a
>   different length.</para>
> 
>+<para>Note that libstdc++ is able to use
>+  <link xmlns:xlink="http://www.w3.org/1999/xlink"
>+  xlink:href="https://github.com/ianlancetaylor/libbacktrace">libbacktrace</link>
>+  to produce backtraces on error. Use <code>-D_GLIBCXX_DEBUG_BACKTRACE</code> to
>+  activate it. You'll also have to link with libbacktrace
>+  (<option>-lbacktrace</option>) 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/doc/xml/manual/using.xml b/libstdc++-v3/doc/xml/manual/using.xml
>index 24543e9526e..9bd0da8c1c5 100644
>--- a/libstdc++-v3/doc/xml/manual/using.xml
>+++ b/libstdc++-v3/doc/xml/manual/using.xml
>@@ -1128,6 +1128,16 @@ g++ -Winvalid-pch -I. -include stdc++.h -H -g -O2 hello.cc -o test.exe
> 	extensions and libstdc++-specific behavior into errors.
>       </para>
>     </listitem></varlistentry>
>+    <varlistentry><term><code>_GLIBCXX_DEBUG_BACKTRACE</code></term>
>+    <listitem>
>+      <para>
>+	Undefined by default. Considered only if <code>_GLIBCXX_DEBUG</code>
>+	is defined. When defined, checks for <link xmlns:xlink="http://www.w3.org/1999/xlink"
>+	xlink:href="https://github.com/ianlancetaylor/libbacktrace">libbacktrace</link>
>+	support and use it to display backtraces on

s/and use it/and uses it/

>+	<link linkend="manual.ext.debug_mode">debug mode</link> assertions.
>+      </para>
>+    </listitem></varlistentry>
>     <varlistentry><term><code>_GLIBCXX_PARALLEL</code></term>
>     <listitem>
>       <para>Undefined by default. When defined, compiles user code
>@@ -1634,6 +1644,17 @@ A quick read of the relevant part of the GCC
>       header will remain compatible between different GCC releases.
>     </para>
>     </section>
>+
>+    <section xml:id="manual.intro.using.linkage.externals" xreflabel="External Libraries"><info><title>External Libraries</title></info>
>+
>+    <para>
>+      libstdc++ <link linkend="manual.ext.debug_mode">debug mode</link> is able to
>+      produce backtraces thanks to <link xmlns:xlink="http://www.w3.org/1999/xlink"
>+      xlink:href="https://github.com/ianlancetaylor/libbacktrace">libbacktrace</link>.
>+      To use the library you should define <code>_GLIBCXX_DEBUG_BACKTRACE</code> along
>+      with <code>_GLIBCXX_DEBUG</code> and link with <option>-lbacktrace</option>.
>+    </para>
>+    </section>
>   </section>
> 
>   <section xml:id="manual.intro.using.concurrency" xreflabel="Concurrency"><info><title>Concurrency</title></info>
>diff --git a/libstdc++-v3/include/debug/formatter.h b/libstdc++-v3/include/debug/formatter.h
>index 7140fed5e83..ec0984e438f 100644
>--- a/libstdc++-v3/include/debug/formatter.h
>+++ b/libstdc++-v3/include/debug/formatter.h
>@@ -31,6 +31,31 @@
> 
> #include <bits/c++config.h>
> 
>+#if defined(_GLIBCXX_DEBUG_BACKTRACE)
>+# if !defined(BACKTRACE_SUPPORTED)

I think this should also depend on __has_include(<backtrace-supported.h>)


#if defined(_GLIBCXX_DEBUG_BACKTRACE)
# if !defined(BACKTRACE_SUPPORTED) && __has_include(<backtrace-supported.h>)
#  include <backtrace-supported.h>
# endif
#endif



>+#  include <backtrace-supported.h>
>+# endif
>+#endif
>+
>+#if defined(_GLIBCXX_DEBUG_BACKTRACE) && BACKTRACE_SUPPORTED
>+# define _GLIBCXX_DEBUG_USE_LIBBACKTRACE 1
>+# include <backtrace.h>
>+
>+namespace __gnu_debug
>+{
>+  typedef backtrace_full_callback __backtrace_full_cb;
>+}
>+#elif defined (_GLIBCXX_USE_C99_STDINT_TR1)
>+# define _GLIBCXX_DEBUG_USE_LIBBACKTRACE 0

The interaction between _GLIBCXX_DEBUG_USE_LIBBACKTRACE==1 and
_GLIBCXX_DEBUG_USE_LIBBACKTRACE==0 is a bit subtle and should be
documented in a comment.

If I'm reading this correctly, we will always define it (at least when
USE_C99_STDINT_TR1 is defined ... which is always), but we don't
always use libbacktrace.

>+# include <stdint.h> // For uintptr_t.
>+
>+namespace __gnu_debug
>+{
>+  typedef int (*__backtrace_full_cb) (void*, uintptr_t, const char*,
>+				      int, const char*);

I don't think we need this type. You can just use void(*)() as the
callback argument.

>+}
>+#endif
>+
> #if __cpp_rtti
> # include <typeinfo>
> # define _GLIBCXX_TYPEID(_Type) &typeid(_Type)
>@@ -155,6 +180,16 @@ namespace __gnu_debug
>     __msg_irreflexive_ordering
>   };
> 
>+#ifdef _GLIBCXX_DEBUG_USE_LIBBACKTRACE
>+
>+  void*
>+  __create_backtrace_state();
>+
>+  void
>+  __render_backtrace(void*, __backtrace_full_cb, void*);
>+
>+#endif
>+
>   class _Error_formatter
>   {
>     // Tags denoting the type of parameter for construction
>@@ -558,12 +593,14 @@ namespace __gnu_debug
>     _M_print_string(const char* __string) const _GLIBCXX_DEPRECATED;
> #endif
> 
>+#ifdef _GLIBCXX_DEBUG_USE_LIBBACKTRACE
>+    void
>+    _M_print_backtrace(__backtrace_full_cb __cb, void* __data) const;
>+#endif
>+
>   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)
>-    { }
>+		     const char* __function);
> 
> #if !_GLIBCXX_INLINE_VERSION
>     void
>@@ -578,6 +615,9 @@ namespace __gnu_debug
>     unsigned int	_M_num_parameters;
>     const char*		_M_text;
>     const char*		_M_function;
>+#ifdef _GLIBCXX_DEBUG_USE_LIBBACKTRACE
>+    void*		_M_backtrace_state;
>+#endif

This member could be defined unconditionally, and just initialize it
to 0 in the constructor if not producing backtraces.

> 
>   public:
>     static _Error_formatter&
>@@ -587,6 +627,36 @@ namespace __gnu_debug
>       return __formatter;
>     }
>   };
>+
>+#if _GLIBCXX_DEBUG_USE_LIBBACKTRACE
>+
>+  void*
>+  __create_backtrace_state()
>+  { return backtrace_create_state(NULL, 0, NULL, NULL); }
>+
>+  void
>+  __render_backtrace(void* __bt, __backtrace_full_cb __cb, void* __data)
>+  { backtrace_full((backtrace_state*)__bt, 1, __cb, 0, __data); }

These two functions need to be 'inline' don't they?

And if you pass void(*)() as the callback you can just cast it back to
the right type here:

   void
   __render_backtrace(void* __bt, void (*__cb)(), void* __data)
   {
     backtrace_full((backtrace_state*)__bt, 1,
                    (backtrace_full_callback)__cb, 0, __data);
   }

That way you don't need the __backtrace_full_cb typedef defined
anywhere. Pass around void(*)() and cast to the right type only when
you actually use it, where we know that libbacktrace is supported.


>+#endif
>+
>+  _Error_formatter::
>+  _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)
>+#ifdef _GLIBCXX_DEBUG_USE_LIBBACKTRACE
>+  , _M_backtrace_state(__create_backtrace_state())
>+#endif
>+  { }
>+
>+#ifdef _GLIBCXX_DEBUG_USE_LIBBACKTRACE
>+  void
>+  _Error_formatter::
>+  _M_print_backtrace(__backtrace_full_cb __cb, void* __data) const
>+  { __render_backtrace(_M_backtrace_state, __cb, __data); }
>+#endif

And these need to be inline too. Maybe just define them in the class body again?

> } // namespace __gnu_debug
> 
> #undef _GLIBCXX_TYPEID
>diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc
>index 5a642097d17..790810fdab4 100644
>--- a/libstdc++-v3/src/c++11/debug.cc
>+++ b/libstdc++-v3/src/c++11/debug.cc
>@@ -34,16 +34,12 @@
> 
> #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 <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"
> 
>@@ -574,7 +570,7 @@ 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)
>     { get_max_length(_M_max_length); }
> 
>     std::size_t	_M_max_length;
>@@ -584,16 +580,17 @@ namespace
>     bool	_M_wordwrap;
>   };
> 
>+  typedef void (*_Print_func_t) (PrintContext&, const char*, ptrdiff_t);

This can use an alias-declaration to be more readable:

   using _Print_func_t = void (PrintContext&, const char*, ptrdiff_t);



>+  void
>+  print_function(PrintContext& ctx, const char* func,
>+		 _Print_func_t print_func)
>+  {
>+    const char cxx1998[] = "__cxx1998::";
>+    const char uglification[] = "__";
>+    for (;;)
>+      {
>+	auto idx1 = strstr(func, cxx1998);
>+	auto idx2 = strstr(func, uglification);
>+	if (idx1 || idx2)

Checking idx1 is redundant, because if the func name contains
"__cxx1998" then it also contains "__"

>+	  {
>+	    if (idx1 && (!idx2 || idx1 <= idx2))

It's impossible for idx1 && !idx2 to be true.

It's also impossible for idx1 < idx2 to be true.

I think this can be:

   if (auto idx = strstr(func, cxx1998))
     {
       ...
     }
   else if (idx = strstr(func, uglification))
     {
       ...
     }
   else


>+	      {
>+		if (idx1 != func)
>+		  print_func(ctx, func, idx1 - func);
>+
>+		func = idx1 + sizeof(cxx1998) - 1;
>+	      }
>+	    else if (idx2)
>+	      {
>+		if (idx2 != func)
>+		  print_func(ctx, func, idx2 - func);
>+
>+		func = idx2 + sizeof(uglification) - 1;
>+	      }
>+
>+	    while (*func && isspace(*func))

This needs to be isspace((unsigned char)*func), see
https://en.cppreference.com/w/cpp/string/byte/isspace#Notes

It's probably unlikely we'll get negative char values in these
backtraces, but let's not risk it.

>+	      ++func;
>+
>+	    if (!*func)
>+	      break;
>+	  }
>+	else
>+	  {
>+	    print_func(ctx, func, -1);
>+	    break;
>+	  }
>       }
>   }
> 
>@@ -657,7 +701,10 @@ 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)
>+	    print_function(ctx, demangled_name, &print_word);
>+	  else
>+	    print_word(ctx, info->name(), -1);
> 	  free(demangled_name);
> 	}
>     }
>@@ -925,61 +972,62 @@ namespace
>   }
> 
>   void
>-  print_string(PrintContext& ctx, const char* string,
>+  print_string(PrintContext& ctx, const char* str, ptrdiff_t nbc,
> 	       const _Parameter* parameters, std::size_t num_parameters)
>   {
>-    const char* start = string;
>-    const int bufsize = 128;
>-    char buf[bufsize];
>-    int bufindex = 0;
>+    const char* start = str;
>+    const char* end = nbc >= 0 ? start + nbc : nullptr;
>+    char buf[64];
> 
>-    while (*start)
>+    while ((end && str != end) || (!end && *str))
>       {
>-	if (isspace(*start))
>+	if (isspace(*str))

Again, this needs to be isspace((unsigned char)*str).

> 	  {
>-	    buf[bufindex++] = *start++;
>-	    buf[bufindex] = '\0';
>-	    print_word(ctx, buf, bufindex);
>-	    bufindex = 0;
>+	    ++str;
>+	    print_word(ctx, start, str - start);
>+	    start = str;
> 	    continue;
> 	  }
> 
>-	if (!num_parameters || *start != '%')
>+	if (!parameters || *str != '%')
> 	  {
> 	    // Normal char or no parameter to look for.
>-	    buf[bufindex++] = *start++;
>+	    ++str;
> 	    continue;
> 	  }
> 
>-	if (*++start == '%')
>+	if (*++str == '%')
> 	  {
> 	    // Escaped '%'
>-	    buf[bufindex++] = *start++;
>+	    print_word(ctx, start, str - start);
>+	    ++str;
>+	    start = str;
> 	    continue;
> 	  }

All this string manipulation code scares me, this is exactly the kind
of thing I'm talking about when I say that running extra code between
detecting UB and terminating the process is risky.

I know we already have this code, but it's exactly the kind of code
where subtle bugs creep in and nobody notices :-(

> 	// We are on a parameter property reference, we need to flush buffer
> 	// first.
>-	if (bufindex != 0)
>+	if (str != start)
> 	  {
>-	    buf[bufindex] = '\0';
>-	    print_word(ctx, buf, bufindex);
>-	    bufindex = 0;
>+	    // Avoid to print the '%'.

s/Avoid to print/Avoid printing/

>+	    if (str - start > 1)
>+	      print_word(ctx, start, str - start - 1);
>+	    start = str;
> 	  }
> 
> 	// Get the parameter number
>-	assert(*start >= '1' && *start <= '9');
>-	size_t param_index = *start - '0' - 1;
>+	assert(*str >= '1' && *str <= '9');
>+	size_t param_index = *str - '0' - 1;
> 	assert(param_index < num_parameters);
> 	const auto& param = parameters[param_index];
> 
> 	// '.' separates the parameter number from the field
> 	// name, if there is one.
>-	++start;
>-	if (*start != '.')
>+	++str;
>+	if (*str != '.')
> 	  {
>-	    assert(*start == ';');
>-	    ++start;
>+	    assert(*str == ';');
>+	    ++str;
> 	    if (param._M_kind == _Parameter::__integer)
> 	      {
> 		int written
>@@ -988,8 +1036,9 @@ namespace
> 		print_word(ctx, buf, written);
> 	      }
> 	    else if (param._M_kind == _Parameter::__string)
>-	      print_string(ctx, param._M_variant._M_string._M_value,
>+	      print_string(ctx, param._M_variant._M_string._M_value, -1,
> 			   parameters, num_parameters);
>+	    start = str;
> 	    continue;
> 	  }
> 
>@@ -997,26 +1046,80 @@ namespace
> 	const int max_field_len = 16;
> 	char field[max_field_len];
> 	int field_idx = 0;
>-	++start;
>-	while (*start != ';')
>+	++str;
>+	while (*str != ';')
> 	  {
>-	    assert(*start);
>+	    assert(*str);
> 	    assert(field_idx < max_field_len - 1);
>-	    field[field_idx++] = *start++;
>+	    field[field_idx++] = *str++;
> 	  }
>-	++start;
>+	++str;
> 	field[field_idx] = '\0';
> 
> 	print_field(ctx, param, field);
>+	start = str;
>       }
> 
>     // Might need to flush.
>-    if (bufindex)
>+    if (str != start)
>+      print_word(ctx, start, str - start);
>+  }
>+
>+  void
>+  print_string(PrintContext& ctx, const char* str, ptrdiff_t nbc)
>+  { print_string(ctx, str, nbc, nullptr, 0); }
>+
>+#ifdef _GLIBCXX_DEBUG_USE_LIBBACKTRACE
>+  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)
>       {
>-	buf[bufindex] = '\0';
>-	print_word(ctx, buf, bufindex);
>+	int status;
>+	char* demangled_name =
>+	  __cxxabiv1::__cxa_demangle(function, NULL, NULL, &status);
>+	if (status == 0)
>+	  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;
>   }
>+#endif
> }
> 
> namespace __gnu_debug
>@@ -1058,41 +1161,27 @@ 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");
>-	}
>-    }
>+#ifdef _GLIBCXX_DEBUG_USE_LIBBACKTRACE
>+    if (_M_backtrace_state)
>+      {
>+	print_literal(ctx, "Backtrace:\n");
>+	_M_print_backtrace(print_backtrace, &ctx);

If libstdc++.so gets compiled on a machine without libacktrace
installed, then _GLIBCXX_DEBUG_USE_LIBBACKTRACE will be defined to 0
here. How do you prevent _M_print_backtrace from being inlined, using
the no-op definition of __render_backtrace defined in this file?

I think you might want to defined _M_print_backtrace as:

__attribute__((__noinline__)) inline void
_Error_formatter::
_M_print_backtrace(void (*__cb)(), void* __data) const
{ __render_backtrace(_M_backtrace_state, (backtrace_full_callback)__cb, __data); }

The __noinline__ attribute will ensure it isn't inlined into
src/c++11/debug.cc and that a definition in the application can be
interposed in place of the one in libstdc++.so

I think.

>+	ctx._M_first_line = true;
>+	print_literal(ctx, "\n");
>+      }
> #endif
> 
>     print_literal(ctx, "Error: ");
> 
>     // Print the error message
>     assert(_M_text);
>-    print_string(ctx, _M_text, _M_parameters, _M_num_parameters);
>+    print_string(ctx, _M_text, -1, _M_parameters, _M_num_parameters);
>     print_literal(ctx, ".\n");
> 
>     // Emit descriptions of the objects involved in the operation
>@@ -1123,6 +1212,18 @@ namespace __gnu_debug
>     abort();
>   }
> 
>+#ifdef _GLIBCXX_DEBUG_USE_LIBBACKTRACE
>+
>+  _GLIBCXX_WEAK_DEFINITION void*
>+  __create_backtrace_state()
>+  { return nullptr; }
>+
>+  _GLIBCXX_WEAK_DEFINITION void
>+  __render_backtrace(void*, __backtrace_full_cb, void*)
>+  { }
>+

What happens if the target doesn't support weak symbols?

Should the use of libbacktrace be conditional on whether weak symbols
are supported?

Or do we even need these functions at all? Would this work?

__attribute__((__noinline__)) inline void
_Error_formatter::
_M_print_backtrace(void (*__cb)(), void* __data) const
{
#if _GLIBCXX_DEBUG_USE_LIBBACKTRACE
     backtrace_full((backtrace_state*)_M_backtrace_state, 1,
                    (backtrace_full_callback)__cb, 0, __data);
#endif
}

And for the constructor:

__attribute__((__noinline__)) inline void
_Error_formatter::
_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)
   , _M_backtrace_state(0)
{
#if _GLIBCXX_DEBUG_USE_LIBBACKTRACE
   _M_backtrace_state = backtrace_create_state(NULL, 0, NULL, NULL);
#endif
}

I think this will mean that the application code will use
backtrace_create_state when constructing an _Error_formatter and then
the library code will call _M_print_backtrace, which will use the
first definition that is found by the linker.


>+#endif
>+
> #if !_GLIBCXX_INLINE_VERSION
>   // Deprecated methods kept for backward compatibility.
>   void
>diff --git a/libstdc++-v3/testsuite/util/testsuite_abi.cc b/libstdc++-v3/testsuite/util/testsuite_abi.cc
>index 3af5dc593c2..2c3204e57a5 100644
>--- a/libstdc++-v3/testsuite/util/testsuite_abi.cc
>+++ b/libstdc++-v3/testsuite/util/testsuite_abi.cc
>@@ -210,6 +210,7 @@ check_version(symbol& test, bool added)
>       known_versions.push_back("GLIBCXX_3.4.27");
>       known_versions.push_back("GLIBCXX_3.4.28");
>       known_versions.push_back("GLIBCXX_3.4.29");
>+      known_versions.push_back("GLIBCXX_3.4.30");
>       known_versions.push_back("GLIBCXX_LDBL_3.4.29");
>       known_versions.push_back("GLIBCXX_IEEE128_3.4.29");
>       known_versions.push_back("CXXABI_1.3");

You also need to update the latestp variable:

       // Check that added symbols are added in the latest pre-release version.
       bool latestp = (test.version_name == "GLIBCXX_3.4.29"

And if you add a new symbol version then you need to update
libtool_VERSION in acinclude.m4 and regenerate the autoconf templates,
see https://gcc.gnu.org/onlinedocs/libstdc++/manual/appendix_porting.html#build_hacking.configure.version




More information about the Libstdc++ mailing list