public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* Re: [PING] Re: [PATCH] MI: PR20684, preserve user selected thread and frame when invoking MI commands
       [not found] <b3e449f0581f9ba05b7523aa1aa4c3685511f0aa.camel@labware.com>
@ 2021-03-02 20:27 ` Jan Vrany
  0 siblings, 0 replies; 7+ messages in thread
From: Jan Vrany @ 2021-03-02 20:27 UTC (permalink / raw)
  To: gdb-patches

Hi Jonah

> 
> I am not a GDB maintainer, so not sure how much I can help here (I
> was
> cc'ed in the original patch email).

I cc'ed you as you're IIRC CDT maintainer and I wanted to make sure
this does not pose problem to CDT. 

> 
> How does this change interact with -thread-select? Eclipse CDT
> expects -thread-select issued on the MI channel to affect the thread
> in the
> CLI channel. In fact this use was added in CDT when the new-ui was
> introduced, while the referenced bug 20684 is actually concerning
> "old ui"
> use case.

Yes, -thread-select still works as expected, that is, it changes 
"current" thread for CLI as following example demonstrates:

*stopped,reason="breakpoint-
hit",disp="keep",bkptno="1",frame={addr="0x0000555555555177",func="chil

d_function",args=[{name="args",value="0x0"}],file="/home/jv/Projects/gd

b/users_jv_patches/gdb/testsuite/gdb.mi/user-selected-context-
sync.c",fullname="/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/

gdb.mi/user-selected-context-sync.c",line="36",arch="i386:x86-
64"},thread-id="2",stopped-threads="all",core="1"
(gdb) 
info thread
&"info thread\n"
~"  Id   Target Id                                            Frame \n"
~"  1    Thread 0x7ffff7dc0740 (LWP 543648) \"user-selected-c\"
futex_wait (private=0, expected=0, futex_word=0x555555558084
<barrier+4>) at ../sysdeps/nptl/futex-internal.h:144\n"
~"* 2    Thread 0x7ffff7dbf700 (LWP 543652) \"user-selected-c\"
child_function (args=0x0) at
/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.mi/user-
selected-context-sync.c:36\n"
~"  3    Thread 0x7ffff75be700 (LWP 543653) \"user-selected-c\"
child_function (args=0x0) at
/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.mi/user-
selected-context-sync.c:36\n"
^done
(gdb) 
-thread-select 3
^done,new-thread-
id="3",frame={level="0",addr="0x0000555555555177",func="child_function"

,args=[{name="args",value="0x0"}],file="/home/jv/Projects/gdb/users_jv_

patches/gdb/testsuite/gdb.mi/user-selected-context-
sync.c",fullname="/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/

gdb.mi/user-selected-context-sync.c",line="36",arch="i386:x86-64"}
(gdb) 
info thread
&"info thread\n"
~"  Id   Target Id                                            Frame \n"
~"  1    Thread 0x7ffff7dc0740 (LWP 543648) \"user-selected-c\"
futex_wait (private=0, expected=0, futex_word=0x555555558084
<barrier+4>) at ../sysdeps/nptl/futex-internal.h:144\n"
~"  2    Thread 0x7ffff7dbf700 (LWP 543652) \"user-selected-c\"
child_function (args=0x0) at
/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.mi/user-
selected-context-sync.c:36\n"
~"* 3    Thread 0x7ffff75be700 (LWP 543653) \"user-selected-c\"
child_function (args=0x0) at
/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.mi/user-
selected-context-sync.c:36\n"
^done
(gdb) 

Best, Jan

> 
> Jonah
> 
> 
> 
> 
> On Tue, 2 Mar 2021 at 05:36, Jan Vrany via Gdb-patches <
> gdb-patches@sourceware.org> wrote:
> 
> > Polite ping.
> > 
> > On Tue, 2021-02-23 at 11:20 +0000, Jan Vrany wrote:
> > > Polite ping.
> > > 
> > > On Tue, 2021-02-09 at 10:08 +0000, Jan Vrany wrote:
> > > > When invoking MI commands with --thread and/or --frame, user
> > > > selected
> > > > thread and frame was not preserved:
> > > > 
> > > >   (gdb)
> > > >   info thread
> > > >   &"info thread\n"
> > > >   ~"  Id   Target Id
> > > > Frame
> > > > \n"
> > > >   ~"* 1    Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-
> > > > c\"
> > > > main
> > > > () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > > > sync.c:60\n"
> > > >   ~"  2    Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-
> > > > c\"
> > > > child_sub_function () at
> > > > /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > > selected-context-sync.c:30\n"
> > > >   ~"  3    Thread 0x7ffff742e700 (LWP 19307) \"user-selected-
> > > > c\"
> > > > child_sub_function () at
> > > > /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > > selected-context-sync.c:30\n"
> > > >   ^done
> > > >   (gdb)
> > > >   info frame
> > > >   &"info frame\n"
> > > >   ~"Stack level 0, frame at 0x7fffffffdf90:\n"
> > > >   ~" rip = 0x555555555207 in main
> > > > (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > > > sync.c:60);
> > > > saved rip = 0x7ffff7c5709b\n"
> > > >   ~" source language c.\n"
> > > >   ~" Arglist at 0x7fffffffdf80, args: \n"
> > > >   ~" Locals at 0x7fffffffdf80, Previous frame's sp is
> > > > 0x7fffffffdf90\n"
> > > >   ~" Saved registers:\n "
> > > >   ~" rbp at 0x7fffffffdf80, rip at 0x7fffffffdf88\n"
> > > >   ^done
> > > >   (gdb)
> > > >   -stack-info-depth --thread 3
> > > >   ^done,depth="4"
> > > >   (gdb)
> > > >   info thread
> > > >   &"info thread\n"
> > > >   ~"  Id   Target Id
> > > > Frame
> > > > \n"
> > > >   ~"  1    Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-
> > > > c\"
> > > > main
> > > > () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > > > sync.c:60\n"
> > > >   ~"  2    Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-
> > > > c\"
> > > > child_sub_function () at
> > > > /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > > selected-context-sync.c:30\n"
> > > >   ~"* 3    Thread 0x7ffff742e700 (LWP 19307) \"user-selected-
> > > > c\"
> > > > child_sub_function () at
> > > > /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > > selected-context-sync.c:30\n"
> > > >   ^done
> > > >   (gdb)
> > > >   info frame
> > > >   &"info frame\n"
> > > >   ~"Stack level 0, frame at 0x7ffff742dee0:\n"
> > > >   ~" rip = 0x555555555169 in child_sub_function
> > > > (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > > > sync.c:30);
> > > > saved rip = 0x555555555188\n"
> > > >   ~" called by frame at 0x7ffff742df00\n"
> > > >   ~" source language c.\n"
> > > >   ~" Arglist at 0x7ffff742ded0, args: \n"
> > > >   ~" Locals at 0x7ffff742ded0, Previous frame's sp is
> > > > 0x7ffff742dee0\n"
> > > >   ~" Saved registers:\n "
> > > >   ~" rbp at 0x7ffff742ded0, rip at 0x7ffff742ded8\n"
> > > >   ^done
> > > >   (gdb)
> > > > 
> > > > This was problematic for frontends that provide access to CLI
> > > > because
> > > > UI
> > > > may silently change the context for CLI commands (as
> > > > demonstrated
> > > > above).
> > > > 
> > > > gdb/Changelog
> > > > 
> > > >         PR 20684
> > > >         * gdb/mi/mi-main.c (mi_cmd_execute): Preserve user
> > > > selected
> > > > thread
> > > >         and frame when invoking MI commands.
> > > > ---
> > > >  gdb/ChangeLog    |  6 ++++++
> > > >  gdb/mi/mi-main.c | 36 +++++++++++++++---------------------
> > > >  2 files changed, 21 insertions(+), 21 deletions(-)
> > > > 
> > > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> > > > index e67668d315c..ea232ff1b3f 100644
> > > > --- a/gdb/ChangeLog
> > > > +++ b/gdb/ChangeLog
> > > > @@ -1,3 +1,9 @@
> > > > +2021-02-09  Jan Vrany  <jan.vrany@labware.com>
> > > > +
> > > > +       PR 20684
> > > > +       * gdb/mi/mi-main.c (mi_cmd_execute): Preserve user
> > > > selected
> > > > thread
> > > > +       and frame when invoking MI commands.
> > > > +
> > > >  2021-02-08  Shahab Vahedi  <shahab@synopsys.com>
> > > > 
> > > >         PR tdep/27369
> > > > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> > > > index 9a14d78e1e2..c5103800314 100644
> > > > --- a/gdb/mi/mi-main.c
> > > > +++ b/gdb/mi/mi-main.c
> > > > @@ -1971,25 +1971,13 @@ mi_execute_command (const char *cmd,
> > > > int
> > > > from_tty)
> > > >              again.  */
> > > >           && !command_notifies_uscc_observer (command.get ()))
> > > >         {
> > > > -         int report_change = 0;
> > > > -
> > > > -         if (command->thread == -1)
> > > > -           {
> > > > -             report_change = (previous_ptid != null_ptid
> > > > -                              && inferior_ptid !=
> > > > previous_ptid
> > > > -                              && inferior_ptid != null_ptid);
> > > > -           }
> > > > -         else if (inferior_ptid != null_ptid)
> > > > -           {
> > > > -             struct thread_info *ti = inferior_thread ();
> > > > -
> > > > -             report_change = (ti->global_num != command-
> > > > > thread);
> > > > -           }
> > > > -
> > > > -         if (report_change)
> > > > -           {
> > > > -            
> > > > gdb::observers::user_selected_context_changed.notify
> > > > -               (USER_SELECTED_THREAD | USER_SELECTED_FRAME);
> > > > +         if (command->thread == -1
> > > > +             && previous_ptid != null_ptid
> > > > +             && inferior_ptid != previous_ptid
> > > > +             && inferior_ptid != null_ptid)
> > > > +            {
> > > > +             
> > > > gdb::observers::user_selected_context_changed.notify
> > > > +                      (USER_SELECTED_THREAD |
> > > > USER_SELECTED_FRAME);
> > > >             }
> > > >         }
> > > >      }
> > > > @@ -2035,6 +2023,7 @@ mi_cmd_execute (struct mi_parse *parse)
> > > >        set_current_program_space (inf->pspace);
> > > >      }
> > > > 
> > > > +  gdb::optional<scoped_restore_current_thread> thread_saver;
> > > >    if (parse->thread != -1)
> > > >      {
> > > >        thread_info *tp = find_thread_global_id (parse->thread);
> > > > @@ -2045,9 +2034,11 @@ mi_cmd_execute (struct mi_parse *parse)
> > > >        if (tp->state == THREAD_EXITED)
> > > >         error (_("Thread id: %d has terminated"), parse-
> > > > > thread);
> > > > 
> > > > +      thread_saver.emplace ();
> > > >        switch_to_thread (tp);
> > > >      }
> > > > 
> > > > +  gdb::optional<scoped_restore_selected_frame> frame_saver;
> > > >    if (parse->frame != -1)
> > > >      {
> > > >        struct frame_info *fid;
> > > > @@ -2055,8 +2046,11 @@ mi_cmd_execute (struct mi_parse *parse)
> > > > 
> > > >        fid = find_relative_frame (get_current_frame (),
> > > > &frame);
> > > >        if (frame == 0)
> > > > -       /* find_relative_frame was successful */
> > > > -       select_frame (fid);
> > > > +        {
> > > > +          /* find_relative_frame was successful */
> > > > +          frame_saver.emplace ();
> > > > +          select_frame (fid);
> > > > +        }
> > > >        else
> > > >         error (_("Invalid frame id: %d"), frame);
> > > >      }
> > > 
> > 
> > 
> > 




^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PING] Re: [PATCH] MI: PR20684, preserve user selected thread and frame when invoking MI commands
  2021-03-17 14:47       ` Jan Vrany
