public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] libstdc++: Add error handler for <stacktrace>
@ 2022-11-29 21:41 Björn Schäpers
  2022-11-30  6:04 ` François Dumont
  2022-11-30 12:31 ` Jonathan Wakely
  0 siblings, 2 replies; 13+ messages in thread
From: Björn Schäpers @ 2022-11-29 21:41 UTC (permalink / raw)
  To: gcc-patches, libstdc++

From: Björn Schäpers <bjoern@hazardy.de>

Not providing an error handler results in a nullpointer dereference when
an error occurs.

libstdc++-v3/ChangeLog

	* include/std/stacktrace: Add __backtrace_error_handler and use
	it in all calls to libbacktrace.
---
 libstdc++-v3/include/std/stacktrace | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/libstdc++-v3/include/std/stacktrace b/libstdc++-v3/include/std/stacktrace
index e7cbbee5638..b786441cbad 100644
--- a/libstdc++-v3/include/std/stacktrace
+++ b/libstdc++-v3/include/std/stacktrace
@@ -85,6 +85,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 #define __cpp_lib_stacktrace 202011L
 
+  inline void
+  __backtrace_error_handler(void*, const char*, int) {}
+
   // [stacktrace.entry], class stacktrace_entry
   class stacktrace_entry
   {
@@ -159,7 +162,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     _S_init()
     {
       static __glibcxx_backtrace_state* __state
-	= __glibcxx_backtrace_create_state(nullptr, 1, nullptr, nullptr);
+	= __glibcxx_backtrace_create_state(nullptr, 1,
+					   __backtrace_error_handler, nullptr);
       return __state;
     }
 
@@ -192,7 +196,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  return __function != nullptr;
       };
       const auto __state = _S_init();
-      if (::__glibcxx_backtrace_pcinfo(__state, _M_pc, +__cb, nullptr, &__data))
+      if (::__glibcxx_backtrace_pcinfo(__state, _M_pc, +__cb,
+				       __backtrace_error_handler, &__data))
 	return true;
       if (__desc && __desc->empty())
 	{
@@ -201,8 +206,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	      if (__symname)
 		*static_cast<_Data*>(__data)->_M_desc = _S_demangle(__symname);
 	  };
-	  if (::__glibcxx_backtrace_syminfo(__state, _M_pc, +__cb2, nullptr,
-					    &__data))
+	  if (::__glibcxx_backtrace_syminfo(__state, _M_pc, +__cb2,
+					    __backtrace_error_handler, &__data))
 	    return true;
 	}
       return false;
