From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9161 invoked by alias); 20 Aug 2014 15:01:50 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 9145 invoked by uid 89); 20 Aug 2014 15:01:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 20 Aug 2014 15:01:35 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s7KF1Cal018191 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 20 Aug 2014 11:01:13 -0400 Received: from blade.nx (ovpn-116-90.ams2.redhat.com [10.36.116.90]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s7KF1Bap008351; Wed, 20 Aug 2014 11:01:12 -0400 Received: by blade.nx (Postfix, from userid 1000) id 2B9CB2640D2; Wed, 20 Aug 2014 16:01:10 +0100 (BST) Date: Wed, 20 Aug 2014 15:01:00 -0000 From: Gary Benson To: Pedro Alves Cc: Doug Evans , gdb-patches@sourceware.org Subject: Re: [PATCH 05/11 v5] Add target/target.h Message-ID: <20140820150110.GA20827@blade.nx> References: <1406888377-25795-1-git-send-email-gbenson@redhat.com> <1406888377-25795-6-git-send-email-gbenson@redhat.com> <21474.27284.80140.944680@ruffy.mtv.corp.google.com> <20140807134840.GC19737@blade.nx> <53F4B55B.3020207@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <53F4B55B.3020207@redhat.com> X-IsSubscribed: yes X-SW-Source: 2014-08/txt/msg00405.txt.bz2 Pedro Alves wrote: > On 08/07/2014 02:48 PM, Gary Benson wrote: > > Doug Evans wrote: > > > Gary Benson writes: > > > > @@ -284,37 +253,18 @@ agent_run_command (int pid, const char *cmd, > > > > int was_non_stop = non_stop; > > > > /* Stop thread PTID. */ > > > > DEBUG_AGENT ("agent: stop helper thread\n"); > > > > -#ifdef GDBSERVER > > > > - { > > > > - struct thread_resume resume_info; > > > > - > > > > - resume_info.thread = ptid; > > > > - resume_info.kind = resume_stop; > > > > - resume_info.sig = GDB_SIGNAL_0; > > > > - (*the_target->resume) (&resume_info, 1); > > > > - } > > > > - > > > > - non_stop = 1; > > > > - mywait (ptid, &status, 0, 0); > > > > -#else > > > > non_stop = 1; > > > > target_stop (ptid); > > > > > > > > memset (&status, 0, sizeof (status)); > > > > target_wait (ptid, &status, 0); > > > > -#endif > > > > non_stop = was_non_stop; > > > > > > The old gdbserver code set non_stop = 1 *after* asking the target to > > > stop, whereas now it'll be done before (right?). Just checking that > > > that's ok. > > > E.g., I see a test for non_stop in linux_resume (which feels weird to be > > > using in this context given that we're talking about target_stop :-)). > > > > Good catch! I did not notice that change. I also don't know if it's > > ok. > > > > In the gdbserver case forcing non_stop to 1 causes need_step_over > > in linux_resume to become maybe set. > > > If non_stop had been 0 > > need_step_over would definitely be NULL. > > That isn't really true, see: > > any_pending = 0; > if (!non_stop) > find_inferior (&all_threads, resume_status_pending_p, &any_pending); #1 > ... > if (!any_pending && supports_breakpoints ()) > need_step_over > = (struct thread_info *) find_inferior (&all_threads, > need_step_over_p, NULL); > > If non_stop is 0, then we execute #1 above, true. But, that may well > return with ANY_PENDING still clear/0, and so 'need_step_over' may end > up set anyway. > > So looks fine to me. > > > So forcing non_stop to 1 > > beforehand like this patch does means a step over might take place > > that would otherwise not have. > > See above. > > > In the GDB case forcing non_stop to 1 before target_stop forces GDB > > to send a SIGSTOP to each LWP. > > Note we're just really just stopping one LWP here, the agent helper > thread, specified in PTID, not all threads. > > > If non_stop had been 0 linux_nat_stop > > would have fallen back to inf_ptrace_stop which sends one SIGINT to > > the process group. > > Yeah, we definitely want SIGSTOP, not SIGINT here. Really, GDB_SIGNAL_0: > SIGSTOP is how we implement "quiesce with no signal" on Linux -- the > SIGSTOP is not visible to the target_wait caller. Unfortunately we have > a mixup of "interrupt/ctrl-c" vs "quiesce" in the interface. Pedro, to mirror your new 'target_continue_ptid (ptid_t ptid)' function suggestion, I thought I might make a new 'target_stop_ptid (ptid_t ptid)' that would handle the stop/wait combination and the non_stop fiddling. That way everything will stay exactly as it is. Does that sound ok to you? Thanks, Gary -- http://gbenson.net/