@ 2021-03-23 13:31         ` Jan Vrany
  0 siblings, 0 replies; 7+ messages in thread
From: Jan Vrany @ 2021-03-23 13:31 UTC (permalink / raw)
  To: gdb-patches

Polite ping.

On Wed, 2021-03-17 at 14:47 +0000, Jan Vrany wrote:
> Polite ping.
> 
> On Tue, 2021-03-09 at 12:04 +0000, Jan Vrany wrote:
> > Polite ping.
> > 
> > On Tue, 2021-03-02 at 10:36 +0000, Jan Vrany wrote:
> > > Polite ping.
> > > 
> > > On Tue, 2021-02-23 at 11:20 +0000, Jan Vrany wrote:
> > > > Polite ping. 
> > > > 
> > > > On Tue, 2021-02-09 at 10:08 +0000, Jan Vrany wrote:
> > > > > When invoking MI commands with --thread and/or --frame, user
> > > > > selected
> > > > > thread and frame was not preserved:
> > > > > 
> > > > >   (gdb)
> > > > >   info thread
> > > > >   &"info thread\n"
> > > > >   ~"  Id   Target
> > > > > Id                                          
> > > > > Frame
> > > > > \n"
> > > > >   ~"* 1    Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-
> > > > > c\"
> > > > > main
> > > > > () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-
> > > > > context-
> > > > > sync.c:60\n"
> > > > >   ~"  2    Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-
> > > > > c\"
> > > > > child_sub_function () at
> > > > > /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > > > selected-context-sync.c:30\n"
> > > > >   ~"  3    Thread 0x7ffff742e700 (LWP 19307) \"user-selected-
> > > > > c\"
> > > > > child_sub_function () at
> > > > > /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > > > selected-context-sync.c:30\n"
> > > > >   ^done
> > > > >   (gdb)
> > > > >   info frame
> > > > >   &"info frame\n"
> > > > >   ~"Stack level 0, frame at 0x7fffffffdf90:\n"
> > > > >   ~" rip = 0x555555555207 in main
> > > > > (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > > > > sync.c:60);
> > > > > saved rip = 0x7ffff7c5709b\n"
> > > > >   ~" source language c.\n"
> > > > >   ~" Arglist at 0x7fffffffdf80, args: \n"
> > > > >   ~" Locals at 0x7fffffffdf80, Previous frame's sp is
> > > > > 0x7fffffffdf90\n"
> > > > >   ~" Saved registers:\n "
> > > > >   ~" rbp at 0x7fffffffdf80, rip at 0x7fffffffdf88\n"
> > > > >   ^done
> > > > >   (gdb)
> > > > >   -stack-info-depth --thread 3
> > > > >   ^done,depth="4"
> > > > >   (gdb)
> > > > >   info thread
> > > > >   &"info thread\n"
> > > > >   ~"  Id   Target
> > > > > Id                                          
> > > > > Frame
> > > > > \n"
> > > > >   ~"  1    Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-
> > > > > c\"
> > > > > main
> > > > > () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-
> > > > > context-
> > > > > sync.c:60\n"
> > > > >   ~"  2    Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-
> > > > > c\"
> > > > > child_sub_function () at
> > > > > /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > > > selected-context-sync.c:30\n"
> > > > >   ~"* 3    Thread 0x7ffff742e700 (LWP 19307) \"user-selected-
> > > > > c\"
> > > > > child_sub_function () at
> > > > > /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > > > selected-context-sync.c:30\n"
> > > > >   ^done
> > > > >   (gdb)
> > > > >   info frame
> > > > >   &"info frame\n"
> > > > >   ~"Stack level 0, frame at 0x7ffff742dee0:\n"
> > > > >   ~" rip = 0x555555555169 in child_sub_function
> > > > > (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > > > > sync.c:30);
> > > > > saved rip = 0x555555555188\n"
> > > > >   ~" called by frame at 0x7ffff742df00\n"
> > > > >   ~" source language c.\n"
> > > > >   ~" Arglist at 0x7ffff742ded0, args: \n"
> > > > >   ~" Locals at 0x7ffff742ded0, Previous frame's sp is
> > > > > 0x7ffff742dee0\n"
> > > > >   ~" Saved registers:\n "
> > > > >   ~" rbp at 0x7ffff742ded0, rip at 0x7ffff742ded8\n"
> > > > >   ^done
> > > > >   (gdb)
> > > > > 
> > > > > This was problematic for frontends that provide access to CLI
> > > > > because
> > > > > UI
> > > > > may silently change the context for CLI commands (as
> > > > > demonstrated
> > > > > above).
> > > > > 
> > > > > gdb/Changelog
> > > > > 
> > > > >         PR 20684
> > > > >         * gdb/mi/mi-main.c (mi_cmd_execute): Preserve user
> > > > > selected
> > > > > thread
> > > > >         and frame when invoking MI commands.
> > > > > ---
> > > > >  gdb/ChangeLog    |  6 ++++++
> > > > >  gdb/mi/mi-main.c | 36 +++++++++++++++---------------------
> > > > >  2 files changed, 21 insertions(+), 21 deletions(-)
> > > > > 
> > > > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> > > > > index e67668d315c..ea232ff1b3f 100644
> > > > > --- a/gdb/ChangeLog
> > > > > +++ b/gdb/ChangeLog
> > > > > @@ -1,3 +1,9 @@
> > > > > +2021-02-09  Jan Vrany  <jan.vrany@labware.com>
> > > > > +
> > > > > +       PR 20684
> > > > > +       * gdb/mi/mi-main.c (mi_cmd_execute): Preserve user
> > > > > selected
> > > > > thread
> > > > > +       and frame when invoking MI commands.
> > > > > +
> > > > >  2021-02-08  Shahab Vahedi  <shahab@synopsys.com>
> > > > >  
> > > > >         PR tdep/27369
> > > > > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> > > > > index 9a14d78e1e2..c5103800314 100644
> > > > > --- a/gdb/mi/mi-main.c
> > > > > +++ b/gdb/mi/mi-main.c
> > > > > @@ -1971,25 +1971,13 @@ mi_execute_command (const char *cmd,
> > > > > int
> > > > > from_tty)
> > > > >              again.  */
> > > > >           && !command_notifies_uscc_observer (command.get
> > > > > ()))
> > > > >         {
> > > > > -         int report_change = 0;
> > > > > -
> > > > > -         if (command->thread == -1)
> > > > > -           {
> > > > > -             report_change = (previous_ptid != null_ptid
> > > > > -                              && inferior_ptid !=
> > > > > previous_ptid
> > > > > -                              && inferior_ptid !=
> > > > > null_ptid);
> > > > > -           }
> > > > > -         else if (inferior_ptid != null_ptid)
> > > > > -           {
> > > > > -             struct thread_info *ti = inferior_thread ();
> > > > > -
> > > > > -             report_change = (ti->global_num != command-
> > > > > > thread);
> > > > > -           }
> > > > > -
> > > > > -         if (report_change)
> > > > > -           {
> > > > > -            
> > > > > gdb::observers::user_selected_context_changed.notify
> > > > > -               (USER_SELECTED_THREAD | USER_SELECTED_FRAME);
> > > > > +         if (command->thread == -1
> > > > > +             && previous_ptid != null_ptid
> > > > > +             && inferior_ptid != previous_ptid
> > > > > +             && inferior_ptid != null_ptid)
> > > > > +            {
> > > > > +             
> > > > > gdb::observers::user_selected_context_changed.notify
> > > > > +                      (USER_SELECTED_THREAD |
> > > > > USER_SELECTED_FRAME);
> > > > >             }
> > > > >         }
> > > > >      }
> > > > > @@ -2035,6 +2023,7 @@ mi_cmd_execute (struct mi_parse *parse)
> > > > >        set_current_program_space (inf->pspace);
> > > > >      }
> > > > >  
> > > > > +  gdb::optional<scoped_restore_current_thread> thread_saver;
> > > > >    if (parse->thread != -1)
> > > > >      {
> > > > >        thread_info *tp = find_thread_global_id (parse-
> > > > > >thread);
> > > > > @@ -2045,9 +2034,11 @@ mi_cmd_execute (struct mi_parse
> > > > > *parse)
> > > > >        if (tp->state == THREAD_EXITED)
> > > > >         error (_("Thread id: %d has terminated"), parse-
> > > > > > thread);
> > > > >  
> > > > > +      thread_saver.emplace ();
> > > > >        switch_to_thread (tp);
> > > > >      }
> > > > >  
> > > > > +  gdb::optional<scoped_restore_selected_frame> frame_saver;
> > > > >    if (parse->frame != -1)
> > > > >      {
> > > > >        struct frame_info *fid;
> > > > > @@ -2055,8 +2046,11 @@ mi_cmd_execute (struct mi_parse
> > > > > *parse)
> > > > >  
> > > > >        fid = find_relative_frame (get_current_frame (),
> > > > > &frame);
> > > > >        if (frame == 0)
> > > > > -       /* find_relative_frame was successful */
> > > > > -       select_frame (fid);
> > > > > +        {
> > > > > +          /* find_relative_frame was successful */
> > > > > +          frame_saver.emplace ();
> > > > > +          select_frame (fid);
> > > > > +        }
> > > > >        else
> > > > >         error (_("Invalid frame id: %d"), frame);
> > > > >      }
> > > > 
> > > 
> > 
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PING] Re: [PATCH] MI: PR20684, preserve user selected thread and frame when invoking MI commands
  2021-03-09 12:04     ` Jan Vrany
