]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commit
gdb: make regcache::raw_update switch to right inferior
authorSimon Marchi <simon.marchi@efficios.com>
Mon, 3 Apr 2023 18:52:06 +0000 (14:52 -0400)
committerSimon Marchi <simon.marchi@efficios.com>
Mon, 17 Apr 2023 17:47:13 +0000 (13:47 -0400)
commit98994c7a18347c7248d2cf01d1dad6772907b813
treebdb49a51dc1b31b24fa1f9c9ea2314cabd8670d8
parent348da4565b5c901e9320c3e2d7f5b62793b48a38
gdb: make regcache::raw_update switch to right inferior

With the following patch, which teaches the amd-dbgapi target to handle
inferiors that fork, we end up with target stacks in the following
state, when an inferior that does not use the GPU forks an inferior that
eventually uses the GPU.

    inf 1            inf 2
    -----            -----
                     amd-dbgapi
    linux-nat        linux-nat
    exec             exec

When a GPU thread from inferior 2 hits a breakpoint, the following
sequence of events would happen, if it was not for the current patch.

 - we start with inferior 1 as current
 - do_target_wait_1 makes inferior 2 current, does a target_wait, which
   returns a stop event for an amd-dbgapi wave (thread).
 - do_target_wait's scoped_restore_current_thread restores inferior 1 as
   current
 - fetch_inferior_event calls switch_to_target_no_thread with linux-nat
   as the process target, since linux-nat is officially the process
   target of inferior 2.  This makes inferior 1 the current inferior, as
   it's the first inferior with that target.
 - In handle_signal_stop, we have:

    ecs->event_thread->suspend.stop_pc
      = regcache_read_pc (get_thread_regcache (ecs->event_thread));

    context_switch (ecs);

   regcache_read_pc executes while inferior 1 is still the current one
   (because it's before the `context_switch`).  This is a problem,
   because the regcache is for a ptid managed by the amd-dbgapi target
   (e.g. (12345, 1, 1)), a ptid that does not make sense for the
   linux-nat target.  The fetch_registers target call goes directly
   to the linux-nat target, which gets confused.
 - We would then get an error like:

     Couldn't get extended state status: No such process.

   ... since linux-nat tries to do a ptrace call on tid 1.

GDB should switch to the inferior the ptid belongs to before doing the
target call to fetch registers, to make sure the call hits the right
target stack (it should be handled by the amd-dbgapi target in this
case).  In fact the following patch does this change, and it would be
enough to fix this specific problem.

However, I propose to change regcache to make it switch to the right
inferior, if needed, before doing target calls.  That makes the
interface as a whole more independent of the global context.

My first attempt at doing this was to find an inferior using the process
stratum target and the ptid that regcache already knows about:

      gdb::optional<scoped_restore_current_thread> restore_thread;
      inferior *inf = find_inferior_ptid (this->target (), this->ptid ());
      if (inf != current_inferior ())
{
  restore_thread.emplace ();
  switch_to_inferior_no_thread (inf);
}

However, this caused some failures in fork-related tests and gdbserver
boards.  When we detach a fork child, we may create a regcache for the
child, but there is no corresponding inferior.  For instance, to restore
the PC after a displaced step over the fork syscall.  So
find_inferior_ptid would return nullptr, and
switch_to_inferior_no_thread would hit a failed assertion.

So, this patch adds to regcache the information "the inferior to switch
to to makes target calls".  In typical cases, it will be the inferior
that matches the regcache's ptid.  But in some cases, like the detached
fork child one, it will be another inferior (in this example, it will be
the fork parent inferior).

The problem that we witnessed was in regcache::raw_update specifically,
but I looked for other regcache methods doing target calls, and added
the same inferior switching code to raw_write too.

In the regcache constructor and in get_thread_arch_aspace_regcache,
"inf_for_target_calls" replaces the process_stratum_target parameter.
We suppose that the process stratum target that would be passed
otherwise is the same that is in inf_for_target_calls's target stack, so
we don't need to pass both in parallel.  The process stratum target is
still used as a key in the `target_pid_ptid_regcache_map` map, but
that's it.

There is one spot that needs to be updated outside of the regcache code,
which is the path that handles the "restore PC after a displaced step in
a fork child we're about to detach" case mentioned above.

regcache_test_data needs to be changed to include full-fledged mock
contexts (because there now needs to be inferiors, not just targets).

Change-Id: Id088569ce106e1f194d9ae7240ff436f11c5e123
Reviewed-By: Pedro Alves <pedro@palves.net>
gdb/infrun.c
gdb/regcache.c
gdb/regcache.h