gdb/debuginfod: cleanup debuginfod earlier
authorAndrew Burgess <aburgess@redhat.com>
Tue, 23 May 2023 10:37:41 +0000 (11:37 +0100)
committerAndrew Burgess <aburgess@redhat.com>
Fri, 9 Jun 2023 14:36:50 +0000 (15:36 +0100)
commitf3eee5861743d635ea3a5be7eae3115f85ec9dae
tree3ff2f49256e98ba2a90889949c863458d847d6a3
parentec5e9488c0a4adecd7502071ae585f045552051b
gdb/debuginfod: cleanup debuginfod earlier

A GDB crash was discovered on Fedora GDB that was tracked back to an
issue with the way that debuginfod is cleaned up.

The bug was reported on Fedora 37, 38, and 39.  Here are the steps to
reproduce:

1. The file /etc/ssl/openssl.cnf contains the following lines:

   [provider_sect]
   default = default_sect
   ##legacy = legacy_sect
   ##
   [default_sect]
   activate = 1

   ##[legacy_sect]
   ##activate = 1

   The bug will occur when the '##' characters are removed so that the
   lines in question look like this:

   [provider_sect]
   default = default_sect
   legacy = legacy_sect

   [default_sect]
   activate = 1

   [legacy_sect]
   activate = 1

2. Clean up any existing debuginfod cache data:

   > rm -rf $HOME/.cache/debuginfod_client

3. Run GDB:

   > gdb -nx -q -iex 'set trace-commands on' \
                -iex 'set debuginfod enabled on' \
                -iex 'set confirm off' \
-ex 'start' -ex 'quit' /bin/ls
   +set debuginfod enabled on
   +set confirm off
   Reading symbols from /bin/ls...
   Downloading separate debug info for /usr/bin/ls
   ... snip ...
   Temporary breakpoint 1, main (argc=1, argv=0x7fffffffde38) at ../src/ls.c:1646
   1646    {
   +quit

   Fatal signal: Segmentation fault
   ----- Backtrace -----
   ... snip ...

So GDB ends up crashing during exit.

What's happening is that when debuginfod is initialised
debuginfod_begin is called (this is in the debuginfod library), this
in turn sets up libcurl, which makes use of openssl.  Somewhere during
this setup process an at_exit function is registered to cleanup some
state.

Back in GDB the debuginfod_client object is managed using this code:

  /* Deleter for a debuginfod_client.  */

  struct debuginfod_client_deleter
  {
    void operator() (debuginfod_client *c)
    {
      debuginfod_end (c);
    }
  };

  using debuginfod_client_up
    = std::unique_ptr<debuginfod_client, debuginfod_client_deleter>;

And then a global debuginfod_client_up is created to hold a pointer to
the debuginfod_client object.  As a global this will be cleaned up
using the standard C++ global object destructor mechanism, which is
run after the at_exit handlers.

However, it is expected that when debuginfod_end is called the
debuginfod_client object will still be in a usable state, that is, we
don't expect the at_exit handlers to have run and started cleaning up
the library state.

To fix this issue we need to ensure that debuginfod_end is called
before the at_exit handlers have a chance to run.

This commit removes the debuginfod_client_up type, and instead has GDB
hold a raw pointer to the debuginfod_client object.  We then make use
of GDB's make_final_cleanup to register a function that will call
debuginfod_end.

As GDB's final cleanups are called before exit is called, this means
that debuginfod_end will be called before the at_exit handlers are
called, and the crash identified above is resolved.

It's not obvious how this issue can easily be tested for. The bug does
not appear to manifest when using a local debuginfod server, so we'd
need to setup something more involved.  For now I'm proposing this
patch without any associated tests.

Co-Authored-By: Mark Wielaard <mark@klomp.org>
Co-Authored-By: Simon Marchi <simark@simark.ca>
Reviewed-By: Tom Tromey <tom@tromey.com>
Reviewed-By: Aaron Merey <amerey@redhat.com>
gdb/debuginfod-support.c