@ 2021-03-17 14:47       ` Jan Vrany
  2021-03-23 13:31         ` Jan Vrany
  0 siblings, 1 reply; 7+ messages in thread
From: Jan Vrany @ 2021-03-17 14:47 UTC (permalink / raw)
  To: gdb-patches

Polite ping.

On Tue, 2021-03-09 at 12:04 +0000, Jan Vrany wrote:
> Polite ping.
> 
> On Tue, 2021-03-02 at 10:36 +0000, Jan Vrany wrote:
> > Polite ping.
> > 
> > On Tue, 2021-02-23 at 11:20 +0000, Jan Vrany wrote:
> > > Polite ping. 
> > > 
> > > On Tue, 2021-02-09 at 10:08 +0000, Jan Vrany wrote:
> > > > When invoking MI commands with --thread and/or --frame, user
> > > > selected
> > > > thread and frame was not preserved:
> > > > 
> > > >   (gdb)
> > > >   info thread
> > > >   &"info thread\n"
> > > >   ~"  Id   Target Id                                          
> > > > Frame
> > > > \n"
> > > >   ~"* 1    Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-
> > > > c\"
> > > > main
> > > > () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > > > sync.c:60\n"
> > > >   ~"  2    Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-
> > > > c\"
> > > > child_sub_function () at
> > > > /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > > selected-context-sync.c:30\n"
> > > >   ~"  3    Thread 0x7ffff742e700 (LWP 19307) \"user-selected-
> > > > c\"
> > > > child_sub_function () at
> > > > /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > > selected-context-sync.c:30\n"
> > > >   ^done
> > > >   (gdb)
> > > >   info frame
> > > >   &"info frame\n"
> > > >   ~"Stack level 0, frame at 0x7fffffffdf90:\n"
> > > >   ~" rip = 0x555555555207 in main
> > > > (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > > > sync.c:60);
> > > > saved rip = 0x7ffff7c5709b\n"
> > > >   ~" source language c.\n"
> > > >   ~" Arglist at 0x7fffffffdf80, args: \n"
> > > >   ~" Locals at 0x7fffffffdf80, Previous frame's sp is
> > > > 0x7fffffffdf90\n"
> > > >   ~" Saved registers:\n "
> > > >   ~" rbp at 0x7fffffffdf80, rip at 0x7fffffffdf88\n"
> > > >   ^done
> > > >   (gdb)
> > > >   -stack-info-depth --thread 3
> > > >   ^done,depth="4"
> > > >   (gdb)
> > > >   info thread
> > > >   &"info thread\n"
> > > >   ~"  Id   Target Id                                          
> > > > Frame
> > > > \n"
> > > >   ~"  1    Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-
> > > > c\"
> > > > main
> > > > () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > > > sync.c:60\n"
> > > >   ~"  2    Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-
> > > > c\"
> > > > child_sub_function () at
> > > > /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > > selected-context-sync.c:30\n"
> > > >   ~"* 3    Thread 0x7ffff742e700 (LWP 19307) \"user-selected-
> > > > c\"
> > > > child_sub_function () at
> > > > /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > > selected-context-sync.c:30\n"
> > > >   ^done
> > > >   (gdb)
> > > >   info frame
> > > >   &"info frame\n"
> > > >   ~"Stack level 0, frame at 0x7ffff742dee0:\n"
> > > >   ~" rip = 0x555555555169 in child_sub_function
> > > > (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > > > sync.c:30);
> > > > saved rip = 0x555555555188\n"
> > > >   ~" called by frame at 0x7ffff742df00\n"
> > > >   ~" source language c.\n"
> > > >   ~" Arglist at 0x7ffff742ded0, args: \n"
> > > >   ~" Locals at 0x7ffff742ded0, Previous frame's sp is
> > > > 0x7ffff742dee0\n"
> > > >   ~" Saved registers:\n "
> > > >   ~" rbp at 0x7ffff742ded0, rip at 0x7ffff742ded8\n"
> > > >   ^done
> > > >   (gdb)
> > > > 
> > > > This was problematic for frontends that provide access to CLI
> > > > because
> > > > UI
> > > > may silently change the context for CLI commands (as
> > > > demonstrated
> > > > above).
> > > > 
> > > > gdb/Changelog
> > > > 
> > > >         PR 20684
> > > >         * gdb/mi/mi-main.c (mi_cmd_execute): Preserve user
> > > > selected
> > > > thread
> > > >         and frame when invoking MI commands.
> > > > ---
> > > >  gdb/ChangeLog    |  6 ++++++
> > > >  gdb/mi/mi-main.c | 36 +++++++++++++++---------------------
> > > >  2 files changed, 21 insertions(+), 21 deletions(-)
> > > > 
> > > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> > > > index e67668d315c..ea232ff1b3f 100644
> > > > --- a/gdb/ChangeLog
> > > > +++ b/gdb/ChangeLog
> > > > @@ -1,3 +1,9 @@
> > > > +2021-02-09  Jan Vrany  <jan.vrany@labware.com>
> > > > +
> > > > +       PR 20684
> > > > +       * gdb/mi/mi-main.c (mi_cmd_execute): Preserve user
> > > > selected
> > > > thread
> > > > +       and frame when invoking MI commands.
> > > > +
> > > >  2021-02-08  Shahab Vahedi  <shahab@synopsys.com>
> > > >  
> > > >         PR tdep/27369
> > > > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> > > > index 9a14d78e1e2..c5103800314 100644
> > > > --- a/gdb/mi/mi-main.c
> > > > +++ b/gdb/mi/mi-main.c
> > > > @@ -1971,25 +1971,13 @@ mi_execute_command (const char *cmd,
> > > > int
> > > > from_tty)
> > > >              again.  */
> > > >           && !command_notifies_uscc_observer (command.get ()))
> > > >         {
> > > > -         int report_change = 0;
> > > > -
> > > > -         if (command->thread == -1)
> > > > -           {
> > > > -             report_change = (previous_ptid != null_ptid
> > > > -                              && inferior_ptid !=
> > > > previous_ptid
> > > > -                              && inferior_ptid != null_ptid);
> > > > -           }
> > > > -         else if (inferior_ptid != null_ptid)
> > > > -           {
> > > > -             struct thread_info *ti = inferior_thread ();
> > > > -
> > > > -             report_change = (ti->global_num != command-
> > > > > thread);
> > > > -           }
> > > > -
> > > > -         if (report_change)
> > > > -           {
> > > > -            
> > > > gdb::observers::user_selected_context_changed.notify
> > > > -               (USER_SELECTED_THREAD | USER_SELECTED_FRAME);
> > > > +         if (command->thread == -1
> > > > +             && previous_ptid != null_ptid
> > > > +             && inferior_ptid != previous_ptid
> > > > +             && inferior_ptid != null_ptid)
> > > > +            {
> > > > +             
> > > > gdb::observers::user_selected_context_changed.notify
> > > > +                      (USER_SELECTED_THREAD |
> > > > USER_SELECTED_FRAME);
> > > >             }
> > > >         }
> > > >      }
> > > > @@ -2035,6 +2023,7 @@ mi_cmd_execute (struct mi_parse *parse)
> > > >        set_current_program_space (inf->pspace);
> > > >      }
> > > >  
> > > > +  gdb::optional<scoped_restore_current_thread> thread_saver;
> > > >    if (parse->thread != -1)
> > > >      {
> > > >        thread_info *tp = find_thread_global_id (parse->thread);
> > > > @@ -2045,9 +2034,11 @@ mi_cmd_execute (struct mi_parse *parse)
> > > >        if (tp->state == THREAD_EXITED)
> > > >         error (_("Thread id: %d has terminated"), parse-
> > > > >thread);
> > > >  
> > > > +      thread_saver.emplace ();
> > > >        switch_to_thread (tp);
> > > >      }
> > > >  
> > > > +  gdb::optional<scoped_restore_selected_frame> frame_saver;
> > > >    if (parse->frame != -1)
> > > >      {
> > > >        struct frame_info *fid;
> > > > @@ -2055,8 +2046,11 @@ mi_cmd_execute (struct mi_parse *parse)
> > > >  
> > > >        fid = find_relative_frame (get_current_frame (),
> > > > &frame);
> > > >        if (frame == 0)
> > > > -       /* find_relative_frame was successful */
> > > > -       select_frame (fid);
> > > > +        {
> > > > +          /* find_relative_frame was successful */
> > > > +          frame_saver.emplace ();
> > > > +          select_frame (fid);
> > > > +        }
> > > >        else
> > > >         error (_("Invalid frame id: %d"), frame);
> > > >      }
> > > 
> > 
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PING] Re: [PATCH] MI: PR20684, preserve user selected thread and frame when invoking MI commands
  2021-03-02 10:36   ` Jan Vrany
  2021-03-02 17:39     ` Jonah Graham
