public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/6] Display GDB backtrace for internal errors
@ 2021-08-19  9:49 Andrew Burgess
  2021-08-19  9:49 ` [PATCH 1/6] gdb: use bool instead of int in struct internal_problem Andrew Burgess
                   ` (8 more replies)
  0 siblings, 9 replies; 34+ messages in thread
From: Andrew Burgess @ 2021-08-19  9:49 UTC (permalink / raw)
  To: gdb-patches

This series does two things:

     1. Imports libbacktrace from gcc to present better backtraces of
     GDB when we hit a fatal signal, and

     2. Prints the backtrace when GDB hits an internal-error (e.g. an
     assert).

The hope here is to try an improve the quality of bug reports, instead
of just getting a report that GDB hit some assert in frame.c, or
value.c, we will (hopefully) have a backtrace included with the bug
report, which _might_ give developers a fighting chance at figuring
out what went wrong.

All feedback welcome,

Thanks,
Andrew

---

Andrew Burgess (6):
  gdb: use bool instead of int in struct internal_problem
  gdb: make use of std::string in utils.c
  gdb: Add a dependency between gdb and libbacktrace
  Copy in libbacktrace from gcc
  gdb: use libbacktrace to create a better backtrace for fatal signals
  gdb: print backtrace for internal error/warning

 Makefile.def                                  |     1 +
 Makefile.in                                   |     1 +
 gdb/Makefile.in                               |    16 +-
 gdb/NEWS                                      |     8 +
 gdb/bt-utils.c                                |   170 +
 gdb/bt-utils.h                                |    69 +
 gdb/config.in                                 |     3 +
 gdb/configure                                 |    32 +
 gdb/configure.ac                              |    23 +
 gdb/doc/gdb.texinfo                           |    13 +
 gdb/event-top.c                               |    47 +-
 .../gdb.base/bt-on-error-and-warning.exp      |   118 +
 gdb/testsuite/gdb.base/bt-on-fatal-signal.exp |    36 -
 gdb/utils.c                                   |   121 +-
 libbacktrace/ChangeLog                        |  1759 ++
 libbacktrace/ChangeLog.jit                    |    14 +
 libbacktrace/Makefile.am                      |   586 +
 libbacktrace/Makefile.in                      |  2528 +++
 libbacktrace/README                           |    23 +
 libbacktrace/aclocal.m4                       |   867 +
 libbacktrace/alloc.c                          |   167 +
 libbacktrace/allocfail.c                      |   136 +
 libbacktrace/allocfail.sh                     |   104 +
 libbacktrace/atomic.c                         |   113 +
 libbacktrace/backtrace-supported.h.in         |    66 +
 libbacktrace/backtrace.c                      |   129 +
 libbacktrace/backtrace.h                      |   206 +
 libbacktrace/btest.c                          |   501 +
 libbacktrace/config.h.in                      |   184 +
 libbacktrace/configure                        | 16893 ++++++++++++++++
 libbacktrace/configure.ac                     |   581 +
 libbacktrace/dwarf.c                          |  4056 ++++
 libbacktrace/edtest.c                         |   120 +
 libbacktrace/edtest2.c                        |    43 +
 libbacktrace/elf.c                            |  4919 +++++
 libbacktrace/fileline.c                       |   346 +
 libbacktrace/filetype.awk                     |    13 +
 .../install-debuginfo-for-buildid.sh.in       |    65 +
 libbacktrace/instrumented_alloc.c             |   114 +
 libbacktrace/internal.h                       |   380 +
 libbacktrace/macho.c                          |  1355 ++
 libbacktrace/mmap.c                           |   331 +
 libbacktrace/mmapio.c                         |   110 +
 libbacktrace/mtest.c                          |   410 +
 libbacktrace/nounwind.c                       |    66 +
 libbacktrace/pecoff.c                         |   935 +
 libbacktrace/posix.c                          |   104 +
 libbacktrace/print.c                          |    92 +
 libbacktrace/read.c                           |   110 +
 libbacktrace/simple.c                         |   108 +
 libbacktrace/sort.c                           |   108 +
 libbacktrace/state.c                          |    72 +
 libbacktrace/stest.c                          |   137 +
 libbacktrace/test_format.c                    |    55 +
 libbacktrace/testlib.c                        |   234 +
 libbacktrace/testlib.h                        |   110 +
 libbacktrace/ttest.c                          |   161 +
 libbacktrace/unittest.c                       |    92 +
 libbacktrace/unknown.c                        |    65 +
 libbacktrace/xcoff.c                          |  1606 ++
 libbacktrace/xztest.c                         |   508 +
 libbacktrace/ztest.c                          |   541 +
 62 files changed, 42760 insertions(+), 121 deletions(-)
 create mode 100644 gdb/bt-utils.c
 create mode 100644 gdb/bt-utils.h
 create mode 100644 gdb/testsuite/gdb.base/bt-on-error-and-warning.exp
 create mode 100644 libbacktrace/ChangeLog
 create mode 100644 libbacktrace/ChangeLog.jit
 create mode 100644 libbacktrace/Makefile.am
 create mode 100644 libbacktrace/Makefile.in
 create mode 100644 libbacktrace/README
 create mode 100644 libbacktrace/aclocal.m4
 create mode 100644 libbacktrace/alloc.c
 create mode 100644 libbacktrace/allocfail.c
 create mode 100755 libbacktrace/allocfail.sh
 create mode 100644 libbacktrace/atomic.c
 create mode 100644 libbacktrace/backtrace-supported.h.in
 create mode 100644 libbacktrace/backtrace.c
 create mode 100644 libbacktrace/backtrace.h
 create mode 100644 libbacktrace/btest.c
 create mode 100644 libbacktrace/config.h.in
 create mode 100755 libbacktrace/configure
 create mode 100644 libbacktrace/configure.ac
 create mode 100644 libbacktrace/dwarf.c
 create mode 100644 libbacktrace/edtest.c
 create mode 100644 libbacktrace/edtest2.c
 create mode 100644 libbacktrace/elf.c
 create mode 100644 libbacktrace/fileline.c
 create mode 100644 libbacktrace/filetype.awk
 create mode 100644 libbacktrace/install-debuginfo-for-buildid.sh.in
 create mode 100644 libbacktrace/instrumented_alloc.c
 create mode 100644 libbacktrace/internal.h
 create mode 100644 libbacktrace/macho.c
 create mode 100644 libbacktrace/mmap.c
 create mode 100644 libbacktrace/mmapio.c
 create mode 100644 libbacktrace/mtest.c
 create mode 100644 libbacktrace/nounwind.c
 create mode 100644 libbacktrace/pecoff.c
 create mode 100644 libbacktrace/posix.c
 create mode 100644 libbacktrace/print.c
 create mode 100644 libbacktrace/read.c
 create mode 100644 libbacktrace/simple.c
 create mode 100644 libbacktrace/sort.c
 create mode 100644 libbacktrace/state.c
 create mode 100644 libbacktrace/stest.c
 create mode 100644 libbacktrace/test_format.c
 create mode 100644 libbacktrace/testlib.c
 create mode 100644 libbacktrace/testlib.h
 create mode 100644 libbacktrace/ttest.c
 create mode 100644 libbacktrace/unittest.c
 create mode 100644 libbacktrace/unknown.c
 create mode 100644 libbacktrace/xcoff.c
 create mode 100644 libbacktrace/xztest.c
 create mode 100644 libbacktrace/ztest.c

-- 
2.25.4


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

* [PATCH 1/6] gdb: use bool instead of int in struct internal_problem
  2021-08-19  9:49 [PATCH 0/6] Display GDB backtrace for internal errors Andrew Burgess
@ 2021-08-19  9:49 ` Andrew Burgess
  2021-08-19 18:33   ` Simon Marchi
  2021-08-19  9:49 ` [PATCH 2/6] gdb: make use of std::string in utils.c Andrew Burgess
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2021-08-19  9:49 UTC (permalink / raw)
  To: gdb-patches

Change struct internal_problem (gdb/utils.c) to use bool instead of
int, update the 3 static instances of this structure that we create to
use true/false instead of 1/0.

I've also updated the comments on struct internal_problem as the
existing comment doesn't seem to be referring to the structure, it
talks about returning something, which doesn't make sense in this
context.

There should be no user visible changes after this commit.
---
 gdb/utils.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/gdb/utils.c b/gdb/utils.c
index 1c226d5d85e..3916ae5a1c9 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -280,16 +280,29 @@ static const char *const internal_problem_modes[] =
   NULL
 };
 
-/* Print a message reporting an internal error/warning.  Ask the user
-   if they want to continue, dump core, or just exit.  Return
-   something to indicate a quit.  */
+/* Data structure used to control how the internal_vproblem function
+   should behave.  An instance of this structure is created for each
+   problem type that GDB supports.  */
 
 struct internal_problem
 {
+  /* The name of this problem type.  This must not contain white space as
+     this string is used to build command names.  */
   const char *name;
-  int user_settable_should_quit;
+
+  /* When this is true then a user command is created (based on NAME) that
+     allows the SHOULD_QUIT field to be modified, otherwise, SHOULD_QUIT
+     can't be changed from its default value by the user.  */
+  bool user_settable_should_quit;
+
+  /* Reference a value from internal_problem_modes to indicate if GDB
+     should quit when it hits a problem of this type.  */
   const char *should_quit;
-  int user_settable_should_dump_core;
+
+  /* Like USER_SETTABLE_SHOULD_QUIT but for SHOULD_DUMP_CORE.  */
+  bool user_settable_should_dump_core;
+
+  /* Like SHOULD_QUIT, but whether GDB should dump core.  */
   const char *should_dump_core;
 };
 
@@ -435,7 +448,7 @@ internal_vproblem (struct internal_problem *problem,
 }
 
 static struct internal_problem internal_error_problem = {
-  "internal-error", 1, internal_problem_ask, 1, internal_problem_ask
+  "internal-error", true, internal_problem_ask, true, internal_problem_ask,
 };
 
 void
@@ -446,7 +459,7 @@ internal_verror (const char *file, int line, const char *fmt, va_list ap)
 }
 
 static struct internal_problem internal_warning_problem = {
-  "internal-warning", 1, internal_problem_ask, 1, internal_problem_ask
+  "internal-warning", true, internal_problem_ask, true, internal_problem_ask,
 };
 
 void
@@ -456,7 +469,7 @@ internal_vwarning (const char *file, int line, const char *fmt, va_list ap)
 }
 
 static struct internal_problem demangler_warning_problem = {
-  "demangler-warning", 1, internal_problem_ask, 0, internal_problem_no
+  "demangler-warning", true, internal_problem_ask, false, internal_problem_no,
 };
 
 void
-- 
2.25.4


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

* [PATCH 2/6] gdb: make use of std::string in utils.c
  2021-08-19  9:49 [PATCH 0/6] Display GDB backtrace for internal errors Andrew Burgess
  2021-08-19  9:49 ` [PATCH 1/6] gdb: use bool instead of int in struct internal_problem Andrew Burgess
@ 2021-08-19  9:49 ` Andrew Burgess
  2021-08-19 18:41   ` Simon Marchi
  2021-08-19  9:49 ` [PATCH 3/6] gdb: Add a dependency between gdb and libbacktrace Andrew Burgess
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2021-08-19  9:49 UTC (permalink / raw)
  To: gdb-patches

Replace use of manual string management (malloc/free) with std::string
when creating commands in utils.c.  Things are a little bit messy as
creating the prefix commands (using add_basic_prefix_cmd and
add_show_prefix_cmd), don't copy the doc string, while creating the
actual set/show commands does copy the doc string, this explains the
extra xstrdup when creating the prefix commands.

There should be no user visible changes after this commit.
---
 gdb/utils.c | 58 ++++++++++++++++++++++++-----------------------------
 1 file changed, 26 insertions(+), 32 deletions(-)

diff --git a/gdb/utils.c b/gdb/utils.c
index 3916ae5a1c9..143c2eddd70 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -508,72 +508,66 @@ add_internal_problem_command (struct internal_problem *problem)
 {
   struct cmd_list_element **set_cmd_list;
   struct cmd_list_element **show_cmd_list;
-  char *set_doc;
-  char *show_doc;
 
   set_cmd_list = XNEW (struct cmd_list_element *);
   show_cmd_list = XNEW (struct cmd_list_element *);
   *set_cmd_list = NULL;
   *show_cmd_list = NULL;
 
-  set_doc = xstrprintf (_("Configure what GDB does when %s is detected."),
-			problem->name);
+  std::string set_doc
+    = string_printf (_("Configure what GDB does when %s is detected."),
+		       problem->name);
 
-  show_doc = xstrprintf (_("Show what GDB does when %s is detected."),
-			 problem->name);
+  std::string show_doc
+    = string_printf (_("Show what GDB does when %s is detected."),
+		       problem->name);
 
-  add_basic_prefix_cmd (problem->name, class_maintenance, set_doc,
-			set_cmd_list,
+  add_basic_prefix_cmd (problem->name, class_maintenance,
+			xstrdup (set_doc.c_str ()), set_cmd_list,
 			0/*allow-unknown*/, &maintenance_set_cmdlist);
 
-  add_show_prefix_cmd (problem->name, class_maintenance, show_doc,
-		       show_cmd_list,
+  add_show_prefix_cmd (problem->name, class_maintenance,
+		       xstrdup (show_doc.c_str ()), show_cmd_list,
 		       0/*allow-unknown*/, &maintenance_show_cmdlist);
 
   if (problem->user_settable_should_quit)
     {
-      set_doc = xstrprintf (_("Set whether GDB should quit "
-			      "when an %s is detected."),
-			    problem->name);
-      show_doc = xstrprintf (_("Show whether GDB will quit "
-			       "when an %s is detected."),
-			     problem->name);
+      set_doc
+	= string_printf (_("Set whether GDB should quit when an %s is "
+			   "detected."), problem->name);
+      show_doc
+	= string_printf (_("Show whether GDB will quit when an %s is "
+			   "detected."), problem->name);
       add_setshow_enum_cmd ("quit", class_maintenance,
 			    internal_problem_modes,
 			    &problem->should_quit,
-			    set_doc,
-			    show_doc,
+			    set_doc.c_str (),
+			    show_doc.c_str (),
 			    NULL, /* help_doc */
 			    NULL, /* setfunc */
 			    NULL, /* showfunc */
 			    set_cmd_list,
 			    show_cmd_list);
-
-      xfree (set_doc);
-      xfree (show_doc);
     }
 
   if (problem->user_settable_should_dump_core)
     {
-      set_doc = xstrprintf (_("Set whether GDB should create a core "
-			      "file of GDB when %s is detected."),
-			    problem->name);
-      show_doc = xstrprintf (_("Show whether GDB will create a core "
-			       "file of GDB when %s is detected."),
-			     problem->name);
+      set_doc
+	= string_printf (_("Set whether GDB should create a core file of "
+			   "GDB when %s is detected."), problem->name);
+      show_doc
+	= string_printf (_("Show whether GDB will create a core file of "
+			   "GDB when %s is detected."), problem->name);
       add_setshow_enum_cmd ("corefile", class_maintenance,
 			    internal_problem_modes,
 			    &problem->should_dump_core,
-			    set_doc,
-			    show_doc,
+			    set_doc.c_str (),
+			    show_doc.c_str (),
 			    NULL, /* help_doc */
 			    NULL, /* setfunc */
 			    NULL, /* showfunc */
 			    set_cmd_list,
 			    show_cmd_list);
-
-      xfree (set_doc);
-      xfree (show_doc);
     }
 }
 
-- 
2.25.4


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

* [PATCH 3/6] gdb: Add a dependency between gdb and libbacktrace
  2021-08-19  9:49 [PATCH 0/6] Display GDB backtrace for internal errors Andrew Burgess
  2021-08-19  9:49 ` [PATCH 1/6] gdb: use bool instead of int in struct internal_problem Andrew Burgess
  2021-08-19  9:49 ` [PATCH 2/6] gdb: make use of std::string in utils.c Andrew Burgess
@ 2021-08-19  9:49 ` Andrew Burgess
  2021-08-19 18:43   ` Simon Marchi
  2021-08-19  9:49 ` [PATCH 4/6] Copy in libbacktrace from gcc Andrew Burgess
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2021-08-19  9:49 UTC (permalink / raw)
  To: gdb-patches

GDB is going to start using libbacktrace, so add a build dependency.

ChangeLog:

	* Makefile.def: Add all-gdb dependency on all-libbacktrace.
	* Makefile.in: Regenerate.
---
 Makefile.def | 1 +
 Makefile.in  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Makefile.def b/Makefile.def
index 5a460f1dbbc..d5b1ee882b6 100644
--- a/Makefile.def
+++ b/Makefile.def
@@ -408,6 +408,7 @@ dependencies = { module=all-gdb; on=all-sim; };
 dependencies = { module=all-gdb; on=all-libdecnumber; };
 dependencies = { module=all-gdb; on=all-libtermcap; };
 dependencies = { module=all-gdb; on=all-libctf; };
+dependencies = { module=all-gdb; on=all-libbacktrace; };
 
 // Host modules specific to gdbserver.
 dependencies = { module=configure-gdbserver; on=all-gnulib; };
diff --git a/Makefile.in b/Makefile.in
index 9b3a5d75735..91b714f6e88 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -52534,6 +52534,7 @@ all-gdb: maybe-all-libiconv
 all-gdb: maybe-all-opcodes
 all-gdb: maybe-all-libdecnumber
 all-gdb: maybe-all-libctf
+all-gdb: maybe-all-libbacktrace
 all-gdbserver: maybe-all-libiberty
 configure-gdbsupport: maybe-configure-intl
 all-gdbsupport: maybe-all-intl
-- 
2.25.4


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

* [PATCH 4/6] Copy in libbacktrace from gcc
  2021-08-19  9:49 [PATCH 0/6] Display GDB backtrace for internal errors Andrew Burgess
                   ` (2 preceding siblings ...)
  2021-08-19  9:49 ` [PATCH 3/6] gdb: Add a dependency between gdb and libbacktrace Andrew Burgess
@ 2021-08-19  9:49 ` Andrew Burgess
  2021-08-27 17:46   ` Tom Tromey
  2021-08-19  9:49 ` [PATCH 5/6] gdb: use libbacktrace to create a better backtrace for fatal signals Andrew Burgess
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2021-08-19  9:49 UTC (permalink / raw)
  To: gdb-patches

This copies in libbacktrace from the gcc repository as it was in the
commit ad3f0ad4bafe377072a53ded468fd9948e659f46.  GDB is going to
start using this library soon.

A dependency between GDB and libbacktrace has already been added to
the top level Makefile, so, after this commit, when building GDB,
libbacktrace will be built first.  However, libbacktrace is not yet
linked into GDB, or used by GDB in any way.

It is possible to stop libbacktrace being built by configuring the
tree with --disable-libbacktrace.
---
 libbacktrace/ChangeLog                        |  1759 ++
 libbacktrace/ChangeLog.jit                    |    14 +
 libbacktrace/Makefile.am                      |   586 +
 libbacktrace/Makefile.in                      |  2528 +++
 libbacktrace/README                           |    23 +
 libbacktrace/aclocal.m4                       |   867 +
 libbacktrace/alloc.c                          |   167 +
 libbacktrace/allocfail.c                      |   136 +
 libbacktrace/allocfail.sh                     |   104 +
 libbacktrace/atomic.c                         |   113 +
 libbacktrace/backtrace-supported.h.in         |    66 +
 libbacktrace/backtrace.c                      |   129 +
 libbacktrace/backtrace.h                      |   206 +
 libbacktrace/btest.c                          |   501 +
 libbacktrace/config.h.in                      |   184 +
 libbacktrace/configure                        | 16893 ++++++++++++++++
 libbacktrace/configure.ac                     |   581 +
 libbacktrace/dwarf.c                          |  4056 ++++
 libbacktrace/edtest.c                         |   120 +
 libbacktrace/edtest2.c                        |    43 +
 libbacktrace/elf.c                            |  4919 +++++
 libbacktrace/fileline.c                       |   346 +
 libbacktrace/filetype.awk                     |    13 +
 .../install-debuginfo-for-buildid.sh.in       |    65 +
 libbacktrace/instrumented_alloc.c             |   114 +
 libbacktrace/internal.h                       |   380 +
 libbacktrace/macho.c                          |  1355 ++
 libbacktrace/mmap.c                           |   331 +
 libbacktrace/mmapio.c                         |   110 +
 libbacktrace/mtest.c                          |   410 +
 libbacktrace/nounwind.c                       |    66 +
 libbacktrace/pecoff.c                         |   935 +
 libbacktrace/posix.c                          |   104 +
 libbacktrace/print.c                          |    92 +
 libbacktrace/read.c                           |   110 +
 libbacktrace/simple.c                         |   108 +
 libbacktrace/sort.c                           |   108 +
 libbacktrace/state.c                          |    72 +
 libbacktrace/stest.c                          |   137 +
 libbacktrace/test_format.c                    |    55 +
 libbacktrace/testlib.c                        |   234 +
 libbacktrace/testlib.h                        |   110 +
 libbacktrace/ttest.c                          |   161 +
 libbacktrace/unittest.c                       |    92 +
 libbacktrace/unknown.c                        |    65 +
 libbacktrace/xcoff.c                          |  1606 ++
 libbacktrace/xztest.c                         |   508 +
 libbacktrace/ztest.c                          |   541 +
 48 files changed, 42223 insertions(+)
 create mode 100644 libbacktrace/ChangeLog
 create mode 100644 libbacktrace/ChangeLog.jit
 create mode 100644 libbacktrace/Makefile.am
 create mode 100644 libbacktrace/Makefile.in
 create mode 100644 libbacktrace/README
 create mode 100644 libbacktrace/aclocal.m4
 create mode 100644 libbacktrace/alloc.c
 create mode 100644 libbacktrace/allocfail.c
 create mode 100755 libbacktrace/allocfail.sh
 create mode 100644 libbacktrace/atomic.c
 create mode 100644 libbacktrace/backtrace-supported.h.in
 create mode 100644 libbacktrace/backtrace.c
 create mode 100644 libbacktrace/backtrace.h
 create mode 100644 libbacktrace/btest.c
 create mode 100644 libbacktrace/config.h.in
 create mode 100755 libbacktrace/configure
 create mode 100644 libbacktrace/configure.ac
 create mode 100644 libbacktrace/dwarf.c
 create mode 100644 libbacktrace/edtest.c
 create mode 100644 libbacktrace/edtest2.c
 create mode 100644 libbacktrace/elf.c
 create mode 100644 libbacktrace/fileline.c
 create mode 100644 libbacktrace/filetype.awk
 create mode 100644 libbacktrace/install-debuginfo-for-buildid.sh.in
 create mode 100644 libbacktrace/instrumented_alloc.c
 create mode 100644 libbacktrace/internal.h
 create mode 100644 libbacktrace/macho.c
 create mode 100644 libbacktrace/mmap.c
 create mode 100644 libbacktrace/mmapio.c
 create mode 100644 libbacktrace/mtest.c
 create mode 100644 libbacktrace/nounwind.c
 create mode 100644 libbacktrace/pecoff.c
 create mode 100644 libbacktrace/posix.c
 create mode 100644 libbacktrace/print.c
 create mode 100644 libbacktrace/read.c
 create mode 100644 libbacktrace/simple.c
 create mode 100644 libbacktrace/sort.c
 create mode 100644 libbacktrace/state.c
 create mode 100644 libbacktrace/stest.c
 create mode 100644 libbacktrace/test_format.c
 create mode 100644 libbacktrace/testlib.c
 create mode 100644 libbacktrace/testlib.h
 create mode 100644 libbacktrace/ttest.c
 create mode 100644 libbacktrace/unittest.c
 create mode 100644 libbacktrace/unknown.c
 create mode 100644 libbacktrace/xcoff.c
 create mode 100644 libbacktrace/xztest.c
 create mode 100644 libbacktrace/ztest.c


### SNIP ####

I've not included the patch here.  I'm just copying in the whole
directory gcc/libbacktrace/ without any modifications.

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

* [PATCH 5/6] gdb: use libbacktrace to create a better backtrace for fatal signals
  2021-08-19  9:49 [PATCH 0/6] Display GDB backtrace for internal errors Andrew Burgess
                   ` (3 preceding siblings ...)
  2021-08-19  9:49 ` [PATCH 4/6] Copy in libbacktrace from gcc Andrew Burgess
@ 2021-08-19  9:49 ` Andrew Burgess
  2021-08-19 18:58   ` Simon Marchi
  2021-08-19  9:49 ` [PATCH 6/6] gdb: print backtrace for internal error/warning Andrew Burgess
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2021-08-19  9:49 UTC (permalink / raw)
  To: gdb-patches

