RE: [PATCH] gdb/remote: use current_inferior in read_ptid if multi-process not supported

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

On Monday, March 7, 2022 5:04 PM, Pedro Alves wrote:
> On 2022-02-09 07:58, Tankut Baris Aktemur via Gdb-patches wrote:
> 
> > --- a/gdb/remote.c
> > +++ b/gdb/remote.c
> > @@ -3145,14 +3145,15 @@ read_ptid (const char *buf, const char **obuf)
> >        return null_ptid;
> >      }
> >
> > -  /* Since the stub is not sending a process id, then default to
> > -     what's in inferior_ptid, unless it's null at this point.  If so,
> > +  /* Since the stub is not sending a process id, default to
> > +     what's current_inferior, unless it's null at this point.  If so,
> >       then since there's no way to know the pid of the reported
> >       threads, use the magic number.  */
> > -  if (inferior_ptid == null_ptid)
> > +  inferior *inf = current_inferior ();
> > +  if (inf == nullptr || inf->pid == 0)
> 
> How can current_inferior() ever by nullptr here?  That should not be possible, unless
> you're in some very early _initialize_foo routine, before the initial inferior was
> created.  There's is always an inferior selected, there's no way to select "no inferior".

You're right.  I remove the null check in v2.
 
> >      pid = magic_null_ptid.pid ();
> >    else
> > -    pid = inferior_ptid.pid ();
> > +    pid = inf->pid;
> >
> >    if (obuf)
> >      *obuf = pp;
> > diff --git a/gdb/testsuite/gdb.multi/attach-no-multi-process.c
> b/gdb/testsuite/gdb.multi/attach-no-multi-process.c
> > new file mode 100644
> > index 00000000000..df09dd69ccc
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.multi/attach-no-multi-process.c
> > @@ -0,0 +1,34 @@
> > +/* This testcase is part of GDB, the GNU debugger.
> > +
> > +   Copyright 2020-2022 Free Software Foundation, Inc.
> > +
> > +   This program is free software; you can redistribute it and/or modify
> > +   it under the terms of the GNU General Public License as published by
> > +   the Free Software Foundation; either version 3 of the License, or
> > +   (at your option) any later version.
> > +
> > +   This program is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +   GNU General Public License for more details.
> > +
> > +   You should have received a copy of the GNU General Public License
> > +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> > +
> > +/* This program is intended to be started outside of gdb, and then
> > +   attached to by GDB.  */
> 
> gdb vs GDB.  Use uppercase in both cases.

Done in v2.
 
> > +
> > +#include <unistd.h>
> > +
> > +int
> > +main ()
> > +{
> > +  /* Don't run forever in case GDB crashes and DejaGNU fails to kill
> > +     this program.  */
> > +  alarm (10);
> > +
> > +  while (1)
> > +    ;
> 
> Please add some "usleep (1);" or some such in the loop to avoid hogging
> the CPU.

Done in v2.

> > +
> > +# The plain remote target can't do multiple inferiors.
> 
> Extended-remote can.  And, you start a separate gdbserver anyhow.  What happens if
> you remove this check?  You'd need to pass
> "set remote multiprocess-feature-packet off" and "maint set target-non-stop"
> via GDBFLAGS instead of as separate commands after starting gdb, but is there
> anything else?

No, there isn't.  I see your point.  I made this change in v2.

Thanks
-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