@ 2021-03-09 12:04     ` Jan Vrany
  2021-03-17 14:47       ` Jan Vrany
  1 sibling, 1 reply; 7+ messages in thread
From: Jan Vrany @ 2021-03-09 12:04 UTC (permalink / raw)
  To: gdb-patches

Polite ping.

On Tue, 2021-03-02 at 10:36 +0000, Jan Vrany wrote:
> Polite ping.
> 
> On Tue, 2021-02-23 at 11:20 +0000, Jan Vrany wrote:
> > Polite ping. 
> > 
> > On Tue, 2021-02-09 at 10:08 +0000, Jan Vrany wrote:
> > > When invoking MI commands with --thread and/or --frame, user
> > > selected
> > > thread and frame was not preserved:
> > > 
> > >   (gdb)
> > >   info thread
> > >   &"info thread\n"
> > >   ~"  Id   Target Id                                          
> > > Frame
> > > \n"
> > >   ~"* 1    Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-c\"
> > > main
> > > () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > > sync.c:60\n"
> > >   ~"  2    Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-c\"
> > > child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > selected-context-sync.c:30\n"
> > >   ~"  3    Thread 0x7ffff742e700 (LWP 19307) \"user-selected-c\"
> > > child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > selected-context-sync.c:30\n"
> > >   ^done
> > >   (gdb)
> > >   info frame
> > >   &"info frame\n"
> > >   ~"Stack level 0, frame at 0x7fffffffdf90:\n"
> > >   ~" rip = 0x555555555207 in main
> > > (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > > sync.c:60);
> > > saved rip = 0x7ffff7c5709b\n"
> > >   ~" source language c.\n"
> > >   ~" Arglist at 0x7fffffffdf80, args: \n"
> > >   ~" Locals at 0x7fffffffdf80, Previous frame's sp is
> > > 0x7fffffffdf90\n"
> > >   ~" Saved registers:\n "
> > >   ~" rbp at 0x7fffffffdf80, rip at 0x7fffffffdf88\n"
> > >   ^done
> > >   (gdb)
> > >   -stack-info-depth --thread 3
> > >   ^done,depth="4"
> > >   (gdb)
> > >   info thread
> > >   &"info thread\n"
> > >   ~"  Id   Target Id                                          
> > > Frame
> > > \n"
> > >   ~"  1    Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-c\"
> > > main
> > > () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > > sync.c:60\n"
> > >   ~"  2    Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-c\"
> > > child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > selected-context-sync.c:30\n"
> > >   ~"* 3    Thread 0x7ffff742e700 (LWP 19307) \"user-selected-c\"
> > > child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > selected-context-sync.c:30\n"
> > >   ^done
> > >   (gdb)
> > >   info frame
> > >   &"info frame\n"
> > >   ~"Stack level 0, frame at 0x7ffff742dee0:\n"
> > >   ~" rip = 0x555555555169 in child_sub_function
> > > (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > > sync.c:30);
> > > saved rip = 0x555555555188\n"
> > >   ~" called by frame at 0x7ffff742df00\n"
> > >   ~" source language c.\n"
> > >   ~" Arglist at 0x7ffff742ded0, args: \n"
> > >   ~" Locals at 0x7ffff742ded0, Previous frame's sp is
> > > 0x7ffff742dee0\n"
> > >   ~" Saved registers:\n "
> > >   ~" rbp at 0x7ffff742ded0, rip at 0x7ffff742ded8\n"
> > >   ^done
> > >   (gdb)
> > > 
> > > This was problematic for frontends that provide access to CLI
> > > because
> > > UI
> > > may silently change the context for CLI commands (as demonstrated
> > > above).
> > > 
> > > gdb/Changelog
> > > 
> > >         PR 20684
> > >         * gdb/mi/mi-main.c (mi_cmd_execute): Preserve user
> > > selected
> > > thread
> > >         and frame when invoking MI commands.
> > > ---
> > >  gdb/ChangeLog    |  6 ++++++
> > >  gdb/mi/mi-main.c | 36 +++++++++++++++---------------------
> > >  2 files changed, 21 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> > > index e67668d315c..ea232ff1b3f 100644
> > > --- a/gdb/ChangeLog
> > > +++ b/gdb/ChangeLog
> > > @@ -1,3 +1,9 @@
> > > +2021-02-09  Jan Vrany  <jan.vrany@labware.com>
> > > +
> > > +       PR 20684
> > > +       * gdb/mi/mi-main.c (mi_cmd_execute): Preserve user
> > > selected
> > > thread
> > > +       and frame when invoking MI commands.
> > > +
> > >  2021-02-08  Shahab Vahedi  <shahab@synopsys.com>
> > >  
> > >         PR tdep/27369
> > > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> > > index 9a14d78e1e2..c5103800314 100644
> > > --- a/gdb/mi/mi-main.c
> > > +++ b/gdb/mi/mi-main.c
> > > @@ -1971,25 +1971,13 @@ mi_execute_command (const char *cmd, int
> > > from_tty)
> > >              again.  */
> > >           && !command_notifies_uscc_observer (command.get ()))
> > >         {
> > > -         int report_change = 0;
> > > -
> > > -         if (command->thread == -1)
> > > -           {
> > > -             report_change = (previous_ptid != null_ptid
> > > -                              && inferior_ptid != previous_ptid
> > > -                              && inferior_ptid != null_ptid);
> > > -           }
> > > -         else if (inferior_ptid != null_ptid)
> > > -           {
> > > -             struct thread_info *ti = inferior_thread ();
> > > -
> > > -             report_change = (ti->global_num != command-
> > > >thread);
> > > -           }
> > > -
> > > -         if (report_change)
> > > -           {
> > > -            
> > > gdb::observers::user_selected_context_changed.notify
> > > -               (USER_SELECTED_THREAD | USER_SELECTED_FRAME);
> > > +         if (command->thread == -1
> > > +             && previous_ptid != null_ptid
> > > +             && inferior_ptid != previous_ptid
> > > +             && inferior_ptid != null_ptid)
> > > +            {
> > > +             
> > > gdb::observers::user_selected_context_changed.notify
> > > +                      (USER_SELECTED_THREAD |
> > > USER_SELECTED_FRAME);
> > >             }
> > >         }
> > >      }
> > > @@ -2035,6 +2023,7 @@ mi_cmd_execute (struct mi_parse *parse)
> > >        set_current_program_space (inf->pspace);
> > >      }
> > >  
> > > +  gdb::optional<scoped_restore_current_thread> thread_saver;
> > >    if (parse->thread != -1)
> > >      {
> > >        thread_info *tp = find_thread_global_id (parse->thread);
> > > @@ -2045,9 +2034,11 @@ mi_cmd_execute (struct mi_parse *parse)
> > >        if (tp->state == THREAD_EXITED)
> > >         error (_("Thread id: %d has terminated"), parse->thread);
> > >  
> > > +      thread_saver.emplace ();
> > >        switch_to_thread (tp);
> > >      }
> > >  
> > > +  gdb::optional<scoped_restore_selected_frame> frame_saver;
> > >    if (parse->frame != -1)
> > >      {
> > >        struct frame_info *fid;
> > > @@ -2055,8 +2046,11 @@ mi_cmd_execute (struct mi_parse *parse)
> > >  
> > >        fid = find_relative_frame (get_current_frame (), &frame);
> > >        if (frame == 0)
> > > -       /* find_relative_frame was successful */
> > > -       select_frame (fid);
> > > +        {
> > > +          /* find_relative_frame was successful */
> > > +          frame_saver.emplace ();
> > > +          select_frame (fid);
> > > +        }
> > >        else
> > >         error (_("Invalid frame id: %d"), frame);
> > >      }
> > 
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PING] Re: [PATCH] MI: PR20684, preserve user selected thread and frame when invoking MI commands
  2021-03-02 10:36   ` Jan Vrany
@ 2021-03-02 17:39     ` Jonah Graham
  2021-03-09 12:04     ` Jan Vrany
  1 sibling, 0 replies; 7+ messages in thread
From: Jonah Graham @ 2021-03-02 17:39 UTC (permalink / raw)
  To: Jan Vrany; +Cc: gdb-patches

Hi Jan,

I am not a GDB maintainer, so not sure how much I can help here (I was
cc'ed in the original patch email).

How does this change interact with -thread-select? Eclipse CDT
expects -thread-select issued on the MI channel to affect the thread in the
CLI channel. In fact this use was added in CDT when the new-ui was
introduced, while the referenced bug 20684 is actually concerning "old ui"
use case.

Jonah




On Tue, 2 Mar 2021 at 05:36, Jan Vrany via Gdb-patches <
gdb-patches@sourceware.org> wrote:

> Polite ping.
>
> On Tue, 2021-02-23 at 11:20 +0000, Jan Vrany wrote:
> > Polite ping.
> >
> > On Tue, 2021-02-09 at 10:08 +0000, Jan Vrany wrote:
> > > When invoking MI commands with --thread and/or --frame, user
> > > selected
> > > thread and frame was not preserved:
> > >
> > >   (gdb)
> > >   info thread
> > >   &"info thread\n"
> > >   ~"  Id   Target Id
> > > Frame
> > > \n"
> > >   ~"* 1    Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-c\"
> > > main
> > > () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > > sync.c:60\n"
> > >   ~"  2    Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-c\"
> > > child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > selected-context-sync.c:30\n"
> > >   ~"  3    Thread 0x7ffff742e700 (LWP 19307) \"user-selected-c\"
> > > child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > selected-context-sync.c:30\n"
> > >   ^done
> > >   (gdb)
> > >   info frame
> > >   &"info frame\n"
> > >   ~"Stack level 0, frame at 0x7fffffffdf90:\n"
> > >   ~" rip = 0x555555555207 in main
> > > (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > > sync.c:60);
> > > saved rip = 0x7ffff7c5709b\n"
> > >   ~" source language c.\n"
> > >   ~" Arglist at 0x7fffffffdf80, args: \n"
> > >   ~" Locals at 0x7fffffffdf80, Previous frame's sp is
> > > 0x7fffffffdf90\n"
> > >   ~" Saved registers:\n "
> > >   ~" rbp at 0x7fffffffdf80, rip at 0x7fffffffdf88\n"
> > >   ^done
> > >   (gdb)
> > >   -stack-info-depth --thread 3
> > >   ^done,depth="4"
> > >   (gdb)
> > >   info thread
> > >   &"info thread\n"
> > >   ~"  Id   Target Id
> > > Frame
> > > \n"
> > >   ~"  1    Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-c\"
> > > main
> > > () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > > sync.c:60\n"
> > >   ~"  2    Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-c\"
> > > child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > selected-context-sync.c:30\n"
> > >   ~"* 3    Thread 0x7ffff742e700 (LWP 19307) \"user-selected-c\"
> > > child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > > selected-context-sync.c:30\n"
> > >   ^done
> > >   (gdb)
> > >   info frame
> > >   &"info frame\n"
> > >   ~"Stack level 0, frame at 0x7ffff742dee0:\n"
> > >   ~" rip = 0x555555555169 in child_sub_function
> > > (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > > sync.c:30);
> > > saved rip = 0x555555555188\n"
> > >   ~" called by frame at 0x7ffff742df00\n"
> > >   ~" source language c.\n"
> > >   ~" Arglist at 0x7ffff742ded0, args: \n"
> > >   ~" Locals at 0x7ffff742ded0, Previous frame's sp is
> > > 0x7ffff742dee0\n"
> > >   ~" Saved registers:\n "
> > >   ~" rbp at 0x7ffff742ded0, rip at 0x7ffff742ded8\n"
> > >   ^done
> > >   (gdb)
> > >
> > > This was problematic for frontends that provide access to CLI
> > > because
> > > UI
> > > may silently change the context for CLI commands (as demonstrated
> > > above).
> > >
> > > gdb/Changelog
> > >
> > >         PR 20684
> > >         * gdb/mi/mi-main.c (mi_cmd_execute): Preserve user selected
> > > thread
> > >         and frame when invoking MI commands.
> > > ---
> > >  gdb/ChangeLog    |  6 ++++++
> > >  gdb/mi/mi-main.c | 36 +++++++++++++++---------------------
> > >  2 files changed, 21 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> > > index e67668d315c..ea232ff1b3f 100644
> > > --- a/gdb/ChangeLog
> > > +++ b/gdb/ChangeLog
> > > @@ -1,3 +1,9 @@
> > > +2021-02-09  Jan Vrany  <jan.vrany@labware.com>
> > > +
> > > +       PR 20684
> > > +       * gdb/mi/mi-main.c (mi_cmd_execute): Preserve user selected
> > > thread
> > > +       and frame when invoking MI commands.
> > > +
> > >  2021-02-08  Shahab Vahedi  <shahab@synopsys.com>
> > >
> > >         PR tdep/27369
> > > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> > > index 9a14d78e1e2..c5103800314 100644
> > > --- a/gdb/mi/mi-main.c
> > > +++ b/gdb/mi/mi-main.c
> > > @@ -1971,25 +1971,13 @@ mi_execute_command (const char *cmd, int
> > > from_tty)
> > >              again.  */
> > >           && !command_notifies_uscc_observer (command.get ()))
> > >         {
> > > -         int report_change = 0;
> > > -
> > > -         if (command->thread == -1)
> > > -           {
> > > -             report_change = (previous_ptid != null_ptid
> > > -                              && inferior_ptid != previous_ptid
> > > -                              && inferior_ptid != null_ptid);
> > > -           }
> > > -         else if (inferior_ptid != null_ptid)
> > > -           {
> > > -             struct thread_info *ti = inferior_thread ();
> > > -
> > > -             report_change = (ti->global_num != command->thread);
> > > -           }
> > > -
> > > -         if (report_change)
> > > -           {
> > > -             gdb::observers::user_selected_context_changed.notify
> > > -               (USER_SELECTED_THREAD | USER_SELECTED_FRAME);
> > > +         if (command->thread == -1
> > > +             && previous_ptid != null_ptid
> > > +             && inferior_ptid != previous_ptid
> > > +             && inferior_ptid != null_ptid)
> > > +            {
> > > +              gdb::observers::user_selected_context_changed.notify
> > > +                      (USER_SELECTED_THREAD |
> > > USER_SELECTED_FRAME);
> > >             }
> > >         }
> > >      }
> > > @@ -2035,6 +2023,7 @@ mi_cmd_execute (struct mi_parse *parse)
> > >        set_current_program_space (inf->pspace);
> > >      }
> > >
> > > +  gdb::optional<scoped_restore_current_thread> thread_saver;
> > >    if (parse->thread != -1)
> > >      {
> > >        thread_info *tp = find_thread_global_id (parse->thread);
> > > @@ -2045,9 +2034,11 @@ mi_cmd_execute (struct mi_parse *parse)
> > >        if (tp->state == THREAD_EXITED)
> > >         error (_("Thread id: %d has terminated"), parse->thread);
> > >
> > > +      thread_saver.emplace ();
> > >        switch_to_thread (tp);
> > >      }
> > >
> > > +  gdb::optional<scoped_restore_selected_frame> frame_saver;
> > >    if (parse->frame != -1)
> > >      {
> > >        struct frame_info *fid;
> > > @@ -2055,8 +2046,11 @@ mi_cmd_execute (struct mi_parse *parse)
> > >
> > >        fid = find_relative_frame (get_current_frame (), &frame);
> > >        if (frame == 0)
> > > -       /* find_relative_frame was successful */
> > > -       select_frame (fid);
> > > +        {
> > > +          /* find_relative_frame was successful */
> > > +          frame_saver.emplace ();
> > > +          select_frame (fid);
> > > +        }
> > >        else
> > >         error (_("Invalid frame id: %d"), frame);
> > >      }
> >
>
>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PING] Re: [PATCH] MI: PR20684, preserve user selected thread and frame when invoking MI commands
  2021-02-23 11:20 ` [PING] " Jan Vrany