GDB recently gained the ability to print a backtrace when a fatal
signal is encountered.  This backtrace is produced using the backtrace
and backtrace_symbols_fd API available in glibc.

However, in order for this API to actually map addresses to symbol
names it is required that the application (GDB) be compiled with
-rdynamic, which GDB is not by default.

As a result, the backtrace produced often looks like this:

  Fatal signal: Bus error
  ----- Backtrace -----
  ./gdb/gdb[0x80ec00]
  ./gdb/gdb[0x80ed56]
  /lib64/libc.so.6(+0x3c6b0)[0x7fc2ce1936b0]
  /lib64/libc.so.6(__poll+0x4f)[0x7fc2ce24da5f]
  ./gdb/gdb[0x15495ba]
  ./gdb/gdb[0x15489b8]
  ./gdb/gdb[0x9b794d]
  ./gdb/gdb[0x9b7a6d]
  ./gdb/gdb[0x9b943b]
  ./gdb/gdb[0x9b94a1]
  ./gdb/gdb[0x4175dd]
  /lib64/libc.so.6(__libc_start_main+0xf3)[0x7fc2ce17e1a3]
  ./gdb/gdb[0x4174de]
  ---------------------

This is OK if you have access to the exact same build of GDB, you can
manually map the addresses back to symbols, however, it is next to
useless if all you have is a backtrace copied into a bug report.

GCC uses libbacktrace for printing a backtrace when it encounters an
error.  In recent commits I added this library into the binutils-gdb
repository, and in this commit I allow this library to be used by
GDB.  Now (when GDB is compiled with debug information) the backtrace
looks like this:

  ----- Backtrace -----
  0x80ee08 gdb_internal_backtrace
  	../../src/gdb/event-top.c:989
  0x80ef0b handle_fatal_signal
  	../../src/gdb/event-top.c:1036
  0x7f24539dd6af ???
  0x7f2453a97a5f ???
  0x154976f gdb_wait_for_event
  	../../src/gdbsupport/event-loop.cc:613
  0x1548b6d _Z16gdb_do_one_eventv
  	../../src/gdbsupport/event-loop.cc:237
  0x9b7b02 start_event_loop
  	../../src/gdb/main.c:421
  0x9b7c22 captured_command_loop
  	../../src/gdb/main.c:481
  0x9b95f0 captured_main
  	../../src/gdb/main.c:1353
  0x9b9656 _Z8gdb_mainP18captured_main_args
  	../../src/gdb/main.c:1368
  0x4175ec main
  	../../src/gdb/gdb.c:32
  ---------------------

Which seems much more useful.

Use of libbacktrace is optional.  If GDB is configured with
--disable-libbacktrace then the libbacktrace directory will not be
built, and GDB will not try to use this library.  In this case GDB
would try to use the old backtrace and backtrace_symbols_fd API.

All of the functions related to writing the backtrace of GDB itself
have been moved into the new files gdb/by-utils.{c,h}.
---
 gdb/Makefile.in  |  16 +++--
 gdb/bt-utils.c   | 170 +++++++++++++++++++++++++++++++++++++++++++++++
 gdb/bt-utils.h   |  69 +++++++++++++++++++
 gdb/config.in    |   3 +
 gdb/configure    |  32 +++++++++
 gdb/configure.ac |  23 +++++++
 gdb/event-top.c  |  47 ++-----------
 7 files changed, 315 insertions(+), 45 deletions(-)
 create mode 100644 gdb/bt-utils.c
 create mode 100644 gdb/bt-utils.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 73a1bf83c85..c33c5e76fc3 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -248,6 +248,10 @@ GDBFLAGS =
 GNULIB_PARENT_DIR = ..
 include $(GNULIB_PARENT_DIR)/gnulib/Makefile.gnulib.inc
 
+# For libbacktrace.
+LIBBACKTRACE_INC=@LIBBACKTRACE_INC@
+LIBBACKTRACE_LIB=@LIBBACKTRACE_LIB@
+
 SUPPORT = ../gdbsupport
 LIBSUPPORT = $(SUPPORT)/libgdbsupport.a
 INCSUPPORT = -I$(srcdir)/.. -I..
@@ -613,9 +617,9 @@ INTERNAL_CFLAGS_BASE = \
 	$(GLOBAL_CFLAGS) $(PROFILE_CFLAGS) \
 	$(GDB_CFLAGS) $(OPCODES_CFLAGS) $(READLINE_CFLAGS) $(ZLIBINC) \
 	$(BFD_CFLAGS) $(INCLUDE_CFLAGS) $(LIBDECNUMBER_CFLAGS) \
-	$(INTL_CFLAGS) $(INCGNU) $(INCSUPPORT) $(ENABLE_CFLAGS) \
-	$(INTERNAL_CPPFLAGS) $(SRCHIGH_CFLAGS) $(TOP_CFLAGS) $(PTHREAD_CFLAGS) \
-	$(DEBUGINFOD_CFLAGS)
+	$(INTL_CFLAGS) $(INCGNU) $(INCSUPPORT) $(LIBBACKTRACE_INC) \
+	$(ENABLE_CFLAGS) $(INTERNAL_CPPFLAGS) $(SRCHIGH_CFLAGS) \
+	$(TOP_CFLAGS) $(PTHREAD_CFLAGS) $(DEBUGINFOD_CFLAGS)
 INTERNAL_WARN_CFLAGS = $(INTERNAL_CFLAGS_BASE) $(GDB_WARN_CFLAGS)
 INTERNAL_CFLAGS = $(INTERNAL_WARN_CFLAGS) $(GDB_WERROR_CFLAGS)
 
@@ -636,12 +640,12 @@ INTERNAL_LDFLAGS = \
 # LIBIBERTY appears twice on purpose.
 CLIBS = $(SIM) $(READLINE) $(OPCODES) $(LIBCTF) $(BFD) $(ZLIB) \
         $(LIBSUPPORT) $(INTL) $(LIBIBERTY) $(LIBDECNUMBER) \
-	$(XM_CLIBS) $(GDBTKLIBS) \
+	$(XM_CLIBS) $(GDBTKLIBS)  $(LIBBACKTRACE_LIB) \
 	@LIBS@ @GUILE_LIBS@ @PYTHON_LIBS@ \
 	$(LIBEXPAT) $(LIBLZMA) $(LIBBABELTRACE) $(LIBIPT) \
 	$(WIN32LIBS) $(LIBGNU) $(LIBGNU_EXTRA_LIBS) $(LIBICONV) \
 	$(LIBMPFR) $(LIBGMP) $(SRCHIGH_LIBS) $(LIBXXHASH) $(PTHREAD_LIBS) \
-	$(DEBUGINFOD_LIBS)
+	$(DEBUGINFOD_LIBS) $(LIBBABELTRACE_LIB)
 CDEPS = $(NAT_CDEPS) $(SIM) $(BFD) $(READLINE_DEPS) $(CTF_DEPS) \
 	$(OPCODES) $(INTL_DEPS) $(LIBIBERTY) $(CONFIG_DEPS) $(LIBGNU) \
 	$(LIBSUPPORT)
@@ -994,6 +998,7 @@ COMMON_SFILES = \
 	break-catch-syscall.c \
 	break-catch-throw.c \
 	breakpoint.c \
+	bt-utils.c \
 	btrace.c \
 	build-id.c \
 	buildsym-legacy.c \
@@ -1255,6 +1260,7 @@ HFILES_NO_SRCDIR = \
 	breakpoint.h \
 	bsd-kvm.h \
 	bsd-uthread.h \
+	bt-utils.h \
 	build-id.h \
 	buildsym-legacy.h \
 	buildsym.h \
diff --git a/gdb/bt-utils.c b/gdb/bt-utils.c
new file mode 100644
index 00000000000..c39e8da5f2c
--- /dev/null
+++ b/gdb/bt-utils.c
@@ -0,0 +1,170 @@
+/* Copyright (C) 2021 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "bt-utils.h"
+#include "command.h"
+#include "gdbcmd.h"
+#include "top.h"
+#include "cli/cli-decode.h"
+
+/* See bt-utils.h.  */
+
+void
+gdb_internal_backtrace_set_cmd (const char *args, int from_tty,
+				cmd_list_element *c)
+{
+  gdb_assert (c->type == set_cmd);
+  gdb_assert (c->var_type == var_boolean);
+  gdb_assert (c->var != nullptr);
+
+#ifndef GDB_PRINT_INTERNAL_BACKTRACE
+  bool *var_ptr = (bool *) c->var;
+
+  if (*var_ptr)
+    {
+      *var_ptr = false;
+      error (_("support for this feature is not compiled into GDB"));
+    }
+#endif
+}
+
+#ifdef GDB_PRINT_INTERNAL_BACKTRACE_USING_LIBBACKTRACE
+
+/* Callback used by libbacktrace if it encounters an error.  */
+
+static void
+libbacktrace_error (void *data, const char *errmsg, int errnum)
+{
+  /* A negative errnum indicates no debug info was available, just
+     skip printing a backtrace in this case.  */
+  if (errnum < 0)
+    return;
+
+  const auto sig_write = [] (const char *msg) -> void
+  {
+    gdb_stderr->write_async_safe (msg, strlen (msg));
+  };
+
+  sig_write ("error creating backtrace: ");
+  sig_write (errmsg);
+  if (errnum > 0)
+    {
+      char buf[10];
+      snprintf (buf, sizeof (buf), ": %d", errnum);
+      buf[sizeof (buf) - 1] = '\0';
+
+      sig_write (buf);
+    }
+  sig_write ("\n");
+}
+
+/* Callback used by libbacktrace to print a single stack frame.  */
+
+static int
+libbacktrace_print (void *data, uintptr_t pc, const char *filename,
+		    int lineno, const char *function)
+{
+  const auto sig_write = [] (const char *msg) -> void
+  {
+    gdb_stderr->write_async_safe (msg, strlen (msg));
+  };
+
+  /* Buffer to print addresses and line numbers into.  An 8-byte address
+     with '0x' prefix and a null terminator requires 20 characters.  This
+     also feels like it should be enough to represent line numbers in most
+     files.  We are also careful to ensure we don't overflow this buffer.  */
+  char buf[20];
+
+  snprintf (buf, sizeof (buf), "0x%lx ", pc);
+  buf[sizeof (buf) - 1] = '\0';
+  sig_write (buf);
+  sig_write (function == nullptr ? "???" : function);
+  if (filename != nullptr)
+    {
+      sig_write ("\n\t");
+      sig_write (filename);
+      sig_write (":");
+      snprintf (buf, sizeof (buf), "%d", lineno);
+      buf[sizeof (buf) - 1] = '\0';
+      sig_write (buf);
+    }
+  sig_write ("\n");
+
+  return function != nullptr && strcmp (function, "main") == 0;
+}
+
+/* Write a backtrace to GDB's stderr in an async safe manor.  This is a
+   backtrace of GDB, not any running inferior, and is to be used when GDB
+   crashes or hits some other error condition.  */
+
+static void
+gdb_internal_backtrace_1 ()
+{
+  static struct backtrace_state *state = nullptr;
+
+  if (state == nullptr)
+    state = backtrace_create_state (nullptr, 0, libbacktrace_error, nullptr);
+
+  backtrace_full (state, 0, libbacktrace_print, libbacktrace_error, nullptr);
+}
+
+#elif defined GDB_PRINT_INTERNAL_BACKTRACE_USING_EXECINFO
+
+/* See the comment on previous version of this function.  */
+
+static void
+gdb_internal_backtrace_1 ()
+{
+  const auto sig_write = [] (const char *msg) -> void
+  {
+    gdb_stderr->write_async_safe (msg, strlen (msg));
+  };
+
+  /* Allow up to 25 frames of backtrace.  */
+  void *buffer[25];
+  int frames = backtrace (buffer, ARRAY_SIZE (buffer));
+
+  backtrace_symbols_fd (buffer, frames, gdb_stderr->fd ());
+  if (frames == ARRAY_SIZE (buffer))
+    sig_write (_("Backtrace might be incomplete.\n"));
+}
+
+#endif
+
+/* See bt-utils.h.  */
+
+void
+gdb_internal_backtrace ()
+{
+  if (current_ui == nullptr)
+    return;
+
+  const auto sig_write = [] (const char *msg) -> void
+  {
+    gdb_stderr->write_async_safe (msg, strlen (msg));
+  };
+
+  sig_write (_("----- Backtrace -----\n"));
+
+  if (gdb_stderr->fd () > -1)
+    gdb_internal_backtrace_1 ();
+  else
+    sig_write (_("Backtrace unavailable\n"));
+
+  sig_write ("---------------------\n");
+}
diff --git a/gdb/bt-utils.h b/gdb/bt-utils.h
new file mode 100644
index 00000000000..433aa23614b
--- /dev/null
+++ b/gdb/bt-utils.h
@@ -0,0 +1,69 @@
+/* Copyright (C) 2021 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Support for printing a backtrace when GDB hits an error.  This is not
+   for printing backtraces of the inferior, but backtraces of GDB itself.  */
+
+#ifdef HAVE_LIBBACKTRACE
+# include "backtrace.h"
+# include "backtrace-supported.h"
+# if BACKTRACE_SUPPORTED && (! BACKTRACE_USES_MALLOC)
+#  define GDB_PRINT_INTERNAL_BACKTRACE
+#  define GDB_PRINT_INTERNAL_BACKTRACE_USING_LIBBACKTRACE
+# endif
+#endif
+
+#if defined HAVE_EXECINFO_H			\
+  && defined HAVE_EXECINFO_BACKTRACE		\
+  && !defined PRINT_BACKTRACE_ON_FATAL_SIGNAL
+# include <execinfo.h>
+# define GDB_PRINT_INTERNAL_BACKTRACE
+# define GDB_PRINT_INTERNAL_BACKTRACE_USING_EXECINFO
+#endif
+
+/* Define GDB_PRINT_INTERNAL_BACKTRACE_INIT_ON.  This is a boolean value
+   that can be used as an initial value for a set/show user setting, where
+   the setting controls printing a GDB internal backtrace.
+
+   If backtrace printing is supported then this will have the value true,
+   but if backtrace printing is not supported then this has the value
+   false.  */
+#ifdef GDB_PRINT_INTERNAL_BACKTRACE
+# define GDB_PRINT_INTERNAL_BACKTRACE_INIT_ON true
+#else
+# define GDB_PRINT_INTERNAL_BACKTRACE_INIT_ON false
+#endif
+
+/* Print a backtrace of the current GDB process to the current
+   gdb_stderr.  The output is done in a signal async manor, so it is safe
+   to call from signal handlers.  */
+
+extern void gdb_internal_backtrace ();
+
+/* A generic function that can be used as the set function for any set
+   command that enables printing of an internal backtrace.  Command C must
+   be a boolean set command.
+
+   If GDB doesn't support printing a backtrace, and the user has tried to
+   set the variable associated with command C to true, then the associated
+   variable will be set back to false, and an error thrown.
+
+   If GDB does support printing a backtrace then this function does
+   nothing.  */
+
+extern void gdb_internal_backtrace_set_cmd (const char *args, int from_tty,
+					    cmd_list_element *c);
diff --git a/gdb/config.in b/gdb/config.in
index af3680c6d95..c61f7a91352 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -228,6 +228,9 @@
 /* Define if you have the babeltrace library. */
 #undef HAVE_LIBBABELTRACE
 
+/* Define if libbacktrace is being used. */
+#undef HAVE_LIBBACKTRACE
+
 /* Define to 1 if debuginfod is enabled. */
 #undef HAVE_LIBDEBUGINFOD
 
diff --git a/gdb/configure b/gdb/configure
index f0b1af4a6ea..7c8335f2576 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -636,6 +636,8 @@ LIBCTF
 LTLIBBABELTRACE
 LIBBABELTRACE
 HAVE_LIBBABELTRACE
+LIBBACKTRACE_LIB
+LIBBACKTRACE_INC
 HAVE_NATIVE_GCORE_HOST
 NAT_GENERATED_FILES
 XM_CLIBS
@@ -925,6 +927,7 @@ with_tcl
 with_tk
 with_x
 enable_sim
+enable_libbacktrace
 with_babeltrace
 with_libbabeltrace_prefix
 with_libbabeltrace_type
@@ -1601,6 +1604,8 @@ Optional Features:
                           gcc is used
   --enable-ubsan          enable undefined behavior sanitizer (auto/yes/no)
   --enable-sim            link gdb with simulator
+  --enable-libbacktrace   use libbacktrace to write a backtrace after a fatal
+                          signal.
   --enable-libctf         Handle .ctf type-info sections [default=yes]
   --enable-unit-tests     Enable the inclusion of unit tests when compiling
                           GDB
@@ -18659,6 +18664,33 @@ _ACEOF
 
 fi
 
+# Setup possible use of libbacktrace.
+# Check whether --enable-libbacktrace was given.
+if test "${enable_libbacktrace+set}" = set; then :
+  enableval=$enable_libbacktrace; case "${enableval}" in
+  yes)  enable_libbacktrace=yes ;;
+  no)   enable_libbacktrace=no  ;;
+  *)    as_fn_error $? "bad value ${enableval} for --enable-libbacktrace option" "$LINENO" 5 ;;
+esac
+else
+  enable_libbacktrace=yes
+fi
+
+
+if test "${enable_libbacktrace}" == "yes"; then
+  LIBBACKTRACE_INC="-I$srcdir/../libbacktrace/ -I../libbacktrace/"
+  LIBBACKTRACE_LIB=../libbacktrace/.libs/libbacktrace.a
+
+$as_echo "#define HAVE_LIBBACKTRACE 1" >>confdefs.h
+
+else
+  LIBBACKTRACE_INC=
+  LIBBACKTRACE_LIB=
+fi
+
+
+
+
 # Check for babeltrace and babeltrace-ctf
 
 # Check whether --with-babeltrace was given.
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 93f11316a14..0d91be59cd6 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -2131,6 +2131,29 @@ if test x"${gdb_osabi}" != x ; then
 		       [Define to the default OS ABI for this configuration.])
 fi
 
