From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f52.google.com (mail-wr1-f52.google.com [209.85.221.52]) by sourceware.org (Postfix) with ESMTPS id 078C93857032 for ; Tue, 2 Mar 2021 20:27:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 078C93857032 Received: by mail-wr1-f52.google.com with SMTP id e10so21019014wro.12 for ; Tue, 02 Mar 2021 12:27:48 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:date:references :user-agent:mime-version:content-transfer-encoding; bh=2puecTkQS2wpdVaw0/sNxBDTC4XzzcLKOEi7Bj4/CE0=; b=ukcDL98kI0iZmk6dMDLkB5dynin19SZOQW/ffZsDliGL+GKqrl3XZ7gBMKr6xCGX0Q qGfopaf4cxFO189VZObnNMzYk4GB/mM6Njdd/79W+zCFweOO+/SJ8GkAhVArQgWrT9dB e0J44atj9FxktJLRlheYnIy+JoXtNCfpJB4aQ5Atjgs28OX/5+TARJvAFBRii/5fRaSg 8G96tUxP6p/DHLjPE2vbV18UqMRlUGVr4uAW0x82+14o/YUd0ENOjDWhqBs/7Wf1Yjhc 3J1VZTkCk2p0u9pz1B807dsg/GEAKOaxb0zyzrvZh3UWNCvF3C2IGhJ8Bz8eCuov+/Eq Y3Qg== X-Gm-Message-State: AOAM532r60y+bi+0XixDwXaSR1EYMRmcKv1HrrIuXtIJyuKkA5m/0cjn HxjXY8cauMuXdzR6asfwm/v5itfm37E= X-Google-Smtp-Source: ABdhPJxV/uJOD0MnVllp5enhYutyba9vP7lA5RNMAU+Z0/msF6ecNVy3YnoLy+A1Kj/xT1hfpjvjDg== X-Received: by 2002:a5d:55c4:: with SMTP id i4mr3800394wrw.84.1614716868067; Tue, 02 Mar 2021 12:27:48 -0800 (PST) Received: from ?IPv6:2a02:c7d:d65a:4200:2236:f008:480a:c873? ([2a02:c7d:d65a:4200:2236:f008:480a:c873]) by smtp.gmail.com with ESMTPSA id r11sm7219636wrm.26.2021.03.02.12.27.47 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Mar 2021 12:27:47 -0800 (PST) Message-ID: <0e0d2da13effefc3005ac610ced32b6b81e248c6.camel@labware.com> Subject: Re: [PING] Re: [PATCH] MI: PR20684, preserve user selected thread and frame when invoking MI commands From: Jan Vrany To: "gdb-patches@sourceware.org" Date: Tue, 02 Mar 2021 20:27:46 +0000 References: Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.38.3-1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.7 required=5.0 tests=BAYES_00, BODY_8BITS, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Mar 2021 20:27:51 -0000 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 ) 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 ) 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  > > > > + > > > > +       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  > > > > > > > >         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 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 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); > > > >      } > > > > > > > > >