Re: [PATCH][_GLIBCXX_DEBUG] libbacktrace integration

[François Dumont via Gcc-patches]

On 07/05/21 4:26 pm, Jonathan Wakely wrote:

On 05/05/21 12:33 +0100, Jonathan Wakely wrote:

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.


Ville pointed out that we'll need to use libbacktrace for
std::stacktrace  anyway, and it would be
useful if/when we add support for C++ Contracts to the lirbary.

So let's integrate libbacktrace into libstdc++ properly. Jakub
suggested doing it how libsanitizer does it, which is to rebuild the
libbacktrace sources as part of the libsanitizer build, using the
preprocessor to rename the symbols so that they use reserved names.
e.g. rename backtrace_full to __glibcxx_backtrace_full or something
like that.

I'll work on getting it building as part of libstdc++ (or maybe as a
separate static library for now, as we do for libstdc++fs.a) and then
you can rework your Debug Mode patch to depend on the private version
of libbacktrace included with libstdc++ (instead of expecting users to
provide it themselves).


Ok, thanks for the info.

I will perhaps extract the non-backtrace related stuff from this patch 
then and I'll rework the backtrace part later.

Re: [PATCH][_GLIBCXX_DEBUG] libbacktrace integration

[Jonathan Wakely via Gcc-patches]

On 05/05/21 12:33 +0100, Jonathan Wakely wrote:

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.


Ville pointed out that we'll need to use libbacktrace for
std::stacktrace  anyway, and it would be
useful if/when we add support for C++ Contracts to the lirbary.

So let's integrate libbacktrace into libstdc++ properly. Jakub
suggested doing it how libsanitizer does it, which is to rebuild the
libbacktrace sources as part of the libsanitizer build, using the
preprocessor to rename the symbols so that they use reserved names.
e.g. rename backtrace_full to __glibcxx_backtrace_full or something
like that.

I'll work on getting it building as part of libstdc++ (or maybe as a
separate static library for now, as we do for libstdc++fs.a) and then
you can rework your Debug Mode patch to depend on the private version
of libbacktrace included with libstdc++ (instead of expecting users to
provide it themselves).

Re: [PATCH][_GLIBCXX_DEBUG] libbacktrace integration

