Re: [PATCH 5/6] gdb: use libbacktrace to create a better backtrace for fatal signals

[Simon Marchi <simon.marchi@polymtl.ca>]

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