Re: [PATCH v4 2/2] gdb: raise and handle NOT_AVAILABLE_ERROR when accessing frame PC

[John Baldwin <jhb@FreeBSD.org>]

On 12/18/23 6:40 AM, Tankut Baris Aktemur wrote:
> This patch can be considered a continuation of
> 
>    commit 4778a5f87d253399083565b4919816f541ebe414
>    Author: Tom de Vries <tdevries@suse.de>
>    Date:   Tue Apr 21 15:45:57 2020 +0200
> 
>      [gdb] Fix hang after ext sigkill
> 
> and
> 
>    commit 47f1aceffa02be4726b854082d7587eb259136e0
>    Author: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
>    Date:   Thu May 14 13:59:54 2020 +0200
> 
>      gdb/infrun: handle already-exited threads when attempting to stop
> 
> If a process dies before GDB reports the exit error to the user, we
> may see the "Couldn't get registers: No such process." error message
> in various places.  For instance:
> 
>    (gdb) start
>    ...
>    (gdb) info inferior
>      Num  Description       Connection           Executable
>    * 1    process 31943     1 (native)           /tmp/a.out
>    (gdb) shell kill -9 31943
>    (gdb) maintenance flush register-cache
>    Register cache flushed.
>    Couldn't get registers: No such process.
>    (gdb) info threads
>      Id   Target Id              Frame
>    * 1    process 31943 "a.out" Couldn't get registers: No such process.
>    (gdb) backtrace
>    Python Exception <class 'gdb.error'>: Couldn't get registers: No such process.
>    Couldn't get registers: No such process.
>    (gdb) inferior 1
>    Couldn't get registers: No such process.
>    (gdb) thread
>    [Current thread is 1 (process 31943)]
>    Couldn't get registers: No such process.
>    (gdb)
> 
> The gdb.threads/killed-outside.exp, gdb.multi/multi-kill.exp, and
> gdb.multi/multi-exit.exp tests also check related scenarios.
> 
> To improve the situation,
> 
> 1. when printing the frame info, catch and process a NOT_AVAILABLE_ERROR.
> 
> 2. when accessing the target to fetch registers, if the operation
>     fails, raise a NOT_AVAILABLE_ERROR instead of a generic error, so
>     that clients can attempt to recover accordingly.  This patch updates
>     the amd64_linux_nat_target and remote_target in this direction.
> 
> With this patch, we obtain the following behavior:
> 
>    (gdb) start
>    ...
>    (gdb) info inferior
>      Num  Description       Connection           Executable
>    * 1    process 748       1 (native)           /tmp/a.out
>    (gdb) shell kill -9 748
>    (gdb) maintenance flush register-cache
>    Register cache flushed.
>    (gdb) info threads
>      Id   Target Id           Frame
>    * 1    process 748 "a.out" <PC register is not available>
>    (gdb) backtrace
>    #0  <PC register is not available>
>    Backtrace stopped: not enough registers or memory available to unwind further
>    (gdb) inferior 1
>    [Switching to inferior 1 [process 748] (/tmp/a.out)]
>    [Switching to thread 1 (process 748)]
>    #0  <PC register is not available>
>    (gdb) thread
>    [Current thread is 1 (process 748)]
>    (gdb)
> 
> Here is another "before/after" case.  Suppose we have two inferiors,
> each having its own remote target underneath.  Before this patch, we
> get the following output:
> 
>    # Create two inferiors on two remote targets, resume both until
>    # termination.  Exit event from one of them is shown first, but the
>    # other also exited -- just not yet shown.
>    (gdb) maint set target-non-stop on
>    (gdb) target remote | gdbserver - ./a.out
>    (gdb) add-inferior -no-connection
>    (gdb) inferior 2
>    (gdb) target remote | gdbserver - ./a.out
>    (gdb) set schedule-multiple on
>    (gdb) continue
>    ...
>    [Inferior 2 (process 22127) exited normally]
>    (gdb) inferior 1
>    [Switching to inferior 1 [process 22111] (target:/tmp/a.out)]
>    [Switching to thread 1.1 (Thread 22111.22111)]
>    Could not read registers; remote failure reply 'E01'
>    (gdb) info threads
>      Id   Target Id                  Frame
>    * 1.1  Thread 22111.22111 "a.out" Could not read registers; remote failure reply 'E01'
>    (gdb) backtrace
>    Python Exception <class 'gdb.error'>: Could not read registers; remote failure reply 'E01'
>    Could not read registers; remote failure reply 'E01'
>    (gdb) thread
>    [Current thread is 1.1 (Thread 22111.22111)]
>    Could not read registers; remote failure reply 'E01'
>    (gdb)
> 
> With this patch, it becomes:
> 
>    ...
>    [Inferior 1 (process 11759) exited normally]
>    (gdb) inferior 2
>    [Switching to inferior 2 [process 13440] (target:/path/to/a.out)]
>    [Switching to thread 2.1 (Thread 13440.13440)]
>    #0  <unavailable> in ?? ()
>    (gdb) info threads
>      Id   Target Id                   Frame
>    * 2.1  Thread 13440.13440 "a.out" <unavailable> in ?? ()
>    (gdb) backtrace
>    #0  <unavailable> in ?? ()
>    Backtrace stopped: not enough registers or memory available to unwind further
>    (gdb) thread
>    [Current thread is 2.1 (Thread 13440.13440)]
>    (gdb)
> 
> Finally, together with its predecessor, this patch also fixes PR gdb/26877.
> 
> Regression-tested on X86_64-Linux.
> ---
>   gdb/amd64-linux-nat.c                         |  5 +-
>   gdb/remote.c                                  | 15 ++--
>   gdb/stack.c                                   | 33 ++++++-
>   gdb/testsuite/gdb.threads/killed-outside.exp  |  8 +-
>   .../gdb.tui/multi-exit-remove-inferior.c      | 21 +++++
>   .../gdb.tui/multi-exit-remove-inferior.exp    | 86 +++++++++++++++++++
>   6 files changed, 156 insertions(+), 12 deletions(-)
>   create mode 100644 gdb/testsuite/gdb.tui/multi-exit-remove-inferior.c
>   create mode 100644 gdb/testsuite/gdb.tui/multi-exit-remove-inferior.exp
> 
> diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
> index f7f9a483def..aa9b10c52d1 100644
> --- a/gdb/amd64-linux-nat.c
> +++ b/gdb/amd64-linux-nat.c
> @@ -223,7 +223,10 @@ amd64_linux_nat_target::fetch_registers (struct regcache *regcache, int regnum)
>         elf_gregset_t regs;
>   
>         if (ptrace (PTRACE_GETREGS, tid, 0, (long) &regs) < 0)
> -	perror_with_name (_("Couldn't get registers"));
> +	{
> +	  std::string msg = perror_string (_("Couldn't get registers"));
> +	  throw_error (NOT_AVAILABLE_ERROR, "%s", msg.c_str ());
> +	}

Should other nat backends for other OS's (and other arches) also make this change when
failing to fetch registers?

-- 
John Baldwin