+# Setup possible use of libbacktrace.
+AC_ARG_ENABLE([libbacktrace],
+[AS_HELP_STRING([--enable-libbacktrace],
+                [use libbacktrace to write a backtrace after a fatal signal.])],
+[case "${enableval}" in
+  yes)  enable_libbacktrace=yes ;;
+  no)   enable_libbacktrace=no  ;;
+  *)    AC_MSG_ERROR(bad value ${enableval} for --enable-libbacktrace option) ;;
+esac],
+enable_libbacktrace=yes)
+
+if test "${enable_libbacktrace}" == "yes"; then
+  LIBBACKTRACE_INC="-I$srcdir/../libbacktrace/ -I../libbacktrace/"
+  LIBBACKTRACE_LIB=../libbacktrace/.libs/libbacktrace.a
+  AC_DEFINE(HAVE_LIBBACKTRACE, 1, [Define if libbacktrace is being used.])
+else
+  LIBBACKTRACE_INC=
+  LIBBACKTRACE_LIB=
+fi
+
+AC_SUBST(LIBBACKTRACE_INC)
+AC_SUBST(LIBBACKTRACE_LIB)
+
 # Check for babeltrace and babeltrace-ctf
 AC_ARG_WITH(babeltrace,
   AS_HELP_STRING([--with-babeltrace], [include babeltrace support (auto/yes/no)]),
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 9233a3650ac..64c624ce4d7 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -41,15 +41,12 @@
 #include "gdbsupport/gdb_select.h"
 #include "gdbsupport/gdb-sigmask.h"
 #include "async-event.h"
+#include "bt-utils.h"
 
 /* readline include files.  */
 #include "readline/readline.h"
 #include "readline/history.h"
 
-#ifdef HAVE_EXECINFO_H
-# include <execinfo.h>
-#endif /* HAVE_EXECINFO_H */
-
 /* readline defines this.  */
 #undef savestring
 
@@ -102,12 +99,7 @@ int call_stdin_event_handler_again_p;
 
 /* When true GDB will produce a minimal backtrace when a fatal signal is
    reached (within GDB code).  */
-static bool bt_on_fatal_signal
-#ifdef HAVE_EXECINFO_BACKTRACE
-  = true;
-#else
-  = false;
-#endif /* HAVE_EXECINFO_BACKTRACE */
+static bool bt_on_fatal_signal = GDB_PRINT_INTERNAL_BACKTRACE_INIT_ON;
 
 /* Implement 'maintenance show backtrace-on-fatal-signal'.  */
 
@@ -118,20 +110,6 @@ show_bt_on_fatal_signal (struct ui_file *file, int from_tty,
   fprintf_filtered (file, _("Backtrace on a fatal signal is %s.\n"), value);
 }
 
-/* Implement 'maintenance set backtrace-on-fatal-signal'.  */
-
-static void
-set_bt_on_fatal_signal (const char *args, int from_tty, cmd_list_element *c)
-{
-#ifndef HAVE_EXECINFO_BACKTRACE
-  if (bt_on_fatal_signal)
-    {
-      bt_on_fatal_signal = false;
-      error (_("support for this feature is not compiled into GDB"));
-    }
-#endif
-}
-
 /* Signal handling variables.  */
 /* Each of these is a pointer to a function that the event loop will
    invoke if the corresponding signal has received.  The real signal
@@ -904,7 +882,7 @@ unblock_signal (int sig)
 static void ATTRIBUTE_NORETURN
 handle_fatal_signal (int sig)
 {
-#ifdef HAVE_EXECINFO_BACKTRACE
+#ifdef GDB_PRINT_INTERNAL_BACKTRACE
   const auto sig_write = [] (const char *msg) -> void
   {
     gdb_stderr->write_async_safe (msg, strlen (msg));
@@ -917,19 +895,8 @@ handle_fatal_signal (int sig)
       sig_write (strsignal (sig));
       sig_write ("\n");
 
-      /* Allow up to 25 frames of backtrace.  */
-      void *buffer[25];
-      int frames = backtrace (buffer, ARRAY_SIZE (buffer));
-      sig_write (_("----- Backtrace -----\n"));
-      if (gdb_stderr->fd () > -1)
-	{
-	  backtrace_symbols_fd (buffer, frames, gdb_stderr->fd ());
-	  if (frames == ARRAY_SIZE (buffer))
-	    sig_write (_("Backtrace might be incomplete.\n"));
-	}
-      else
-	sig_write (_("Backtrace unavailable\n"));
-      sig_write ("---------------------\n");
+      gdb_internal_backtrace ();
+
       sig_write (_("A fatal error internal to GDB has been detected, "
 		   "further\ndebugging is not possible.  GDB will now "
 		   "terminate.\n\n"));
@@ -944,7 +911,7 @@ handle_fatal_signal (int sig)
 
       gdb_stderr->flush ();
     }
-#endif /* HAVE_EXECINF_BACKTRACE */
+#endif
 
   /* If possible arrange for SIG to have its default behaviour (which
      should be to terminate the current process), unblock SIG, and reraise
@@ -1449,7 +1416,7 @@ Use \"on\" to enable, \"off\" to disable.\n\
 If enabled, GDB will produce a minimal backtrace if it encounters a fatal\n\
 signal from within GDB itself.  This is a mechanism to help diagnose\n\
 crashes within GDB, not a mechanism for debugging inferiors."),
-			   set_bt_on_fatal_signal,
+			   gdb_internal_backtrace_set_cmd,
 			   show_bt_on_fatal_signal,
 			   &maintenance_set_cmdlist,
 			   &maintenance_show_cmdlist);
-- 
2.25.4


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

* [PATCH 6/6] gdb: print backtrace for internal error/warning
  2021-08-19  9:49 [PATCH 0/6] Display GDB backtrace for internal errors Andrew Burgess
                   ` (4 preceding siblings ...)
  2021-08-19  9:49 ` [PATCH 5/6] gdb: use libbacktrace to create a better backtrace for fatal signals Andrew Burgess
@ 2021-08-19  9:49 ` Andrew Burgess
  2021-08-19 19:01   ` Simon Marchi
  2021-08-30 14:16 ` [PATCH 0/6] Display GDB backtrace for internal errors Tom de Vries
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2021-08-19  9:49 UTC (permalink / raw)
  To: gdb-patches

This commit builds on previous work to allow GDB to print a backtrace
of itself when GDB encounters an internal-error or internal-warning.

There's not many places where we call internal_warning, and I guess in
most cases the user would probably continue their debug session.  And
so, in order to avoid cluttering up the output, by default, printing
of a backtrace is off for internal-warnings.

In contrast, printing of a backtrace is on by default for
internal-errors, as I figure that in most cases hitting an
internal-error is going to be the end of the debug session.

Whether a backtrace is printed or not can be controlled with the new
settings:

  maintenance set internal-error backtrace on|off
  maintenance show internal-error backtrace

  maintenance set internal-warning backtrace on|off
  maintenance show internal-warning backtrace

Here is an example of what an internal-error now looks like with the
backtrace included:

  (gdb) maintenance internal-error blah
  ../../src.dev-3/gdb/maint.c:82: internal-error: blah
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  ----- Backtrace -----
  0x5c61ca gdb_internal_backtrace_1
  	../../src.dev-3/gdb/bt-utils.c:123
  0x5c626d _Z22gdb_internal_backtracev
  	../../src.dev-3/gdb/bt-utils.c:165
  0xe33237 internal_vproblem
  	../../src.dev-3/gdb/utils.c:393
  0xe33539 _Z15internal_verrorPKciS0_P13__va_list_tag
  	../../src.dev-3/gdb/utils.c:470
  0x1549652 _Z14internal_errorPKciS0_z
  	../../src.dev-3/gdbsupport/errors.cc:55
  0x9c7982 maintenance_internal_error
  	../../src.dev-3/gdb/maint.c:82
  0x636f57 do_simple_func
  	../../src.dev-3/gdb/cli/cli-decode.c:97
   .... snip, lots more backtrace lines ....
  ---------------------
  ../../src.dev-3/gdb/maint.c:82: internal-error: blah
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Quit this debugging session? (y or n) y

  This is a bug, please report it.  For instructions, see:
  <https://www.gnu.org/software/gdb/bugs/>.

  ../../src.dev-3/gdb/maint.c:82: internal-error: blah
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Create a core file of GDB? (y or n) n

My hope is that this backtrace might make it slightly easier to
diagnose GDB issues if all that is provided is the console output, I
find that we frequently get reports of an assert being hit that is
located in pretty generic code (frame.c, value.c, etc) and it is not
always obvious how we might have arrived at the assert.
---
 gdb/NEWS                                      |   8 ++
 gdb/doc/gdb.texinfo                           |  13 ++
 .../gdb.base/bt-on-error-and-warning.exp      | 118 ++++++++++++++++++
 gdb/testsuite/gdb.base/bt-on-fatal-signal.exp |  36 ------
 gdb/utils.c                                   |  36 +++++-
 5 files changed, 174 insertions(+), 37 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/bt-on-error-and-warning.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index ec3058ea118..2317c5fe25d 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -10,6 +10,14 @@ maint show backtrace-on-fatal-signal
   fatal signal.  This only supported on some platforms where the
   backtrace and backtrace_symbols_fd functions are available.
 
+maint set internal-error backtrace on|off
+maint show internal-error backtrace
+maint set internal-warning backtrace on|off
+maint show internal-warning backtrace
+  GDB can now print a backtrace of itself when it encounters either an
+  internal-error, or an internal-warning.  This is on by default for
+  internal-error and off by default for internal-warning.
+
 *** Changes in GDB 11
 
 * The 'set disassembler-options' command now supports specifying options
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 58479ef3ed6..5e3e5fd1241 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -39231,6 +39231,19 @@
 disabled.
 @end table
 
+@kindex maint set internal-error
+@kindex maint show internal-error
+@kindex maint set internal-warning
+@kindex maint show internal-warning
+@item maint set internal-error backtrace @r{[}on|off@r{]}
+@itemx maint show internal-error backtrace
+@itemx maint set internal-warning backtrace @r{[}on|off@r{]}
+@itemx maint show internal-warning backtrace
+When @value{GDBN} reports an internal problem (error or warning) it is
+possible to have a backtrace of @value{GDBN} printed to stderr.  This
+is @samp{on} by default for @code{internal-error} and @samp{off} by
+default for @code{internal-warning}.
+
 @kindex maint packet
 @item maint packet @var{text}
 If @value{GDBN} is talking to an inferior via the serial protocol,
diff --git a/gdb/testsuite/gdb.base/bt-on-error-and-warning.exp b/gdb/testsuite/gdb.base/bt-on-error-and-warning.exp
new file mode 100644
index 00000000000..d988cf742b4
--- /dev/null
+++ b/gdb/testsuite/gdb.base/bt-on-error-and-warning.exp
@@ -0,0 +1,118 @@
+# Copyright 2021 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test that GDB can print a backtrace when it encounters an internal
+# error or an internal warning, and that this functionality can be
+# switched off.
+
+standard_testfile bt-on-fatal-signal.c
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
+    return -1
+}
+
+# Check we can run to main.  If this works this time then we just
+# assume that it will work later on (when we repeatedly restart GDB).
+if ![runto_main] then {
+    untested "run to main"
+    return -1
+}
+
+# Check that the backtrace-on-fatal-signal feature is supported.  If
+# this target doesn't have the backtrace function available then
+# trying to turn this on will give an error, in which case we just
+# skip this test.
+gdb_test_multiple "maint set internal-error backtrace on" \
+    "check backtrace is supported" {
+    -re "support for this feature is not compiled into GDB" {
+	untested "feature not supported"
+	return -1
+    }
+    -re "$gdb_prompt $" {
+	pass $gdb_test_name
+    }
+}
+
+# MODE should be either 'on' or 'off', while PROBLEM_TYPE should be
+# 'internal-error' or 'internal-warning'.  This proc sets the
+# backtrace printing for PROBLEM_TYPE to MODE, then uses 'maint
+# PROBLEM_TYPE foobar' to raise a fake error or warning.
+#
+# We then check that a backtrace either is, or isn't printed, inline
+# with MODE.
+proc run_test {problem_type mode} {
+
+    with_test_prefix "problem=${problem_type}, mode=${mode}" {
+	gdb_test_no_output "maint set ${problem_type} backtrace ${mode}"
+
+	set header_lines 0
+	set bt_lines 0
+
+	gdb_test_multiple "maint ${problem_type} foobar" "scan for backtrace" {
+	    -early -re "^\r\n" {
+		exp_continue
+	    }
+	    -early -re "^maint ${problem_type} foobar\r\n" {
+		exp_continue
+	    }
+	    -early -re "^\[^\r\n\]+: ${problem_type}: foobar\r\n" {
+		incr header_lines
+		exp_continue
+	    }
+	    -early -re "^A problem internal to GDB has been detected,\r\n" {
+		incr header_lines
+		exp_continue
+	    }
+	    -early -re "^further debugging may prove unreliable\\.\r\n" {
+		incr header_lines
+		exp_continue
+	    }
+	    -early -re "^----- Backtrace -----\r\n" {
+		incr bt_lines
+		exp_continue
+	    }
+	    -early -re "^\[^-\].+\r\n---------------------\r\n" {
+		incr bt_lines
+		exp_continue
+	    }
+	    eof {
+		fail ${gdb_test_name}
+		return
+	    }
+	    -re "$::gdb_prompt $" {
+		pass ${gdb_test_name}
+	    }
+	}
+
+	gdb_assert { ${header_lines} == 3 }
+	if { $mode == "on" } {
+	    gdb_assert { ${bt_lines} == 2 }
+	} else {
+	    gdb_assert { ${bt_lines} == 0 }
+	}
+    }
+}
+
+# For each problem type (error or warning) raise a fake problem using
+# the maintenance commands and check that a backtrace is (or isn't)
+# printed, depending on the user setting.
+foreach problem_type { internal-error internal-warning } {
+    gdb_test_no_output "maint set ${problem_type} corefile no"
+    gdb_test_no_output "maint set ${problem_type} quit no"
+
+    foreach mode { on off } {
+	run_test ${problem_type} ${mode}
+    }
+}
diff --git a/gdb/testsuite/gdb.base/bt-on-fatal-signal.exp b/gdb/testsuite/gdb.base/bt-on-fatal-signal.exp
index 1f0d61f00ed..8875d00fdb1 100644
--- a/gdb/testsuite/gdb.base/bt-on-fatal-signal.exp
+++ b/gdb/testsuite/gdb.base/bt-on-fatal-signal.exp
@@ -135,39 +135,3 @@ foreach test_data {{SEGV "Segmentation fault"} \
 	gdb_exit
     }
 }
-
-# Check that when we get an internal error and choose to dump core, we
-# don't print a backtrace to the console.
-with_test_prefix "internal-error" {
-    # Restart GDB.
-    clean_restart $binfile
-
-    set saw_bt_start false
-
-    gdb_test_multiple "maint internal-error foo" "" {
-	-early -re "internal-error: foo\r\n" {
-	    exp_continue
-	}
-	-early -re "^A problem internal to GDB has been detected,\r\n" {
-	    exp_continue
-	}
-	-early -re "^further debugging may prove unreliable\\.\r\n" {
-	    exp_continue
-	}
-	-early -re "^Quit this debugging session\\? \\(y or n\\)" {
-	    send_gdb "y\n"
-	    exp_continue
-	}
-	-early -re "^Create a core file of GDB\\? \\(y or n\\)" {
-	    send_gdb "y\n"
-	    exp_continue
-	}
-	-early -re "----- Backtrace -----\r\n" {
-	    set saw_bt_start true
-	    exp_continue
-	}
-	eof {
-	    gdb_assert { [expr ! $saw_bt_start] }
-	}
-    }
-}
diff --git a/gdb/utils.c b/gdb/utils.c
index 143c2eddd70..0ab032e48b2 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -75,6 +75,7 @@
 #include "gdbarch.h"
 #include "cli-out.h"
 #include "gdbsupport/gdb-safe-ctype.h"
+#include "bt-utils.h"
 
 void (*deprecated_error_begin_hook) (void);
 
@@ -304,6 +305,13 @@ struct internal_problem
 
   /* Like SHOULD_QUIT, but whether GDB should dump core.  */
   const char *should_dump_core;
+
+  /* Like USER_SETTABLE_SHOULD_QUIT but for SHOULD_PRINT_BACKTRACE.  */
+  bool user_settable_should_print_backtrace;
+
+  /* When this is true GDB will print a backtrace when a problem of this
+     type is encountered.  */
+  bool should_print_backtrace;
 };
 
 /* Report a problem, internal to GDB, to the user.  Once the problem
@@ -377,9 +385,13 @@ internal_vproblem (struct internal_problem *problem,
   /* Emit the message unless query will emit it below.  */
   if (problem->should_quit != internal_problem_ask
       || !confirm
-      || !filtered_printing_initialized ())
+      || !filtered_printing_initialized ()
+      || problem->should_print_backtrace)
     fprintf_unfiltered (gdb_stderr, "%s\n", reason.c_str ());
 
