* Making mi -add-inferior command consistent with ci @ 2022-02-21 21:08 Muhammad Umair Sair 2022-02-22 14:08 ` Andrew Burgess 0 siblings, 1 reply; 5+ messages in thread From: Muhammad Umair Sair @ 2022-02-21 21:08 UTC (permalink / raw) To: gdb-patches From 3729f798d1f75a74eefe0c240d8c4c9849dac308 Mon Sep 17 00:00:00 2001 From: Umair Sair <umair_sair@hotmail.com> Date: Tue, 22 Feb 2022 01:40:06 +0500 Subject: [PATCH] Making mi -add-inferior command consistent with ci With multi-target debugging support in gdb, add-inferior command without arguments in console interpreter adds a new inferior and sets the connection of previously selected inferior as the connection of new inferior as well. But the same command for machine interpreter is not setting the connection for new inferior. This commit fixes this issue and makes the console and machine interpreter behavior consistent for add-inferior command. --- gdb/ChangeLog | 9 +++++++++ gdb/inferior.c | 4 ++-- gdb/inferior.h | 5 ++++- gdb/mi/mi-main.c | 4 +++- 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index dfee3dad157..1f342cc628f 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,12 @@ +2022-02-22 Umair Sair <umair_sair@hotmail.com> + + * gdb/inferior.c (switch_to_inferior_and_push_target): allowing + access to this function outside of file. + * gdb/inferior.h (switch_to_inferior_and_push_target): added extern + entry for function in header. + * gdb/mi/mi-main.c (mi_cmd_add_inferior): new inferior should use + the connection from previously selected inferior. + 2021-04-23 Tom Tromey <tromey@adacore.com> PR gdb/27743: diff --git a/gdb/inferior.c b/gdb/inferior.c index 4fdddbcb55b..16e4279bc1a 100644 --- a/gdb/inferior.c +++ b/gdb/inferior.c @@ -1,6 +1,6 @@ /* Multi-process control for GDB, the GNU debugger. - Copyright (C) 2008-2021 Free Software Foundation, Inc. + Copyright (C) 2008-2022 Free Software Foundation, Inc. This file is part of GDB. @@ -740,7 +740,7 @@ add_inferior_with_spaces (void) NO_CONNECTION is true, push the process_stratum_target of ORG_INF to NEW_INF. */ -static void +void switch_to_inferior_and_push_target (inferior *new_inf, bool no_connection, inferior *org_inf) { diff --git a/gdb/inferior.h b/gdb/inferior.h index fb7d0c2bc1a..d4abd00489a 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -1,7 +1,7 @@ /* Variables that describe the inferior process running under GDB: Where it is, why it stopped, and how to step it. - Copyright (C) 1986-2021 Free Software Foundation, Inc. + Copyright (C) 1986-2022 Free Software Foundation, Inc. This file is part of GDB. @@ -686,4 +686,7 @@ extern struct inferior *add_inferior_with_spaces (void); /* Print the current selected inferior. */ extern void print_selected_inferior (struct ui_out *uiout); +extern void switch_to_inferior_and_push_target (inferior *new_inf, + bool no_connection, inferior *org_inf); + #endif /* !defined (INFERIOR_H) */ diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index e38520a98e6..65c0aa5be0b 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -1,6 +1,6 @@ /* MI Command Set. - Copyright (C) 2000-2021 Free Software Foundation, Inc. + Copyright (C) 2000-2022 Free Software Foundation, Inc. Contributed by Cygnus Solutions (a Red Hat company). @@ -1702,6 +1702,8 @@ mi_cmd_add_inferior (const char *command, char **argv, int argc) inf = add_inferior_with_spaces (); + switch_to_inferior_and_push_target (inf, false, current_inferior ()); + current_uiout->field_fmt ("inferior", "i%d", inf->num); } -- 2.31.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Making mi -add-inferior command consistent with ci 2022-02-21 21:08 Making mi -add-inferior command consistent with ci Muhammad Umair Sair @ 2022-02-22 14:08 ` Andrew Burgess 2022-02-22 16:56 ` Muhammad Umair Sair 0 siblings, 1 reply; 5+ messages in thread From: Andrew Burgess @ 2022-02-22 14:08 UTC (permalink / raw) To: Muhammad Umair Sair, gdb-patches Muhammad Umair Sair via Gdb-patches <gdb-patches@sourceware.org> writes: > From 3729f798d1f75a74eefe0c240d8c4c9849dac308 Mon Sep 17 00:00:00 2001 > From: Umair Sair <umair_sair@hotmail.com> > Date: Tue, 22 Feb 2022 01:40:06 +0500 > Subject: [PATCH] Making mi -add-inferior command consistent with ci > > With multi-target debugging support in gdb, add-inferior command without > arguments in console interpreter adds a new inferior and sets the connection > of previously selected inferior as the connection of new inferior as well. > But the same command for machine interpreter is not setting the connection > for new inferior. This commit fixes this issue and makes the console and > machine interpreter behavior consistent for add-inferior command. We're usually pretty conservative about changing MI behaviour in non backward compatible ways. I'm not entirely sure that this change wont cause problems for an existing f/e that is using -add-inferior? Maybe I'm worrying about nothing? I'm open to being convinced. Another option would be to add a new option, e.g.: -add-inferior [--connection NUM] or -add-inferior [--inherit-connection] > --- > gdb/ChangeLog | 9 +++++++++ > gdb/inferior.c | 4 ++-- > gdb/inferior.h | 5 ++++- > gdb/mi/mi-main.c | 4 +++- > 4 files changed, 18 insertions(+), 4 deletions(-) > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index dfee3dad157..1f342cc628f 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,3 +1,12 @@ > +2022-02-22 Umair Sair <umair_sair@hotmail.com> > + > + * gdb/inferior.c (switch_to_inferior_and_push_target): allowing > + access to this function outside of file. > + * gdb/inferior.h (switch_to_inferior_and_push_target): added extern > + entry for function in header. > + * gdb/mi/mi-main.c (mi_cmd_add_inferior): new inferior should use > + the connection from previously selected inferior. > + We no longer maintain ChangeLog in gdb, so this part is not needed. You're welcome to keep the ChangeLog text in the commit message in _addition_ to the long form description, but this is not a requirement. > 2021-04-23 Tom Tromey <tromey@adacore.com> > > PR gdb/27743: > diff --git a/gdb/inferior.c b/gdb/inferior.c > index 4fdddbcb55b..16e4279bc1a 100644 > --- a/gdb/inferior.c > +++ b/gdb/inferior.c > @@ -1,6 +1,6 @@ > /* Multi-process control for GDB, the GNU debugger. > > - Copyright (C) 2008-2021 Free Software Foundation, Inc. > + Copyright (C) 2008-2022 Free Software Foundation, Inc. > Not sure what's going on here. The patch shows all the dates changing, but all the files you've changed already have 2022 in the upstream master branch. Thanks, Andrew > This file is part of GDB. > > @@ -740,7 +740,7 @@ add_inferior_with_spaces (void) > NO_CONNECTION is true, push the process_stratum_target of ORG_INF > to NEW_INF. */ > > -static void > +void > switch_to_inferior_and_push_target (inferior *new_inf, > bool no_connection, inferior *org_inf) > { > diff --git a/gdb/inferior.h b/gdb/inferior.h > index fb7d0c2bc1a..d4abd00489a 100644 > --- a/gdb/inferior.h > +++ b/gdb/inferior.h > @@ -1,7 +1,7 @@ > /* Variables that describe the inferior process running under GDB: > Where it is, why it stopped, and how to step it. > > - Copyright (C) 1986-2021 Free Software Foundation, Inc. > + Copyright (C) 1986-2022 Free Software Foundation, Inc. > > This file is part of GDB. > > @@ -686,4 +686,7 @@ extern struct inferior *add_inferior_with_spaces (void); > /* Print the current selected inferior. */ > extern void print_selected_inferior (struct ui_out *uiout); > > +extern void switch_to_inferior_and_push_target (inferior *new_inf, > + bool no_connection, inferior *org_inf); > + > #endif /* !defined (INFERIOR_H) */ > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c > index e38520a98e6..65c0aa5be0b 100644 > --- a/gdb/mi/mi-main.c > +++ b/gdb/mi/mi-main.c > @@ -1,6 +1,6 @@ > /* MI Command Set. > > - Copyright (C) 2000-2021 Free Software Foundation, Inc. > + Copyright (C) 2000-2022 Free Software Foundation, Inc. > > Contributed by Cygnus Solutions (a Red Hat company). > > @@ -1702,6 +1702,8 @@ mi_cmd_add_inferior (const char *command, char **argv, int argc) > > inf = add_inferior_with_spaces (); > > + switch_to_inferior_and_push_target (inf, false, current_inferior ()); > + > current_uiout->field_fmt ("inferior", "i%d", inf->num); > } > > -- > 2.31.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Making mi -add-inferior command consistent with ci 2022-02-22 14:08 ` Andrew Burgess @ 2022-02-22 16:56 ` Muhammad Umair Sair 2022-02-23 14:37 ` Andrew Burgess 0 siblings, 1 reply; 5+ messages in thread From: Muhammad Umair Sair @ 2022-02-22 16:56 UTC (permalink / raw) To: Andrew Burgess, gdb-patches On 2/22/22 7:08 PM, Andrew Burgess wrote: > Muhammad Umair Sair via Gdb-patches <gdb-patches@sourceware.org> writes: > >> From 3729f798d1f75a74eefe0c240d8c4c9849dac308 Mon Sep 17 00:00:00 2001 >> From: Umair Sair <umair_sair@hotmail.com> >> Date: Tue, 22 Feb 2022 01:40:06 +0500 >> Subject: [PATCH] Making mi -add-inferior command consistent with ci >> >> With multi-target debugging support in gdb, add-inferior command without >> arguments in console interpreter adds a new inferior and sets the connection >> of previously selected inferior as the connection of new inferior as well. >> But the same command for machine interpreter is not setting the connection >> for new inferior. This commit fixes this issue and makes the console and >> machine interpreter behavior consistent for add-inferior command. > > We're usually pretty conservative about changing MI behaviour in non > backward compatible ways. I'm not entirely sure that this change wont > cause problems for an existing f/e that is using -add-inferior? > > Maybe I'm worrying about nothing? I'm open to being convinced. The older change broke it in non backward compatibility way and this commit is fixing it. I faced issues compared to older versions of gdb and on investigation I found this issue. Following is the commit where we supported multi-target. https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=5b6d1e4fa4fc6827c7b3f0e99ff120dfa14d65d2 > > Another option would be to add a new option, e.g.: > > -add-inferior [--connection NUM] > > or > > -add-inferior [--inherit-connection] > IMO we should support the same options as in console. Currently we don't allow any arguments for -add-inferior MI command though console add-inferior command allows several arguments. I was thinking to support at least "--no-connection" argument which is also supported in console. IMO we can add it separate commit and keep this commit for fixing the regression. WDYS? > >> --- >> gdb/ChangeLog | 9 +++++++++ >> gdb/inferior.c | 4 ++-- >> gdb/inferior.h | 5 ++++- >> gdb/mi/mi-main.c | 4 +++- >> 4 files changed, 18 insertions(+), 4 deletions(-) >> >> diff --git a/gdb/ChangeLog b/gdb/ChangeLog >> index dfee3dad157..1f342cc628f 100644 >> --- a/gdb/ChangeLog >> +++ b/gdb/ChangeLog >> @@ -1,3 +1,12 @@ >> +2022-02-22 Umair Sair <umair_sair@hotmail.com> >> + >> + * gdb/inferior.c (switch_to_inferior_and_push_target): allowing >> + access to this function outside of file. >> + * gdb/inferior.h (switch_to_inferior_and_push_target): added extern >> + entry for function in header. >> + * gdb/mi/mi-main.c (mi_cmd_add_inferior): new inferior should use >> + the connection from previously selected inferior. >> + > > We no longer maintain ChangeLog in gdb, so this part is not needed. > You're welcome to keep the ChangeLog text in the commit message in > _addition_ to the long form description, but this is not a requirement. > Yes. I actually created this patch for gdb-10-branch. Is there any policy to submit patches for older releases? Or should I submit patch for master? >> 2021-04-23 Tom Tromey <tromey@adacore.com> >> >> PR gdb/27743: >> diff --git a/gdb/inferior.c b/gdb/inferior.c >> index 4fdddbcb55b..16e4279bc1a 100644 >> --- a/gdb/inferior.c >> +++ b/gdb/inferior.c >> @@ -1,6 +1,6 @@ >> /* Multi-process control for GDB, the GNU debugger. >> >> - Copyright (C) 2008-2021 Free Software Foundation, Inc. >> + Copyright (C) 2008-2022 Free Software Foundation, Inc. >> > > Not sure what's going on here. The patch shows all the dates changing, > but all the files you've changed already have 2022 in the upstream > master branch. The patch was for gdb-10-branch. I'll update patch according to the responses of queries above. Thanks, Umair Sair > > Thanks, > Andrew > >> This file is part of GDB. >> >> @@ -740,7 +740,7 @@ add_inferior_with_spaces (void) >> NO_CONNECTION is true, push the process_stratum_target of ORG_INF >> to NEW_INF. */ >> >> -static void >> +void >> switch_to_inferior_and_push_target (inferior *new_inf, >> bool no_connection, inferior *org_inf) >> { >> diff --git a/gdb/inferior.h b/gdb/inferior.h >> index fb7d0c2bc1a..d4abd00489a 100644 >> --- a/gdb/inferior.h >> +++ b/gdb/inferior.h >> @@ -1,7 +1,7 @@ >> /* Variables that describe the inferior process running under GDB: >> Where it is, why it stopped, and how to step it. >> >> - Copyright (C) 1986-2021 Free Software Foundation, Inc. >> + Copyright (C) 1986-2022 Free Software Foundation, Inc. >> >> This file is part of GDB. >> >> @@ -686,4 +686,7 @@ extern struct inferior *add_inferior_with_spaces (void); >> /* Print the current selected inferior. */ >> extern void print_selected_inferior (struct ui_out *uiout); >> >> +extern void switch_to_inferior_and_push_target (inferior *new_inf, >> + bool no_connection, inferior *org_inf); >> + >> #endif /* !defined (INFERIOR_H) */ >> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c >> index e38520a98e6..65c0aa5be0b 100644 >> --- a/gdb/mi/mi-main.c >> +++ b/gdb/mi/mi-main.c >> @@ -1,6 +1,6 @@ >> /* MI Command Set. >> >> - Copyright (C) 2000-2021 Free Software Foundation, Inc. >> + Copyright (C) 2000-2022 Free Software Foundation, Inc. >> >> Contributed by Cygnus Solutions (a Red Hat company). >> >> @@ -1702,6 +1702,8 @@ mi_cmd_add_inferior (const char *command, char **argv, int argc) >> >> inf = add_inferior_with_spaces (); >> >> + switch_to_inferior_and_push_target (inf, false, current_inferior ()); >> + >> current_uiout->field_fmt ("inferior", "i%d", inf->num); >> } >> >> -- >> 2.31.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Making mi -add-inferior command consistent with ci 2022-02-22 16:56 ` Muhammad Umair Sair @ 2022-02-23 14:37 ` Andrew Burgess 2022-02-24 17:27 ` Muhammad Umair Sair 0 siblings, 1 reply; 5+ messages in thread From: Andrew Burgess @ 2022-02-23 14:37 UTC (permalink / raw) To: Muhammad Umair Sair, gdb-patches Muhammad Umair Sair via Gdb-patches <gdb-patches@sourceware.org> writes: > On 2/22/22 7:08 PM, Andrew Burgess wrote: >> Muhammad Umair Sair via Gdb-patches <gdb-patches@sourceware.org> writes: >> >>> From 3729f798d1f75a74eefe0c240d8c4c9849dac308 Mon Sep 17 00:00:00 2001 >>> From: Umair Sair <umair_sair@hotmail.com> >>> Date: Tue, 22 Feb 2022 01:40:06 +0500 >>> Subject: [PATCH] Making mi -add-inferior command consistent with ci >>> >>> With multi-target debugging support in gdb, add-inferior command without >>> arguments in console interpreter adds a new inferior and sets the connection >>> of previously selected inferior as the connection of new inferior as well. >>> But the same command for machine interpreter is not setting the connection >>> for new inferior. This commit fixes this issue and makes the console and >>> machine interpreter behavior consistent for add-inferior command. >> >> We're usually pretty conservative about changing MI behaviour in non >> backward compatible ways. I'm not entirely sure that this change wont >> cause problems for an existing f/e that is using -add-inferior? >> >> Maybe I'm worrying about nothing? I'm open to being convinced. > > The older change broke it in non backward compatibility way and this > commit is fixing it. I faced issues compared to older versions of gdb > and on investigation I found this issue. Following is the commit where > we supported multi-target. Ah, OK. I didn't get that from the commit message. Maybe it would be worth expanding the commit message to make it clearer that this is a regression. Ideally, include an example of how GDB used to behave, and the now broken behaviour, highlighting how things have regressed. > > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=5b6d1e4fa4fc6827c7b3f0e99ff120dfa14d65d2 > >> >> Another option would be to add a new option, e.g.: >> >> -add-inferior [--connection NUM] >> >> or >> >> -add-inferior [--inherit-connection] >> > > IMO we should support the same options as in console. Currently we don't > allow any arguments for -add-inferior MI command though console > add-inferior command allows several arguments. I was thinking to support > at least "--no-connection" argument which is also supported in console. > IMO we can add it separate commit and keep this commit for fixing the > regression. WDYS? I'd agree that ideally the same functionality would be available via the MI as from the CLI. As I said in my previous commit we try very hard to maintain backward compatibility for the MI, so I'm pretty sure there are some MI commands where the default command behaviour is not the same as the CLI, and there are extra/different flags for the MI to allow CLI like behaviour. Though I can't think of any examples off the top of my head, so maybe I'm wrong. I 100% agree that adding new flags should be done in a separate commit though. Additionally, I forgot to mention in the first reviewl; we really should have a test for this behaviour. Thanks, Andrew > >> >>> --- >>> gdb/ChangeLog | 9 +++++++++ >>> gdb/inferior.c | 4 ++-- >>> gdb/inferior.h | 5 ++++- >>> gdb/mi/mi-main.c | 4 +++- >>> 4 files changed, 18 insertions(+), 4 deletions(-) >>> >>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog >>> index dfee3dad157..1f342cc628f 100644 >>> --- a/gdb/ChangeLog >>> +++ b/gdb/ChangeLog >>> @@ -1,3 +1,12 @@ >>> +2022-02-22 Umair Sair <umair_sair@hotmail.com> >>> + >>> + * gdb/inferior.c (switch_to_inferior_and_push_target): allowing >>> + access to this function outside of file. >>> + * gdb/inferior.h (switch_to_inferior_and_push_target): added extern >>> + entry for function in header. >>> + * gdb/mi/mi-main.c (mi_cmd_add_inferior): new inferior should use >>> + the connection from previously selected inferior. >>> + >> >> We no longer maintain ChangeLog in gdb, so this part is not needed. >> You're welcome to keep the ChangeLog text in the commit message in >> _addition_ to the long form description, but this is not a requirement. >> > > Yes. I actually created this patch for gdb-10-branch. Is there any > policy to submit patches for older releases? Or should I submit patch > for master? > >>> 2021-04-23 Tom Tromey <tromey@adacore.com> >>> >>> PR gdb/27743: >>> diff --git a/gdb/inferior.c b/gdb/inferior.c >>> index 4fdddbcb55b..16e4279bc1a 100644 >>> --- a/gdb/inferior.c >>> +++ b/gdb/inferior.c >>> @@ -1,6 +1,6 @@ >>> /* Multi-process control for GDB, the GNU debugger. >>> >>> - Copyright (C) 2008-2021 Free Software Foundation, Inc. >>> + Copyright (C) 2008-2022 Free Software Foundation, Inc. >>> >> >> Not sure what's going on here. The patch shows all the dates changing, >> but all the files you've changed already have 2022 in the upstream >> master branch. > > The patch was for gdb-10-branch. I'll update patch according to the > responses of queries above. > > Thanks, > Umair Sair > >> >> Thanks, >> Andrew >> >>> This file is part of GDB. >>> >>> @@ -740,7 +740,7 @@ add_inferior_with_spaces (void) >>> NO_CONNECTION is true, push the process_stratum_target of ORG_INF >>> to NEW_INF. */ >>> >>> -static void >>> +void >>> switch_to_inferior_and_push_target (inferior *new_inf, >>> bool no_connection, inferior *org_inf) >>> { >>> diff --git a/gdb/inferior.h b/gdb/inferior.h >>> index fb7d0c2bc1a..d4abd00489a 100644 >>> --- a/gdb/inferior.h >>> +++ b/gdb/inferior.h >>> @@ -1,7 +1,7 @@ >>> /* Variables that describe the inferior process running under GDB: >>> Where it is, why it stopped, and how to step it. >>> >>> - Copyright (C) 1986-2021 Free Software Foundation, Inc. >>> + Copyright (C) 1986-2022 Free Software Foundation, Inc. >>> >>> This file is part of GDB. >>> >>> @@ -686,4 +686,7 @@ extern struct inferior *add_inferior_with_spaces (void); >>> /* Print the current selected inferior. */ >>> extern void print_selected_inferior (struct ui_out *uiout); >>> >>> +extern void switch_to_inferior_and_push_target (inferior *new_inf, >>> + bool no_connection, inferior *org_inf); >>> + >>> #endif /* !defined (INFERIOR_H) */ >>> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c >>> index e38520a98e6..65c0aa5be0b 100644 >>> --- a/gdb/mi/mi-main.c >>> +++ b/gdb/mi/mi-main.c >>> @@ -1,6 +1,6 @@ >>> /* MI Command Set. >>> >>> - Copyright (C) 2000-2021 Free Software Foundation, Inc. >>> + Copyright (C) 2000-2022 Free Software Foundation, Inc. >>> >>> Contributed by Cygnus Solutions (a Red Hat company). >>> >>> @@ -1702,6 +1702,8 @@ mi_cmd_add_inferior (const char *command, char **argv, int argc) >>> >>> inf = add_inferior_with_spaces (); >>> >>> + switch_to_inferior_and_push_target (inf, false, current_inferior ()); >>> + >>> current_uiout->field_fmt ("inferior", "i%d", inf->num); >>> } >>> >>> -- >>> 2.31.1 >> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Making mi -add-inferior command consistent with ci 2022-02-23 14:37 ` Andrew Burgess @ 2022-02-24 17:27 ` Muhammad Umair Sair 0 siblings, 0 replies; 5+ messages in thread From: Muhammad Umair Sair @ 2022-02-24 17:27 UTC (permalink / raw) To: Andrew Burgess, gdb-patches Submitted a new patch. https://sourceware.org/pipermail/gdb-patches/2022-February/186105.html - Umair On 2/23/22 7:37 PM, Andrew Burgess wrote: > Muhammad Umair Sair via Gdb-patches <gdb-patches@sourceware.org> writes: > >> On 2/22/22 7:08 PM, Andrew Burgess wrote: >>> Muhammad Umair Sair via Gdb-patches <gdb-patches@sourceware.org> writes: >>> >>>> From 3729f798d1f75a74eefe0c240d8c4c9849dac308 Mon Sep 17 00:00:00 2001 >>>> From: Umair Sair <umair_sair@hotmail.com> >>>> Date: Tue, 22 Feb 2022 01:40:06 +0500 >>>> Subject: [PATCH] Making mi -add-inferior command consistent with ci >>>> >>>> With multi-target debugging support in gdb, add-inferior command without >>>> arguments in console interpreter adds a new inferior and sets the connection >>>> of previously selected inferior as the connection of new inferior as well. >>>> But the same command for machine interpreter is not setting the connection >>>> for new inferior. This commit fixes this issue and makes the console and >>>> machine interpreter behavior consistent for add-inferior command. >>> >>> We're usually pretty conservative about changing MI behaviour in non >>> backward compatible ways. I'm not entirely sure that this change wont >>> cause problems for an existing f/e that is using -add-inferior? >>> >>> Maybe I'm worrying about nothing? I'm open to being convinced. >> >> The older change broke it in non backward compatibility way and this >> commit is fixing it. I faced issues compared to older versions of gdb >> and on investigation I found this issue. Following is the commit where >> we supported multi-target. > > Ah, OK. I didn't get that from the commit message. Maybe it would be > worth expanding the commit message to make it clearer that this is a > regression. Ideally, include an example of how GDB used to behave, and > the now broken behaviour, highlighting how things have regressed. > >> >> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=5b6d1e4fa4fc6827c7b3f0e99ff120dfa14d65d2 >> >>> >>> Another option would be to add a new option, e.g.: >>> >>> -add-inferior [--connection NUM] >>> >>> or >>> >>> -add-inferior [--inherit-connection] >>> >> >> IMO we should support the same options as in console. Currently we don't >> allow any arguments for -add-inferior MI command though console >> add-inferior command allows several arguments. I was thinking to support >> at least "--no-connection" argument which is also supported in console. >> IMO we can add it separate commit and keep this commit for fixing the >> regression. WDYS? > > I'd agree that ideally the same functionality would be available via the > MI as from the CLI. As I said in my previous commit we try very hard to > maintain backward compatibility for the MI, so I'm pretty sure there are > some MI commands where the default command behaviour is not the same as > the CLI, and there are extra/different flags for the MI to allow CLI > like behaviour. Though I can't think of any examples off the top of my > head, so maybe I'm wrong. > > I 100% agree that adding new flags should be done in a separate commit > though. > > Additionally, I forgot to mention in the first reviewl; we really should > have a test for this behaviour. > > Thanks, > Andrew > >> >>> >>>> --- >>>> gdb/ChangeLog | 9 +++++++++ >>>> gdb/inferior.c | 4 ++-- >>>> gdb/inferior.h | 5 ++++- >>>> gdb/mi/mi-main.c | 4 +++- >>>> 4 files changed, 18 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog >>>> index dfee3dad157..1f342cc628f 100644 >>>> --- a/gdb/ChangeLog >>>> +++ b/gdb/ChangeLog >>>> @@ -1,3 +1,12 @@ >>>> +2022-02-22 Umair Sair <umair_sair@hotmail.com> >>>> + >>>> + * gdb/inferior.c (switch_to_inferior_and_push_target): allowing >>>> + access to this function outside of file. >>>> + * gdb/inferior.h (switch_to_inferior_and_push_target): added extern >>>> + entry for function in header. >>>> + * gdb/mi/mi-main.c (mi_cmd_add_inferior): new inferior should use >>>> + the connection from previously selected inferior. >>>> + >>> >>> We no longer maintain ChangeLog in gdb, so this part is not needed. >>> You're welcome to keep the ChangeLog text in the commit message in >>> _addition_ to the long form description, but this is not a requirement. >>> >> >> Yes. I actually created this patch for gdb-10-branch. Is there any >> policy to submit patches for older releases? Or should I submit patch >> for master? >> >>>> 2021-04-23 Tom Tromey <tromey@adacore.com> >>>> >>>> PR gdb/27743: >>>> diff --git a/gdb/inferior.c b/gdb/inferior.c >>>> index 4fdddbcb55b..16e4279bc1a 100644 >>>> --- a/gdb/inferior.c >>>> +++ b/gdb/inferior.c >>>> @@ -1,6 +1,6 @@ >>>> /* Multi-process control for GDB, the GNU debugger. >>>> >>>> - Copyright (C) 2008-2021 Free Software Foundation, Inc. >>>> + Copyright (C) 2008-2022 Free Software Foundation, Inc. >>>> >>> >>> Not sure what's going on here. The patch shows all the dates changing, >>> but all the files you've changed already have 2022 in the upstream >>> master branch. >> >> The patch was for gdb-10-branch. I'll update patch according to the >> responses of queries above. >> >> Thanks, >> Umair Sair >> >>> >>> Thanks, >>> Andrew >>> >>>> This file is part of GDB. >>>> >>>> @@ -740,7 +740,7 @@ add_inferior_with_spaces (void) >>>> NO_CONNECTION is true, push the process_stratum_target of ORG_INF >>>> to NEW_INF. */ >>>> >>>> -static void >>>> +void >>>> switch_to_inferior_and_push_target (inferior *new_inf, >>>> bool no_connection, inferior *org_inf) >>>> { >>>> diff --git a/gdb/inferior.h b/gdb/inferior.h >>>> index fb7d0c2bc1a..d4abd00489a 100644 >>>> --- a/gdb/inferior.h >>>> +++ b/gdb/inferior.h >>>> @@ -1,7 +1,7 @@ >>>> /* Variables that describe the inferior process running under GDB: >>>> Where it is, why it stopped, and how to step it. >>>> >>>> - Copyright (C) 1986-2021 Free Software Foundation, Inc. >>>> + Copyright (C) 1986-2022 Free Software Foundation, Inc. >>>> >>>> This file is part of GDB. >>>> >>>> @@ -686,4 +686,7 @@ extern struct inferior *add_inferior_with_spaces (void); >>>> /* Print the current selected inferior. */ >>>> extern void print_selected_inferior (struct ui_out *uiout); >>>> >>>> +extern void switch_to_inferior_and_push_target (inferior *new_inf, >>>> + bool no_connection, inferior *org_inf); >>>> + >>>> #endif /* !defined (INFERIOR_H) */ >>>> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c >>>> index e38520a98e6..65c0aa5be0b 100644 >>>> --- a/gdb/mi/mi-main.c >>>> +++ b/gdb/mi/mi-main.c >>>> @@ -1,6 +1,6 @@ >>>> /* MI Command Set. >>>> >>>> - Copyright (C) 2000-2021 Free Software Foundation, Inc. >>>> + Copyright (C) 2000-2022 Free Software Foundation, Inc. >>>> >>>> Contributed by Cygnus Solutions (a Red Hat company). >>>> >>>> @@ -1702,6 +1702,8 @@ mi_cmd_add_inferior (const char *command, char **argv, int argc) >>>> >>>> inf = add_inferior_with_spaces (); >>>> >>>> + switch_to_inferior_and_push_target (inf, false, current_inferior ()); >>>> + >>>> current_uiout->field_fmt ("inferior", "i%d", inf->num); >>>> } >>>> >>>> -- >>>> 2.31.1 >>> > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-02-24 17:27 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-21 21:08 Making mi -add-inferior command consistent with ci Muhammad Umair Sair 2022-02-22 14:08 ` Andrew Burgess 2022-02-22 16:56 ` Muhammad Umair Sair 2022-02-23 14:37 ` Andrew Burgess 2022-02-24 17:27 ` Muhammad Umair Sair
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).