Message ID | 83k2vas7f1.fsf@gnu.org |
---|---|
State | New, archived |
Headers |
Received: (qmail 76644 invoked by alias); 11 Jun 2015 13:26:23 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 76634 invoked by uid 89); 11 Jun 2015 13:26:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.1 required=5.0 tests=AWL, BAYES_50, RCVD_IN_DNSWL_NONE, SPF_SOFTFAIL autolearn=no version=3.3.2 X-HELO: mtaout23.012.net.il Received: from mtaout23.012.net.il (HELO mtaout23.012.net.il) (80.179.55.175) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 11 Jun 2015 13:26:20 +0000 Received: from conversion-daemon.a-mtaout23.012.net.il by a-mtaout23.012.net.il (HyperSendmail v2007.08) id <0NPS00L007HJLV00@a-mtaout23.012.net.il> for gdb-patches@sourceware.org; Thu, 11 Jun 2015 16:26:18 +0300 (IDT) Received: from HOME-C4E4A596F7 ([87.69.4.28]) by a-mtaout23.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0NPS00LRA7ZTM310@a-mtaout23.012.net.il>; Thu, 11 Jun 2015 16:26:18 +0300 (IDT) Date: Thu, 11 Jun 2015 16:26:10 +0300 From: Eli Zaretskii <eliz@gnu.org> Subject: Re: Inadvertently run inferior threads In-reply-to: <83y4jrsgui.fsf@gnu.org> To: Pedro Alves <palves@redhat.com> Cc: gdb-patches@sourceware.org Reply-to: Eli Zaretskii <eliz@gnu.org> Message-id: <83k2vas7f1.fsf@gnu.org> References: <83h9tq3zu3.fsf@gnu.org> <55043A63.6020103@redhat.com> <8361a339xd.fsf@gnu.org> <5504555C.804@redhat.com> <550458E0.10206@redhat.com> <83y4jrsgui.fsf@gnu.org> X-IsSubscribed: yes |
Commit Message
Eli Zaretskii
June 11, 2015, 1:26 p.m. UTC
[I'm taking this to gdb-patches@, as I'm suggesting a patch.] > Date: Wed, 10 Jun 2015 18:50:13 +0300 > From: Eli Zaretskii <eliz@gnu.org> > Cc: gdb@sourceware.org > > So 'set_running' is called, and it is called with minus_one_ptid, > which then has the effect of marking all the threads as running. > > What I don't understand is why doesn't the breakpoint we set at exit > from the inferior function countermand that. I do see the effect of > that breakpoint if I turn on infrun debugging: > > infrun: target_wait (-1, status) = > infrun: 4608 [Thread 4608.0x4900], > infrun: status->kind = stopped, signal = GDB_SIGNAL_TRAP > infrun: TARGET_WAITKIND_STOPPED > infrun: stop_pc = 0x88ba9f > infrun: BPSTAT_WHAT_STOP_SILENT > infrun: stop_waiting > > Why don't we mark all threads as stopped when we hit the breakpoint? > is that because of #3 above? Answering myself: yes. > Any ideas how to solve this annoying problem? One idea is implemented in the patch below. I tested it in a MinGW-compiled GDB doing native debugging, and it completely solved the problem for me: I see additional threads started during an infcall, but none of those dreaded "the selected thread is running" error messages. OK to commit (with a suitable ChangeLog entry)?
Comments
Ping! With the GDB 7.10 branching date looming, could we please decide ASAP if the patch I proposed for this annoying problem is good to go in? TIA. > Date: Thu, 11 Jun 2015 16:26:10 +0300 > From: Eli Zaretskii <eliz@gnu.org> > Cc: gdb-patches@sourceware.org > > [I'm taking this to gdb-patches@, as I'm suggesting a patch.] > > > Date: Wed, 10 Jun 2015 18:50:13 +0300 > > From: Eli Zaretskii <eliz@gnu.org> > > Cc: gdb@sourceware.org > > > > So 'set_running' is called, and it is called with minus_one_ptid, > > which then has the effect of marking all the threads as running. > > > > What I don't understand is why doesn't the breakpoint we set at exit > > from the inferior function countermand that. I do see the effect of > > that breakpoint if I turn on infrun debugging: > > > > infrun: target_wait (-1, status) = > > infrun: 4608 [Thread 4608.0x4900], > > infrun: status->kind = stopped, signal = GDB_SIGNAL_TRAP > > infrun: TARGET_WAITKIND_STOPPED > > infrun: stop_pc = 0x88ba9f > > infrun: BPSTAT_WHAT_STOP_SILENT > > infrun: stop_waiting > > > > Why don't we mark all threads as stopped when we hit the breakpoint? > > is that because of #3 above? > > Answering myself: yes. > > > Any ideas how to solve this annoying problem? > > One idea is implemented in the patch below. I tested it in a > MinGW-compiled GDB doing native debugging, and it completely solved > the problem for me: I see additional threads started during an > infcall, but none of those dreaded "the selected thread is running" > error messages. > > OK to commit (with a suitable ChangeLog entry)? > > --- gdb/infrun.c~0 2015-01-13 15:14:47 +0200 > +++ gdb/infrun.c 2015-06-11 07:13:54 +0300 > @@ -6508,8 +6508,14 @@ normal_stop (void) > running, all without informing the user/frontend about state > transition changes. If this is actually a call command, then the > thread was originally already stopped, so there's no state to > - finish either. */ > - if (target_has_execution && inferior_thread ()->control.in_infcall) > + finish either. Howevever, if target doesn't support asynchronous > + execution, we must mark all of its threads as stopped, because > + that's what such targets do when the thread running in an infcall > + stops. (The cleanup will call finish_thread_state with > + minus_one_ptid in that case.) */ > + if (target_has_execution > + && inferior_thread ()->control.in_infcall > + && target_can_async_p ()) > discard_cleanups (old_chain); > else > do_cleanups (old_chain); >
On 06/13/2015 12:00 PM, Eli Zaretskii wrote: > Ping! With the GDB 7.10 branching date looming, could we please > decide ASAP if the patch I proposed for this annoying problem is good > to go in? Off hand, I don't think the patch is correct. I'll need a bit more to think this through and give a better response. I'm not seeing why target_can_async targets would be special here: target_can_async targets also stop all threads when they report an event out of target_wait. I'll go respond to some of your other mails. Meanwhile, the way to make sure that we don't forget considering fixing something for the release, is to list it in the wiki's release page: https://sourceware.org/gdb/wiki/GDB_7.10_Release Thanks, Pedro Alves
> Date: Mon, 15 Jun 2015 13:58:13 +0100 > From: Pedro Alves <palves@redhat.com> > CC: gdb-patches@sourceware.org > > On 06/13/2015 12:00 PM, Eli Zaretskii wrote: > > Ping! With the GDB 7.10 branching date looming, could we please > > decide ASAP if the patch I proposed for this annoying problem is good > > to go in? > > Off hand, I don't think the patch is correct. I'll need a bit more > to think this through and give a better response. I'm not seeing why > target_can_async targets would be special here: target_can_async targets > also stop all threads when they report an event out of target_wait. One difference is that in the former case, the call to set_running marks _all_ the threads running. But I'm okay with making this a MinGW-specific change, since other platforms need to do something extraordinary to get into this situation, while on Windows this happens out of my or the inferior's control, and its effect on the debugging session is so devastating. > Meanwhile, the way to make sure that we don't forget > considering fixing something for the release, is to list it > in the wiki's release page: > > https://sourceware.org/gdb/wiki/GDB_7.10_Release Done. I hope I formatted the entry according to expectations.
--- gdb/infrun.c~0 2015-01-13 15:14:47 +0200 +++ gdb/infrun.c 2015-06-11 07:13:54 +0300 @@ -6508,8 +6508,14 @@ normal_stop (void) running, all without informing the user/frontend about state transition changes. If this is actually a call command, then the thread was originally already stopped, so there's no state to - finish either. */ - if (target_has_execution && inferior_thread ()->control.in_infcall) + finish either. Howevever, if target doesn't support asynchronous + execution, we must mark all of its threads as stopped, because + that's what such targets do when the thread running in an infcall + stops. (The cleanup will call finish_thread_state with + minus_one_ptid in that case.) */ + if (target_has_execution + && inferior_thread ()->control.in_infcall + && target_can_async_p ()) discard_cleanups (old_chain); else do_cleanups (old_chain);