+  if (problem->should_print_backtrace)
+    gdb_internal_backtrace ();
+
   if (problem->should_quit == internal_problem_ask)
     {
       /* Default (yes/batch case) is to quit GDB.  When in batch mode
@@ -449,6 +461,7 @@ internal_vproblem (struct internal_problem *problem,
 
 static struct internal_problem internal_error_problem = {
   "internal-error", true, internal_problem_ask, true, internal_problem_ask,
+  true, GDB_PRINT_INTERNAL_BACKTRACE_INIT_ON
 };
 
 void
@@ -460,6 +473,7 @@ internal_verror (const char *file, int line, const char *fmt, va_list ap)
 
 static struct internal_problem internal_warning_problem = {
   "internal-warning", true, internal_problem_ask, true, internal_problem_ask,
+  true, false
 };
 
 void
@@ -470,6 +484,7 @@ internal_vwarning (const char *file, int line, const char *fmt, va_list ap)
 
 static struct internal_problem demangler_warning_problem = {
   "demangler-warning", true, internal_problem_ask, false, internal_problem_no,
+  false, false
 };
 
 void
@@ -569,6 +584,25 @@ add_internal_problem_command (struct internal_problem *problem)
 			    set_cmd_list,
 			    show_cmd_list);
     }
+
+  if (problem->user_settable_should_print_backtrace)
+    {
+      set_doc
+	= string_printf (_("Set whether GDB should print a backtrace of "
+			   "GDB when %s is detected."), problem->name);
+      show_doc
+	= string_printf (_("Show whether GDB will print a backtrace of "
+			   "GDB when %s is detected."), problem->name);
+      add_setshow_boolean_cmd ("backtrace", class_maintenance,
+			       &problem->should_print_backtrace,
+			       set_doc.c_str (),
+			       show_doc.c_str (),
+			       NULL, /* help_doc */
+			       gdb_internal_backtrace_set_cmd,
+			       NULL, /* showfunc */
+			       set_cmd_list,
+			       show_cmd_list);
+    }
 }
 
 /* Return a newly allocated string, containing the PREFIX followed
-- 
2.25.4


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

* Re: [PATCH 1/6] gdb: use bool instead of int in struct internal_problem
  2021-08-19  9:49 ` [PATCH 1/6] gdb: use bool instead of int in struct internal_problem Andrew Burgess
@ 2021-08-19 18:33   ` Simon Marchi
  2021-09-07 14:11     ` Andrew Burgess
  0 siblings, 1 reply; 34+ messages in thread
From: Simon Marchi @ 2021-08-19 18:33 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 2021-08-19 5:49 a.m., Andrew Burgess wrote:
> Change struct internal_problem (gdb/utils.c) to use bool instead of
> int, update the 3 static instances of this structure that we create to
> use true/false instead of 1/0.
> 
> I've also updated the comments on struct internal_problem as the
> existing comment doesn't seem to be referring to the structure, it
> talks about returning something, which doesn't make sense in this
> context.
> 
> There should be no user visible changes after this commit.

That LGTM on its own.

Simon

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

* Re: [PATCH 2/6] gdb: make use of std::string in utils.c
  2021-08-19  9:49 ` [PATCH 2/6] gdb: make use of std::string in utils.c Andrew Burgess
@ 2021-08-19 18:41   ` Simon Marchi
  2021-09-07 14:12     ` Andrew Burgess
  0 siblings, 1 reply; 34+ messages in thread
From: Simon Marchi @ 2021-08-19 18:41 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches



On 2021-08-19 5:49 a.m., Andrew Burgess wrote:
> Replace use of manual string management (malloc/free) with std::string
> when creating commands in utils.c.  Things are a little bit messy as
> creating the prefix commands (using add_basic_prefix_cmd and
> add_show_prefix_cmd), don't copy the doc string, while creating the
> actual set/show commands does copy the doc string, this explains the
> extra xstrdup when creating the prefix commands.

Indeed, all of this is really not consistent.  I guess we would need to
create versions of the add_*_cmd that take a
gdb::unique_xmalloc_pointer<char> and set
cmd_list_element::doc_allocated.

> There should be no user visible changes after this commit.
> ---
>  gdb/utils.c | 58 ++++++++++++++++++++++++-----------------------------
>  1 file changed, 26 insertions(+), 32 deletions(-)
> 
> diff --git a/gdb/utils.c b/gdb/utils.c
> index 3916ae5a1c9..143c2eddd70 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -508,72 +508,66 @@ add_internal_problem_command (struct internal_problem *problem)
>  {
>    struct cmd_list_element **set_cmd_list;
>    struct cmd_list_element **show_cmd_list;
> -  char *set_doc;
> -  char *show_doc;
>  
>    set_cmd_list = XNEW (struct cmd_list_element *);
>    show_cmd_list = XNEW (struct cmd_list_element *);
>    *set_cmd_list = NULL;
>    *show_cmd_list = NULL;
>  
> -  set_doc = xstrprintf (_("Configure what GDB does when %s is detected."),
> -			problem->name);
> +  std::string set_doc
> +    = string_printf (_("Configure what GDB does when %s is detected."),
> +		       problem->name);
>  
> -  show_doc = xstrprintf (_("Show what GDB does when %s is detected."),
> -			 problem->name);
> +  std::string show_doc
> +    = string_printf (_("Show what GDB does when %s is detected."),
> +		       problem->name);
>  
> -  add_basic_prefix_cmd (problem->name, class_maintenance, set_doc,
> -			set_cmd_list,
> +  add_basic_prefix_cmd (problem->name, class_maintenance,
> +			xstrdup (set_doc.c_str ()), set_cmd_list,
>  			0/*allow-unknown*/, &maintenance_set_cmdlist);
>  
> -  add_show_prefix_cmd (problem->name, class_maintenance, show_doc,
> -		       show_cmd_list,
> +  add_show_prefix_cmd (problem->name, class_maintenance,
> +		       xstrdup (show_doc.c_str ()), show_cmd_list,
>  		       0/*allow-unknown*/, &maintenance_show_cmdlist);

I guess you could keep using xstrprintf for these to avoid the extra
copy...  but what you have LGTM either way.

Simon

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

* Re: [PATCH 3/6] gdb: Add a dependency between gdb and libbacktrace
  2021-08-19  9:49 ` [PATCH 3/6] gdb: Add a dependency between gdb and libbacktrace Andrew Burgess
@ 2021-08-19 18:43   ` Simon Marchi
  2021-08-27 17:44     ` Tom Tromey
  0 siblings, 1 reply; 34+ messages in thread
From: Simon Marchi @ 2021-08-19 18:43 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 2021-08-19 5:49 a.m., Andrew Burgess wrote:
> GDB is going to start using libbacktrace, so add a build dependency.
> 
> ChangeLog:
> 
> 	* Makefile.def: Add all-gdb dependency on all-libbacktrace.
> 	* Makefile.in: Regenerate.
> ---
>  Makefile.def | 1 +
>  Makefile.in  | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/Makefile.def b/Makefile.def
> index 5a460f1dbbc..d5b1ee882b6 100644
> --- a/Makefile.def
> +++ b/Makefile.def
> @@ -408,6 +408,7 @@ dependencies = { module=all-gdb; on=all-sim; };
>  dependencies = { module=all-gdb; on=all-libdecnumber; };
>  dependencies = { module=all-gdb; on=all-libtermcap; };
>  dependencies = { module=all-gdb; on=all-libctf; };
> +dependencies = { module=all-gdb; on=all-libbacktrace; };
>  
>  // Host modules specific to gdbserver.
>  dependencies = { module=configure-gdbserver; on=all-gnulib; };
> diff --git a/Makefile.in b/Makefile.in
> index 9b3a5d75735..91b714f6e88 100644
> --- a/Makefile.in
> +++ b/Makefile.in
> @@ -52534,6 +52534,7 @@ all-gdb: maybe-all-libiconv
>  all-gdb: maybe-all-opcodes
>  all-gdb: maybe-all-libdecnumber
>  all-gdb: maybe-all-libctf
> +all-gdb: maybe-all-libbacktrace
>  all-gdbserver: maybe-all-libiberty
>  configure-gdbsupport: maybe-configure-intl
>  all-gdbsupport: maybe-all-intl
> 

I was surprised that this worked, even though libbacktrace isn't
imported yet.

LGTM, but would this need to be sent to binutils as well (maybe just a
FYI, a formality)?

Simon

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

* Re: [PATCH 5/6] gdb: use libbacktrace to create a better backtrace for fatal signals
  2021-08-19  9:49 ` [PATCH 5/6] gdb: use libbacktrace to create a better backtrace for fatal signals Andrew Burgess
@ 2021-08-19 18:58   ` Simon Marchi
  0 siblings, 0 replies; 34+ messages in thread
From: Simon Marchi @ 2021-08-19 18:58 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 2021-08-19 5:49 a.m., Andrew Burgess wrote:
> GDB recently gained the ability to print a backtrace when a fatal
> signal is encountered.  This backtrace is produced using the backtrace
> and backtrace_symbols_fd API available in glibc.
> 
> However, in order for this API to actually map addresses to symbol
> names it is required that the application (GDB) be compiled with
> -rdynamic, which GDB is not by default.
> 
> As a result, the backtrace produced often looks like this:
> 
>   Fatal signal: Bus error
>   ----- Backtrace -----
>   ./gdb/gdb[0x80ec00]
>   ./gdb/gdb[0x80ed56]
>   /lib64/libc.so.6(+0x3c6b0)[0x7fc2ce1936b0]
>   /lib64/libc.so.6(__poll+0x4f)[0x7fc2ce24da5f]
>   ./gdb/gdb[0x15495ba]
>   ./gdb/gdb[0x15489b8]
>   ./gdb/gdb[0x9b794d]
>   ./gdb/gdb[0x9b7a6d]
>   ./gdb/gdb[0x9b943b]
>   ./gdb/gdb[0x9b94a1]
>   ./gdb/gdb[0x4175dd]
>   /lib64/libc.so.6(__libc_start_main+0xf3)[0x7fc2ce17e1a3]
>   ./gdb/gdb[0x4174de]
>   ---------------------

I haven't seen these backtraces on my builds, I wonder why.  It probably
means GDB never crashes!

> This is OK if you have access to the exact same build of GDB, you can
> manually map the addresses back to symbols, however, it is next to
> useless if all you have is a backtrace copied into a bug report.
> 
> GCC uses libbacktrace for printing a backtrace when it encounters an
> error.  In recent commits I added this library into the binutils-gdb
> repository, and in this commit I allow this library to be used by
> GDB.  Now (when GDB is compiled with debug information) the backtrace
> looks like this:
> 
>   ----- Backtrace -----
>   0x80ee08 gdb_internal_backtrace
>   	../../src/gdb/event-top.c:989
>   0x80ef0b handle_fatal_signal
>   	../../src/gdb/event-top.c:1036
>   0x7f24539dd6af ???
>   0x7f2453a97a5f ???
>   0x154976f gdb_wait_for_event
>   	../../src/gdbsupport/event-loop.cc:613
>   0x1548b6d _Z16gdb_do_one_eventv
>   	../../src/gdbsupport/event-loop.cc:237
>   0x9b7b02 start_event_loop
>   	../../src/gdb/main.c:421
>   0x9b7c22 captured_command_loop
>   	../../src/gdb/main.c:481
>   0x9b95f0 captured_main
>   	../../src/gdb/main.c:1353
>   0x9b9656 _Z8gdb_mainP18captured_main_args
>   	../../src/gdb/main.c:1368
>   0x4175ec main
>   	../../src/gdb/gdb.c:32
>   ---------------------
> 
> Which seems much more useful.
> 
> Use of libbacktrace is optional.  If GDB is configured with
> --disable-libbacktrace then the libbacktrace directory will not be
> built, and GDB will not try to use this library.  In this case GDB
> would try to use the old backtrace and backtrace_symbols_fd API.
> 
> All of the functions related to writing the backtrace of GDB itself
> have been moved into the new files gdb/by-utils.{c,h}.

bt-utils

Will we want to have this in GDBserver as well?  If so, could you put
the bits that can be shared in gdbsupport?  That would mean the code to
produce the backtrace, but not the code that deals with
cmd_list_element, for example.

> @@ -636,12 +640,12 @@ INTERNAL_LDFLAGS = \
>  # LIBIBERTY appears twice on purpose.
>  CLIBS = $(SIM) $(READLINE) $(OPCODES) $(LIBCTF) $(BFD) $(ZLIB) \
>          $(LIBSUPPORT) $(INTL) $(LIBIBERTY) $(LIBDECNUMBER) \
> -	$(XM_CLIBS) $(GDBTKLIBS) \
> +	$(XM_CLIBS) $(GDBTKLIBS)  $(LIBBACKTRACE_LIB) \
>  	@LIBS@ @GUILE_LIBS@ @PYTHON_LIBS@ \
>  	$(LIBEXPAT) $(LIBLZMA) $(LIBBABELTRACE) $(LIBIPT) \
>  	$(WIN32LIBS) $(LIBGNU) $(LIBGNU_EXTRA_LIBS) $(LIBICONV) \
>  	$(LIBMPFR) $(LIBGMP) $(SRCHIGH_LIBS) $(LIBXXHASH) $(PTHREAD_LIBS) \
> -	$(DEBUGINFOD_LIBS)
> +	$(DEBUGINFOD_LIBS) $(LIBBABELTRACE_LIB)

Why are you adding LIBBABELTRACE_LIB here?

> +/* Callback used by libbacktrace if it encounters an error.  */
> +
> +static void
> +libbacktrace_error (void *data, const char *errmsg, int errnum)
> +{
> +  /* A negative errnum indicates no debug info was available, just
> +     skip printing a backtrace in this case.  */
> +  if (errnum < 0)
> +    return;
> +
> +  const auto sig_write = [] (const char *msg) -> void
> +  {
> +    gdb_stderr->write_async_safe (msg, strlen (msg));
> +  };
> +
> +  sig_write ("error creating backtrace: ");
> +  sig_write (errmsg);
> +  if (errnum > 0)
> +    {
> +      char buf[10];
> +      snprintf (buf, sizeof (buf), ": %d", errnum);
> +      buf[sizeof (buf) - 1] = '\0';

I think snprintf always writes the null terminator itself, even if the
output is truncated.

Other than that, it all LGTM, thanks.

Simon

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

* Re: [PATCH 6/6] gdb: print backtrace for internal error/warning
  2021-08-19  9:49 ` [PATCH 6/6] gdb: print backtrace for internal error/warning Andrew Burgess
@ 2021-08-19 19:01   ` Simon Marchi
  0 siblings, 0 replies; 34+ messages in thread
From: Simon Marchi @ 2021-08-19 19:01 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 2021-08-19 5:49 a.m., Andrew Burgess wrote:
> This commit builds on previous work to allow GDB to print a backtrace
> of itself when GDB encounters an internal-error or internal-warning.
> 
> There's not many places where we call internal_warning, and I guess in
> most cases the user would probably continue their debug session.  And
> so, in order to avoid cluttering up the output, by default, printing
> of a backtrace is off for internal-warnings.
> 
> In contrast, printing of a backtrace is on by default for
> internal-errors, as I figure that in most cases hitting an
> internal-error is going to be the end of the debug session.
> 
> Whether a backtrace is printed or not can be controlled with the new
> settings:
> 
>   maintenance set internal-error backtrace on|off
>   maintenance show internal-error backtrace
> 
>   maintenance set internal-warning backtrace on|off
>   maintenance show internal-warning backtrace
> 
> Here is an example of what an internal-error now looks like with the
> backtrace included:
> 
>   (gdb) maintenance internal-error blah
>   ../../src.dev-3/gdb/maint.c:82: internal-error: blah
>   A problem internal to GDB has been detected,
>   further debugging may prove unreliable.
>   ----- Backtrace -----
>   0x5c61ca gdb_internal_backtrace_1
>   	../../src.dev-3/gdb/bt-utils.c:123
>   0x5c626d _Z22gdb_internal_backtracev
>   	../../src.dev-3/gdb/bt-utils.c:165
>   0xe33237 internal_vproblem
>   	../../src.dev-3/gdb/utils.c:393
>   0xe33539 _Z15internal_verrorPKciS0_P13__va_list_tag
>   	../../src.dev-3/gdb/utils.c:470
>   0x1549652 _Z14internal_errorPKciS0_z
>   	../../src.dev-3/gdbsupport/errors.cc:55
>   0x9c7982 maintenance_internal_error
>   	../../src.dev-3/gdb/maint.c:82
>   0x636f57 do_simple_func
>   	../../src.dev-3/gdb/cli/cli-decode.c:97
>    .... snip, lots more backtrace lines ....
>   ---------------------
>   ../../src.dev-3/gdb/maint.c:82: internal-error: blah
>   A problem internal to GDB has been detected,
>   further debugging may prove unreliable.
>   Quit this debugging session? (y or n) y
> 
>   This is a bug, please report it.  For instructions, see:
>   <https://www.gnu.org/software/gdb/bugs/>.
> 
>   ../../src.dev-3/gdb/maint.c:82: internal-error: blah
>   A problem internal to GDB has been detected,
>   further debugging may prove unreliable.
>   Create a core file of GDB? (y or n) n
> 
> My hope is that this backtrace might make it slightly easier to
> diagnose GDB issues if all that is provided is the console output, I
> find that we frequently get reports of an assert being hit that is
> located in pretty generic code (frame.c, value.c, etc) and it is not
> always obvious how we might have arrived at the assert.

Thanks for doing this, I think it will be very useful.

Simon

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

* Re: [PATCH 3/6] gdb: Add a dependency between gdb and libbacktrace
  2021-08-19 18:43   ` Simon Marchi
@ 2021-08-27 17:44     ` Tom Tromey
  2021-08-30 20:33       ` Andrew Burgess
  0 siblings, 1 reply; 34+ messages in thread
From: Tom Tromey @ 2021-08-27 17:44 UTC (permalink / raw)
  To: Simon Marchi via Gdb-patches

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> I was surprised that this worked, even though libbacktrace isn't
Simon> imported yet.

These top level dependencies are optional, which is why stuff like
building libiconv in-tree works out.  It's weird though.

Simon> LGTM, but would this need to be sent to binutils as well (maybe just a
Simon> FYI, a formality)?

I think top-level changes normally have to go to gcc.

Tom

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

* Re: [PATCH 4/6] Copy in libbacktrace from gcc
  2021-08-19  9:49 ` [PATCH 4/6] Copy in libbacktrace from gcc Andrew Burgess
@ 2021-08-27 17:46   ` Tom Tromey
  2021-08-30 20:34     ` Andrew Burgess
  0 siblings, 1 reply; 34+ messages in thread
From: Tom Tromey @ 2021-08-27 17:46 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> This copies in libbacktrace from the gcc repository as it was in the
Andrew> commit ad3f0ad4bafe377072a53ded468fd9948e659f46.  GDB is going to
Andrew> start using this library soon.

Seems reasonable to me.

Tom

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

* Re: [PATCH 0/6] Display GDB backtrace for internal errors
  2021-08-19  9:49 [PATCH 0/6] Display GDB backtrace for internal errors Andrew Burgess
                   ` (5 preceding siblings ...)
  2021-08-19  9:49 ` [PATCH 6/6] gdb: print backtrace for internal error/warning Andrew Burgess
@ 2021-08-30 14:16 ` Tom de Vries
  2021-08-30 20:35   ` Andrew Burgess
  2021-08-31 11:17 ` Florian Weimer
  2021-09-28 11:26 ` [PUSHED " Andrew Burgess
  8 siblings, 1 reply; 34+ messages in thread
From: Tom de Vries @ 2021-08-30 14:16 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 8/19/21 11:49 AM, Andrew Burgess wrote:
> This series does two things:
> 
>      1. Imports libbacktrace from gcc to present better backtraces of
>      GDB when we hit a fatal signal, and
> 
>      2. Prints the backtrace when GDB hits an internal-error (e.g. an
>      assert).
> 
> The hope here is to try an improve the quality of bug reports, instead
> of just getting a report that GDB hit some assert in frame.c, or
> value.c, we will (hopefully) have a backtrace included with the bug
> report, which _might_ give developers a fighting chance at figuring
> out what went wrong.
> 

Nice :)

This means you can claim PR26377 - "generate backtrace upon assert", I'm
guessing in the last patch of the series.

Thanks,
- Tom

> All feedback welcome,
> 
> Thanks,
> Andrew
> 
> ---
> 
> Andrew Burgess (6):
>   gdb: use bool instead of int in struct internal_problem
>   gdb: make use of std::string in utils.c
>   gdb: Add a dependency between gdb and libbacktrace
>   Copy in libbacktrace from gcc
>   gdb: use libbacktrace to create a better backtrace for fatal signals
>   gdb: print backtrace for internal error/warning
> 
>  Makefile.def                                  |     1 +
>  Makefile.in                                   |     1 +
>  gdb/Makefile.in                               |    16 +-
>  gdb/NEWS                                      |     8 +
>  gdb/bt-utils.c                                |   170 +
>  gdb/bt-utils.h                                |    69 +
>  gdb/config.in                                 |     3 +
>  gdb/configure                                 |    32 +
>  gdb/configure.ac                              |    23 +
>  gdb/doc/gdb.texinfo                           |    13 +
>  gdb/event-top.c                               |    47 +-
>  .../gdb.base/bt-on-error-and-warning.exp      |   118 +
>  gdb/testsuite/gdb.base/bt-on-fatal-signal.exp |    36 -
>  gdb/utils.c                                   |   121 +-
>  libbacktrace/ChangeLog                        |  1759 ++
>  libbacktrace/ChangeLog.jit                    |    14 +
>  libbacktrace/Makefile.am                      |   586 +
>  libbacktrace/Makefile.in                      |  2528 +++
>  libbacktrace/README                           |    23 +
>  libbacktrace/aclocal.m4                       |   867 +
>  libbacktrace/alloc.c                          |   167 +
>  libbacktrace/allocfail.c                      |   136 +
>  libbacktrace/allocfail.sh                     |   104 +
>  libbacktrace/atomic.c                         |   113 +
>  libbacktrace/backtrace-supported.h.in         |    66 +
>  libbacktrace/backtrace.c                      |   129 +
>  libbacktrace/backtrace.h                      |   206 +
>  libbacktrace/btest.c                          |   501 +
>  libbacktrace/config.h.in                      |   184 +
>  libbacktrace/configure                        | 16893 ++++++++++++++++
>  libbacktrace/configure.ac                     |   581 +
>  libbacktrace/dwarf.c                          |  4056 ++++
>  libbacktrace/edtest.c                         |   120 +
>  libbacktrace/edtest2.c                        |    43 +
>  libbacktrace/elf.c                            |  4919 +++++
>  libbacktrace/fileline.c                       |   346 +
>  libbacktrace/filetype.awk                     |    13 +
>  .../install-debuginfo-for-buildid.sh.in       |    65 +
>  libbacktrace/instrumented_alloc.c             |   114 +
>  libbacktrace/internal.h                       |   380 +
>  libbacktrace/macho.c                          |  1355 ++
>  libbacktrace/mmap.c                           |   331 +
>  libbacktrace/mmapio.c                         |   110 +
>  libbacktrace/mtest.c                          |   410 +
>  libbacktrace/nounwind.c                       |    66 +
>  libbacktrace/pecoff.c                         |   935 +
>  libbacktrace/posix.c                          |   104 +
>  libbacktrace/print.c                          |    92 +
>  libbacktrace/read.c                           |   110 +
>  libbacktrace/simple.c                         |   108 +
>  libbacktrace/sort.c                           |   108 +
>  libbacktrace/state.c                          |    72 +
>  libbacktrace/stest.c                          |   137 +
>  libbacktrace/test_format.c                    |    55 +
>  libbacktrace/testlib.c                        |   234 +
>  libbacktrace/testlib.h                        |   110 +
>  libbacktrace/ttest.c                          |   161 +
>  libbacktrace/unittest.c                       |    92 +
>  libbacktrace/unknown.c                        |    65 +
>  libbacktrace/xcoff.c                          |  1606 ++
>  libbacktrace/xztest.c                         |   508 +
>  libbacktrace/ztest.c                          |   541 +
>  62 files changed, 42760 insertions(+), 121 deletions(-)
>  create mode 100644 gdb/bt-utils.c
>  create mode 100644 gdb/bt-utils.h
>  create mode 100644 gdb/testsuite/gdb.base/bt-on-error-and-warning.exp
>  create mode 100644 libbacktrace/ChangeLog
>  create mode 100644 libbacktrace/ChangeLog.jit
>  create mode 100644 libbacktrace/Makefile.am
>  create mode 100644 libbacktrace/Makefile.in
>  create mode 100644 libbacktrace/README
>  create mode 100644 libbacktrace/aclocal.m4
>  create mode 100644 libbacktrace/alloc.c
>  create mode 100644 libbacktrace/allocfail.c
>  create mode 100755 libbacktrace/allocfail.sh
>  create mode 100644 libbacktrace/atomic.c
>  create mode 100644 libbacktrace/backtrace-supported.h.in
>  create mode 100644 libbacktrace/backtrace.c
>  create mode 100644 libbacktrace/backtrace.h
>  create mode 100644 libbacktrace/btest.c
>  create mode 100644 libbacktrace/config.h.in
>  create mode 100755 libbacktrace/configure
>  create mode 100644 libbacktrace/configure.ac
>  create mode 100644 libbacktrace/dwarf.c
>  create mode 100644 libbacktrace/edtest.c
>  create mode 100644 libbacktrace/edtest2.c
>  create mode 100644 libbacktrace/elf.c
>  create mode 100644 libbacktrace/fileline.c
>  create mode 100644 libbacktrace/filetype.awk
>  create mode 100644 libbacktrace/install-debuginfo-for-buildid.sh.in
>  create mode 100644 libbacktrace/instrumented_alloc.c
>  create mode 100644 libbacktrace/internal.h
>  create mode 100644 libbacktrace/macho.c
>  create mode 100644 libbacktrace/mmap.c
>  create mode 100644 libbacktrace/mmapio.c
>  create mode 100644 libbacktrace/mtest.c
>  create mode 100644 libbacktrace/nounwind.c
>  create mode 100644 libbacktrace/pecoff.c
>  create mode 100644 libbacktrace/posix.c
>  create mode 100644 libbacktrace/print.c
>  create mode 100644 libbacktrace/read.c
>  create mode 100644 libbacktrace/simple.c
>  create mode 100644 libbacktrace/sort.c
>  create mode 100644 libbacktrace/state.c
>  create mode 100644 libbacktrace/stest.c
>  create mode 100644 libbacktrace/test_format.c
>  create mode 100644 libbacktrace/testlib.c
>  create mode 100644 libbacktrace/testlib.h
>  create mode 100644 libbacktrace/ttest.c
>  create mode 100644 libbacktrace/unittest.c
>  create mode 100644 libbacktrace/unknown.c
>  create mode 100644 libbacktrace/xcoff.c
>  create mode 100644 libbacktrace/xztest.c
>  create mode 100644 libbacktrace/ztest.c
> 

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

* Re: [PATCH 3/6] gdb: Add a dependency between gdb and libbacktrace
  2021-08-27 17:44     ` Tom Tromey
@ 2021-08-30 20:33       ` Andrew Burgess
  0 siblings, 0 replies; 34+ messages in thread
From: Andrew Burgess @ 2021-08-30 20:33 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Simon Marchi via Gdb-patches, Simon Marchi

* Tom Tromey <tom@tromey.com> [2021-08-27 11:44:54 -0600]:

> >>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> I was surprised that this worked, even though libbacktrace isn't
> Simon> imported yet.
> 
> These top level dependencies are optional, which is why stuff like
> building libiconv in-tree works out.  It's weird though.
> 
> Simon> LGTM, but would this need to be sent to binutils as well (maybe just a
> Simon> FYI, a formality)?
> 
> I think top-level changes normally have to go to gcc.

Thanks, I've posted this patch to the GCC list and will wait for it to
get merged before moving ahead with this series.

Thanks,
Andrew

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

* Re: [PATCH 4/6] Copy in libbacktrace from gcc
  2021-08-27 17:46   ` Tom Tromey
@ 2021-08-30 20:34     ` Andrew Burgess
  0 siblings, 0 replies; 34+ messages in thread
From: Andrew Burgess @ 2021-08-30 20:34 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

* Tom Tromey <tom@tromey.com> [2021-08-27 11:46:13 -0600]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> This copies in libbacktrace from the gcc repository as it was in the
> Andrew> commit ad3f0ad4bafe377072a53ded468fd9948e659f46.  GDB is going to
> Andrew> start using this library soon.
> 
> Seems reasonable to me.

I've sent an RFC to the binutils list to make them aware I plan to
merge this in.  I'll give them some time to raise any
questions/feedback before I go ahead with this series.

Thanks,
Andrew

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

* Re: [PATCH 0/6] Display GDB backtrace for internal errors
  2021-08-30 14:16 ` [PATCH 0/6] Display GDB backtrace for internal errors Tom de Vries
@ 2021-08-30 20:35   ` Andrew Burgess
  0 siblings, 0 replies; 34+ messages in thread
From: Andrew Burgess @ 2021-08-30 20:35 UTC (permalink / raw)
  To: Tom de Vries; +Cc: gdb-patches

* Tom de Vries <tdevries@suse.de> [2021-08-30 16:16:09 +0200]:

> On 8/19/21 11:49 AM, Andrew Burgess wrote:
> > This series does two things:
> > 
> >      1. Imports libbacktrace from gcc to present better backtraces of
> >      GDB when we hit a fatal signal, and
> > 
> >      2. Prints the backtrace when GDB hits an internal-error (e.g. an
> >      assert).
> > 
> > The hope here is to try an improve the quality of bug reports, instead
> > of just getting a report that GDB hit some assert in frame.c, or
> > value.c, we will (hopefully) have a backtrace included with the bug
> > report, which _might_ give developers a fighting chance at figuring
> > out what went wrong.
> > 
> 
> Nice :)
> 
> This means you can claim PR26377 - "generate backtrace upon assert", I'm
> guessing in the last patch of the series.

Thanks for pointing that out, I'll add a link to that bug in the
commit message.

Andrew

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

* Re: [PATCH 0/6] Display GDB backtrace for internal errors
  2021-08-19  9:49 [PATCH 0/6] Display GDB backtrace for internal errors Andrew Burgess
                   ` (6 preceding siblings ...)
  2021-08-30 14:16 ` [PATCH 0/6] Display GDB backtrace for internal errors Tom de Vries
@ 2021-08-31 11:17 ` Florian Weimer
  2021-09-28 11:26 ` [PUSHED " Andrew Burgess
  8 siblings, 0 replies; 34+ messages in thread
From: Florian Weimer @ 2021-08-31 11:17 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

* Andrew Burgess:

> This series does two things:
>
>      1. Imports libbacktrace from gcc to present better backtraces of
>      GDB when we hit a fatal signal, and
>
>      2. Prints the backtrace when GDB hits an internal-error (e.g. an
>      assert).

Ideally, we would add this capability to bash, so that if a process dies
with certain signals, the shell prints a backtrace, possibly with help
from a core-catching daemon.  Backtrace generation from outside the
process is much more reliable.  It also avoids introducing further
opportunities for exploitation after memory corruption is detected.

In my opinion, it does not make much sense to add this functionality to
more and more GNU programs.

Thanks,
Florian


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

* Re: [PATCH 1/6] gdb: use bool instead of int in struct internal_problem
  2021-08-19 18:33   ` Simon Marchi
@ 2021-09-07 14:11     ` Andrew Burgess
  0 siblings, 0 replies; 34+ messages in thread
From: Andrew Burgess @ 2021-09-07 14:11 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

* Simon Marchi <simon.marchi@polymtl.ca> [2021-08-19 14:33:48 -0400]:

> On 2021-08-19 5:49 a.m., Andrew Burgess wrote:
> > Change struct internal_problem (gdb/utils.c) to use bool instead of
> > int, update the 3 static instances of this structure that we create to
> > use true/false instead of 1/0.
> > 
> > I've also updated the comments on struct internal_problem as the
> > existing comment doesn't seem to be referring to the structure, it
> > talks about returning something, which doesn't make sense in this
> > context.
> > 
> > There should be no user visible changes after this commit.
> 
> That LGTM on its own.

Thanks, I've now pushed this patch.

Andrew

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

* Re: [PATCH 2/6] gdb: make use of std::string in utils.c
  2021-08-19 18:41   ` Simon Marchi
@ 2021-09-07 14:12     ` Andrew Burgess
  0 siblings, 0 replies; 34+ messages in thread
From: Andrew Burgess @ 2021-09-07 14:12 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

* Simon Marchi <simon.marchi@polymtl.ca> [2021-08-19 14:41:41 -0400]:

> 
> 
> On 2021-08-19 5:49 a.m., Andrew Burgess wrote:
> > Replace use of manual string management (malloc/free) with std::string
> > when creating commands in utils.c.  Things are a little bit messy as
> > creating the prefix commands (using add_basic_prefix_cmd and
> > add_show_prefix_cmd), don't copy the doc string, while creating the
> > actual set/show commands does copy the doc string, this explains the
> > extra xstrdup when creating the prefix commands.
> 
> Indeed, all of this is really not consistent.  I guess we would need to
> create versions of the add_*_cmd that take a
> gdb::unique_xmalloc_pointer<char> and set
> cmd_list_element::doc_allocated.
> 
> > There should be no user visible changes after this commit.
> > ---
> >  gdb/utils.c | 58 ++++++++++++++++++++++++-----------------------------
> >  1 file changed, 26 insertions(+), 32 deletions(-)
> > 
> > diff --git a/gdb/utils.c b/gdb/utils.c
> > index 3916ae5a1c9..143c2eddd70 100644
> > --- a/gdb/utils.c
> > +++ b/gdb/utils.c
> > @@ -508,72 +508,66 @@ add_internal_problem_command (struct internal_problem *problem)
> >  {
> >    struct cmd_list_element **set_cmd_list;
> >    struct cmd_list_element **show_cmd_list;
> > -  char *set_doc;
> > -  char *show_doc;
> >  
> >    set_cmd_list = XNEW (struct cmd_list_element *);
> >    show_cmd_list = XNEW (struct cmd_list_element *);
> >    *set_cmd_list = NULL;
> >    *show_cmd_list = NULL;
> >  
> > -  set_doc = xstrprintf (_("Configure what GDB does when %s is detected."),
> > -			problem->name);
> > +  std::string set_doc
> > +    = string_printf (_("Configure what GDB does when %s is detected."),
> > +		       problem->name);
> >  
> > -  show_doc = xstrprintf (_("Show what GDB does when %s is detected."),
> > -			 problem->name);
> > +  std::string show_doc
> > +    = string_printf (_("Show what GDB does when %s is detected."),
> > +		       problem->name);
> >  
> > -  add_basic_prefix_cmd (problem->name, class_maintenance, set_doc,
> > -			set_cmd_list,
> > +  add_basic_prefix_cmd (problem->name, class_maintenance,
> > +			xstrdup (set_doc.c_str ()), set_cmd_list,
> >  			0/*allow-unknown*/, &maintenance_set_cmdlist);
> >  
> > -  add_show_prefix_cmd (problem->name, class_maintenance, show_doc,
> > -		       show_cmd_list,
> > +  add_show_prefix_cmd (problem->name, class_maintenance,
> > +		       xstrdup (show_doc.c_str ()), show_cmd_list,
> >  		       0/*allow-unknown*/, &maintenance_show_cmdlist);
> 
> I guess you could keep using xstrprintf for these to avoid the extra
> copy...  but what you have LGTM either way.

I've taken your advice.  Below is what I pushed.

Thanks,
Andrew

--

commit 747656685b3e8477868478cd927fbb2834937aff
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Tue Aug 17 13:29:22 2021 +0100

    gdb: make use of std::string in utils.c
    
    Replace some of the manual string management (malloc/free) with
    std::string when creating commands in utils.c.
    
    Things are a little bit messy as, creating the prefix commands (using
    add_basic_prefix_cmd and add_show_prefix_cmd), doesn't copy the doc
    string, while creating the actual set/show commands (using
    add_setshow_enum_cmd) does copy the doc string.
    
    As a result, I have retained the use of xstrprintf when creating the
    prefix command doc strings, but switched to using std::string when
    creating the actual set/show commands.
    
    There should be no user visible changes after this commit.

diff --git a/gdb/utils.c b/gdb/utils.c
index 0009cb10d87..0a7c270b40d 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -508,19 +508,21 @@ add_internal_problem_command (struct internal_problem *problem)
 {
   struct cmd_list_element **set_cmd_list;
   struct cmd_list_element **show_cmd_list;
-  char *set_doc;
-  char *show_doc;
 
   set_cmd_list = XNEW (struct cmd_list_element *);
   show_cmd_list = XNEW (struct cmd_list_element *);
   *set_cmd_list = NULL;
   *show_cmd_list = NULL;
 
-  set_doc = xstrprintf (_("Configure what GDB does when %s is detected."),
-			problem->name);
-
-  show_doc = xstrprintf (_("Show what GDB does when %s is detected."),
-			 problem->name);
+  /* The add_basic_prefix_cmd and add_show_prefix_cmd functions take
+     ownership of the string passed in, which is why we don't need to free
+     set_doc and show_doc in this function.  */
+  const char *set_doc
+    = xstrprintf (_("Configure what GDB does when %s is detected."),
+		  problem->name);
+  const char *show_doc
+    = xstrprintf (_("Show what GDB does when %s is detected."),
+		  problem->name);
 
   add_basic_prefix_cmd (problem->name, class_maintenance, set_doc,
 			set_cmd_list,
@@ -532,48 +534,42 @@ add_internal_problem_command (struct internal_problem *problem)
 
   if (problem->user_settable_should_quit)
     {
-      set_doc = xstrprintf (_("Set whether GDB should quit "
-			      "when an %s is detected."),
-			    problem->name);
-      show_doc = xstrprintf (_("Show whether GDB will quit "
-			       "when an %s is detected."),
-			     problem->name);
+      std::string set_quit_doc
+	= string_printf (_("Set whether GDB should quit when an %s is "
+			   "detected."), problem->name);
+      std::string show_quit_doc
+	= string_printf (_("Show whether GDB will quit when an %s is "
+			   "detected."), problem->name);
       add_setshow_enum_cmd ("quit", class_maintenance,
 			    internal_problem_modes,
 			    &problem->should_quit,
-			    set_doc,
-			    show_doc,
+			    set_quit_doc.c_str (),
+			    show_quit_doc.c_str (),
 			    NULL, /* help_doc */
 			    NULL, /* setfunc */
 			    NULL, /* showfunc */
 			    set_cmd_list,
 			    show_cmd_list);
-
-      xfree (set_doc);
-      xfree (show_doc);
     }
 
   if (problem->user_settable_should_dump_core)
     {
-      set_doc = xstrprintf (_("Set whether GDB should create a core "
-			      "file of GDB when %s is detected."),
-			    problem->name);
-      show_doc = xstrprintf (_("Show whether GDB will create a core "
-			       "file of GDB when %s is detected."),
-			     problem->name);
+      std::string set_core_doc
+	= string_printf (_("Set whether GDB should create a core file of "
+			   "GDB when %s is detected."), problem->name);
+      std::string show_core_doc
+	= string_printf (_("Show whether GDB will create a core file of "
+			   "GDB when %s is detected."), problem->name);
       add_setshow_enum_cmd ("corefile", class_maintenance,
 			    internal_problem_modes,
 			    &problem->should_dump_core,
-			    set_doc,
-			    show_doc,
+			    set_core_doc.c_str (),
+			    show_core_doc.c_str (),
 			    NULL, /* help_doc */
 			    NULL, /* setfunc */
 			    NULL, /* showfunc */
 			    set_cmd_list,
 			    show_cmd_list);
-
-      xfree (set_doc);
-      xfree (show_doc);
     }
 }
 

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

* [PUSHED 0/6] Display GDB backtrace for internal errors
  2021-08-19  9:49 [PATCH 0/6] Display GDB backtrace for internal errors Andrew Burgess
                   ` (7 preceding siblings ...)
  2021-08-31 11:17 ` Florian Weimer
@ 2021-09-28 11:26 ` Andrew Burgess
  2021-09-28 11:26   ` [PUSHED 1/6] top-level configure: setup target_configdirs based on repository Andrew Burgess
                     ` (5 more replies)
  8 siblings, 6 replies; 34+ messages in thread
From: Andrew Burgess @ 2021-09-28 11:26 UTC (permalink / raw)
  To: gdb-patches

Patches #1 and #2 have already been merged into GCC, the commits in
this series are just me committing the same patches over here.

Patch #3 imports the latest libbacktrace from GCC, there's nothing
I've written in here.

Patch #4 is new, a minor update to src-release.sh to ensure that
libbacktrace is included in the release tar file.

Patch #5 and #6 are just rebased from the original series, no other
changes.

Thanks,
Andrew


---

Andrew Burgess (6):
  top-level configure: setup target_configdirs based on repository
  gdb: Add a dependency between gdb and libbacktrace
  Copy in libbacktrace from gcc
  src-release.sh: add libbacktrace to GDB_SUPPORT_DIRS
  gdb: use libbacktrace to create a better backtrace for fatal signals
  gdb: print backtrace for internal error/warning

 ChangeLog                                     |     4 +
 Makefile.def                                  |     1 +
 Makefile.in                                   |     1 +
 configure                                     |    10 +
 configure.ac                                  |    10 +
 gdb/Makefile.in                               |    16 +-
 gdb/NEWS                                      |     8 +
 gdb/bt-utils.c                                |   170 +
 gdb/bt-utils.h                                |    69 +
 gdb/config.in                                 |     3 +
 gdb/configure                                 |    32 +
 gdb/configure.ac                              |    23 +
 gdb/doc/gdb.texinfo                           |    13 +
 gdb/event-top.c                               |    47 +-
 .../gdb.base/bt-on-error-and-warning.exp      |   118 +
 gdb/testsuite/gdb.base/bt-on-fatal-signal.exp |    36 -
 gdb/utils.c                                   |    36 +-
 libbacktrace/ChangeLog                        |  1774 ++
 libbacktrace/ChangeLog.jit                    |    14 +
 libbacktrace/Makefile.am                      |   586 +
 libbacktrace/Makefile.in                      |  2528 +++
 libbacktrace/README                           |    23 +
 libbacktrace/aclocal.m4                       |   867 +
 libbacktrace/alloc.c                          |   167 +
 libbacktrace/allocfail.c                      |   136 +
 libbacktrace/allocfail.sh                     |   104 +
 libbacktrace/atomic.c                         |   113 +
 libbacktrace/backtrace-supported.h.in         |    66 +
 libbacktrace/backtrace.c                      |   129 +
 libbacktrace/backtrace.h                      |   206 +
 libbacktrace/btest.c                          |   501 +
 libbacktrace/config.h.in                      |   184 +
 libbacktrace/configure                        | 16893 ++++++++++++++++
 libbacktrace/configure.ac                     |   581 +
 libbacktrace/dwarf.c                          |  4056 ++++
 libbacktrace/edtest.c                         |   120 +
 libbacktrace/edtest2.c                        |    43 +
 libbacktrace/elf.c                            |  4919 +++++
 libbacktrace/fileline.c                       |   346 +
 libbacktrace/filetype.awk                     |    13 +
 .../install-debuginfo-for-buildid.sh.in       |    65 +
 libbacktrace/instrumented_alloc.c             |   114 +
 libbacktrace/internal.h                       |   380 +
 libbacktrace/macho.c                          |  1355 ++
 libbacktrace/mmap.c                           |   331 +
 libbacktrace/mmapio.c                         |   110 +
 libbacktrace/mtest.c                          |   410 +
 libbacktrace/nounwind.c                       |    66 +
 libbacktrace/pecoff.c                         |   935 +
 libbacktrace/posix.c                          |   104 +
 libbacktrace/print.c                          |    92 +
 libbacktrace/read.c                           |   110 +
 libbacktrace/simple.c                         |   108 +
 libbacktrace/sort.c                           |   108 +
 libbacktrace/state.c                          |    72 +
 libbacktrace/stest.c                          |   137 +
 libbacktrace/test_format.c                    |    55 +
 libbacktrace/testlib.c                        |   234 +
 libbacktrace/testlib.h                        |   110 +
 libbacktrace/ttest.c                          |   161 +
 libbacktrace/unittest.c                       |    92 +
 libbacktrace/unknown.c                        |    65 +
 libbacktrace/xcoff.c                          |  1607 ++
 libbacktrace/xztest.c                         |   508 +
 libbacktrace/ztest.c                          |   541 +
 src-release.sh                                |     2 +-
 66 files changed, 42755 insertions(+), 83 deletions(-)
 create mode 100644 gdb/bt-utils.c
 create mode 100644 gdb/bt-utils.h
 create mode 100644 gdb/testsuite/gdb.base/bt-on-error-and-warning.exp
 create mode 100644 libbacktrace/ChangeLog
 create mode 100644 libbacktrace/ChangeLog.jit
 create mode 100644 libbacktrace/Makefile.am
 create mode 100644 libbacktrace/Makefile.in
 create mode 100644 libbacktrace/README
 create mode 100644 libbacktrace/aclocal.m4
 create mode 100644 libbacktrace/alloc.c
 create mode 100644 libbacktrace/allocfail.c
 create mode 100755 libbacktrace/allocfail.sh
 create mode 100644 libbacktrace/atomic.c
 create mode 100644 libbacktrace/backtrace-supported.h.in
 create mode 100644 libbacktrace/backtrace.c
 create mode 100644 libbacktrace/backtrace.h
 create mode 100644 libbacktrace/btest.c
 create mode 100644 libbacktrace/config.h.in
 create mode 100755 libbacktrace/configure
 create mode 100644 libbacktrace/configure.ac
 create mode 100644 libbacktrace/dwarf.c
 create mode 100644 libbacktrace/edtest.c
 create mode 100644 libbacktrace/edtest2.c
 create mode 100644 libbacktrace/elf.c
 create mode 100644 libbacktrace/fileline.c
 create mode 100644 libbacktrace/filetype.awk
 create mode 100644 libbacktrace/install-debuginfo-for-buildid.sh.in
 create mode 100644 libbacktrace/instrumented_alloc.c
 create mode 100644 libbacktrace/internal.h
 create mode 100644 libbacktrace/macho.c
 create mode 100644 libbacktrace/mmap.c
 create mode 100644 libbacktrace/mmapio.c
 create mode 100644 libbacktrace/mtest.c
 create mode 100644 libbacktrace/nounwind.c
 create mode 100644 libbacktrace/pecoff.c
 create mode 100644 libbacktrace/posix.c
 create mode 100644 libbacktrace/print.c
 create mode 100644 libbacktrace/read.c
 create mode 100644 libbacktrace/simple.c
 create mode 100644 libbacktrace/sort.c
 create mode 100644 libbacktrace/state.c
 create mode 100644 libbacktrace/stest.c
 create mode 100644 libbacktrace/test_format.c
 create mode 100644 libbacktrace/testlib.c
 create mode 100644 libbacktrace/testlib.h
 create mode 100644 libbacktrace/ttest.c
 create mode 100644 libbacktrace/unittest.c
 create mode 100644 libbacktrace/unknown.c
 create mode 100644 libbacktrace/xcoff.c
 create mode 100644 libbacktrace/xztest.c
 create mode 100644 libbacktrace/ztest.c

-- 
2.25.4


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

* [PUSHED 1/6] top-level configure: setup target_configdirs based on repository
  2021-09-28 11:26 ` [PUSHED " Andrew Burgess
@ 2021-09-28 11:26   ` Andrew Burgess
  2021-09-28 11:26   ` [PUSHED 2/6] gdb: Add a dependency between gdb and libbacktrace Andrew Burgess
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 34+ messages in thread
From: Andrew Burgess @ 2021-09-28 11:26 UTC (permalink / raw)
  To: gdb-patches

The top-level configure script is shared between the gcc repository
and the binutils-gdb repository.

The target_configdirs variable in the configure.ac script, defines
sub-directories that contain components that should be built for the
target using the target tools.

Some components, e.g. zlib, are built as both host and target
libraries.

This causes problems for binutils-gdb.  If we run 'make all' in the
binutils-gdb repository we end up trying to build a target version of
the zlib library, which requires the target compiler be available.
Often the target compiler isn't immediately available, and so the
build fails.

The problem with zlib impacted a previous attempt to synchronise the
top-level configure scripts from gcc to binutils-gdb, see this thread:

  https://sourceware.org/pipermail/binutils/2019-May/107094.html

And I'm in the process of importing libbacktrace in to binutils-gdb,
which is also a host and target library, and triggers the same issues.

I believe that for binutils-gdb, at least at the moment, there are no
target libraries that we need to build.

In the configure script we build three lists of things we want to
build, $configdirs, $build_configdirs, and $target_configdirs, we also
build two lists of things we don't want to build, $skipdirs and
$noconfigdirs.  We then remove anything that is in the lists of things
not to build, from the list of things that should be built.

My proposal is to add everything in target_configdirs into skipdirs,
if the source tree doesn't contain a gcc/ sub-directory.  The result
is that for binutils-gdb no target tools or libraries will be built,
while for the gcc repository, nothing should change.

If a user builds a unified source tree, then the target tools and
libraries should still be built as the gcc/ directory will be present.

I've tested a build of gcc on x86-64, and the same set of target
libraries still seem to get built.  On binutils-gdb this change
resolves the issues with 'make all'.

ChangeLog:

	* configure: Regenerate.
	* configure.ac (skipdirs): Add the contents of target_configdirs if
	we are not building gcc.
---
 configure    | 10 ++++++++++
 configure.ac | 10 ++++++++++
 2 files changed, 20 insertions(+)

diff --git a/configure b/configure
index fcec657de7a..787fcf89100 100755
--- a/configure
+++ b/configure
@@ -6848,6 +6848,16 @@ case ,${enable_languages}, in
     ;;
 esac
 
+# If gcc/ is not in the source tree then we'll not be building a
+# target compiler, assume in that case we don't want to build any
+# target libraries or tools.
+#
+# This was added primarily for the benefit for binutils-gdb who reuse
+# this configure script, but don't always have target tools available.
+if test ! -d ${srcdir}/gcc; then
+   skipdirs="${skipdirs} ${target_configdirs}"
+fi
+
 # Remove the entries in $skipdirs and $noconfigdirs from $configdirs,
 # $build_configdirs and $target_configdirs.
 # If we have the source for $noconfigdirs entries, add them to $notsupp.
diff --git a/configure.ac b/configure.ac
index 1b1aa80f7c7..2b10e9a1b02 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2233,6 +2233,16 @@ case ,${enable_languages}, in
     ;;
 esac
 
+# If gcc/ is not in the source tree then we'll not be building a
+# target compiler, assume in that case we don't want to build any
+# target libraries or tools.
+#
+# This was added primarily for the benefit for binutils-gdb who reuse
+# this configure script, but don't always have target tools available.
+if test ! -d ${srcdir}/gcc; then
+   skipdirs="${skipdirs} ${target_configdirs}"
+fi
+
 # Remove the entries in $skipdirs and $noconfigdirs from $configdirs,
 # $build_configdirs and $target_configdirs.
 # If we have the source for $noconfigdirs entries, add them to $notsupp.
-- 
2.25.4


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

* [PUSHED 2/6] gdb: Add a dependency between gdb and libbacktrace
  2021-09-28 11:26 ` [PUSHED " Andrew Burgess
  2021-09-28 11:26   ` [PUSHED 1/6] top-level configure: setup target_configdirs based on repository Andrew Burgess
@ 2021-09-28 11:26   ` Andrew Burgess
  2021-09-28 11:26   ` [PUSHED 4/6] src-release.sh: add libbacktrace to GDB_SUPPORT_DIRS Andrew Burgess
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 34+ messages in thread
From: Andrew Burgess @ 2021-09-28 11:26 UTC (permalink / raw)
  To: gdb-patches

GDB is going to start using libbacktrace, so add a build dependency.

ChangeLog:

	* Makefile.def: Add all-gdb dependency on all-libbacktrace.
	* Makefile.in: Regenerate.
---
 Makefile.def | 1 +
 Makefile.in  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Makefile.def b/Makefile.def
index 5a460f1dbbc..d5b1ee882b6 100644
--- a/Makefile.def
+++ b/Makefile.def
@@ -408,6 +408,7 @@ dependencies = { module=all-gdb; on=all-sim; };
 dependencies = { module=all-gdb; on=all-libdecnumber; };
 dependencies = { module=all-gdb; on=all-libtermcap; };
 dependencies = { module=all-gdb; on=all-libctf; };
+dependencies = { module=all-gdb; on=all-libbacktrace; };
 
 // Host modules specific to gdbserver.
 dependencies = { module=configure-gdbserver; on=all-gnulib; };
diff --git a/Makefile.in b/Makefile.in
index 9b3a5d75735..91b714f6e88 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -52534,6 +52534,7 @@ all-gdb: maybe-all-libiconv
 all-gdb: maybe-all-opcodes
 all-gdb: maybe-all-libdecnumber
 all-gdb: maybe-all-libctf
+all-gdb: maybe-all-libbacktrace
 all-gdbserver: maybe-all-libiberty
 configure-gdbsupport: maybe-configure-intl
 all-gdbsupport: maybe-all-intl
-- 
2.25.4


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

* [PUSHED 4/6] src-release.sh: add libbacktrace to GDB_SUPPORT_DIRS
  2021-09-28 11:26 ` [PUSHED " Andrew Burgess
  2021-09-28 11:26   ` [PUSHED 1/6] top-level configure: setup target_configdirs based on repository Andrew Burgess
  2021-09-28 11:26   ` [PUSHED 2/6] gdb: Add a dependency between gdb and libbacktrace Andrew Burgess
@ 2021-09-28 11:26   ` Andrew Burgess
  2021-09-28 11:26   ` [PUSHED 5/6] gdb: use libbacktrace to create a better backtrace for fatal signals Andrew Burgess
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 34+ messages in thread
From: Andrew Burgess @ 2021-09-28 11:26 UTC (permalink / raw)
  To: gdb-patches

After the previous commit that imported libbacktrace from gcc, this
commit updates src-release.sh so that the libbacktrace directory is
included in the gdb release tar file.

ChangeLog:

	* src-release.sh (GDB_SUPPPORT_DIRS): Add libbacktrace.
---
 ChangeLog      | 4 ++++
 src-release.sh | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index afa6629abd1..247deffb4c2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2021-09-28  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* src-release.sh (GDB_SUPPPORT_DIRS): Add libbacktrace.
+
 2021-09-27  Nick Alcock  <nick.alcock@oracle.com>
 
 	PR libctf/27967
diff --git a/src-release.sh b/src-release.sh
index d24a63c99b0..86f6802f96a 100755
--- a/src-release.sh
+++ b/src-release.sh
@@ -313,7 +313,7 @@ gas_release()
     tar_compress $package $tool "$GAS_SUPPORT_DIRS" "$compressors"
 }
 
-GDB_SUPPORT_DIRS="bfd include libiberty libctf opcodes readline sim intl libdecnumber cpu zlib contrib gnulib gdbsupport gdbserver"
+GDB_SUPPORT_DIRS="bfd include libiberty libctf opcodes readline sim intl libdecnumber cpu zlib contrib gnulib gdbsupport gdbserver libbacktrace"
 gdb_release()
 {
     compressors=$1
-- 
2.25.4


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

* [PUSHED 5/6] gdb: use libbacktrace to create a better backtrace for fatal signals
  2021-09-28 11:26 ` [PUSHED " Andrew Burgess
                     ` (2 preceding siblings ...)
  2021-09-28 11:26   ` [PUSHED 4/6] src-release.sh: add libbacktrace to GDB_SUPPORT_DIRS Andrew Burgess
@ 2021-09-28 11:26   ` Andrew Burgess
  2021-09-28 18:55     ` Pedro Alves
  2021-09-29  3:09     ` Simon Marchi
  2021-09-28 11:26   ` [PUSHED 6/6] gdb: print backtrace for internal error/warning Andrew Burgess
  2021-09-28 12:20   ` [PUSHED 3/6] Copy in libbacktrace from gcc Andrew Burgess
  5 siblings, 2 replies; 34+ messages in thread
From: Andrew Burgess @ 2021-09-28 11:26 UTC (permalink / raw)
  To: gdb-patches

GDB recently gained the ability to print a backtrace when a fatal
signal is encountered.  This backtrace is produced using the backtrace
and backtrace_symbols_fd API available in glibc.

However, in order for this API to actually map addresses to symbol
names it is required that the application (GDB) be compiled with
-rdynamic, which GDB is not by default.

As a result, the backtrace produced often looks like this:

  Fatal signal: Bus error
  ----- Backtrace -----
  ./gdb/gdb[0x80ec00]
  ./gdb/gdb[0x80ed56]
  /lib64/libc.so.6(+0x3c6b0)[0x7fc2ce1936b0]
  /lib64/libc.so.6(__poll+0x4f)[0x7fc2ce24da5f]
  ./gdb/gdb[0x15495ba]
  ./gdb/gdb[0x15489b8]
  ./gdb/gdb[0x9b794d]
  ./gdb/gdb[0x9b7a6d]
  ./gdb/gdb[0x9b943b]
  ./gdb/gdb[0x9b94a1]
  ./gdb/gdb[0x4175dd]
  /lib64/libc.so.6(__libc_start_main+0xf3)[0x7fc2ce17e1a3]
  ./gdb/gdb[0x4174de]
  ---------------------

This is OK if you have access to the exact same build of GDB, you can
manually map the addresses back to symbols, however, it is next to
useless if all you have is a backtrace copied into a bug report.

GCC uses libbacktrace for printing a backtrace when it encounters an
error.  In recent commits I added this library into the binutils-gdb
repository, and in this commit I allow this library to be used by
GDB.  Now (when GDB is compiled with debug information) the backtrace
looks like this:

  ----- Backtrace -----
  0x80ee08 gdb_internal_backtrace
  	../../src/gdb/event-top.c:989
  0x80ef0b handle_fatal_signal
  	../../src/gdb/event-top.c:1036
  0x7f24539dd6af ???
  0x7f2453a97a5f ???
  0x154976f gdb_wait_for_event
  	../../src/gdbsupport/event-loop.cc:613
  0x1548b6d _Z16gdb_do_one_eventv
  	../../src/gdbsupport/event-loop.cc:237
  0x9b7b02 start_event_loop
  	../../src/gdb/main.c:421
  0x9b7c22 captured_command_loop
  	../../src/gdb/main.c:481
  0x9b95f0 captured_main
  	../../src/gdb/main.c:1353
  0x9b9656 _Z8gdb_mainP18captured_main_args
  	../../src/gdb/main.c:1368
  0x4175ec main
  	../../src/gdb/gdb.c:32
  ---------------------

Which seems much more useful.

Use of libbacktrace is optional.  If GDB is configured with
--disable-libbacktrace then the libbacktrace directory will not be
built, and GDB will not try to use this library.  In this case GDB
would try to use the old backtrace and backtrace_symbols_fd API.

All of the functions related to writing the backtrace of GDB itself
have been moved into the new files gdb/by-utils.{c,h}.
---
 gdb/Makefile.in  |  16 +++--
 gdb/bt-utils.c   | 170 +++++++++++++++++++++++++++++++++++++++++++++++
 gdb/bt-utils.h   |  69 +++++++++++++++++++
 gdb/config.in    |   3 +
 gdb/configure    |  32 +++++++++
 gdb/configure.ac |  23 +++++++
 gdb/event-top.c  |  47 ++-----------
 7 files changed, 315 insertions(+), 45 deletions(-)
 create mode 100644 gdb/bt-utils.c
 create mode 100644 gdb/bt-utils.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 320d3326a81..5a3bb952279 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -248,6 +248,10 @@ GDBFLAGS =
 GNULIB_PARENT_DIR = ..
 include $(GNULIB_PARENT_DIR)/gnulib/Makefile.gnulib.inc
 
+# For libbacktrace.
+LIBBACKTRACE_INC=@LIBBACKTRACE_INC@
+LIBBACKTRACE_LIB=@LIBBACKTRACE_LIB@
+
 SUPPORT = ../gdbsupport
 LIBSUPPORT = $(SUPPORT)/libgdbsupport.a
 INCSUPPORT = -I$(srcdir)/.. -I..
@@ -614,9 +618,9 @@ INTERNAL_CFLAGS_BASE = \
 	$(GLOBAL_CFLAGS) $(PROFILE_CFLAGS) \
 	$(GDB_CFLAGS) $(OPCODES_CFLAGS) $(READLINE_CFLAGS) $(ZLIBINC) \
 	$(BFD_CFLAGS) $(INCLUDE_CFLAGS) $(LIBDECNUMBER_CFLAGS) \
-	$(INTL_CFLAGS) $(INCGNU) $(INCSUPPORT) $(ENABLE_CFLAGS) \
-	$(INTERNAL_CPPFLAGS) $(SRCHIGH_CFLAGS) $(TOP_CFLAGS) $(PTHREAD_CFLAGS) \
-	$(DEBUGINFOD_CFLAGS)
+	$(INTL_CFLAGS) $(INCGNU) $(INCSUPPORT) $(LIBBACKTRACE_INC) \
+	$(ENABLE_CFLAGS) $(INTERNAL_CPPFLAGS) $(SRCHIGH_CFLAGS) \
+	$(TOP_CFLAGS) $(PTHREAD_CFLAGS) $(DEBUGINFOD_CFLAGS)
 INTERNAL_WARN_CFLAGS = $(INTERNAL_CFLAGS_BASE) $(GDB_WARN_CFLAGS)
 INTERNAL_CFLAGS = $(INTERNAL_WARN_CFLAGS) $(GDB_WERROR_CFLAGS)
 
@@ -637,12 +641,12 @@ INTERNAL_LDFLAGS = \
 # LIBIBERTY appears twice on purpose.
 CLIBS = $(SIM) $(READLINE) $(OPCODES) $(LIBCTF) $(BFD) $(ZLIB) \
         $(LIBSUPPORT) $(INTL) $(LIBIBERTY) $(LIBDECNUMBER) \
-	$(XM_CLIBS) $(GDBTKLIBS) \
+	$(XM_CLIBS) $(GDBTKLIBS)  $(LIBBACKTRACE_LIB) \
 	@LIBS@ @GUILE_LIBS@ @PYTHON_LIBS@ \
 	$(LIBEXPAT) $(LIBLZMA) $(LIBBABELTRACE) $(LIBIPT) \
 	$(WIN32LIBS) $(LIBGNU) $(LIBGNU_EXTRA_LIBS) $(LIBICONV) \
 	$(LIBMPFR) $(LIBGMP) $(SRCHIGH_LIBS) $(LIBXXHASH) $(PTHREAD_LIBS) \
-	$(DEBUGINFOD_LIBS)
+	$(DEBUGINFOD_LIBS) $(LIBBABELTRACE_LIB)
 CDEPS = $(NAT_CDEPS) $(SIM) $(BFD) $(READLINE_DEPS) $(CTF_DEPS) \
 	$(OPCODES) $(INTL_DEPS) $(LIBIBERTY) $(CONFIG_DEPS) $(LIBGNU) \
 	$(LIBSUPPORT)
@@ -995,6 +999,7 @@ COMMON_SFILES = \
 	break-catch-syscall.c \
 	break-catch-throw.c \
 	breakpoint.c \
+	bt-utils.c \
 	btrace.c \
 	build-id.c \
 	buildsym-legacy.c \
@@ -1256,6 +1261,7 @@ HFILES_NO_SRCDIR = \
 	breakpoint.h \
 	bsd-kvm.h \
 	bsd-uthread.h \
+	bt-utils.h \
 	build-id.h \
 	buildsym-legacy.h \
 	buildsym.h \
diff --git a/gdb/bt-utils.c b/gdb/bt-utils.c
new file mode 100644
index 00000000000..b5e0a0ed004
--- /dev/null
+++ b/gdb/bt-utils.c
@@ -0,0 +1,170 @@
+/* Copyright (C) 2021 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "bt-utils.h"
+#include "command.h"
+#include "gdbcmd.h"
+#include "top.h"
+#include "cli/cli-decode.h"
+
+/* See bt-utils.h.  */
+
+void
+gdb_internal_backtrace_set_cmd (const char *args, int from_tty,
+				cmd_list_element *c)
+{
+  gdb_assert (c->type == set_cmd);
+  gdb_assert (c->var_type == var_boolean);
+  gdb_assert (c->var != nullptr);
+
+#ifndef GDB_PRINT_INTERNAL_BACKTRACE
+  bool *var_ptr = (bool *) c->var;
+
+  if (*var_ptr)
+    {
+      *var_ptr = false;
+      error (_("support for this feature is not compiled into GDB"));
+    }
+#endif
+}
+
+#ifdef GDB_PRINT_INTERNAL_BACKTRACE_USING_LIBBACKTRACE
+
+/* Callback used by libbacktrace if it encounters an error.  */
+
+static void
+libbacktrace_error (void *data, const char *errmsg, int errnum)
+{
+  /* A negative errnum indicates no debug info was available, just
+     skip printing a backtrace in this case.  */
+  if (errnum < 0)
+    return;
+
+  const auto sig_write = [] (const char *msg) -> void
+  {
+    gdb_stderr->write_async_safe (msg, strlen (msg));
+  };
+
+  sig_write ("error creating backtrace: ");
+  sig_write (errmsg);
+  if (errnum > 0)
+    {
+      char buf[20];
+      snprintf (buf, sizeof (buf), ": %d", errnum);
+      buf[sizeof (buf) - 1] = '\0';
+
+      sig_write (buf);
+    }
+  sig_write ("\n");
+}
+
+/* Callback used by libbacktrace to print a single stack frame.  */
+
+static int
+libbacktrace_print (void *data, uintptr_t pc, const char *filename,
+		    int lineno, const char *function)
+{
+  const auto sig_write = [] (const char *msg) -> void
+  {
+    gdb_stderr->write_async_safe (msg, strlen (msg));
+  };
+
+  /* Buffer to print addresses and line numbers into.  An 8-byte address
+     with '0x' prefix and a null terminator requires 20 characters.  This
+     also feels like it should be enough to represent line numbers in most
+     files.  We are also careful to ensure we don't overflow this buffer.  */
+  char buf[20];
+
+  snprintf (buf, sizeof (buf), "0x%lx ", pc);
+  buf[sizeof (buf) - 1] = '\0';
+  sig_write (buf);
+  sig_write (function == nullptr ? "???" : function);
+  if (filename != nullptr)
+    {
+      sig_write ("\n\t");
+      sig_write (filename);
+      sig_write (":");
+      snprintf (buf, sizeof (buf), "%d", lineno);
+      buf[sizeof (buf) - 1] = '\0';
+      sig_write (buf);
+    }
+  sig_write ("\n");
+
+  return function != nullptr && strcmp (function, "main") == 0;
+}
+
+/* Write a backtrace to GDB's stderr in an async safe manor.  This is a
+   backtrace of GDB, not any running inferior, and is to be used when GDB
+   crashes or hits some other error condition.  */
+
+static void
+gdb_internal_backtrace_1 ()
+{
+  static struct backtrace_state *state = nullptr;
+
+  if (state == nullptr)
+    state = backtrace_create_state (nullptr, 0, libbacktrace_error, nullptr);
+
+  backtrace_full (state, 0, libbacktrace_print, libbacktrace_error, nullptr);
+}
+
+#elif defined GDB_PRINT_INTERNAL_BACKTRACE_USING_EXECINFO
+
+/* See the comment on previous version of this function.  */
+
+static void
+gdb_internal_backtrace_1 ()
+{
+  const auto sig_write = [] (const char *msg) -> void
+  {
+    gdb_stderr->write_async_safe (msg, strlen (msg));
+  };
+
+  /* Allow up to 25 frames of backtrace.  */
+  void *buffer[25];
+  int frames = backtrace (buffer, ARRAY_SIZE (buffer));
+
+  backtrace_symbols_fd (buffer, frames, gdb_stderr->fd ());
+  if (frames == ARRAY_SIZE (buffer))
+    sig_write (_("Backtrace might be incomplete.\n"));
+}
+
+#endif
+
+/* See bt-utils.h.  */
+
+void
+gdb_internal_backtrace ()
+{
+  if (current_ui == nullptr)
+    return;
+
+  const auto sig_write = [] (const char *msg) -> void
+  {
+    gdb_stderr->write_async_safe (msg, strlen (msg));
+  };
+
+  sig_write (_("----- Backtrace -----\n"));
+
+  if (gdb_stderr->fd () > -1)
+    gdb_internal_backtrace_1 ();
+  else
+    sig_write (_("Backtrace unavailable\n"));
+
+  sig_write ("---------------------\n");
+}
diff --git a/gdb/bt-utils.h b/gdb/bt-utils.h
new file mode 100644
index 00000000000..433aa23614b
--- /dev/null
+++ b/gdb/bt-utils.h
@@ -0,0 +1,69 @@
+/* Copyright (C) 2021 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Support for printing a backtrace when GDB hits an error.  This is not
+   for printing backtraces of the inferior, but backtraces of GDB itself.  */
+
+#ifdef HAVE_LIBBACKTRACE
+# include "backtrace.h"
+# include "backtrace-supported.h"
+# if BACKTRACE_SUPPORTED && (! BACKTRACE_USES_MALLOC)
+#  define GDB_PRINT_INTERNAL_BACKTRACE
+#  define GDB_PRINT_INTERNAL_BACKTRACE_USING_LIBBACKTRACE
+# endif
+#endif
+
+#if defined HAVE_EXECINFO_H			\
+  && defined HAVE_EXECINFO_BACKTRACE		\
+  && !defined PRINT_BACKTRACE_ON_FATAL_SIGNAL
+# include <execinfo.h>
+# define GDB_PRINT_INTERNAL_BACKTRACE
+# define GDB_PRINT_INTERNAL_BACKTRACE_USING_EXECINFO
+#endif
+
+/* Define GDB_PRINT_INTERNAL_BACKTRACE_INIT_ON.  This is a boolean value
+   that can be used as an initial value for a set/show user setting, where
+   the setting controls printing a GDB internal backtrace.
+
+   If backtrace printing is supported then this will have the value true,
+   but if backtrace printing is not supported then this has the value
+   false.  */
+#ifdef GDB_PRINT_INTERNAL_BACKTRACE
+# define GDB_PRINT_INTERNAL_BACKTRACE_INIT_ON true
+#else
+# define GDB_PRINT_INTERNAL_BACKTRACE_INIT_ON false
+#endif
+
+/* Print a backtrace of the current GDB process to the current
+   gdb_stderr.  The output is done in a signal async manor, so it is safe
+   to call from signal handlers.  */
+
+extern void gdb_internal_backtrace ();
+
+/* A generic function that can be used as the set function for any set
+   command that enables printing of an internal backtrace.  Command C must
+   be a boolean set command.
+
+   If GDB doesn't support printing a backtrace, and the user has tried to
+   set the variable associated with command C to true, then the associated
+   variable will be set back to false, and an error thrown.
+
+   If GDB does support printing a backtrace then this function does
+   nothing.  */
+
+extern void gdb_internal_backtrace_set_cmd (const char *args, int from_tty,
+					    cmd_list_element *c);
diff --git a/gdb/config.in b/gdb/config.in
index af3680c6d95..c61f7a91352 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -228,6 +228,9 @@
 /* Define if you have the babeltrace library. */
 #undef HAVE_LIBBABELTRACE
 
+/* Define if libbacktrace is being used. */
+#undef HAVE_LIBBACKTRACE
+
 /* Define to 1 if debuginfod is enabled. */
 #undef HAVE_LIBDEBUGINFOD
 
diff --git a/gdb/configure b/gdb/configure
index f0b1af4a6ea..7c8335f2576 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -636,6 +636,8 @@ LIBCTF
 LTLIBBABELTRACE
 LIBBABELTRACE
 HAVE_LIBBABELTRACE
+LIBBACKTRACE_LIB
+LIBBACKTRACE_INC
 HAVE_NATIVE_GCORE_HOST
 NAT_GENERATED_FILES
 XM_CLIBS
@@ -925,6 +927,7 @@ with_tcl
 with_tk
 with_x
 enable_sim
+enable_libbacktrace
 with_babeltrace
 with_libbabeltrace_prefix
 with_libbabeltrace_type
@@ -1601,6 +1604,8 @@ Optional Features:
                           gcc is used
   --enable-ubsan          enable undefined behavior sanitizer (auto/yes/no)
   --enable-sim            link gdb with simulator
+  --enable-libbacktrace   use libbacktrace to write a backtrace after a fatal
+                          signal.
   --enable-libctf         Handle .ctf type-info sections [default=yes]
   --enable-unit-tests     Enable the inclusion of unit tests when compiling
                           GDB
@@ -18659,6 +18664,33 @@ _ACEOF
 
 fi
 
+# Setup possible use of libbacktrace.
+# Check whether --enable-libbacktrace was given.
+if test "${enable_libbacktrace+set}" = set; then :
+  enableval=$enable_libbacktrace; case "${enableval}" in
+  yes)  enable_libbacktrace=yes ;;
+  no)   enable_libbacktrace=no  ;;
+  *)    as_fn_error $? "bad value ${enableval} for --enable-libbacktrace option" "$LINENO" 5 ;;
+esac
+else
+  enable_libbacktrace=yes
+fi
+
+
+if test "${enable_libbacktrace}" == "yes"; then
+  LIBBACKTRACE_INC="-I$srcdir/../libbacktrace/ -I../libbacktrace/"
+  LIBBACKTRACE_LIB=../libbacktrace/.libs/libbacktrace.a
+
+$as_echo "#define HAVE_LIBBACKTRACE 1" >>confdefs.h
+
+else
+  LIBBACKTRACE_INC=
+  LIBBACKTRACE_LIB=
+fi
+
+
+
+
 # Check for babeltrace and babeltrace-ctf
 
 # Check whether --with-babeltrace was given.
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 93f11316a14..0d91be59cd6 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -2131,6 +2131,29 @@ if test x"${gdb_osabi}" != x ; then
 		       [Define to the default OS ABI for this configuration.])
 fi
 
