gdb/remote: iterate on pspace inferiors in remote_new_objfile
authorSimon Marchi <simon.marchi@polymtl.ca>
Sat, 30 Apr 2022 03:21:14 +0000 (23:21 -0400)
committerSimon Marchi <simon.marchi@polymtl.ca>
Wed, 4 May 2022 12:15:42 +0000 (08:15 -0400)
commit06c7226ea19c1880fc160b8890c3a5574d490709
tree75079da9b5c4ec7ac081824123f1777c10b21fcb
parent3569f4ab7e4bb8bf34db35b623da0c5774b4ee90
gdb/remote: iterate on pspace inferiors in remote_new_objfile

Commit 152a17495663 ("gdb: prune inferiors at end of
fetch_inferior_event, fix intermittent failure of
gdb.threads/fork-plus-threads.exp") broke some tests with the
native-gdbserver board, such as:

    (gdb) PASS: gdb.base/step-over-syscall.exp: detach-on-fork=off: follow-fork=child: break cond on target : vfork: break marker
    continue^M
    Continuing.^M
    terminate called after throwing an instance of 'gdb_exception_error'^M

I can manually reproduce the issue by running (just the commands that
the test does as a one liner):

    $ ./gdb -q --data-directory=data-directory \
          testsuite/outputs/gdb.base/step-over-syscall/step-over-vfork \
  -ex "tar rem | ../gdbserver/gdbserver - testsuite/outputs/gdb.base/step-over-syscall/step-over-vfork" \
  -ex "b main" \
  -ex c \
  -ex "d 1" \
  -ex "set displaced-stepping off" \
  -ex "b *0x7ffff7d7ac5a if main == 0" \
  -ex "set detach-on-fork off" \
  -ex "set follow-fork-mode child" \
  -ex c \
  -ex "inferior 1" \
  -ex "b marker" \
  -ex c

... where 0x7ffff7d7ac5a is the exact address of the vfork syscall
(which can be found by looking at gdb.log).

The important part of the above is that a vfork syscall creates inferior
2, then inferior 2 executes until exit, then we switch back to inferior
1 and try to resume it.

The uncaught exception happens here:

    #4  0x00005596969d81a9 in error (fmt=0x559692da9e40 "Cannot execute this command while the target is running.\nUse the \"interrupt\" command to stop the target\nand then try again.")
        at /home/simark/src/binutils-gdb/gdbsupport/errors.cc:43
    #5  0x0000559695af6f66 in remote_target::putpkt_binary (this=0x617000038080, buf=0x559692da4380 "qSymbol::", cnt=9) at /home/simark/src/binutils-gdb/gdb/remote.c:9560
    #6  0x0000559695af6aaf in remote_target::putpkt (this=0x617000038080, buf=0x559692da4380 "qSymbol::") at /home/simark/src/binutils-gdb/gdb/remote.c:9518
    #7  0x0000559695ab50dc in remote_target::remote_check_symbols (this=0x617000038080) at /home/simark/src/binutils-gdb/gdb/remote.c:5141
    #8  0x0000559695b3cccf in remote_new_objfile (objfile=0x0) at /home/simark/src/binutils-gdb/gdb/remote.c:14600
    #9  0x0000559693bc52a9 in std::__invoke_impl<void, void (*&)(objfile*), objfile*> (__f=@0x61b0000167f8: 0x559695b3cb1d <remote_new_objfile(objfile*)>) at /usr/include/c++/11.2.0/bits/invoke.h:61
    #10 0x0000559693bb2848 in std::__invoke_r<void, void (*&)(objfile*), objfile*> (__fn=@0x61b0000167f8: 0x559695b3cb1d <remote_new_objfile(objfile*)>) at /usr/include/c++/11.2.0/bits/invoke.h:111
    #11 0x0000559693b8dddf in std::_Function_handler<void (objfile*), void (*)(objfile*)>::_M_invoke(std::_Any_data const&, objfile*&&) (__functor=..., __args#0=@0x7ffe0bae0590: 0x0) at /usr/include/c++/11.2.0/bits/std_function.h:291
    #12 0x00005596956374b2 in std::function<void (objfile*)>::operator()(objfile*) const (this=0x61b0000167f8, __args#0=0x0) at /usr/include/c++/11.2.0/bits/std_function.h:560
    #13 0x0000559695633c64 in gdb::observers::observable<objfile*>::notify (this=0x55969ef5c480 <gdb::observers::new_objfile>, args#0=0x0) at /home/simark/src/binutils-gdb/gdb/../gdbsupport/observable.h:150
    #14 0x0000559695df6cc2 in clear_symtab_users (add_flags=...) at /home/simark/src/binutils-gdb/gdb/symfile.c:2873
    #15 0x000055969574c263 in program_space::~program_space (this=0x6120000c8a40, __in_chrg=<optimized out>) at /home/simark/src/binutils-gdb/gdb/progspace.c:154
    #16 0x0000559694fc086b in delete_inferior (inf=0x61700003bf80) at /home/simark/src/binutils-gdb/gdb/inferior.c:205
    #17 0x0000559694fc341f in prune_inferiors () at /home/simark/src/binutils-gdb/gdb/inferior.c:390
    #18 0x0000559695017ada in fetch_inferior_event () at /home/simark/src/binutils-gdb/gdb/infrun.c:4293
    #19 0x0000559694f629e6 in inferior_event_handler (event_type=INF_REG_EVENT) at /home/simark/src/binutils-gdb/gdb/inf-loop.c:41
    #20 0x0000559695b3b0e3 in remote_async_serial_handler (scb=0x6250001ef100, context=0x6170000380a8) at /home/simark/src/binutils-gdb/gdb/remote.c:14466
    #21 0x0000559695c59eb7 in run_async_handler_and_reschedule (scb=0x6250001ef100) at /home/simark/src/binutils-gdb/gdb/ser-base.c:138
    #22 0x0000559695c5a42a in fd_event (error=0, context=0x6250001ef100) at /home/simark/src/binutils-gdb/gdb/ser-base.c:189
    #23 0x00005596969d9ebf in handle_file_event (file_ptr=0x60700005af40, ready_mask=1) at /home/simark/src/binutils-gdb/gdbsupport/event-loop.cc:574
    #24 0x00005596969da7fa in gdb_wait_for_event (block=0) at /home/simark/src/binutils-gdb/gdbsupport/event-loop.cc:700
    #25 0x00005596969d8539 in gdb_do_one_event () at /home/simark/src/binutils-gdb/gdbsupport/event-loop.cc:212

If I enable "set debug infrun" just before the last continue, we see:

    (gdb) continue
    Continuing.
    [infrun] clear_proceed_status_thread: 965604.965604.0
    [infrun] proceed: enter
      [infrun] proceed: addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT
      [infrun] scoped_disable_commit_resumed: reason=proceeding
      [infrun] start_step_over: enter
        [infrun] start_step_over: stealing global queue of threads to step, length = 0
        [infrun] operator(): step-over queue now empty
      [infrun] start_step_over: exit
      [infrun] resume_1: step=0, signal=GDB_SIGNAL_0, trap_expected=0, current thread [965604.965604.0] at 0x7ffff7d7ac5c
      [infrun] do_target_resume: resume_ptid=965604.0.0, step=0, sig=GDB_SIGNAL_0
      [infrun] prepare_to_wait: prepare_to_wait
      [infrun] reset: reason=proceeding
      [infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target remote
      [infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target remote
    [infrun] proceed: exit
    [infrun] fetch_inferior_event: enter
      [infrun] scoped_disable_commit_resumed: reason=handling event
      [infrun] do_target_wait: Found 2 inferiors, starting at #1
      [infrun] random_pending_event_thread: None found.
      [infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
      [infrun] print_target_wait_results:   965604.965604.0 [Thread 965604.965604],
      [infrun] print_target_wait_results:   status->kind = VFORK_DONE
      [infrun] handle_inferior_event: status->kind = VFORK_DONE
      [infrun] context_switch: Switching context from 0.0.0 to 965604.965604.0
      [infrun] handle_vfork_done: not waiting for a vfork-done event
      [infrun] start_step_over: enter
        [infrun] start_step_over: stealing global queue of threads to step, length = 0
        [infrun] operator(): step-over queue now empty
      [infrun] start_step_over: exit
      [infrun] resume_1: step=0, signal=GDB_SIGNAL_0, trap_expected=0, current thread [965604.965604.0] at 0x7ffff7d7ac5c
      [infrun] do_target_resume: resume_ptid=965604.0.0, step=0, sig=GDB_SIGNAL_0
      [infrun] prepare_to_wait: prepare_to_wait
      [infrun] reset: reason=handling event
      [infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target remote
      [infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target remote
    terminate called after throwing an instance of 'gdb_exception_error'

What happens is:

 - After doing the "continue" on inferior 1, the remote target gives us
   a VFORK_DONE event.  The core ignores it and resumes inferior 1.
 - Since prune_inferiors is now called after each handled event, in
   fetch_inferior_event, it is called after we handled that VFORK_DONE
   event and resumed inferior 1.
 - Inferior 2 is pruned, which (see backtrace above) causes its program
   space to be deleted, which clears the symtabs for that program space,
   which calls the new_objfile observable and remote_new_objfile
   observer (with a nullptr objfile, to indicate that the previously
   loaded symbols have been discarded), which calls
   remote_check_symbols.

remote_check_symbols is the function that sends the qSymbol packet, to
let the remote side ask for symbol addresses.  The problem is that the
remote target is working in all-stop / sync mode and is currently
resumed.  It has sent a vCont packet to resume the target and is waiting
for a stop reply.  It can't send any packets in the mean time.  That
causes the exception to be thrown.

This wasn't a problem before, when prune_inferiors was called in
normal_stop, because it was always called at a time the target was not
resumed.

An important observation here is that the new_objfile observable is
invoked for a change in inferior 2's program space (inferior 2's program
space is the current program space).  Inferior 2 isn't bound to any
process on the remote side (it has exited, that's why it's being
pruned).  It doesn't make sense to try to send a qSymbol packet for a
process that doesn't exist on the remote side.  remote_check_symbols
actually attempts to avoid that:

   /* The remote side has no concept of inferiors that aren't running
     yet, it only knows about running processes.  If we're connected
     but our current inferior is not running, we should not invite the
     remote target to request symbol lookups related to its
     (unrelated) current process.  */
  if (!target_has_execution ())
    return;

The problem here is that while inferior 2's program space is the current
program space, inferior 1 is the current inferior.  So the check above
passes, since inferior has execution.  We therefore try to send a
qSymbol packet for inferior 1 in reaction to a change in inferior 2's
program space, that's wrong.

This exposes a conceptual flaw in remote_new_objfile.  The "new_objfile"
event concerns a specific program space, which can concern multiple
inferiors, as inferiors can share a program space.  We shouldn't
consider the current inferior at all, but instead all inferiors bound to
the affected program space.  Especially since the current inferior can
be unrelated to the current program space at that point.

To be clear, we are in this state because ~program_space sets itself as
the current program space, but there is no more inferior having that
program space to switch to, inferior 2 has already been unlinked.

To fix this, make remote_new_objfile iterate on all inferiors bound to
the affected program space.  Remove the target_has_execution check from
remote_check_symbols, replace it with an assert.  All callers must
ensure that the current inferior has execution before calling it.

Change-Id: Ica643145bcc03115248290fd310cadab8ec8371c
gdb/remote.c