Re: [PATCH v3] inferior without argument prints detail of current inferior

Lancelot SIX via Gdb-patches
2021-01-10T17:37:55Z

On Sun, Jan 10, 2021 at 11:00:28AM -0500, Simon Marchi wrote:
>
>
> On 2021-01-09 4:24 p.m., Lancelot SIX via Gdb-patches wrote:
> > This patch makes the inferior command display information about the
> > current inferior when called with no argument. This behavior is similar
> > to the one of the thread command.
> >
> > This contribution is inspired by an item in
> > https://sourceware.org/gdb/wiki/ProjectIdeas
> >
> > Before patch:
> >
> > (gdb) info inferior
> > Num Description Connection Executable
> > * 1 process 19221 1 (native) /home/lsix/tmp/a.out
> > 2 process 19239 1 (native) /home/lsix/tmp/a.out
> > (gdb) inferior 2
> > [Switching to inferior 2 [process 19239] (/home/lsix/tmp/a.out)]
> > [Switching to thread 2.1 (process 19239)]
> > #0 0x0000000000401146 in main ()
> > (gdb) inferior
> > Argument required (expression to compute).
> >
> > After patch:
> >
> > (gdb) info inferior
> > Num Description Connection Executable
> > * 1 process 18699 1 (native) /home/lsix/tmp/a.out
> > 2 process 18705 1 (native) /home/lsix/tmp/a.out
> > (gdb) inferior 2
> > [Switching to inferior 2 [process 18705] (/home/lsix/tmp/a.out)]
> > [Switching to thread 2.1 (process 18705)]
> > #0 0x0000000000401146 in main ()
> > (gdb) inferior
> > [Current inferior is 2 [process 18705] (/home/lsix/tmp/a.out)]
> >
> > Changes from V1:
> > * Add test.
> >
> > Change from V2:
> > * Fix gdb/doc/ChangeLog entry.
> > * My copyright assignment has been signed.
> >
> > gdb/doc/ChangeLog:
> >
> > * gdb.texinfo (Inferiors Connections and Programs): Document the
> > inferior command when used without argument.
> >
> > gdb/ChangeLog:
> >
> > * inferior.c (inferior_command): When no argument is given to the
> > inferior command, display info about the currently selected
> > inferior.
> >
> > gdb/testsuite/ChangeLog:
> >
> > * gdb.base/inferior-noarg.c: New test.
> > * gdb.base/inferior-noarg.exp: New test.
> >
> > ---
> > ---
> > gdb/doc/gdb.texinfo | 16 +++++++
> > gdb/inferior.c | 58 +++++++++++++++--------
> > gdb/testsuite/gdb.base/inferior-noarg.c | 22 +++++++++
> > gdb/testsuite/gdb.base/inferior-noarg.exp | 36 ++++++++++++++
> > 4 files changed, 111 insertions(+), 21 deletions(-)
> > create mode 100644 gdb/testsuite/gdb.base/inferior-noarg.c
> > create mode 100644 gdb/testsuite/gdb.base/inferior-noarg.exp
> >
> > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> > index 69fa6b709b..a97a342e9d 100644
> > --- a/gdb/doc/gdb.texinfo
> > +++ b/gdb/doc/gdb.texinfo
> > @@ -3202,6 +3202,22 @@ For example,
> > 2 process 2307 2 (extended-remote host:10000) hello
> > @end smallexample
> >
> > +To get informations about the current inferior, use @code{inferior}:
> > +
> > +@table @code
> > +@kindex inferior
> > +@item inferior
> > +Shows information about the current inferior.
> > +
> > +For example,
> > +@end table
> > +@c end table here to get a little more width for example
> > +
> > +@smallexample
> > +(@value{GDBP}) inferior
> > +[Current inferior is 1 [process 3401] (helloworld)]
> > +@end smallexample
> > +
> > To find out what open target connections exist at any moment, use
> > @w{@code{info connections}}:
> >
> > diff --git a/gdb/inferior.c b/gdb/inferior.c
> > index 3ff0512c38..49f869a4c7 100644
> > --- a/gdb/inferior.c
> > +++ b/gdb/inferior.c
> > @@ -635,34 +635,50 @@ inferior_command (const char *args, int from_tty)
> > struct inferior *inf;
> > int num;
> >
> > - num = parse_and_eval_long (args);
> > -
> > - inf = find_inferior_id (num);
> > - if (inf == NULL)
> > - error (_("Inferior ID %d not known."), num);
> > -
> > - if (inf->pid != 0)
> > + if (args == nullptr)
> > {
> > - if (inf != current_inferior ())
> > - {
> > - thread_info *tp = any_thread_of_inferior (inf);
> > - if (tp == NULL)
> > - error (_("Inferior has no threads."));
> > + inf = current_inferior ();
> > + gdb_assert (inf != nullptr);
> > + const char *filename = inf->pspace->exec_filename.get ();
> >
> > - switch_to_thread (tp);
> > - }
> > + if (filename == nullptr)
> > + filename = _("<noexec>");
> >
> > - gdb::observers::user_selected_context_changed.notify
> > - (USER_SELECTED_INFERIOR
> > - | USER_SELECTED_THREAD
> > - | USER_SELECTED_FRAME);
> > + printf_filtered (_("[Current inferior is %d [%s] (%s)]\n"),
> > + inf->num, inferior_pid_to_str (inf->pid).c_str (),
> > + filename);
> > }
> > else
> > {
> > - switch_to_inferior_no_thread (inf);
> > + num = parse_and_eval_long (args);
> > +
> > + inf = find_inferior_id (num);
> > + if (inf == NULL)
> > + error (_("Inferior ID %d not known."), num);
> > +
> > + if (inf->pid != 0)
> > + {
> > + if (inf != current_inferior ())
> > + {
> > + thread_info *tp = any_thread_of_inferior (inf);
> > + if (tp == NULL)
> > + error (_("Inferior has no threads."));
> >
> > - gdb::observers::user_selected_context_changed.notify
> > - (USER_SELECTED_INFERIOR);
> > + switch_to_thread (tp);
> > + }
> > +
> > + gdb::observers::user_selected_context_changed.notify
> > + (USER_SELECTED_INFERIOR
> > + | USER_SELECTED_THREAD
> > + | USER_SELECTED_FRAME);
> > + }
> > + else
> > + {
> > + switch_to_inferior_no_thread (inf);
> > +
> > + gdb::observers::user_selected_context_changed.notify
> > + (USER_SELECTED_INFERIOR);
> > + }
> > }
> > }
> >
> > diff --git a/gdb/testsuite/gdb.base/inferior-noarg.c b/gdb/testsuite/gdb.base/inferior-noarg.c
> > new file mode 100644
> > index 0000000000..9d7b2f1a4c
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/inferior-noarg.c
> > @@ -0,0 +1,22 @@
> > +/* This testcase is part of GDB, the GNU debugger.
> > +
> > + Copyright 2020 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/>. */
> > +
> > +int
> > +main (void)
> > +{
> > + return 0;
> > +}
> > diff --git a/gdb/testsuite/gdb.base/inferior-noarg.exp b/gdb/testsuite/gdb.base/inferior-noarg.exp
> > new file mode 100644
> > index 0000000000..38842aa7d8
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/inferior-noarg.exp
> > @@ -0,0 +1,36 @@
> > +# Copyright 2020 Free Software Foundation, Inc.
>
> Change both copyright years to 2021.
>
> > +
> > +# 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 test case checks that the 'inferior' command, when given no
> > +# argument, displays information about the inferior currently active.
> > +
> > +standard_testfile
> > +
> > +if { [prepare_for_testing "failed to prepare" \
> > + ${testfile} ${srcfile}] } {
> > + return
> > +}
> > +
> > +gdb_test "inferior" "\[Current inferior is 1 \[<null>\] (.*)\]" "inferior not running"
> > +
> > +if { ![runto_main] } {
> > + untested "could not run to main"
> > + return
> > +}
> > +
> > +gdb_test "inferior" "\[Current inferior is 1 \[process .*\] (.*)\]" "inferior running"
>
> I'm worried that this regexp is a little bit too specific, as targets can
> implement the `pid_to_str` method and decide to call their "process"
> something else. "process 1234" is just the default, done by normal_pid_to_str.
>
> I'm fine with merging it though, as that will probably work with all in-tree
> targets. The only one I found where it wouldn't work in bsd_kvm_target, which
> does:
>
> std::string
> bsd_kvm_target::pid_to_str (ptid_t ptid)
> {
> return "<kvm>";
> }
>

I’ll send shortly an updated version that removes this behavior
assumption and accept anything pid_to_str can throw at it.

> But I don't think the testsuite is ran against that target anywhere... and in
> any case it's an easy fix if we stumble on a target for which it doesn't work.
>
> Do you have push access? If you plan on sending more patches, it would make
> sense for you to have it, so you can push your own patches once approved. Let
> me know if you'd like to do that.

No, I do not, nor do I have an account on sourceware.org. Given that
I’ll try to continue to contribute (to the extend of what my spare time
allows me to), I guess this would make sence! Should I follow the
procedure describe in https://sourceware.org/cgi-bin/pdw/ps_form.cgi and
provide your email as reference?

Thanks
Lancelot.

>
> Simon