+# Setup possible use of libbacktrace.
+AC_ARG_ENABLE([libbacktrace],
+[AS_HELP_STRING([--enable-libbacktrace],
+                [use libbacktrace to write a backtrace after a fatal signal.])],
+[case "${enableval}" in
+  yes)  enable_libbacktrace=yes ;;
+  no)   enable_libbacktrace=no  ;;
+  *)    AC_MSG_ERROR(bad value ${enableval} for --enable-libbacktrace option) ;;
+esac],
+enable_libbacktrace=yes)
+
+if test "${enable_libbacktrace}" == "yes"; then
+  LIBBACKTRACE_INC="-I$srcdir/../libbacktrace/ -I../libbacktrace/"
+  LIBBACKTRACE_LIB=../libbacktrace/.libs/libbacktrace.a
+  AC_DEFINE(HAVE_LIBBACKTRACE, 1, [Define if libbacktrace is being used.])
+else
+  LIBBACKTRACE_INC=
+  LIBBACKTRACE_LIB=
+fi
+
+AC_SUBST(LIBBACKTRACE_INC)
+AC_SUBST(LIBBACKTRACE_LIB)
+
 # Check for babeltrace and babeltrace-ctf
 AC_ARG_WITH(babeltrace,
   AS_HELP_STRING([--with-babeltrace], [include babeltrace support (auto/yes/no)]),
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 9233a3650ac..64c624ce4d7 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -41,15 +41,12 @@
 #include "gdbsupport/gdb_select.h"
 #include "gdbsupport/gdb-sigmask.h"
 #include "async-event.h"
+#include "bt-utils.h"
 
 /* readline include files.  */
 #include "readline/readline.h"
 #include "readline/history.h"
 
-#ifdef HAVE_EXECINFO_H
-# include <execinfo.h>
-#endif /* HAVE_EXECINFO_H */
-
 /* readline defines this.  */
 #undef savestring
 
@@ -102,12 +99,7 @@ int call_stdin_event_handler_again_p;
 
 /* When true GDB will produce a minimal backtrace when a fatal signal is
    reached (within GDB code).  */
-static bool bt_on_fatal_signal
-#ifdef HAVE_EXECINFO_BACKTRACE
-  = true;
-#else
-  = false;
-#endif /* HAVE_EXECINFO_BACKTRACE */
+static bool bt_on_fatal_signal = GDB_PRINT_INTERNAL_BACKTRACE_INIT_ON;
 
 /* Implement 'maintenance show backtrace-on-fatal-signal'.  */
 
@@ -118,20 +110,6 @@ show_bt_on_fatal_signal (struct ui_file *file, int from_tty,
   fprintf_filtered (file, _("Backtrace on a fatal signal is %s.\n"), value);
 }
 
-/* Implement 'maintenance set backtrace-on-fatal-signal'.  */
-
-static void
-set_bt_on_fatal_signal (const char *args, int from_tty, cmd_list_element *c)
-{
-#ifndef HAVE_EXECINFO_BACKTRACE
-  if (bt_on_fatal_signal)
-    {
-      bt_on_fatal_signal = false;
-      error (_("support for this feature is not compiled into GDB"));
-    }
-#endif
-}
-
 /* Signal handling variables.  */
 /* Each of these is a pointer to a function that the event loop will
    invoke if the corresponding signal has received.  The real signal
@@ -904,7 +882,7 @@ unblock_signal (int sig)
 static void ATTRIBUTE_NORETURN
 handle_fatal_signal (int sig)
 {
-#ifdef HAVE_EXECINFO_BACKTRACE
+#ifdef GDB_PRINT_INTERNAL_BACKTRACE
   const auto sig_write = [] (const char *msg) -> void
   {
     gdb_stderr->write_async_safe (msg, strlen (msg));
@@ -917,19 +895,8 @@ handle_fatal_signal (int sig)
       sig_write (strsignal (sig));
       sig_write ("\n");
 
-      /* Allow up to 25 frames of backtrace.  */
-      void *buffer[25];
-      int frames = backtrace (buffer, ARRAY_SIZE (buffer));
-      sig_write (_("----- Backtrace -----\n"));
-      if (gdb_stderr->fd () > -1)
-	{
-	  backtrace_symbols_fd (buffer, frames, gdb_stderr->fd ());
-	  if (frames == ARRAY_SIZE (buffer))
-	    sig_write (_("Backtrace might be incomplete.\n"));
-	}
-      else
-	sig_write (_("Backtrace unavailable\n"));
-      sig_write ("---------------------\n");
+      gdb_internal_backtrace ();
+
       sig_write (_("A fatal error internal to GDB has been detected, "
 		   "further\ndebugging is not possible.  GDB will now "
 		   "terminate.\n\n"));
@@ -944,7 +911,7 @@ handle_fatal_signal (int sig)
 
       gdb_stderr->flush ();
     }