@@ -252,7 +257,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	if (auto __cb = __ret._M_prepare()) [[likely]]
 	  {
 	    auto __state = stacktrace_entry::_S_init();
-	    if (__glibcxx_backtrace_simple(__state, 1, __cb, nullptr,
+	    if (__glibcxx_backtrace_simple(__state, 1, __cb,
+					   __backtrace_error_handler,
 					   std::__addressof(__ret)))
 	      __ret._M_clear();
 	  }
@@ -270,7 +276,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	if (auto __cb = __ret._M_prepare()) [[likely]]
 	  {
 	    auto __state = stacktrace_entry::_S_init();
-	    if (__glibcxx_backtrace_simple(__state, __skip + 1, __cb, nullptr,
+	    if (__glibcxx_backtrace_simple(__state, __skip + 1, __cb,
+					   __backtrace_error_handler,
 					   std::__addressof(__ret)))
 	      __ret._M_clear();
 	  }
@@ -294,7 +301,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  {
 	    auto __state = stacktrace_entry::_S_init();
 	    int __err = __glibcxx_backtrace_simple(__state, __skip + 1, __cb,
-						   nullptr,
+						   __backtrace_error_handler,
 						   std::__addressof(__ret));
 	    if (__err < 0)
 	      __ret._M_clear();
-- 
2.38.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] libstdc++: Add error handler for <stacktrace>
  2022-11-29 21:41 [PATCH] libstdc++: Add error handler for <stacktrace> Björn Schäpers
@ 2022-11-30  6:04 ` François Dumont
  2022-11-30 11:54   ` Jonathan Wakely
  2022-11-30 19:20   ` Björn Schäpers
  2022-11-30 12:31 ` Jonathan Wakely
  1 sibling, 2 replies; 13+ messages in thread
From: François Dumont @ 2022-11-30  6:04 UTC (permalink / raw)
  To: Björn Schäpers, gcc-patches, libstdc++

[-- Attachment #1: Type: text/plain, Size: 3474 bytes --]

Good catch, then we also need this patch.

I still need to test it thought, just to make sure it compiles. Unless 
you have a nice way to force call to the error callback ?

François

On 29/11/22 22:41, Björn Schäpers wrote:
> From: Björn Schäpers <bjoern@hazardy.de>
>
> Not providing an error handler results in a nullpointer dereference when
> an error occurs.
>
> libstdc++-v3/ChangeLog
>
> 	* include/std/stacktrace: Add __backtrace_error_handler and use
> 	it in all calls to libbacktrace.
> ---
>   libstdc++-v3/include/std/stacktrace | 21 ++++++++++++++-------
>   1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/stacktrace b/libstdc++-v3/include/std/stacktrace
> index e7cbbee5638..b786441cbad 100644
> --- a/libstdc++-v3/include/std/stacktrace
> +++ b/libstdc++-v3/include/std/stacktrace
> @@ -85,6 +85,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>   
>   #define __cpp_lib_stacktrace 202011L
>   
> +  inline void
> +  __backtrace_error_handler(void*, const char*, int) {}
> +
>     // [stacktrace.entry], class stacktrace_entry
>     class stacktrace_entry
>     {
> @@ -159,7 +162,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       _S_init()
>       {
>         static __glibcxx_backtrace_state* __state
> -	= __glibcxx_backtrace_create_state(nullptr, 1, nullptr, nullptr);
> +	= __glibcxx_backtrace_create_state(nullptr, 1,
> +					   __backtrace_error_handler, nullptr);
>         return __state;
>       }
>   
> @@ -192,7 +196,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>   	  return __function != nullptr;
>         };
>         const auto __state = _S_init();
> -      if (::__glibcxx_backtrace_pcinfo(__state, _M_pc, +__cb, nullptr, &__data))
> +      if (::__glibcxx_backtrace_pcinfo(__state, _M_pc, +__cb,
> +				       __backtrace_error_handler, &__data))
>   	return true;
>         if (__desc && __desc->empty())
>   	{
> @@ -201,8 +206,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>   	      if (__symname)
>   		*static_cast<_Data*>(__data)->_M_desc = _S_demangle(__symname);
>   	  };
> -	  if (::__glibcxx_backtrace_syminfo(__state, _M_pc, +__cb2, nullptr,
> -					    &__data))
> +	  if (::__glibcxx_backtrace_syminfo(__state, _M_pc, +__cb2,
> +					    __backtrace_error_handler, &__data))
>   	    return true;
>   	}
>         return false;
> @@ -252,7 +257,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>   	if (auto __cb = __ret._M_prepare()) [[likely]]
>   	  {
>   	    auto __state = stacktrace_entry::_S_init();
> -	    if (__glibcxx_backtrace_simple(__state, 1, __cb, nullptr,
> +	    if (__glibcxx_backtrace_simple(__state, 1, __cb,
> +					   __backtrace_error_handler,
>   					   std::__addressof(__ret)))
>   	      __ret._M_clear();
>   	  }
> @@ -270,7 +276,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>   	if (auto __cb = __ret._M_prepare()) [[likely]]
>   	  {
>   	    auto __state = stacktrace_entry::_S_init();
> -	    if (__glibcxx_backtrace_simple(__state, __skip + 1, __cb, nullptr,
> +	    if (__glibcxx_backtrace_simple(__state, __skip + 1, __cb,
> +					   __backtrace_error_handler,
>   					   std::__addressof(__ret)))
>   	      __ret._M_clear();
>   	  }
> @@ -294,7 +301,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>   	  {
>   	    auto __state = stacktrace_entry::_S_init();
>   	    int __err = __glibcxx_backtrace_simple(__state, __skip + 1, __cb,
> -						   nullptr,
> +						   __backtrace_error_handler,
>   						   std::__addressof(__ret));
>   	    if (__err < 0)
>   	      __ret._M_clear();


[-- Attachment #2: debug.patch --]
[-- Type: text/x-patch, Size: 960 bytes --]

diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc
index 9eda38023f7..30cf24ca891 100644
--- a/libstdc++-v3/src/c++11/debug.cc
+++ b/libstdc++-v3/src/c++11/debug.cc
@@ -1141,6 +1141,21 @@ namespace
 
     return ret;
   }
+
+  void
+  print_backtrace_error(void* data, const char* msg, int errnum)
+  {
+    const int bufsize = 64;
+    char buf[bufsize];
+
+    PrintContext& ctx = *static_cast<PrintContext*>(data);
+
+    int written = __builtin_sprintf(buf, "%d", errnum);
+    print_word(ctx, buf, written);
+    print_literal(ctx, " - ");
+    print_word(ctx, msg);
+    print_literal(ctx, "\n");
+  }
 #endif
 }
 
@@ -1193,7 +1208,7 @@ namespace __gnu_debug
       {
 	print_literal(ctx, "Backtrace:\n");
 	_M_backtrace_full(
-	  _M_backtrace_state, 1, print_backtrace, nullptr, &ctx);
+	  _M_backtrace_state, 1, print_backtrace, print_backtrace_error, &ctx);
 	ctx._M_first_line = true;
 	print_literal(ctx, "\n");
       }

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] libstdc++: Add error handler for <stacktrace>
  2022-11-30  6:04 ` François Dumont
@ 2022-11-30 11:54   ` Jonathan Wakely
  2022-11-30 11:57     ` Jonathan Wakely
  2022-11-30 19:20   ` Björn Schäpers
  1 sibling, 1 reply; 13+ messages in thread
From: Jonathan Wakely @ 2022-11-30 11:54 UTC (permalink / raw)
  To: François Dumont; +Cc: Björn Schäpers, gcc-patches, libstdc++


[-- Attachment #1.1: Type: text/plain, Size: 655 bytes --]

On Wed, 30 Nov 2022 at 06:04, François Dumont via Libstdc++ <
libstdc++@gcc.gnu.org> wrote:

> Good catch, then we also need this patch.
>

Is it worth printing an error? If we can't show the backtrace because of an
error, we can just print nothing there.

We also need to pass an error handler to the
__glibcxx_backtrace_create_state call in formatter.h.

Now that I look at this code again, why do we need the _M_backtrace_full
member? It's always set to the same thing, why can't we just call that
function directly?

And I think we should use threaded=1 for the
__glibcxx_backtrace_create_state call.

So like the attached patch.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3797 bytes --]

commit b1ab3ca54c58e9e5505929b58bd311aca4458cda
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Nov 30 11:48:35 2022

    libstdc++: Pass error handler to libbacktrace functions
    
    Also remove a redundant data member, which also allows removing several
    namespace scope declarations.
    
    libstdc++-v3/ChangeLog:
    
            * include/debug/formatter.h [_GLIBCXX_DEBUG_BACKTRACE]
            (_Error_formatter::_Error_formatter): Pass error handler to
            __glibcxx_backtrace_create_state.
            (_Error_formatter::_M_backtrace_full): Remove data member.
            (_Error_formatter::_S_err): Define empty function.
            * src/c++11/debug.cc (_Error_formatter::_M_error): Call
            __glibcxx_backtrace_full directly.

diff --git a/libstdc++-v3/include/debug/formatter.h b/libstdc++-v3/include/debug/formatter.h
index f120163c6d4..2dd90bdf23b 100644
--- a/libstdc++-v3/include/debug/formatter.h
+++ b/libstdc++-v3/include/debug/formatter.h
@@ -32,34 +32,11 @@
 #include <bits/c++config.h>
 
 #if _GLIBCXX_HAVE_STACKTRACE
-struct __glibcxx_backtrace_state;
-
 extern "C"
-{
-  __glibcxx_backtrace_state*
-  __glibcxx_backtrace_create_state(const char*, int,
-				   void(*)(void*, const char*, int),
-				   void*);
-
-  typedef int (*__glibcxx_backtrace_full_callback) (
-    void*, __UINTPTR_TYPE__, const char *, int, const char*);
-
-  typedef void (*__glibcxx_backtrace_error_callback) (
-    void*, const char*, int);
-
-  typedef int (*__glibcxx_backtrace_full_func) (
-    __glibcxx_backtrace_state*, int,
-    __glibcxx_backtrace_full_callback,
-    __glibcxx_backtrace_error_callback,
-    void*);
-
-  int
-  __glibcxx_backtrace_full(
-    __glibcxx_backtrace_state*, int,
-    __glibcxx_backtrace_full_callback,
-    __glibcxx_backtrace_error_callback,
-    void*);
-}
+struct __glibcxx_backtrace_state*
+__glibcxx_backtrace_create_state(const char*, int,
+				 void(*)(void*, const char*, int),
+				 void*);
 #endif
 
 #if __cpp_rtti
@@ -609,8 +586,7 @@ namespace __gnu_debug
     , _M_function(__function)
 #if _GLIBCXX_HAVE_STACKTRACE
 # ifdef _GLIBCXX_DEBUG_BACKTRACE
-    , _M_backtrace_state(__glibcxx_backtrace_create_state(0, 0, 0, 0))
-    , _M_backtrace_full(&__glibcxx_backtrace_full)
+    , _M_backtrace_state(__glibcxx_backtrace_create_state(0, 1, _S_err, 0))
 # else
     , _M_backtrace_state()
 # endif
@@ -632,7 +608,8 @@ namespace __gnu_debug
     const char*		_M_function;
 #if _GLIBCXX_HAVE_STACKTRACE
     __glibcxx_backtrace_state*		_M_backtrace_state;
-    __glibcxx_backtrace_full_func	_M_backtrace_full;
+
+    static void _S_err(void*, const char*, int) { }
 #endif
 
   public:
diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc
index 9eda38023f7..2f273ec2c93 100644
--- a/libstdc++-v3/src/c++11/debug.cc
+++ b/libstdc++-v3/src/c++11/debug.cc
@@ -1084,6 +1084,20 @@ namespace
   { print_string(ctx, str, nbc, nullptr, 0); }
 
 #if _GLIBCXX_HAVE_STACKTRACE
+extern "C"
+{
+  using __glibcxx_backtrace_full_callback
+    = int (*)(void*, __UINTPTR_TYPE__, const char *, int, const char*);
+
+  using __glibcxx_backtrace_error_callback = void (*)(void*, const char*, int);
+
+  int
+  __glibcxx_backtrace_full(__glibcxx_backtrace_state*, int,
+			   __glibcxx_backtrace_full_callback,
+			   __glibcxx_backtrace_error_callback,
+			   void*);
+}
+
   void
   print_raw(PrintContext& ctx, const char* str, ptrdiff_t nbc)
   {
@@ -1192,8 +1206,8 @@ namespace __gnu_debug
     if (_M_backtrace_state)
       {
 	print_literal(ctx, "Backtrace:\n");
-	_M_backtrace_full(
-	  _M_backtrace_state, 1, print_backtrace, nullptr, &ctx);
+	__glibcxx_backtrace_full(
+	  _M_backtrace_state, 1, print_backtrace, _S_err, &ctx);
 	ctx._M_first_line = true;
 	print_literal(ctx, "\n");
       }

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] libstdc++: Add error handler for <stacktrace>
  2022-11-30 11:54   ` Jonathan Wakely
@ 2022-11-30 11:57     ` Jonathan Wakely
  2022-11-30 13:07       ` Jonathan Wakely
  0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Wakely @ 2022-11-30 11:57 UTC (permalink / raw)
  To: François Dumont; +Cc: Björn Schäpers, gcc-patches, libstdc++

[-- Attachment #1: Type: text/plain, Size: 911 bytes --]

On Wed, 30 Nov 2022 at 11:54, Jonathan Wakely <jwakely@redhat.com> wrote:

>
>
> On Wed, 30 Nov 2022 at 06:04, François Dumont via Libstdc++ <
> libstdc++@gcc.gnu.org> wrote:
>
>> Good catch, then we also need this patch.
>>
>
> Is it worth printing an error? If we can't show the backtrace because of
> an error, we can just print nothing there.
>
> We also need to pass an error handler to the
> __glibcxx_backtrace_create_state call in formatter.h.
>
> Now that I look at this code again, why do we need the _M_backtrace_full
> member? It's always set to the same thing, why can't we just call that
> function directly?
>

Oh right, I remember now ... because otherwise the libstdc++.so library
needs the definition of __glibcxx_backtrace_full.



> And I think we should use threaded=1 for the
> __glibcxx_backtrace_create_state call.
>
> So like the attached patch.
>
>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] libstdc++: Add error handler for <stacktrace>
  2022-11-29 21:41 [PATCH] libstdc++: Add error handler for <stacktrace> Björn Schäpers
  2022-11-30  6:04 ` François Dumont
@ 2022-11-30 12:31 ` Jonathan Wakely
  2022-11-30 13:05   ` Jonathan Wakely
  1 sibling, 1 reply; 13+ messages in thread
From: Jonathan Wakely @ 2022-11-30 12:31 UTC (permalink / raw)
  To: Björn Schäpers; +Cc: gcc-patches, libstdc++

[-- Attachment #1: Type: text/plain, Size: 713 bytes --]

On Tue, 29 Nov 2022 at 21:41, Björn Schäpers wrote:
>
> From: Björn Schäpers <bjoern@hazardy.de>
>
> Not providing an error handler results in a nullpointer dereference when
> an error occurs.


Thanks for the patch. This looks small enough to not require legal
paperwork, but if you intend to make further contributions (and I hope
you do!) please note the process at https://gcc.gnu.org/dco.html or
complete the paperwork for a copyright assignment to the FSF,
whichever you prefer.

I'm going to test and commit the attached patch, which replaces your
__backtrace_error_handler with a static member function so that we
don't make the name visible in the global namespace.

Thanks again!

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3161 bytes --]

commit 0e32de31f34a88b941acd2f471ba6e8e945372cf
Author: Björn Schäpers <bjoern@hazardy.de>
Date:   Wed Nov 30 12:04:16 2022

    libstdc++: Add error handler for <stacktrace>
    
    Not providing an error handler results in a null pointer dereference
    when an error occurs.
    
    Co-authored-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            * include/std/stacktrace (stacktrace_entry::_S_err_handler): New
            static function.
            (stacktrace_entry, basic_stacktrace): Pass &_S_err_handler to
            all calls to libbacktrace.

diff --git a/libstdc++-v3/include/std/stacktrace b/libstdc++-v3/include/std/stacktrace
index e7cbbee5638..ec3335e89d8 100644
--- a/libstdc++-v3/include/std/stacktrace
+++ b/libstdc++-v3/include/std/stacktrace
@@ -155,11 +155,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     template<typename _Allocator> friend class basic_stacktrace;
 
+    static void _S_err_handler(void*, const char*, int) { }
+
     static __glibcxx_backtrace_state*
     _S_init()
     {
       static __glibcxx_backtrace_state* __state
-	= __glibcxx_backtrace_create_state(nullptr, 1, nullptr, nullptr);
+	= __glibcxx_backtrace_create_state(nullptr, 1, _S_err_handler, nullptr);
       return __state;
     }
 
@@ -192,7 +194,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  return __function != nullptr;
       };
       const auto __state = _S_init();
-      if (::__glibcxx_backtrace_pcinfo(__state, _M_pc, +__cb, nullptr, &__data))
+      if (::__glibcxx_backtrace_pcinfo(__state, _M_pc, +__cb, _S_err_handler,
+				       &__data))
 	return true;
       if (__desc && __desc->empty())
 	{
@@ -201,8 +204,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	      if (__symname)
 		*static_cast<_Data*>(__data)->_M_desc = _S_demangle(__symname);
 	  };
-	  if (::__glibcxx_backtrace_syminfo(__state, _M_pc, +__cb2, nullptr,
-					    &__data))
+	  if (::__glibcxx_backtrace_syminfo(__state, _M_pc, +__cb2,
+					    _S_err_handler, &__data))
 	    return true;
 	}
       return false;
@@ -252,7 +255,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	if (auto __cb = __ret._M_prepare()) [[likely]]
 	  {
 	    auto __state = stacktrace_entry::_S_init();
-	    if (__glibcxx_backtrace_simple(__state, 1, __cb, nullptr,
+	    if (__glibcxx_backtrace_simple(__state, 1, __cb,
+					   stacktrace_entry::_S_err_handler,
 					   std::__addressof(__ret)))
 	      __ret._M_clear();
 	  }
@@ -270,7 +274,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	if (auto __cb = __ret._M_prepare()) [[likely]]
 	  {
 	    auto __state = stacktrace_entry::_S_init();
-	    if (__glibcxx_backtrace_simple(__state, __skip + 1, __cb, nullptr,
+	    if (__glibcxx_backtrace_simple(__state, __skip + 1, __cb,
+					   stacktrace_entry::_S_err_handler,
 					   std::__addressof(__ret)))
 	      __ret._M_clear();
 	  }
@@ -294,7 +299,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  {
 	    auto __state = stacktrace_entry::_S_init();
 	    int __err = __glibcxx_backtrace_simple(__state, __skip + 1, __cb,
-						   nullptr,
+						   stacktrace_entry::_S_err_handler,
 						   std::__addressof(__ret));
 	    if (__err < 0)
 	      __ret._M_clear();

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] libstdc++: Add error handler for <stacktrace>
  2022-11-30 12:31 ` Jonathan Wakely
@ 2022-11-30 13:05   ` Jonathan Wakely
  0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Wakely @ 2022-11-30 13:05 UTC (permalink / raw)
  To: Björn Schäpers; +Cc: libstdc++, gcc Patches

[-- Attachment #1: Type: text/plain, Size: 889 bytes --]

Resending with the typo in the mailing list address fixed...

On Wed, 30 Nov 2022 at 12:31, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On Tue, 29 Nov 2022 at 21:41, Björn Schäpers wrote:
> >
> > From: Björn Schäpers <bjoern@hazardy.de>
> >
> > Not providing an error handler results in a nullpointer dereference when
> > an error occurs.
>
>
> Thanks for the patch. This looks small enough to not require legal
> paperwork, but if you intend to make further contributions (and I hope
> you do!) please note the process at https://gcc.gnu.org/dco.html or
> complete the paperwork for a copyright assignment to the FSF,
> whichever you prefer.
>
> I'm going to test and commit the attached patch, which replaces your
> __backtrace_error_handler with a static member function so that we
> don't make the name visible in the global namespace.
>
> Thanks again!

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3161 bytes --]

commit 0e32de31f34a88b941acd2f471ba6e8e945372cf
Author: Björn Schäpers <bjoern@hazardy.de>
Date:   Wed Nov 30 12:04:16 2022

    libstdc++: Add error handler for <stacktrace>
    
    Not providing an error handler results in a null pointer dereference
    when an error occurs.
    
    Co-authored-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            * include/std/stacktrace (stacktrace_entry::_S_err_handler): New
            static function.
            (stacktrace_entry, basic_stacktrace): Pass &_S_err_handler to
            all calls to libbacktrace.

diff --git a/libstdc++-v3/include/std/stacktrace b/libstdc++-v3/include/std/stacktrace
index e7cbbee5638..ec3335e89d8 100644
--- a/libstdc++-v3/include/std/stacktrace
+++ b/libstdc++-v3/include/std/stacktrace
@@ -155,11 +155,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     template<typename _Allocator> friend class basic_stacktrace;
 
+    static void _S_err_handler(void*, const char*, int) { }
+
     static __glibcxx_backtrace_state*
     _S_init()
     {
       static __glibcxx_backtrace_state* __state
-	= __glibcxx_backtrace_create_state(nullptr, 1, nullptr, nullptr);
+	= __glibcxx_backtrace_create_state(nullptr, 1, _S_err_handler, nullptr);
       return __state;
     }
 
@@ -192,7 +194,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  return __function != nullptr;
       };
       const auto __state = _S_init();
-      if (::__glibcxx_backtrace_pcinfo(__state, _M_pc, +__cb, nullptr, &__data))
+      if (::__glibcxx_backtrace_pcinfo(__state, _M_pc, +__cb, _S_err_handler,
+				       &__data))
 	return true;
       if (__desc && __desc->empty())
 	{
@@ -201,8 +204,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	      if (__symname)
 		*static_cast<_Data*>(__data)->_M_desc = _S_demangle(__symname);
 	  };
-	  if (::__glibcxx_backtrace_syminfo(__state, _M_pc, +__cb2, nullptr,
-					    &__data))
+	  if (::__glibcxx_backtrace_syminfo(__state, _M_pc, +__cb2,
+					    _S_err_handler, &__data))
 	    return true;
 	}
       return false;
@@ -252,7 +255,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	if (auto __cb = __ret._M_prepare()) [[likely]]
 	  {
 	    auto __state = stacktrace_entry::_S_init();
-	    if (__glibcxx_backtrace_simple(__state, 1, __cb, nullptr,
+	    if (__glibcxx_backtrace_simple(__state, 1, __cb,
+					   stacktrace_entry::_S_err_handler,
 					   std::__addressof(__ret)))
 	      __ret._M_clear();
 	  }
@@ -270,7 +274,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	if (auto __cb = __ret._M_prepare()) [[likely]]
 	  {
 	    auto __state = stacktrace_entry::_S_init();
-	    if (__glibcxx_backtrace_simple(__state, __skip + 1, __cb, nullptr,
+	    if (__glibcxx_backtrace_simple(__state, __skip + 1, __cb,
+					   stacktrace_entry::_S_err_handler,
 					   std::__addressof(__ret)))
 	      __ret._M_clear();
 	  }
@@ -294,7 +299,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  {
 	    auto __state = stacktrace_entry::_S_init();
 	    int __err = __glibcxx_backtrace_simple(__state, __skip + 1, __cb,
-						   nullptr,
+						   stacktrace_entry::_S_err_handler,
 						   std::__addressof(__ret));
 	    if (__err < 0)
 	      __ret._M_clear();

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] libstdc++: Add error handler for <stacktrace>
  2022-11-30 11:57     ` Jonathan Wakely
@ 2022-11-30 13:07       ` Jonathan Wakely
  2022-11-30 18:00         ` François Dumont
  2022-11-30 18:17         ` François Dumont
  0 siblings, 2 replies; 13+ messages in thread
From: Jonathan Wakely @ 2022-11-30 13:07 UTC (permalink / raw)
  To: François Dumont; +Cc: Björn Schäpers, libstdc++, gcc Patches

[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]

On Wed, 30 Nov 2022 at 11:57, Jonathan Wakely <jwakely@redhat.com> wrote:
>
>
>
> On Wed, 30 Nov 2022 at 11:54, Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>>
>>
>> On Wed, 30 Nov 2022 at 06:04, François Dumont via Libstdc++ <libstdc++@gcc.gnu.org> wrote:
>>>
>>> Good catch, then we also need this patch.
>>
>>
>> Is it worth printing an error? If we can't show the backtrace because of an error, we can just print nothing there.
>>
>> We also need to pass an error handler to the __glibcxx_backtrace_create_state call in formatter.h.
>>
>> Now that I look at this code again, why do we need the _M_backtrace_full member? It's always set to the same thing, why can't we just call that function directly?
>
>
> Oh right, I remember now ... because otherwise the libstdc++.so library needs the definition of __glibcxx_backtrace_full.

I'm testing the attached patch.


>
>>
>> And I think we should use threaded=1 for the __glibcxx_backtrace_create_state call.
>>
>> So like the attached patch.
>>
>>

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3502 bytes --]

commit 6c9cc05dc097f6ee66f18731a6247cce36823d54
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Nov 30 12:32:53 2022

    libstdc++: Pass error handler to libbacktrace functions
    
    Also pass threaded=1 to __glibcxx_backtrace_create_state and remove some
    of the namespace scope declarations.
    
    libstdc++-v3/ChangeLog:
    
            * include/debug/formatter.h [_GLIBCXX_DEBUG_BACKTRACE]
            (_Error_formatter::_Error_formatter): Pass error handler to
            __glibcxx_backtrace_create_state. Pass 1 for threaded argument.
            (_Error_formatter::_S_err): Define empty function.
            * src/c++11/debug.cc (_Error_formatter::_M_error): Pass error
            handler to __glibcxx_backtrace_full.

diff --git a/libstdc++-v3/include/debug/formatter.h b/libstdc++-v3/include/debug/formatter.h
index f120163c6d4..e8a83a21bde 100644
--- a/libstdc++-v3/include/debug/formatter.h
+++ b/libstdc++-v3/include/debug/formatter.h
@@ -32,32 +32,17 @@
 #include <bits/c++config.h>
 
 #if _GLIBCXX_HAVE_STACKTRACE
-struct __glibcxx_backtrace_state;
-
 extern "C"
 {
-  __glibcxx_backtrace_state*
+  struct __glibcxx_backtrace_state*
   __glibcxx_backtrace_create_state(const char*, int,
 				   void(*)(void*, const char*, int),
 				   void*);
-
-  typedef int (*__glibcxx_backtrace_full_callback) (
-    void*, __UINTPTR_TYPE__, const char *, int, const char*);
-
-  typedef void (*__glibcxx_backtrace_error_callback) (
-    void*, const char*, int);
-
-  typedef int (*__glibcxx_backtrace_full_func) (
-    __glibcxx_backtrace_state*, int,
-    __glibcxx_backtrace_full_callback,
-    __glibcxx_backtrace_error_callback,
-    void*);
-
   int
   __glibcxx_backtrace_full(
-    __glibcxx_backtrace_state*, int,
-    __glibcxx_backtrace_full_callback,
-    __glibcxx_backtrace_error_callback,
+    struct __glibcxx_backtrace_state*, int,
+    int (*)(void*, __UINTPTR_TYPE__, const char *, int, const char*),
+    void (*)(void*, const char*, int),
     void*);
 }
 #endif
@@ -609,10 +594,10 @@ namespace __gnu_debug
     , _M_function(__function)
 #if _GLIBCXX_HAVE_STACKTRACE
 # ifdef _GLIBCXX_DEBUG_BACKTRACE
-    , _M_backtrace_state(__glibcxx_backtrace_create_state(0, 0, 0, 0))
+    , _M_backtrace_state(__glibcxx_backtrace_create_state(0, 1, _S_err, 0))
     , _M_backtrace_full(&__glibcxx_backtrace_full)
 # else
-    , _M_backtrace_state()
+    , _M_backtrace_state(0)
 # endif
 #endif
     { }
@@ -631,8 +616,12 @@ namespace __gnu_debug
     const char*		_M_text;
     const char*		_M_function;
 #if _GLIBCXX_HAVE_STACKTRACE
-    __glibcxx_backtrace_state*		_M_backtrace_state;
-    __glibcxx_backtrace_full_func	_M_backtrace_full;
+    struct __glibcxx_backtrace_state*		_M_backtrace_state;
+    // TODO: Remove _M_backtrace_full after __glibcxx_backtrace_full is moved
+    // from libstdc++_libbacktrace.a to libstdc++.so:
+    __decltype(&__glibcxx_backtrace_full)	_M_backtrace_full;
+
+    static void _S_err(void*, const char*, int) { }
 #endif
 
   public:
diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc
index 9eda38023f7..c08eaa7f921 100644
--- a/libstdc++-v3/src/c++11/debug.cc
+++ b/libstdc++-v3/src/c++11/debug.cc
@@ -1193,7 +1193,7 @@ namespace __gnu_debug
       {
 	print_literal(ctx, "Backtrace:\n");
 	_M_backtrace_full(
-	  _M_backtrace_state, 1, print_backtrace, nullptr, &ctx);
+	  _M_backtrace_state, 1, print_backtrace, _S_err, &ctx);
 	ctx._M_first_line = true;
 	print_literal(ctx, "\n");
       }

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] libstdc++: Add error handler for <stacktrace>
  2022-11-30 13:07       ` Jonathan Wakely
@ 2022-11-30 18:00         ` François Dumont
  2022-12-06 21:44           ` Jonathan Wakely
  2022-11-30 18:17         ` François Dumont
  1 sibling, 1 reply; 13+ messages in thread
From: François Dumont @ 2022-11-30 18:00 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: Björn Schäpers, libstdc++, gcc Patches

On 30/11/22 14:07, Jonathan Wakely wrote:
> On Wed, 30 Nov 2022 at 11:57, Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>>
>> On Wed, 30 Nov 2022 at 11:54, Jonathan Wakely <jwakely@redhat.com> wrote:
>>>
>>>
>>> On Wed, 30 Nov 2022 at 06:04, François Dumont via Libstdc++ <libstdc++@gcc.gnu.org> wrote:
>>>> Good catch, then we also need this patch.
>>>
>>> Is it worth printing an error? If we can't show the backtrace because of an error, we can just print nothing there.

No strong opinion on that but if we do not print anything the output 
will be:

Backtrace:

Error: ...

I just considered that it did not cost much to report the issue to the 
user that defined _GLIBCXX_DEBUG_BACKTRACE and so is expecting a backtrace.

Maybe printing "Backtrace:\n" could be done in the normal callback 
leaving the user with the feeling that _GLIBCXX_DEBUG_BACKTRACE does not 
work.

>>>
>>> We also need to pass an error handler to the __glibcxx_backtrace_create_state call in formatter.h.
>>>
>>> Now that I look at this code again, why do we need the _M_backtrace_full member? It's always set to the same thing, why can't we just call that function directly?
>>
>> Oh right, I remember now ... because otherwise the libstdc++.so library needs the definition of __glibcxx_backtrace_full.
> I'm testing the attached patch.
>
>
>>> And I think we should use threaded=1 for the __glibcxx_backtrace_create_state call.
>>>
>>> So like the attached patch.
>>>
>>>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] libstdc++: Add error handler for <stacktrace>
  2022-11-30 13:07       ` Jonathan Wakely
  2022-11-30 18:00         ` François Dumont
@ 2022-11-30 18:17         ` François Dumont
  1 sibling, 0 replies; 13+ messages in thread
From: François Dumont @ 2022-11-30 18:17 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: Björn Schäpers, libstdc++, gcc Patches

On 30/11/22 14:07, Jonathan Wakely wrote:
> On Wed, 30 Nov 2022 at 11:57, Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>>
>> On Wed, 30 Nov 2022 at 11:54, Jonathan Wakely <jwakely@redhat.com> wrote:
>>>
>>>
>>> On Wed, 30 Nov 2022 at 06:04, François Dumont via Libstdc++ <libstdc++@gcc.gnu.org> wrote:
>>>> Good catch, then we also need this patch.
>>>
>>> Is it worth printing an error? If we can't show the backtrace because of an error, we can just print nothing there.
>>>
>>> We also need to pass an error handler to the __glibcxx_backtrace_create_state call in formatter.h.
>>>
>>> Now that I look at this code again, why do we need the _M_backtrace_full member? It's always set to the same thing, why can't we just call that function directly?
>>
>> Oh right, I remember now ... because otherwise the libstdc++.so library needs the definition of __glibcxx_backtrace_full.
> I'm testing the attached patch.
>
>
>>> And I think we should use threaded=1 for the __glibcxx_backtrace_create_state call.

You mean that 2 threads could try to assert at the same time.

I don't know what's the rule on the static _Error_formatter instance in 
_S_at. If we have a strong guaranty that only 1 instance will be created 
then I understand why we need threaded=1. Even if in this case the 2 
threads will report the same stacktrace.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] libstdc++: Add error handler for <stacktrace>
  2022-11-30  6:04 ` François Dumont
  2022-11-30 11:54   ` Jonathan Wakely
@ 2022-11-30 19:20   ` Björn Schäpers
  1 sibling, 0 replies; 13+ messages in thread
From: Björn Schäpers @ 2022-11-30 19:20 UTC (permalink / raw)
  To: François Dumont, gcc-patches, libstdc++

One could (for a manual test) always change libbacktrace to call the callback. 
Or invoke it on a platform where libbacktrace can't figure out the executable 
path on its own, like currently windows.

As for an automated test, I have no idea how to enforce that, without changing 
the code to be tested.

Björn.

Am 30.11.2022 um 07:04 schrieb François Dumont:
> Good catch, then we also need this patch.
> 
> I still need to test it thought, just to make sure it compiles. Unless you have 
> a nice way to force call to the error callback ?
> 
> François
> 
> On 29/11/22 22:41, Björn Schäpers wrote:
>> From: Björn Schäpers <bjoern@hazardy.de>
>>
>> Not providing an error handler results in a nullpointer dereference when
>> an error occurs.
>>
>> libstdc++-v3/ChangeLog
>>
>>     * include/std/stacktrace: Add __backtrace_error_handler and use
>>     it in all calls to libbacktrace.
>> ---
>>   libstdc++-v3/include/std/stacktrace | 21 ++++++++++++++-------
>>   1 file changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/libstdc++-v3/include/std/stacktrace 
>> b/libstdc++-v3/include/std/stacktrace
>> index e7cbbee5638..b786441cbad 100644
>> --- a/libstdc++-v3/include/std/stacktrace
>> +++ b/libstdc++-v3/include/std/stacktrace
>> @@ -85,6 +85,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>   #define __cpp_lib_stacktrace 202011L
>> +  inline void
>> +  __backtrace_error_handler(void*, const char*, int) {}
>> +
>>     // [stacktrace.entry], class stacktrace_entry
>>     class stacktrace_entry
>>     {
>> @@ -159,7 +162,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>       _S_init()
>>       {
>>         static __glibcxx_backtrace_state* __state
>> -    = __glibcxx_backtrace_create_state(nullptr, 1, nullptr, nullptr);
>> +    = __glibcxx_backtrace_create_state(nullptr, 1,
>> +                       __backtrace_error_handler, nullptr);
>>         return __state;
>>       }
>> @@ -192,7 +196,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>         return __function != nullptr;
>>         };
>>         const auto __state = _S_init();
>> -      if (::__glibcxx_backtrace_pcinfo(__state, _M_pc, +__cb, nullptr, &__data))
>> +      if (::__glibcxx_backtrace_pcinfo(__state, _M_pc, +__cb,
>> +                       __backtrace_error_handler, &__data))
>>       return true;
>>         if (__desc && __desc->empty())
>>       {
>> @@ -201,8 +206,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>             if (__symname)
>>           *static_cast<_Data*>(__data)->_M_desc = _S_demangle(__symname);
>>         };
>> -      if (::__glibcxx_backtrace_syminfo(__state, _M_pc, +__cb2, nullptr,
>> -                        &__data))
>> +      if (::__glibcxx_backtrace_syminfo(__state, _M_pc, +__cb2,
>> +                        __backtrace_error_handler, &__data))
>>           return true;
>>       }
>>         return false;
>> @@ -252,7 +257,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>       if (auto __cb = __ret._M_prepare()) [[likely]]
>>         {
>>           auto __state = stacktrace_entry::_S_init();
>> -        if (__glibcxx_backtrace_simple(__state, 1, __cb, nullptr,
>> +        if (__glibcxx_backtrace_simple(__state, 1, __cb,
>> +                       __backtrace_error_handler,
>>                          std::__addressof(__ret)))
>>             __ret._M_clear();
>>         }
>> @@ -270,7 +276,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>       if (auto __cb = __ret._M_prepare()) [[likely]]
>>         {
>>           auto __state = stacktrace_entry::_S_init();
>> -        if (__glibcxx_backtrace_simple(__state, __skip + 1, __cb, nullptr,
>> +        if (__glibcxx_backtrace_simple(__state, __skip + 1, __cb,
>> +                       __backtrace_error_handler,
>>                          std::__addressof(__ret)))
>>             __ret._M_clear();
>>         }
>> @@ -294,7 +301,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>         {
>>           auto __state = stacktrace_entry::_S_init();
>>           int __err = __glibcxx_backtrace_simple(__state, __skip + 1, __cb,
>> -                           nullptr,
>> +                           __backtrace_error_handler,
>>                              std::__addressof(__ret));
>>           if (__err < 0)
>>             __ret._M_clear();
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] libstdc++: Add error handler for <stacktrace>
  2022-11-30 18:00         ` François Dumont
@ 2022-12-06 21:44           ` Jonathan Wakely
  2022-12-07 17:58             ` François Dumont
  0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Wakely @ 2022-12-06 21:44 UTC (permalink / raw)
  To: François Dumont; +Cc: Björn Schäpers, libstdc++, gcc Patches

[-- Attachment #1: Type: text/plain, Size: 1068 bytes --]

On Wed, 30 Nov 2022 at 18:00, François Dumont <frs.dumont@gmail.com> wrote:
>
> On 30/11/22 14:07, Jonathan Wakely wrote:
> > On Wed, 30 Nov 2022 at 11:57, Jonathan Wakely <jwakely@redhat.com> wrote:
> >>
> >>
> >> On Wed, 30 Nov 2022 at 11:54, Jonathan Wakely <jwakely@redhat.com> wrote:
> >>>
> >>>
> >>> On Wed, 30 Nov 2022 at 06:04, François Dumont via Libstdc++ <libstdc++@gcc.gnu.org> wrote:
> >>>> Good catch, then we also need this patch.
> >>>
> >>> Is it worth printing an error? If we can't show the backtrace because of an error, we can just print nothing there.
>
> No strong opinion on that but if we do not print anything the output
> will be:
>
> Backtrace:
>
> Error: ...
>
> I just considered that it did not cost much to report the issue to the
> user that defined _GLIBCXX_DEBUG_BACKTRACE and so is expecting a backtrace.
>
> Maybe printing "Backtrace:\n" could be done in the normal callback
> leaving the user with the feeling that _GLIBCXX_DEBUG_BACKTRACE does not
> work.

OK, how's this?

Tested x86_64-linux.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 4115 bytes --]

commit aadf83283f0d3e1d473ac17595ba73d0210bb9ba
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Nov 30 12:32:53 2022

    libstdc++: Pass error handler to libbacktrace functions
    
    Also pass threaded=1 to __glibcxx_backtrace_create_state and remove some
    of the namespace scope declarations in the header.
    
    Co-authored-by: François Dumont <frs.dumont@gmail.com>
    
    libstdc++-v3/ChangeLog:
    
            * include/debug/formatter.h [_GLIBCXX_DEBUG_BACKTRACE]
            (_Error_formatter::_Error_formatter): Pass error handler to
            __glibcxx_backtrace_create_state. Pass 1 for threaded argument.
            (_Error_formatter::_S_err): Define empty function.
            * src/c++11/debug.cc (_Error_formatter::_M_error): Pass error
            handler to __glibcxx_backtrace_full.

diff --git a/libstdc++-v3/include/debug/formatter.h b/libstdc++-v3/include/debug/formatter.h
index f120163c6d4..e8a83a21bde 100644
--- a/libstdc++-v3/include/debug/formatter.h
+++ b/libstdc++-v3/include/debug/formatter.h
@@ -32,32 +32,17 @@
 #include <bits/c++config.h>
 
 #if _GLIBCXX_HAVE_STACKTRACE
-struct __glibcxx_backtrace_state;
-
 extern "C"
 {
-  __glibcxx_backtrace_state*
+  struct __glibcxx_backtrace_state*
   __glibcxx_backtrace_create_state(const char*, int,
 				   void(*)(void*, const char*, int),
 				   void*);
-
-  typedef int (*__glibcxx_backtrace_full_callback) (
-    void*, __UINTPTR_TYPE__, const char *, int, const char*);
-
-  typedef void (*__glibcxx_backtrace_error_callback) (
-    void*, const char*, int);
-
-  typedef int (*__glibcxx_backtrace_full_func) (
-    __glibcxx_backtrace_state*, int,
-    __glibcxx_backtrace_full_callback,
-    __glibcxx_backtrace_error_callback,
-    void*);
-
   int
   __glibcxx_backtrace_full(
-    __glibcxx_backtrace_state*, int,
-    __glibcxx_backtrace_full_callback,
-    __glibcxx_backtrace_error_callback,
+    struct __glibcxx_backtrace_state*, int,
+    int (*)(void*, __UINTPTR_TYPE__, const char *, int, const char*),
+    void (*)(void*, const char*, int),
     void*);
 }
 #endif
@@ -609,10 +594,10 @@ namespace __gnu_debug
     , _M_function(__function)
 #if _GLIBCXX_HAVE_STACKTRACE
 # ifdef _GLIBCXX_DEBUG_BACKTRACE
-    , _M_backtrace_state(__glibcxx_backtrace_create_state(0, 0, 0, 0))
+    , _M_backtrace_state(__glibcxx_backtrace_create_state(0, 1, _S_err, 0))
     , _M_backtrace_full(&__glibcxx_backtrace_full)
 # else
-    , _M_backtrace_state()
+    , _M_backtrace_state(0)
 # endif
 #endif
     { }
@@ -631,8 +616,12 @@ namespace __gnu_debug
     const char*		_M_text;
     const char*		_M_function;
 #if _GLIBCXX_HAVE_STACKTRACE
-    __glibcxx_backtrace_state*		_M_backtrace_state;
-    __glibcxx_backtrace_full_func	_M_backtrace_full;
+    struct __glibcxx_backtrace_state*		_M_backtrace_state;
+    // TODO: Remove _M_backtrace_full after __glibcxx_backtrace_full is moved
+    // from libstdc++_libbacktrace.a to libstdc++.so:
+    __decltype(&__glibcxx_backtrace_full)	_M_backtrace_full;
+
+    static void _S_err(void*, const char*, int) { }
 #endif
 
   public:
diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc
index 9eda38023f7..83996ffe622 100644
--- a/libstdc++-v3/src/c++11/debug.cc
+++ b/libstdc++-v3/src/c++11/debug.cc
@@ -1141,6 +1141,23 @@ namespace
 
     return ret;
   }
+
+  void
+  print_backtrace_error(void* data, const char* msg, int errnum)
+  {
+    PrintContext& ctx = *static_cast<PrintContext*>(data);
+
+    print_literal(ctx, "Backtrace unavailable: ");
+    print_word(ctx, msg ? msg : "<unknown error>");
+    if (errnum > 0)
+      {
+	char buf[64];
+	int written = __builtin_sprintf(buf, " (errno=%d)\n", errnum);
+	print_word(ctx, buf, written);
+      }
+    else
+      print_literal(ctx, "\n");
+  }
 #endif
 }
 
@@ -1193,7 +1210,7 @@ namespace __gnu_debug
       {
 	print_literal(ctx, "Backtrace:\n");
 	_M_backtrace_full(
-	  _M_backtrace_state, 1, print_backtrace, nullptr, &ctx);
+	  _M_backtrace_state, 1, print_backtrace, print_backtrace_error, &ctx);
 	ctx._M_first_line = true;
 	print_literal(ctx, "\n");
       }

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] libstdc++: Add error handler for <stacktrace>
  2022-12-06 21:44           ` Jonathan Wakely
@ 2022-12-07 17:58             ` François Dumont
  2022-12-07 20:06               ` Jonathan Wakely
  0 siblings, 1 reply; 13+ messages in thread
From: François Dumont @ 2022-12-07 17:58 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: Björn Schäpers, libstdc++, gcc Patches

Looks perfect to me, thanks.

On 06/12/22 22:44, Jonathan Wakely wrote:
> On Wed, 30 Nov 2022 at 18:00, François Dumont <frs.dumont@gmail.com> wrote:
>> On 30/11/22 14:07, Jonathan Wakely wrote:
>>> On Wed, 30 Nov 2022 at 11:57, Jonathan Wakely <jwakely@redhat.com> wrote:
>>>>
>>>> On Wed, 30 Nov 2022 at 11:54, Jonathan Wakely <jwakely@redhat.com> wrote:
>>>>>
>>>>> On Wed, 30 Nov 2022 at 06:04, François Dumont via Libstdc++ <libstdc++@gcc.gnu.org> wrote:
>>>>>> Good catch, then we also need this patch.
>>>>> Is it worth printing an error? If we can't show the backtrace because of an error, we can just print nothing there.
>> No strong opinion on that but if we do not print anything the output
>> will be:
>>
>> Backtrace:
>>
>> Error: ...
>>
>> I just considered that it did not cost much to report the issue to the
>> user that defined _GLIBCXX_DEBUG_BACKTRACE and so is expecting a backtrace.
>>
>> Maybe printing "Backtrace:\n" could be done in the normal callback
>> leaving the user with the feeling that _GLIBCXX_DEBUG_BACKTRACE does not
>> work.
> OK, how's this?
>
> Tested x86_64-linux.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] libstdc++: Add error handler for <stacktrace>
  2022-12-07 17:58             ` François Dumont
@ 2022-12-07 20:06               ` Jonathan Wakely
  0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Wakely @ 2022-12-07 20:06 UTC (permalink / raw)
  To: François Dumont; +Cc: Björn Schäpers, libstdc++, gcc Patches

On Wed, 7 Dec 2022 at 17:58, François Dumont <frs.dumont@gmail.com> wrote:
>
> Looks perfect to me, thanks.

OK thanks, it's pushed to trunk now.


>
> On 06/12/22 22:44, Jonathan Wakely wrote:
> > On Wed, 30 Nov 2022 at 18:00, François Dumont <frs.dumont@gmail.com> wrote:
> >> On 30/11/22 14:07, Jonathan Wakely wrote:
> >>> On Wed, 30 Nov 2022 at 11:57, Jonathan Wakely <jwakely@redhat.com> wrote:
> >>>>
> >>>> On Wed, 30 Nov 2022 at 11:54, Jonathan Wakely <jwakely@redhat.com> wrote:
> >>>>>
> >>>>> On Wed, 30 Nov 2022 at 06:04, François Dumont via Libstdc++ <libstdc++@gcc.gnu.org> wrote:
> >>>>>> Good catch, then we also need this patch.
> >>>>> Is it worth printing an error? If we can't show the backtrace because of an error, we can just print nothing there.
> >> No strong opinion on that but if we do not print anything the output
> >> will be:
> >>
> >> Backtrace:
> >>
> >> Error: ...
> >>
> >> I just considered that it did not cost much to report the issue to the
> >> user that defined _GLIBCXX_DEBUG_BACKTRACE and so is expecting a backtrace.
> >>
> >> Maybe printing "Backtrace:\n" could be done in the normal callback
> >> leaving the user with the feeling that _GLIBCXX_DEBUG_BACKTRACE does not
> >> work.
> > OK, how's this?
> >
> > Tested x86_64-linux.
>
>


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-12-07 20:07 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-29 21:41 [PATCH] libstdc++: Add error handler for <stacktrace> Björn Schäpers
2022-11-30  6:04 ` François Dumont
2022-11-30 11:54   ` Jonathan Wakely
2022-11-30 11:57     ` Jonathan Wakely
2022-11-30 13:07       ` Jonathan Wakely
2022-11-30 18:00         ` François Dumont
2022-12-06 21:44           ` Jonathan Wakely
2022-12-07 17:58             ` François Dumont
2022-12-07 20:06               ` Jonathan Wakely
2022-11-30 18:17         ` François Dumont
2022-11-30 19:20   ` Björn Schäpers
2022-11-30 12:31 ` Jonathan Wakely
2022-11-30 13:05   ` Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).