@ 2021-03-02 10:36   ` Jan Vrany
  2021-03-02 17:39     ` Jonah Graham
  2021-03-09 12:04     ` Jan Vrany
  0 siblings, 2 replies; 7+ messages in thread
From: Jan Vrany @ 2021-03-02 10:36 UTC (permalink / raw)
  To: gdb-patches

Polite ping.

On Tue, 2021-02-23 at 11:20 +0000, Jan Vrany wrote:
> Polite ping. 
> 
> On Tue, 2021-02-09 at 10:08 +0000, Jan Vrany wrote:
> > When invoking MI commands with --thread and/or --frame, user
> > selected
> > thread and frame was not preserved:
> > 
> >   (gdb)
> >   info thread
> >   &"info thread\n"
> >   ~"  Id   Target Id                                          
> > Frame
> > \n"
> >   ~"* 1    Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-c\"
> > main
> > () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > sync.c:60\n"
> >   ~"  2    Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-c\"
> > child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > selected-context-sync.c:30\n"
> >   ~"  3    Thread 0x7ffff742e700 (LWP 19307) \"user-selected-c\"
> > child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > selected-context-sync.c:30\n"
> >   ^done
> >   (gdb)
> >   info frame
> >   &"info frame\n"
> >   ~"Stack level 0, frame at 0x7fffffffdf90:\n"
> >   ~" rip = 0x555555555207 in main
> > (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > sync.c:60);
> > saved rip = 0x7ffff7c5709b\n"
> >   ~" source language c.\n"
> >   ~" Arglist at 0x7fffffffdf80, args: \n"
> >   ~" Locals at 0x7fffffffdf80, Previous frame's sp is
> > 0x7fffffffdf90\n"
> >   ~" Saved registers:\n "
> >   ~" rbp at 0x7fffffffdf80, rip at 0x7fffffffdf88\n"
> >   ^done
> >   (gdb)
> >   -stack-info-depth --thread 3
> >   ^done,depth="4"
> >   (gdb)
> >   info thread
> >   &"info thread\n"
> >   ~"  Id   Target Id                                          
> > Frame
> > \n"
> >   ~"  1    Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-c\"
> > main
> > () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > sync.c:60\n"
> >   ~"  2    Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-c\"
> > child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > selected-context-sync.c:30\n"
> >   ~"* 3    Thread 0x7ffff742e700 (LWP 19307) \"user-selected-c\"
> > child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> > selected-context-sync.c:30\n"
> >   ^done
> >   (gdb)
> >   info frame
> >   &"info frame\n"
> >   ~"Stack level 0, frame at 0x7ffff742dee0:\n"
> >   ~" rip = 0x555555555169 in child_sub_function
> > (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> > sync.c:30);
> > saved rip = 0x555555555188\n"
> >   ~" called by frame at 0x7ffff742df00\n"
> >   ~" source language c.\n"
> >   ~" Arglist at 0x7ffff742ded0, args: \n"
> >   ~" Locals at 0x7ffff742ded0, Previous frame's sp is
> > 0x7ffff742dee0\n"
> >   ~" Saved registers:\n "
> >   ~" rbp at 0x7ffff742ded0, rip at 0x7ffff742ded8\n"
> >   ^done
> >   (gdb)
> > 
> > This was problematic for frontends that provide access to CLI
> > because
> > UI
> > may silently change the context for CLI commands (as demonstrated
> > above).
> > 
> > gdb/Changelog
> > 
> >         PR 20684
> >         * gdb/mi/mi-main.c (mi_cmd_execute): Preserve user selected
> > thread
> >         and frame when invoking MI commands.
> > ---
> >  gdb/ChangeLog    |  6 ++++++
> >  gdb/mi/mi-main.c | 36 +++++++++++++++---------------------
> >  2 files changed, 21 insertions(+), 21 deletions(-)
> > 
> > diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> > index e67668d315c..ea232ff1b3f 100644
> > --- a/gdb/ChangeLog
> > +++ b/gdb/ChangeLog
> > @@ -1,3 +1,9 @@
> > +2021-02-09  Jan Vrany  <jan.vrany@labware.com>
> > +
> > +       PR 20684
> > +       * gdb/mi/mi-main.c (mi_cmd_execute): Preserve user selected
> > thread
> > +       and frame when invoking MI commands.
> > +
> >  2021-02-08  Shahab Vahedi  <shahab@synopsys.com>
> >  
> >         PR tdep/27369
> > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> > index 9a14d78e1e2..c5103800314 100644
> > --- a/gdb/mi/mi-main.c
> > +++ b/gdb/mi/mi-main.c
> > @@ -1971,25 +1971,13 @@ mi_execute_command (const char *cmd, int
> > from_tty)
> >              again.  */
> >           && !command_notifies_uscc_observer (command.get ()))
> >         {
> > -         int report_change = 0;
> > -
> > -         if (command->thread == -1)
> > -           {
> > -             report_change = (previous_ptid != null_ptid
> > -                              && inferior_ptid != previous_ptid
> > -                              && inferior_ptid != null_ptid);
> > -           }
> > -         else if (inferior_ptid != null_ptid)
> > -           {
> > -             struct thread_info *ti = inferior_thread ();
> > -
> > -             report_change = (ti->global_num != command->thread);
> > -           }
> > -
> > -         if (report_change)
> > -           {
> > -             gdb::observers::user_selected_context_changed.notify
> > -               (USER_SELECTED_THREAD | USER_SELECTED_FRAME);
> > +         if (command->thread == -1
> > +             && previous_ptid != null_ptid
> > +             && inferior_ptid != previous_ptid
> > +             && inferior_ptid != null_ptid)
> > +            {
> > +              gdb::observers::user_selected_context_changed.notify
> > +                      (USER_SELECTED_THREAD |
> > USER_SELECTED_FRAME);
> >             }
> >         }
> >      }
> > @@ -2035,6 +2023,7 @@ mi_cmd_execute (struct mi_parse *parse)
> >        set_current_program_space (inf->pspace);
> >      }
> >  
> > +  gdb::optional<scoped_restore_current_thread> thread_saver;
> >    if (parse->thread != -1)
> >      {
> >        thread_info *tp = find_thread_global_id (parse->thread);
> > @@ -2045,9 +2034,11 @@ mi_cmd_execute (struct mi_parse *parse)
> >        if (tp->state == THREAD_EXITED)
> >         error (_("Thread id: %d has terminated"), parse->thread);
> >  
> > +      thread_saver.emplace ();
> >        switch_to_thread (tp);
> >      }
> >  
> > +  gdb::optional<scoped_restore_selected_frame> frame_saver;
> >    if (parse->frame != -1)
> >      {
> >        struct frame_info *fid;
> > @@ -2055,8 +2046,11 @@ mi_cmd_execute (struct mi_parse *parse)
> >  
> >        fid = find_relative_frame (get_current_frame (), &frame);
> >        if (frame == 0)
> > -       /* find_relative_frame was successful */
> > -       select_frame (fid);
> > +        {
> > +          /* find_relative_frame was successful */
> > +          frame_saver.emplace ();
> > +          select_frame (fid);
> > +        }
> >        else
> >         error (_("Invalid frame id: %d"), frame);
> >      }
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PING] Re: [PATCH] MI: PR20684, preserve user selected thread and frame when invoking MI commands
  2021-02-09 10:08 Jan Vrany
@ 2021-02-23 11:20 ` Jan Vrany
  2021-03-02 10:36   ` Jan Vrany
  0 siblings, 1 reply; 7+ messages in thread
From: Jan Vrany @ 2021-02-23 11:20 UTC (permalink / raw)
  To: gdb-patches

Polite ping. 

On Tue, 2021-02-09 at 10:08 +0000, Jan Vrany wrote:
> When invoking MI commands with --thread and/or --frame, user selected
> thread and frame was not preserved:
> 
>   (gdb)
>   info thread
>   &"info thread\n"
>   ~"  Id   Target Id                                           Frame
> \n"
>   ~"* 1    Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-c\" main
> () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> sync.c:60\n"
>   ~"  2    Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-c\"
> child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> selected-context-sync.c:30\n"
>   ~"  3    Thread 0x7ffff742e700 (LWP 19307) \"user-selected-c\"
> child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> selected-context-sync.c:30\n"
>   ^done
>   (gdb)
>   info frame
>   &"info frame\n"
>   ~"Stack level 0, frame at 0x7fffffffdf90:\n"
>   ~" rip = 0x555555555207 in main
> (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:60);
> saved rip = 0x7ffff7c5709b\n"
>   ~" source language c.\n"
>   ~" Arglist at 0x7fffffffdf80, args: \n"
>   ~" Locals at 0x7fffffffdf80, Previous frame's sp is
> 0x7fffffffdf90\n"
>   ~" Saved registers:\n "
>   ~" rbp at 0x7fffffffdf80, rip at 0x7fffffffdf88\n"
>   ^done
>   (gdb)
>   -stack-info-depth --thread 3
>   ^done,depth="4"
>   (gdb)
>   info thread
>   &"info thread\n"
>   ~"  Id   Target Id                                           Frame
> \n"
>   ~"  1    Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-c\" main
> () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-
> sync.c:60\n"
>   ~"  2    Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-c\"
> child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> selected-context-sync.c:30\n"
>   ~"* 3    Thread 0x7ffff742e700 (LWP 19307) \"user-selected-c\"
> child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-
> selected-context-sync.c:30\n"
>   ^done
>   (gdb)
>   info frame
>   &"info frame\n"
>   ~"Stack level 0, frame at 0x7ffff742dee0:\n"
>   ~" rip = 0x555555555169 in child_sub_function
> (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30);
> saved rip = 0x555555555188\n"
>   ~" called by frame at 0x7ffff742df00\n"
>   ~" source language c.\n"
>   ~" Arglist at 0x7ffff742ded0, args: \n"
>   ~" Locals at 0x7ffff742ded0, Previous frame's sp is
> 0x7ffff742dee0\n"
>   ~" Saved registers:\n "
>   ~" rbp at 0x7ffff742ded0, rip at 0x7ffff742ded8\n"
>   ^done
>   (gdb)
> 
> This was problematic for frontends that provide access to CLI because
> UI
> may silently change the context for CLI commands (as demonstrated
> above).
> 
> gdb/Changelog
> 
>         PR 20684
>         * gdb/mi/mi-main.c (mi_cmd_execute): Preserve user selected
> thread
>         and frame when invoking MI commands.
> ---
>  gdb/ChangeLog    |  6 ++++++
>  gdb/mi/mi-main.c | 36 +++++++++++++++---------------------
>  2 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index e67668d315c..ea232ff1b3f 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,9 @@
> +2021-02-09  Jan Vrany  <jan.vrany@labware.com>
> +
> +       PR 20684
> +       * gdb/mi/mi-main.c (mi_cmd_execute): Preserve user selected
> thread
> +       and frame when invoking MI commands.
> +
>  2021-02-08  Shahab Vahedi  <shahab@synopsys.com>
>  
>         PR tdep/27369
> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index 9a14d78e1e2..c5103800314 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -1971,25 +1971,13 @@ mi_execute_command (const char *cmd, int
> from_tty)
>              again.  */
>           && !command_notifies_uscc_observer (command.get ()))
>         {
> -         int report_change = 0;
> -
> -         if (command->thread == -1)
> -           {
> -             report_change = (previous_ptid != null_ptid
> -                              && inferior_ptid != previous_ptid
> -                              && inferior_ptid != null_ptid);
> -           }
> -         else if (inferior_ptid != null_ptid)
> -           {
> -             struct thread_info *ti = inferior_thread ();
> -
> -             report_change = (ti->global_num != command->thread);
> -           }
> -
> -         if (report_change)
> -           {
> -             gdb::observers::user_selected_context_changed.notify
> -               (USER_SELECTED_THREAD | USER_SELECTED_FRAME);
> +         if (command->thread == -1
> +             && previous_ptid != null_ptid
> +             && inferior_ptid != previous_ptid
> +             && inferior_ptid != null_ptid)
> +            {
> +              gdb::observers::user_selected_context_changed.notify
> +                      (USER_SELECTED_THREAD | USER_SELECTED_FRAME);
>             }
>         }
>      }
> @@ -2035,6 +2023,7 @@ mi_cmd_execute (struct mi_parse *parse)
>        set_current_program_space (inf->pspace);
>      }
>  
> +  gdb::optional<scoped_restore_current_thread> thread_saver;
>    if (parse->thread != -1)
>      {
>        thread_info *tp = find_thread_global_id (parse->thread);
> @@ -2045,9 +2034,11 @@ mi_cmd_execute (struct mi_parse *parse)
>        if (tp->state == THREAD_EXITED)
>         error (_("Thread id: %d has terminated"), parse->thread);
>  
> +      thread_saver.emplace ();
>        switch_to_thread (tp);
>      }
>  
> +  gdb::optional<scoped_restore_selected_frame> frame_saver;
>    if (parse->frame != -1)
>      {
>        struct frame_info *fid;
> @@ -2055,8 +2046,11 @@ mi_cmd_execute (struct mi_parse *parse)
>  
>        fid = find_relative_frame (get_current_frame (), &frame);
>        if (frame == 0)
> -       /* find_relative_frame was successful */
> -       select_frame (fid);
> +        {
> +          /* find_relative_frame was successful */
> +          frame_saver.emplace ();
> +          select_frame (fid);
> +        }
>        else
>         error (_("Invalid frame id: %d"), frame);
>      }



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-03-23 13:31 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <b3e449f0581f9ba05b7523aa1aa4c3685511f0aa.camel@labware.com>
2021-03-02 20:27 ` [PING] Re: [PATCH] MI: PR20684, preserve user selected thread and frame when invoking MI commands Jan Vrany
2021-02-09 10:08 Jan Vrany
2021-02-23 11:20 ` [PING] " Jan Vrany
2021-03-02 10:36   ` Jan Vrany
2021-03-02 17:39     ` Jonah Graham
2021-03-09 12:04     ` Jan Vrany
2021-03-17 14:47       ` Jan Vrany
2021-03-23 13:31         ` Jan Vrany

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).