-#endif /* HAVE_EXECINF_BACKTRACE */
+#endif
 
   /* If possible arrange for SIG to have its default behaviour (which
      should be to terminate the current process), unblock SIG, and reraise
@@ -1449,7 +1416,7 @@ Use \"on\" to enable, \"off\" to disable.\n\
 If enabled, GDB will produce a minimal backtrace if it encounters a fatal\n\
 signal from within GDB itself.  This is a mechanism to help diagnose\n\
 crashes within GDB, not a mechanism for debugging inferiors."),
-			   set_bt_on_fatal_signal,
+			   gdb_internal_backtrace_set_cmd,
 			   show_bt_on_fatal_signal,
 			   &maintenance_set_cmdlist,
 			   &maintenance_show_cmdlist);
-- 
2.25.4


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

* [PUSHED 6/6] gdb: print backtrace for internal error/warning
  2021-09-28 11:26 ` [PUSHED " Andrew Burgess
                     ` (3 preceding siblings ...)
  2021-09-28 11:26   ` [PUSHED 5/6] gdb: use libbacktrace to create a better backtrace for fatal signals Andrew Burgess
@ 2021-09-28 11:26   ` Andrew Burgess
  2021-09-28 12:26     ` Eli Zaretskii
  2021-09-28 12:20   ` [PUSHED 3/6] Copy in libbacktrace from gcc Andrew Burgess
  5 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2021-09-28 11:26 UTC (permalink / raw)
  To: gdb-patches

This commit builds on previous work to allow GDB to print a backtrace
of itself when GDB encounters an internal-error or internal-warning.
This fixes PR gdb/26377.

There's not many places where we call internal_warning, and I guess in
most cases the user would probably continue their debug session.  And
so, in order to avoid cluttering up the output, by default, printing
of a backtrace is off for internal-warnings.

In contrast, printing of a backtrace is on by default for
internal-errors, as I figure that in most cases hitting an
internal-error is going to be the end of the debug session.

Whether a backtrace is printed or not can be controlled with the new
settings:

  maintenance set internal-error backtrace on|off
  maintenance show internal-error backtrace

  maintenance set internal-warning backtrace on|off
  maintenance show internal-warning backtrace

Here is an example of what an internal-error now looks like with the
backtrace included:

  (gdb) maintenance internal-error blah
  ../../src.dev-3/gdb/maint.c:82: internal-error: blah
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  ----- Backtrace -----
  0x5c61ca gdb_internal_backtrace_1
  	../../src.dev-3/gdb/bt-utils.c:123
  0x5c626d _Z22gdb_internal_backtracev
  	../../src.dev-3/gdb/bt-utils.c:165
  0xe33237 internal_vproblem
  	../../src.dev-3/gdb/utils.c:393
  0xe33539 _Z15internal_verrorPKciS0_P13__va_list_tag
  	../../src.dev-3/gdb/utils.c:470
  0x1549652 _Z14internal_errorPKciS0_z
  	../../src.dev-3/gdbsupport/errors.cc:55
  0x9c7982 maintenance_internal_error
  	../../src.dev-3/gdb/maint.c:82
  0x636f57 do_simple_func
  	../../src.dev-3/gdb/cli/cli-decode.c:97
   .... snip, lots more backtrace lines ....
  ---------------------
  ../../src.dev-3/gdb/maint.c:82: internal-error: blah
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Quit this debugging session? (y or n) y

  This is a bug, please report it.  For instructions, see:
  <https://www.gnu.org/software/gdb/bugs/>.

  ../../src.dev-3/gdb/maint.c:82: internal-error: blah
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Create a core file of GDB? (y or n) n

My hope is that this backtrace might make it slightly easier to
diagnose GDB issues if all that is provided is the console output, I
find that we frequently get reports of an assert being hit that is
located in pretty generic code (frame.c, value.c, etc) and it is not
always obvious how we might have arrived at the assert.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26377
---
 gdb/NEWS                                      |   8 ++
 gdb/doc/gdb.texinfo                           |  13 ++
 .../gdb.base/bt-on-error-and-warning.exp      | 118 ++++++++++++++++++
 gdb/testsuite/gdb.base/bt-on-fatal-signal.exp |  36 ------
 gdb/utils.c                                   |  36 +++++-
 5 files changed, 174 insertions(+), 37 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/bt-on-error-and-warning.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index d7c29c82edb..1e25cb8cc36 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -19,6 +19,14 @@ show source open
   to open and read source code files, which can be useful if the files
   are located over a slow network connection.
 
+maint set internal-error backtrace on|off
+maint show internal-error backtrace
+maint set internal-warning backtrace on|off
+maint show internal-warning backtrace
+  GDB can now print a backtrace of itself when it encounters either an
+  internal-error, or an internal-warning.  This is on by default for
+  internal-error and off by default for internal-warning.
+
 * Python API
 
   ** New function gdb.add_history(), which takes a gdb.Value object
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index fcfdc26ac1a..d4e4174be5d 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -39257,6 +39257,19 @@
 disabled.
 @end table
 
+@kindex maint set internal-error
+@kindex maint show internal-error
+@kindex maint set internal-warning
+@kindex maint show internal-warning
+@item maint set internal-error backtrace @r{[}on|off@r{]}
+@itemx maint show internal-error backtrace
+@itemx maint set internal-warning backtrace @r{[}on|off@r{]}
+@itemx maint show internal-warning backtrace
+When @value{GDBN} reports an internal problem (error or warning) it is
+possible to have a backtrace of @value{GDBN} printed to stderr.  This
+is @samp{on} by default for @code{internal-error} and @samp{off} by
+default for @code{internal-warning}.
+
 @kindex maint packet
 @item maint packet @var{text}
 If @value{GDBN} is talking to an inferior via the serial protocol,
diff --git a/gdb/testsuite/gdb.base/bt-on-error-and-warning.exp b/gdb/testsuite/gdb.base/bt-on-error-and-warning.exp
new file mode 100644
index 00000000000..d988cf742b4
--- /dev/null
+++ b/gdb/testsuite/gdb.base/bt-on-error-and-warning.exp
@@ -0,0 +1,118 @@
+# Copyright 2021 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test that GDB can print a backtrace when it encounters an internal
+# error or an internal warning, and that this functionality can be
+# switched off.
+
+standard_testfile bt-on-fatal-signal.c
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
+    return -1
+}
+
+# Check we can run to main.  If this works this time then we just
+# assume that it will work later on (when we repeatedly restart GDB).
+if ![runto_main] then {
+    untested "run to main"
+    return -1
+}
+
+# Check that the backtrace-on-fatal-signal feature is supported.  If
+# this target doesn't have the backtrace function available then
+# trying to turn this on will give an error, in which case we just
+# skip this test.
+gdb_test_multiple "maint set internal-error backtrace on" \
+    "check backtrace is supported" {
+    -re "support for this feature is not compiled into GDB" {
+	untested "feature not supported"
+	return -1
+    }
+    -re "$gdb_prompt $" {
+	pass $gdb_test_name
+    }
+}
+
+# MODE should be either 'on' or 'off', while PROBLEM_TYPE should be
+# 'internal-error' or 'internal-warning'.  This proc sets the
+# backtrace printing for PROBLEM_TYPE to MODE, then uses 'maint
+# PROBLEM_TYPE foobar' to raise a fake error or warning.
+#
+# We then check that a backtrace either is, or isn't printed, inline
+# with MODE.
+proc run_test {problem_type mode} {
+
+    with_test_prefix "problem=${problem_type}, mode=${mode}" {
+	gdb_test_no_output "maint set ${problem_type} backtrace ${mode}"
+
+	set header_lines 0
+	set bt_lines 0
+
+	gdb_test_multiple "maint ${problem_type} foobar" "scan for backtrace" {
+	    -early -re "^\r\n" {
+		exp_continue
+	    }
+	    -early -re "^maint ${problem_type} foobar\r\n" {
+		exp_continue
+	    }
+	    -early -re "^\[^\r\n\]+: ${problem_type}: foobar\r\n" {
+		incr header_lines
+		exp_continue
+	    }
+	    -early -re "^A problem internal to GDB has been detected,\r\n" {
+		incr header_lines
+		exp_continue
+	    }
+	    -early -re "^further debugging may prove unreliable\\.\r\n" {
+		incr header_lines
+		exp_continue
+	    }
+	    -early -re "^----- Backtrace -----\r\n" {
+		incr bt_lines
+		exp_continue
+	    }
+	    -early -re "^\[^-\].+\r\n---------------------\r\n" {
+		incr bt_lines
+		exp_continue
+	    }
+	    eof {
+		fail ${gdb_test_name}
+		return
+	    }
+	    -re "$::gdb_prompt $" {
+		pass ${gdb_test_name}
+	    }
+	}
+
+	gdb_assert { ${header_lines} == 3 }
+	if { $mode == "on" } {
+	    gdb_assert { ${bt_lines} == 2 }
+	} else {
+	    gdb_assert { ${bt_lines} == 0 }
+	}
+    }
+}
+
+# For each problem type (error or warning) raise a fake problem using
+# the maintenance commands and check that a backtrace is (or isn't)
+# printed, depending on the user setting.
+foreach problem_type { internal-error internal-warning } {
+    gdb_test_no_output "maint set ${problem_type} corefile no"
+    gdb_test_no_output "maint set ${problem_type} quit no"
+
+    foreach mode { on off } {
+	run_test ${problem_type} ${mode}
+    }
+}
diff --git a/gdb/testsuite/gdb.base/bt-on-fatal-signal.exp b/gdb/testsuite/gdb.base/bt-on-fatal-signal.exp
index 1f0d61f00ed..8875d00fdb1 100644
--- a/gdb/testsuite/gdb.base/bt-on-fatal-signal.exp
+++ b/gdb/testsuite/gdb.base/bt-on-fatal-signal.exp
@@ -135,39 +135,3 @@ foreach test_data {{SEGV "Segmentation fault"} \
 	gdb_exit
     }
 }
-
-# Check that when we get an internal error and choose to dump core, we
-# don't print a backtrace to the console.
-with_test_prefix "internal-error" {
-    # Restart GDB.
-    clean_restart $binfile
-
-    set saw_bt_start false
-
-    gdb_test_multiple "maint internal-error foo" "" {
-	-early -re "internal-error: foo\r\n" {
-	    exp_continue
-	}
-	-early -re "^A problem internal to GDB has been detected,\r\n" {
-	    exp_continue
-	}
-	-early -re "^further debugging may prove unreliable\\.\r\n" {
-	    exp_continue
-	}
-	-early -re "^Quit this debugging session\\? \\(y or n\\)" {
-	    send_gdb "y\n"
-	    exp_continue
-	}
-	-early -re "^Create a core file of GDB\\? \\(y or n\\)" {
-	    send_gdb "y\n"
-	    exp_continue
-	}
-	-early -re "----- Backtrace -----\r\n" {
-	    set saw_bt_start true
-	    exp_continue
-	}
-	eof {
-	    gdb_assert { [expr ! $saw_bt_start] }
-	}
-    }
-}
diff --git a/gdb/utils.c b/gdb/utils.c
index 0a7c270b40d..f6f90d7365b 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -75,6 +75,7 @@
 #include "gdbarch.h"
 #include "cli-out.h"
 #include "gdbsupport/gdb-safe-ctype.h"
+#include "bt-utils.h"
 
 void (*deprecated_error_begin_hook) (void);
 
@@ -304,6 +305,13 @@ struct internal_problem
 
   /* Like SHOULD_QUIT, but whether GDB should dump core.  */
   const char *should_dump_core;
