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

["Aktemur, Tankut Baris" <tankut.baris.aktemur@intel.com>]

On Thursday, December 21, 2023 7:42 AM, Eli Zaretskii wrote:
> > Date: Wed, 20 Dec 2023 14:00:03 -0800
> > From: John Baldwin <jhb@FreeBSD.org>
> >
> > > --- 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?
> 
> That's probably a good idea, yes.

Yes.  I didn't do that, however, because I don't have a chance to do regression
testing for the other targets.

> But I also wonder whether the difference between
> 
>   Couldn't get registers: No such process.
> 
> or
> 
>   Could not read registers; remote failure reply 'E01'
> 
> and
> 
>   <PC register is not available>
> 
> or
> 
>   Backtrace stopped: not enough registers or memory available to unwind further
> 
> is such a significant improvement in terms of UX.  Both the "before"
> and "after" messages allude to issues that are extremely technical and
> obscure, IMO.  If anything, "No such process" sounds better to me than
> "<PC register is not available>", because the former explicitly
> explains the reason in high-level terms understood by anyone, while
> the latter alludes to the PC register, which is a GDB abstraction, and
> the fact that some register is not available doesn't necessarily tell
> me what is wrong in practical terms.
> 
> IOW, it seems to me that, when we catch the error, we ought to produce
> some meaningful message, and with this change we don't yet do that.
> 
> Does this make sense?

We can remember the reason in regcache for why a particular register became
unavailable and use that reason in the error messages.  With such a change,
the output for the first case would become

      (gdb) start
      ...
      (gdb) info inferior
        Num  Description       Connection           Executable
      * 1    process 1017286   1 (native)           /tmp/a.out
      (gdb) shell kill -9 1017286
      (gdb) maintenance flush register-cache
      Register cache flushed.
      (gdb) info threads
        Id   Target Id                                   Frame
      * 1    Thread 0x7ffff7d87740 (LWP 1017286) "a.out" <Couldn't get registers: No such process>
      (gdb) backtrace
      #0  <Couldn't get registers: No such process>
      Backtrace stopped: not enough registers or memory available to unwind further
      (gdb) inferior 1
      [Switching to inferior 1 [process 1017286] (/tmp/a.out)]
      [Switching to thread 1 (Thread 0x7ffff7d87740 (LWP 1017286))]
      #0  <Couldn't get registers: No such process>
      (gdb) thread
      [Current thread is 1 (Thread 0x7ffff7d87740 (LWP 1017286))]
      (gdb)

Does this look acceptable?

Regards
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928