Do not print anything when self-backtrace unavailable

Message ID 20220105154803.2124458-1-tromey@adacore.com
State New
Headers show
Series
  • Do not print anything when self-backtrace unavailable
Related show

Commit Message

Simon Marchi via Gdb-patches Jan. 5, 2022, 3:48 p.m.
Right now, gdb's self-backtrace feature will still print something
when a backtrace is unavailable:

   sig_write (_("----- Backtrace -----\n"));
[...]
     sig_write (_("Backtrace unavailable\n"));
    sig_write ("---------------------\n");

However, if GDB_PRINT_INTERNAL_BACKTRACE is undefined, it seems better
to me to print nothing at all.

This patch implements this change.  It also makes a couple of other
small changes in this same module: it adds a header guard to
bt-utils.h, and it protects the definitions of
gdb_internal_backtrace_1 with a check of GDB_PRINT_INTERNAL_BACKTRACE.
---
 gdb/bt-utils.c | 6 ++++--
 gdb/bt-utils.h | 5 +++++
 2 files changed, 9 insertions(+), 2 deletions(-)

-- 
2.31.1

Comments

Simon Marchi via Gdb-patches Jan. 5, 2022, 4:36 p.m. | #1
* Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> [2022-01-05 08:48:03 -0700]:

> Right now, gdb's self-backtrace feature will still print something

> when a backtrace is unavailable:

> 

>    sig_write (_("----- Backtrace -----\n"));

> [...]

>      sig_write (_("Backtrace unavailable\n"));

>     sig_write ("---------------------\n");

> 

> However, if GDB_PRINT_INTERNAL_BACKTRACE is undefined, it seems better

> to me to print nothing at all.

> 

> This patch implements this change.  It also makes a couple of other

> small changes in this same module: it adds a header guard to

> bt-utils.h, and it protects the definitions of

> gdb_internal_backtrace_1 with a check of GDB_PRINT_INTERNAL_BACKTRACE.

> ---

>  gdb/bt-utils.c | 6 ++++--

>  gdb/bt-utils.h | 5 +++++

>  2 files changed, 9 insertions(+), 2 deletions(-)

> 

> diff --git a/gdb/bt-utils.c b/gdb/bt-utils.c

> index d45631551af..60cc59dc382 100644

> --- a/gdb/bt-utils.c

> +++ b/gdb/bt-utils.c

> @@ -41,6 +41,7 @@ gdb_internal_backtrace_set_cmd (const char *args, int from_tty,

>  #endif

>  }

>  

> +#ifdef GDB_PRINT_INTERNAL_BACKTRACE

>  #ifdef GDB_PRINT_INTERNAL_BACKTRACE_USING_LIBBACKTRACE

>  

>  /* Callback used by libbacktrace if it encounters an error.  */

> @@ -143,6 +144,7 @@ gdb_internal_backtrace_1 ()

>  }

>  

>  #endif

> +#endif /* GDB_PRINT_INTERNAL_BACKTRACE */


If you're adding this extra level of protection then I think we should
structure it like:

  #ifdef GDB_PRINT_INTERNAL_BACKTRACE
  #ifdef GDB_PRINT_INTERNAL_BACKTRACE_USING_LIBBACKTRACE
  ...
  #elif defined GDB_PRINT_INTERNAL_BACKTRACE_USING_EXECINFO
  ...
  #else
  #error "unexpected internal backtrace policy"
  #endif
  #endif /* GDB_PRINT_INTERNAL_BACKTRACE */

if that makes sense?

>  

>  /* See bt-utils.h.  */

>  

> @@ -152,6 +154,7 @@ gdb_internal_backtrace ()

>    if (current_ui == nullptr)

>      return;

>  

> +#ifdef GDB_PRINT_INTERNAL_BACKTRACE

>    const auto sig_write = [] (const char *msg) -> void

>    {

>      gdb_stderr->write_async_safe (msg, strlen (msg));

> @@ -159,12 +162,11 @@ gdb_internal_backtrace ()

>  

>    sig_write (_("----- Backtrace -----\n"));

>  

> -#ifdef GDB_PRINT_INTERNAL_BACKTRACE

>    if (gdb_stderr->fd () > -1)

>      gdb_internal_backtrace_1 ();

>    else

> -#endif

>      sig_write (_("Backtrace unavailable\n"));

>  

>    sig_write ("---------------------\n");

> +#endif

>  }

> diff --git a/gdb/bt-utils.h b/gdb/bt-utils.h

> index 42edc94cef0..835a508a89d 100644

> --- a/gdb/bt-utils.h

> +++ b/gdb/bt-utils.h