+
+  /* Like USER_SETTABLE_SHOULD_QUIT but for SHOULD_PRINT_BACKTRACE.  */
+  bool user_settable_should_print_backtrace;
+
+  /* When this is true GDB will print a backtrace when a problem of this
+     type is encountered.  */
+  bool should_print_backtrace;
 };
 
 /* Report a problem, internal to GDB, to the user.  Once the problem
@@ -377,9 +385,13 @@ internal_vproblem (struct internal_problem *problem,
   /* Emit the message unless query will emit it below.  */
   if (problem->should_quit != internal_problem_ask
       || !confirm
-      || !filtered_printing_initialized ())
+      || !filtered_printing_initialized ()
+      || problem->should_print_backtrace)
     fprintf_unfiltered (gdb_stderr, "%s\n", reason.c_str ());
 
+  if (problem->should_print_backtrace)
+    gdb_internal_backtrace ();
+
   if (problem->should_quit == internal_problem_ask)
     {
       /* Default (yes/batch case) is to quit GDB.  When in batch mode
@@ -449,6 +461,7 @@ internal_vproblem (struct internal_problem *problem,
 
 static struct internal_problem internal_error_problem = {
   "internal-error", true, internal_problem_ask, true, internal_problem_ask,
+  true, GDB_PRINT_INTERNAL_BACKTRACE_INIT_ON
 };
 
 void
@@ -460,6 +473,7 @@ internal_verror (const char *file, int line, const char *fmt, va_list ap)
 
 static struct internal_problem internal_warning_problem = {
   "internal-warning", true, internal_problem_ask, true, internal_problem_ask,
+  true, false
 };
 
 void
@@ -470,6 +484,7 @@ internal_vwarning (const char *file, int line, const char *fmt, va_list ap)
 
 static struct internal_problem demangler_warning_problem = {
   "demangler-warning", true, internal_problem_ask, false, internal_problem_no,
+  false, false
 };
 
 void
@@ -571,6 +586,25 @@ add_internal_problem_command (struct internal_problem *problem)
 			    set_cmd_list,
 			    show_cmd_list);
     }
+
+  if (problem->user_settable_should_print_backtrace)
+    {
+      std::string set_bt_doc
+	= string_printf (_("Set whether GDB should print a backtrace of "
+			   "GDB when %s is detected."), problem->name);
+      std::string show_bt_doc
+	= string_printf (_("Show whether GDB will print a backtrace of "
+			   "GDB when %s is detected."), problem->name);
+      add_setshow_boolean_cmd ("backtrace", class_maintenance,
+			       &problem->should_print_backtrace,
+			       set_bt_doc.c_str (),
+			       show_bt_doc.c_str (),
+			       NULL, /* help_doc */
+			       gdb_internal_backtrace_set_cmd,
+			       NULL, /* showfunc */
+			       set_cmd_list,
+			       show_cmd_list);
+    }
 }
 
 /* Return a newly allocated string, containing the PREFIX followed
-- 
2.25.4


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

* [PUSHED 3/6] Copy in libbacktrace from gcc
  2021-09-28 11:26 ` [PUSHED " Andrew Burgess
                     ` (4 preceding siblings ...)
  2021-09-28 11:26   ` [PUSHED 6/6] gdb: print backtrace for internal error/warning Andrew Burgess
@ 2021-09-28 12:20   ` Andrew Burgess
  5 siblings, 0 replies; 34+ messages in thread
From: Andrew Burgess @ 2021-09-28 12:20 UTC (permalink / raw)
  To: gdb-patches

This copies in libbacktrace from the gcc repository as it was in the
commit 62e420293a293608f383d9b9c7f2debd666e9fc9.  GDB is going to
start using this library soon.

A dependency between GDB and libbacktrace has already been added to
the top level Makefile, so, after this commit, when building GDB,
libbacktrace will be built first.  However, libbacktrace is not yet
linked into GDB, or used by GDB in any way.

It is possible to stop libbacktrace being built by configuring the
tree with --disable-libbacktrace.

This commit does NOT update src-release.sh, that will be done in the
next commit, this commit ONLY imports libbacktrace from gcc.  This
means that if you try to make a release of GDB from exactly this
commit then the release tar file will not include libbacktrace.
However, as libbacktrace is an optional dependency this is fine.

---

<The patch is not included here as it's pretty large.>


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

* Re: [PUSHED 6/6] gdb: print backtrace for internal error/warning
  2021-09-28 11:26   ` [PUSHED 6/6] gdb: print backtrace for internal error/warning Andrew Burgess
@ 2021-09-28 12:26     ` Eli Zaretskii
  2021-09-29  8:34       ` Andrew Burgess
  0 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2021-09-28 12:26 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Date: Tue, 28 Sep 2021 12:26:24 +0100
> 
>   ../../src.dev-3/gdb/maint.c:82: internal-error: blah
>   A problem internal to GDB has been detected,
>   further debugging may prove unreliable.
>   Create a core file of GDB? (y or n) n
> 
> My hope is that this backtrace might make it slightly easier to
> diagnose GDB issues if all that is provided is the console output, I
> find that we frequently get reports of an assert being hit that is
> located in pretty generic code (frame.c, value.c, etc) and it is not
> always obvious how we might have arrived at the assert.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26377
> ---
>  gdb/NEWS                                      |   8 ++
>  gdb/doc/gdb.texinfo                           |  13 ++
>  .../gdb.base/bt-on-error-and-warning.exp      | 118 ++++++++++++++++++
>  gdb/testsuite/gdb.base/bt-on-fatal-signal.exp |  36 ------
>  gdb/utils.c                                   |  36 +++++-
>  5 files changed, 174 insertions(+), 37 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/bt-on-error-and-warning.exp
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index d7c29c82edb..1e25cb8cc36 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -19,6 +19,14 @@ show source open
>    to open and read source code files, which can be useful if the files
>    are located over a slow network connection.
>  
> +maint set internal-error backtrace on|off
> +maint show internal-error backtrace
> +maint set internal-warning backtrace on|off
> +maint show internal-warning backtrace
> +  GDB can now print a backtrace of itself when it encounters either an
> +  internal-error, or an internal-warning.  This is on by default for
> +  internal-error and off by default for internal-warning.

Does this depend on libbacktrace?  If so, should that be mentioned in
the documentation?

> +When @value{GDBN} reports an internal problem (error or warning) it is
> +possible to have a backtrace of @value{GDBN} printed to stderr.  This

I suggest "standard error stream" instead of "stderr".

The documentation parts are okay with these nits taken care of.
Thanks.

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

* Re: [PUSHED 5/6] gdb: use libbacktrace to create a better backtrace for fatal signals
  2021-09-28 11:26   ` [PUSHED 5/6] gdb: use libbacktrace to create a better backtrace for fatal signals Andrew Burgess
@ 2021-09-28 18:55     ` Pedro Alves
  2021-09-29  8:21       ` Andrew Burgess
  2021-09-29  3:09     ` Simon Marchi
  1 sibling, 1 reply; 34+ messages in thread
From: Pedro Alves @ 2021-09-28 18:55 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

Hi Andrew,

I read this now (for the first time, I completely missed v1!), and spotted a couple typos:

On 2021-09-28 12:26 p.m., Andrew Burgess wrote:

> +  return function != nullptr && strcmp (function, "main") == 0;
> +}
> +
> +/* Write a backtrace to GDB's stderr in an async safe manor.  This is a

typo: manor -> manner.

> +/* Print a backtrace of the current GDB process to the current
> +   gdb_stderr.  The output is done in a signal async manor, so it is safe

likewise.


Thanks for doing all this.

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

* Re: [PUSHED 5/6] gdb: use libbacktrace to create a better backtrace for fatal signals
  2021-09-28 11:26   ` [PUSHED 5/6] gdb: use libbacktrace to create a better backtrace for fatal signals Andrew Burgess
  2021-09-28 18:55     ` Pedro Alves
@ 2021-09-29  3:09     ` Simon Marchi
  2021-09-29  9:56       ` Andrew Burgess
  1 sibling, 1 reply; 34+ messages in thread
From: Simon Marchi @ 2021-09-29  3:09 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

> +void
> +gdb_internal_backtrace ()
> +{
> +  if (current_ui == nullptr)
> +    return;
> +
> +  const auto sig_write = [] (const char *msg) -> void
> +  {
> +    gdb_stderr->write_async_safe (msg, strlen (msg));
> +  };
> +
> +  sig_write (_("----- Backtrace -----\n"));
> +
> +  if (gdb_stderr->fd () > -1)
> +    gdb_internal_backtrace_1 ();
> +  else
> +    sig_write (_("Backtrace unavailable\n"));

I happen to try to build GDB on NetBSD, and I get:

      CXX    bt-utils.o
    /home/simark/src/binutils-gdb/gdb/bt-utils.c: In function 'void gdb_internal_backtrace()':
    /home/simark/src/binutils-gdb/gdb/bt-utils.c:165:5: error: 'gdb_internal_backtrace_1' was not declared in this scope
         gdb_internal_backtrace_1 ();
         ^~~~~~~~~~~~~~~~~~~~~~~~

Indeed, it looks like gdb_internal_backtrace_1 is never defined if
!GDB_PRINT_INTERNAL_BACKTRACE_USING_EXECINFO and
!GDB_PRINT_INTERNAL_BACKTRACE_USING_LIBBACKTRACE.

You can reproduce on Linux by deleting HAVE_LIBBACKTRACE and
HAVE_EXECINFO_BACKTRACE from your generated config.h.

Simon

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

* Re: [PUSHED 5/6] gdb: use libbacktrace to create a better backtrace for fatal signals
  2021-09-28 18:55     ` Pedro Alves
@ 2021-09-29  8:21       ` Andrew Burgess
  0 siblings, 0 replies; 34+ messages in thread
From: Andrew Burgess @ 2021-09-29  8:21 UTC (permalink / raw)
  To: gdb-patches

* Pedro Alves <pedro@palves.net> [2021-09-28 19:55:11 +0100]:

> Hi Andrew,
> 
> I read this now (for the first time, I completely missed v1!), and spotted a couple typos:
> 
> On 2021-09-28 12:26 p.m., Andrew Burgess wrote:
> 
> > +  return function != nullptr && strcmp (function, "main") == 0;
> > +}
> > +
> > +/* Write a backtrace to GDB's stderr in an async safe manor.  This is a
> 
> typo: manor -> manner.
> 
> > +/* Print a backtrace of the current GDB process to the current
> > +   gdb_stderr.  The output is done in a signal async manor, so it is safe
> 
> likewise.
> 
> 
> Thanks for doing all this.

Thanks, I fixed these.  I also found an additional instance of this
mistake which I fixed.

Below is what I pushed.

Thanks,
Andrew

---

commit 80656a8e4b32353025f7f4cb1279168f082fbf7b
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Wed Sep 29 09:16:52 2021 +0100

    gdb: fix manor -> manner typo in some comments
    
    In a recent commit I used 'manor' in some comments rather than
    'manner'.  This commit fixes those two mistakes.
    
    I also looked through the gdb/ tree and found one additional instance
    of this mistake that this commit also fixes.

diff --git a/gdb/bt-utils.c b/gdb/bt-utils.c
index b5e0a0ed004..8f826bde6b4 100644
--- a/gdb/bt-utils.c
+++ b/gdb/bt-utils.c
@@ -108,7 +108,7 @@ libbacktrace_print (void *data, uintptr_t pc, const char *filename,
   return function != nullptr && strcmp (function, "main") == 0;
 }
 
-/* Write a backtrace to GDB's stderr in an async safe manor.  This is a
+/* Write a backtrace to GDB's stderr in an async safe manner.  This is a
    backtrace of GDB, not any running inferior, and is to be used when GDB
    crashes or hits some other error condition.  */
 