[Jonathan Wakely via Gcc-patches]

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 .
    [_GLIBCXX_DEBUG_BACKTRACE && BACKTRACE_SUPPORTED]:
    Include .
    [(!_GLIBCXX_DEBUG_BACKTRACE || !BACKTRACE_SUPPORTED) &&
    _GLIBCXX_USE_C99_STDINT_TR1]: Include .
    [_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 .
    (_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.
  GLIBCXX_DEBUG_MESSAGE_LENGTH can be used to request a
  different length.

+Note that libstdc++ is able to use
+  http://www.w3.org/1999/xlink";
+  
xlink:href="https://github.com/ianlancetaylor/libbacktrace";>libbacktrace
+  to produce backtraces on error. Use -D_GLIBCXX_DEBUG_BACKTRACE 
to
+  activate it. You'll also have to link with libbacktrace
+  (-lbacktrace) to build your application.


Using a 
Specific Debug Container
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.
  

+_GLIBCXX_DEBUG_BACKTRACE
+
+  
+   Undefined by default. Considered only if _GLIBCXX_DEBUG
+   is defined. When defined, checks for http://www.w3.org/1999/xlink";
+   
xlink:href="https://github.com/ianlancetaylor/libbacktrace

Re: [PATCH][_GLIBCXX_DEBUG] libbacktrace integration

[Jonathan Wakely via Gcc-patches]

On 04/05/21 08:03 +0200, François Dumont wrote:

On 03/05/21 11:06 pm, Jonathan Wakely wrote:

On 03/05/21 22:17 +0200, François Dumont via Libstdc++ wrote:

Is it too early to consider this patch ? Or just lack of time ?


I haven't had time to review it yet, but my general feeling hasn't
changed. I still don't like the idea of executing additional code
after undefined behaviour is detected. I've been convinced by glibc
folk that every bit of code run when the program state is corrupt
increases the risk that it can be exploited by an attacker.




Ok, I must have miss (or forgot) this feedback.


See https://gcc.gnu.org/pipermail/libstdc++/2018-December/048061.html


Well, isn't it the current situation of the whole _GLIBCXX_DEBUG mode ?


Yes, but adding more code makes it worse.

For me _GLIBCXX_DEBUG mode purpose is to detect UB situation and to 
assert _before_ any UB code is run.


Yes, it stops running the user code, but then runs its own code to
format the message to show to the user. The more code that runs when
the program is in an inconsistent/undefined state, the more likely it
is that some of that code can be exploited to do something bad.

Moreover it is optional. This is a feature to use when _GLIBCXX_DEBUG 
is telling you that you have a problem in your code but you just 
cannot find where it is called.


Which you can do with a debugger. When debug mode calls abort() it
will stop the program in a debugger, or produce a core file that can
be examined in a debugger.

The stacktrace is a convenience, not providing anything that couldn't
be done already.

Anyway, I'll review the patch...

Re: [PATCH][_GLIBCXX_DEBUG] libbacktrace integration

[François Dumont via Gcc-patches]

On 03/05/21 11:06 pm, Jonathan Wakely wrote:

On 03/05/21 22:17 +0200, François Dumont via Libstdc++ wrote:

Is it too early to consider this patch ? Or just lack of time ?


I haven't had time to review it yet, but my general feeling hasn't
changed. I still don't like the idea of executing additional code
after undefined behaviour is detected. I've been convinced by glibc
folk that every bit of code run when the program state is corrupt
increases the risk that it can be exploited by an attacker.




Ok, I must have miss (or forgot) this feedback.

Well, isn't it the current situation of the whole _GLIBCXX_DEBUG mode ?

For me _GLIBCXX_DEBUG mode purpose is to detect UB situation and to 
assert _before_ any UB code is run.


Moreover it is optional. This is a feature to use when _GLIBCXX_DEBUG is 
telling you that you have a problem in your code but you just cannot 
find where it is called.

Re: [PATCH][_GLIBCXX_DEBUG] libbacktrace integration

[Jonathan Wakely via Gcc-patches]

On 03/05/21 22:17 +0200, François Dumont via Libstdc++ wrote:

Is it too early to consider this patch ? Or just lack of time ?


I haven't had time to review it yet, but my general feeling hasn't
changed. I still don't like the idea of executing additional code
after undefined behaviour is detected. I've been convinced by glibc
folk that every bit of code run when the program state is corrupt
increases the risk that it can be exploited by an attacker.

Re: [PATCH][_GLIBCXX_DEBUG] libbacktrace integration

[François Dumont via Gcc-patches]

Is it too early to consider this patch ? Or just lack of time ?

Considering the patch I would really appreciate that, if validated, it 
gets in as early as possible in next release.


Thanks,
François

On 24/04/21 3:46 pm, François Dumont 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 .
    [_GLIBCXX_DEBUG_BACKTRACE && BACKTRACE_SUPPORTED]:
    Include .
    [(!_GLIBCXX_DEBUG_BACKTRACE || !BACKTRACE_SUPPORTED) &&
    _GLIBCXX_USE_C99_STDINT_TR1]: Include .
    [_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 .
    (_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

[PATCH][_GLIBCXX_DEBUG] libbacktrace integration

[François Dumont via Gcc-patches]

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 .
    [_GLIBCXX_DEBUG_BACKTRACE && BACKTRACE_SUPPORTED]:
    Include .
    [(!_GLIBCXX_DEBUG_BACKTRACE || !BACKTRACE_SUPPORTED) &&
    _GLIBCXX_USE_C99_STDINT_TR1]: Include .
    [_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 .
    (_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.
   GLIBCXX_DEBUG_MESSAGE_LENGTH can be used to request a
   different length.
 
+Note that libstdc++ is able to use
+  http://www.w3.org/1999/xlink";
+  xlink:href="https://github.com/ianlancetaylor/libbacktrace";>libbacktrace
+  to produce backtraces on error. Use -D_GLIBCXX_DEBUG_BACKTRACE to
+  activate it. You'll also have to link with libbacktrace
+  (-lbacktrace) to build your application.
 
 
 Using a Specific Debug Container
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.
   
 
+_GLIBCXX_DEBUG_BACKTRACE
+
+  
+	Undefined by default. Considered only if _GLIBCXX_DEBUG
+	is defined. When defined, checks for http://www.w3.org/1999/xlink";
+	xlink:href="https://github.com/ianlancetaylor/libbacktrace";>libbacktrace
+	support and use it to display backtraces on
+	debug mode assertion