> @@ -18,6 +18,9 @@

>  /* Support for printing a backtrace when GDB hits an error.  This is not

>     for printing backtraces of the inferior, but backtraces of GDB itself.  */

>  

> +#ifndef GDB_BT_UTILS_H

> +#define GDB_BT_UTILS_H


It doesn't really matter, but the GDB style seems to be to use the
filename without the GDB_ prefix.  Can we follow that style here?

Otherwise, this looks good.

Thanks,
Andrew


> +

>  #ifdef HAVE_LIBBACKTRACE

>  # include "backtrace.h"

>  # include "backtrace-supported.h"

> @@ -67,3 +70,5 @@ extern void gdb_internal_backtrace ();

>  

>  extern void gdb_internal_backtrace_set_cmd (const char *args, int from_tty,

>  					    cmd_list_element *c);

> +

> +#endif /* GDB_BT_UTILS_H */

> -- 

> 2.31.1

>
Simon Marchi via Gdb-patches Jan. 5, 2022, 5:07 p.m. | #2
Andrew> If you're adding this extra level of protection then I think we should
Andrew> structure it like:

Andrew>   #ifdef GDB_PRINT_INTERNAL_BACKTRACE
Andrew>   #ifdef GDB_PRINT_INTERNAL_BACKTRACE_USING_LIBBACKTRACE
Andrew>   ...
Andrew>   #elif defined GDB_PRINT_INTERNAL_BACKTRACE_USING_EXECINFO
Andrew>   ...
Andrew>   #else
Andrew>   #error "unexpected internal backtrace policy"
Andrew>   #endif
Andrew>   #endif /* GDB_PRINT_INTERNAL_BACKTRACE */

Andrew> if that makes sense?

Yep.

>> +#ifndef GDB_BT_UTILS_H

>> +#define GDB_BT_UTILS_H


Andrew> It doesn't really matter, but the GDB style seems to be to use the
Andrew> filename without the GDB_ prefix.  Can we follow that style here?

Andrew> Otherwise, this looks good.

Thanks.  I'm going to check it in with those change.

Tom

Patch

diff --git a/gdb/bt-utils.c b/gdb/bt-utils.c
index d45631551af..60cc59dc382 100644
--- a/gdb/bt-utils.c
+++ b/gdb/bt-utils.c
@@ -41,6 +41,7 @@  gdb_internal_backtrace_set_cmd (const char *args, int from_tty,
 #endif
 }
 
+#ifdef GDB_PRINT_INTERNAL_BACKTRACE
 #ifdef GDB_PRINT_INTERNAL_BACKTRACE_USING_LIBBACKTRACE
 
 /* Callback used by libbacktrace if it encounters an error.  */
@@ -143,6 +144,7 @@  gdb_internal_backtrace_1 ()
 }
 
 #endif
+#endif /* GDB_PRINT_INTERNAL_BACKTRACE */
 
 /* See bt-utils.h.  */
 
@@ -152,6 +154,7 @@  gdb_internal_backtrace ()
   if (current_ui == nullptr)
     return;
 
+#ifdef GDB_PRINT_INTERNAL_BACKTRACE
   const auto sig_write = [] (const char *msg) -> void
   {
     gdb_stderr->write_async_safe (msg, strlen (msg));
@@ -159,12 +162,11 @@  gdb_internal_backtrace ()
 
   sig_write (_("----- Backtrace -----\n"));
 
-#ifdef GDB_PRINT_INTERNAL_BACKTRACE
   if (gdb_stderr->fd () > -1)
     gdb_internal_backtrace_1 ();
   else
-#endif
     sig_write (_("Backtrace unavailable\n"));
 
   sig_write ("---------------------\n");
+#endif
 }
diff --git a/gdb/bt-utils.h b/gdb/bt-utils.h
index 42edc94cef0..835a508a89d 100644
--- a/gdb/bt-utils.h
+++ b/gdb/bt-utils.h
@@ -18,6 +18,9 @@ 
 /* Support for printing a backtrace when GDB hits an error.  This is not
    for printing backtraces of the inferior, but backtraces of GDB itself.  */
 
+#ifndef GDB_BT_UTILS_H
+#define GDB_BT_UTILS_H
+
 #ifdef HAVE_LIBBACKTRACE
 # include "backtrace.h"
 # include "backtrace-supported.h"
@@ -67,3 +70,5 @@  extern void gdb_internal_backtrace ();
 
 extern void gdb_internal_backtrace_set_cmd (const char *args, int from_tty,
 					    cmd_list_element *c);
+
+#endif /* GDB_BT_UTILS_H */