gdb/testsuite: gdb.server/server-kill.exp 'info frame' before kill_server
authorAndrew Burgess <aburgess@redhat.com>
Wed, 15 Mar 2023 14:18:37 +0000 (14:18 +0000)
committerAndrew Burgess <aburgess@redhat.com>
Mon, 3 Apr 2023 11:05:04 +0000 (12:05 +0100)
commit71f18376db954e95a44a9281d05699a228070f77
treea97d61ae23bd818d69c160a3f23de769324d420c
parent4c148f65fc148918d0be15607938770ad8c46e36
gdb/testsuite: gdb.server/server-kill.exp 'info frame' before kill_server

This commit follows on from the following two commits:

  commit 80dc83fd0e70f4d522a534bc601df5e05b81d564
  Date:   Fri Jun 11 11:30:47 2021 +0100

      gdb/remote: handle target dying just before a stepi

And:

  commit 079f190d4cfc6aa9c934b00a9134bc0fcc172d53
  Date:   Thu Mar 9 10:45:03 2023 +0100

      [gdb/testsuite] Fix gdb.server/server-kill.exp for remote target

The first of these commits fixed an issue in GDB and tried to extend
the gdb.server/server-kill.exp test to cover the GDB fix.

Unfortunately, the changes to gdb.server/server-kill.exp were not
correct, and were causing problems when trying to run with the
remote-gdbserver-on-localhost board file.

The second commit reverts some of the gdb.server/server-kill.exp
changes introduced in the first commit so that the test will now work
correctly with the remote-gdbserver-on-localhost board file.

The second commit is just about GDB's testing infrastructure -- it's
not about the original fix to GDB from the first commit, the actual
GDB change was fine.

While reviewing the second commit I wanted to check that the problem
fixed in the first commit is still being tested by the
gdb.server/server-kill.exp script, so I reverted the change to
breakpoint.c that is the core of the first commit and ran the test
script ..... and saw no failures.

The first commit is about GDB discovering that gdbserver has died
while trying to insert a breakpoint.  As soon as GDB spots that
gdbserver is gone we mourn the remote inferior, which ends up deleting
all the breakpoints associated with the remote inferiors.  We then
throw an exception which is caught in the insert breakpoints code, and
we try to display an error that includes the breakpoint number
.... but the breakpoint has already been deleted ... and so GDB
crashes.

After digging a little, what I found is that today, when the test does
'stepi' the first thing we end up doing is calculating the frame-id as
part of the stepi logic, it is during this frame-id calculation that
we mourn the remote inferior, delete the breakpoints, and throw an
exception.  The exception is caught by the top level interpreter loop,
and so we never try to print the breakpoint number which is what
caused the original crash.

If I add an 'info frame' command to the test script, prior to killing
gdbserver, then now when we 'stepi' GDB already has the frame-id
calculated, and the first thing we do is try to insert the
breakpoints, this will trigger the original bug.

In order to reproduce this experiment you'll need to change a function
in breakpoint.c, like this:

  static void
  rethrow_on_target_close_error (const gdb_exception &e)
  {
    return;
  }

Then run gdb.server/server-kill.exp with and without this patch.  You
should find that without this patch there are zero test failures,
while with this patch there will be one failure like this:

  (gdb) PASS: gdb.server/server-kill.exp: test_stepi: info frame
  Executing on target: kill -9 4513    (timeout = 300)
  builtin_spawn -ignore SIGHUP kill -9 4513
  stepi
  ../../src/gdb/breakpoint.c:2863: internal-error: insert_bp_location: Assertion `bl->owner != nullptr' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  ----- Backtrace -----
  ...
gdb/testsuite/gdb.server/server-kill.exp