diff --git a/gdb/bt-utils.h b/gdb/bt-utils.h
index 433aa23614b..a406041ec64 100644
--- a/gdb/bt-utils.h
+++ b/gdb/bt-utils.h
@@ -49,7 +49,7 @@
 #endif
 
 /* Print a backtrace of the current GDB process to the current
-   gdb_stderr.  The output is done in a signal async manor, so it is safe
+   gdb_stderr.  The output is done in a signal async manner, so it is safe
    to call from signal handlers.  */
 
 extern void gdb_internal_backtrace ();
diff --git a/gdb/language.h b/gdb/language.h
index cec3ab03ed6..69101bd4074 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -574,7 +574,7 @@ struct language_defn
   /* Return false if the language has first-class arrays.  Return true if
      there are no array values, and array objects decay to pointers, as in
      C.  The default is true as currently most supported languages behave
-     in this manor.  */
+     in this manner.  */
 
   virtual bool c_style_arrays_p () const
   { return true; }

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

* Re: [PUSHED 6/6] gdb: print backtrace for internal error/warning
  2021-09-28 12:26     ` Eli Zaretskii
@ 2021-09-29  8:34       ` Andrew Burgess
  0 siblings, 0 replies; 34+ messages in thread
From: Andrew Burgess @ 2021-09-29  8:34 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

* Eli Zaretskii <eliz@gnu.org> [2021-09-28 15:26:47 +0300]:

> > From: Andrew Burgess <andrew.burgess@embecosm.com>
> > Date: Tue, 28 Sep 2021 12:26:24 +0100
> > 
> >   ../../src.dev-3/gdb/maint.c:82: internal-error: blah
> >   A problem internal to GDB has been detected,
> >   further debugging may prove unreliable.
> >   Create a core file of GDB? (y or n) n
> > 
> > My hope is that this backtrace might make it slightly easier to
> > diagnose GDB issues if all that is provided is the console output, I
> > find that we frequently get reports of an assert being hit that is
> > located in pretty generic code (frame.c, value.c, etc) and it is not
> > always obvious how we might have arrived at the assert.
> > 
> > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26377
> > ---
> >  gdb/NEWS                                      |   8 ++
> >  gdb/doc/gdb.texinfo                           |  13 ++
> >  .../gdb.base/bt-on-error-and-warning.exp      | 118 ++++++++++++++++++
> >  gdb/testsuite/gdb.base/bt-on-fatal-signal.exp |  36 ------
> >  gdb/utils.c                                   |  36 +++++-
> >  5 files changed, 174 insertions(+), 37 deletions(-)
> >  create mode 100644 gdb/testsuite/gdb.base/bt-on-error-and-warning.exp
> > 
> > diff --git a/gdb/NEWS b/gdb/NEWS
> > index d7c29c82edb..1e25cb8cc36 100644
> > --- a/gdb/NEWS
> > +++ b/gdb/NEWS
> > @@ -19,6 +19,14 @@ show source open
> >    to open and read source code files, which can be useful if the files
> >    are located over a slow network connection.
> >  
> > +maint set internal-error backtrace on|off
> > +maint show internal-error backtrace
> > +maint set internal-warning backtrace on|off
> > +maint show internal-warning backtrace
> > +  GDB can now print a backtrace of itself when it encounters either an
> > +  internal-error, or an internal-warning.  This is on by default for
> > +  internal-error and off by default for internal-warning.
> 
> Does this depend on libbacktrace?  If so, should that be mentioned in
> the documentation?

I'm not sure.  libbacktrace is now included within the binutils-gdb
repository, so it's not like this is a dependency that users will need
to hunt down and install on their own.

It is true that libbacktrace could potentially not be supported/built
on a particular target, but then GDB falls back to the libc
backtrace() API, so libbacktrace isn't a hard requirement for getting
such a backtrace, it's just one possible requirement.

Would you suggest that this dependency situation be described in the
actual .texinfo docs?  Or just mentioned in the NEWS file?

If you can give some more guidance, I'll try to draft some suitable
words.

> 
> > +When @value{GDBN} reports an internal problem (error or warning) it is
> > +possible to have a backtrace of @value{GDBN} printed to stderr.  This
> 
> I suggest "standard error stream" instead of "stderr".

Thanks, I fixed the two places I said 'stderr' with the patch below
which I pushed.

Thanks,
Andrew

---

commit 4180173142185f0967dfefb131e4843a17779c86
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Wed Sep 29 09:25:03 2021 +0100

    gdb/doc: use 'standard error stream' instead of 'stderr' in some places
    
    With this commit:
    
      commit 91f2597bd24d171c1337a4629f8237aa47c59082
      Date:   Thu Aug 12 18:24:59 2021 +0100
    
          gdb: print backtrace for internal error/warning
    
    I included some references to 'stderr', which, it was pointed out,
    would be better written as 'standard error stream'.  See:
    
      https://sourceware.org/pipermail/gdb-patches/2021-September/182225.html
    
    This commit replaces the two instances of 'stderr' that I introduced.

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index d4e4174be5d..c156a1d6739 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -39266,9 +39266,9 @@
 @itemx maint set internal-warning backtrace @r{[}on|off@r{]}
 @itemx maint show internal-warning backtrace
 When @value{GDBN} reports an internal problem (error or warning) it is
-possible to have a backtrace of @value{GDBN} printed to stderr.  This
-is @samp{on} by default for @code{internal-error} and @samp{off} by
-default for @code{internal-warning}.
+possible to have a backtrace of @value{GDBN} printed to the standard
+error stream.  This is @samp{on} by default for @code{internal-error}
+and @samp{off} by default for @code{internal-warning}.
 
 @kindex maint packet
 @item maint packet @var{text}
@@ -39768,9 +39768,9 @@
 @itemx maint show backtrace-on-fatal-signal
 When this setting is @code{on}, if @value{GDBN} itself terminates with
 a fatal signal (e.g.@: SIGSEGV), then a limited backtrace will be
-printed to stderr.  This backtrace can be used to help diagnose
-crashes within @value{GDBN} in situations where a user is unable to
-share a corefile with the @value{GDBN} developers.
+printed to the standard error stream.  This backtrace can be used to
+help diagnose crashes within @value{GDBN} in situations where a user
+is unable to share a corefile with the @value{GDBN} developers.
 
 If the functionality to provide this backtrace is not available for
 the platform on which GDB is running then this feature will be

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

* Re: [PUSHED 5/6] gdb: use libbacktrace to create a better backtrace for fatal signals
  2021-09-29  3:09     ` Simon Marchi
@ 2021-09-29  9:56       ` Andrew Burgess
  0 siblings, 0 replies; 34+ messages in thread
From: Andrew Burgess @ 2021-09-29  9:56 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

* Simon Marchi <simon.marchi@polymtl.ca> [2021-09-28 23:09:20 -0400]:

> > +void
> > +gdb_internal_backtrace ()
> > +{
> > +  if (current_ui == nullptr)
> > +    return;
> > +
> > +  const auto sig_write = [] (const char *msg) -> void
> > +  {
> > +    gdb_stderr->write_async_safe (msg, strlen (msg));
> > +  };
> > +
> > +  sig_write (_("----- Backtrace -----\n"));
> > +
> > +  if (gdb_stderr->fd () > -1)
> > +    gdb_internal_backtrace_1 ();
> > +  else
> > +    sig_write (_("Backtrace unavailable\n"));
> 
> I happen to try to build GDB on NetBSD, and I get:
> 
>       CXX    bt-utils.o
>     /home/simark/src/binutils-gdb/gdb/bt-utils.c: In function 'void gdb_internal_backtrace()':
>     /home/simark/src/binutils-gdb/gdb/bt-utils.c:165:5: error: 'gdb_internal_backtrace_1' was not declared in this scope
>          gdb_internal_backtrace_1 ();
>          ^~~~~~~~~~~~~~~~~~~~~~~~
> 
> Indeed, it looks like gdb_internal_backtrace_1 is never defined if
> !GDB_PRINT_INTERNAL_BACKTRACE_USING_EXECINFO and
> !GDB_PRINT_INTERNAL_BACKTRACE_USING_LIBBACKTRACE.
> 
> You can reproduce on Linux by deleting HAVE_LIBBACKTRACE and
> HAVE_EXECINFO_BACKTRACE from your generated config.h.

Sorry for breaking this.

I pushed the patch below to resolve this issue.

Let me know if you see any further problems.

Thanks,
Andrew

--

commit 74ea3b51c3b76fc0ccd46cc755e6e85e4570515b
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Wed Sep 29 10:26:59 2021 +0100

    gdb: fix build when libbacktrace and execinfo backtrace are not available
    
    In this commit:
    
      commit abbbd4a3e0ca51132e7fb31a43f896d29894dae0
      Date:   Wed Aug 11 13:24:33 2021 +0100
    
          gdb: use libbacktrace to create a better backtrace for fatal signals
    
    The build of GDB was broken iff, the execinfo backtrace API is not
    available, and, libbacktrace is either disabled, or not usable.  In
    this case you'll see build errors like this:
    
          CXX    bt-utils.o
        /home/username/src/binutils-gdb/gdb/bt-utils.c: In function 'void gdb_internal_backtrace()':
        /home/username/src/binutils-gdb/gdb/bt-utils.c:165:5: error: 'gdb_internal_backtrace_1' was not declared in this scope
             gdb_internal_backtrace_1 ();
             ^~~~~~~~~~~~~~~~~~~~~~~~
    
    This commit fixes the issue by guarding the call to
    gdb_internal_backtrace_1 with '#ifdef GDB_PRINT_INTERNAL_BACKTRACE',
    which is only defined when one of the backtrace libraries are
    available.

diff --git a/gdb/bt-utils.c b/gdb/bt-utils.c
index 8f826bde6b4..79e6e090d42 100644
--- a/gdb/bt-utils.c
+++ b/gdb/bt-utils.c
@@ -161,9 +161,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");

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

end of thread, other threads:[~2021-09-29  9:56 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-19  9:49 [PATCH 0/6] Display GDB backtrace for internal errors Andrew Burgess
2021-08-19  9:49 ` [PATCH 1/6] gdb: use bool instead of int in struct internal_problem Andrew Burgess
2021-08-19 18:33   ` Simon Marchi
2021-09-07 14:11     ` Andrew Burgess
2021-08-19  9:49 ` [PATCH 2/6] gdb: make use of std::string in utils.c Andrew Burgess
2021-08-19 18:41   ` Simon Marchi
2021-09-07 14:12     ` Andrew Burgess
2021-08-19  9:49 ` [PATCH 3/6] gdb: Add a dependency between gdb and libbacktrace Andrew Burgess
2021-08-19 18:43   ` Simon Marchi
2021-08-27 17:44     ` Tom Tromey
2021-08-30 20:33       ` Andrew Burgess
2021-08-19  9:49 ` [PATCH 4/6] Copy in libbacktrace from gcc Andrew Burgess
2021-08-27 17:46   ` Tom Tromey
2021-08-30 20:34     ` Andrew Burgess
2021-08-19  9:49 ` [PATCH 5/6] gdb: use libbacktrace to create a better backtrace for fatal signals Andrew Burgess
2021-08-19 18:58   ` Simon Marchi
2021-08-19  9:49 ` [PATCH 6/6] gdb: print backtrace for internal error/warning Andrew Burgess
2021-08-19 19:01   ` Simon Marchi
2021-08-30 14:16 ` [PATCH 0/6] Display GDB backtrace for internal errors Tom de Vries
2021-08-30 20:35   ` Andrew Burgess
2021-08-31 11:17 ` Florian Weimer
2021-09-28 11:26 ` [PUSHED " Andrew Burgess
2021-09-28 11:26   ` [PUSHED 1/6] top-level configure: setup target_configdirs based on repository Andrew Burgess
2021-09-28 11:26   ` [PUSHED 2/6] gdb: Add a dependency between gdb and libbacktrace Andrew Burgess
2021-09-28 11:26   ` [PUSHED 4/6] src-release.sh: add libbacktrace to GDB_SUPPORT_DIRS Andrew Burgess
2021-09-28 11:26   ` [PUSHED 5/6] gdb: use libbacktrace to create a better backtrace for fatal signals Andrew Burgess
2021-09-28 18:55     ` Pedro Alves
2021-09-29  8:21       ` Andrew Burgess
2021-09-29  3:09     ` Simon Marchi
2021-09-29  9:56       ` Andrew Burgess
2021-09-28 11:26   ` [PUSHED 6/6] gdb: print backtrace for internal error/warning Andrew Burgess
2021-09-28 12:26     ` Eli Zaretskii
2021-09-29  8:34       ` Andrew Burgess
2021-09-28 12:20   ` [PUSHED 3/6] Copy in libbacktrace from gcc Andrew Burgess

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).