From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1251 invoked by alias); 7 Aug 2014 13:48:46 -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 1241 invoked by uid 89); 7 Aug 2014 13:48:45 -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,SPF_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; Thu, 07 Aug 2014 13:48:44 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s77DmfhB008887 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 7 Aug 2014 09:48:41 -0400 Received: from blade.nx (ovpn-116-43.ams2.redhat.com [10.36.116.43]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s77Dmeee025120; Thu, 7 Aug 2014 09:48:41 -0400 Received: by blade.nx (Postfix, from userid 1000) id 3A6C62640DF; Thu, 7 Aug 2014 14:48:40 +0100 (BST) Date: Thu, 07 Aug 2014 13:48:00 -0000 From: Gary Benson To: Doug Evans Cc: gdb-patches@sourceware.org, Pedro Alves , Tom Tromey Subject: Re: [PATCH 05/11 v5] Add target/target.h Message-ID: <20140807134840.GC19737@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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <21474.27284.80140.944680@ruffy.mtv.corp.google.com> X-IsSubscribed: yes X-SW-Source: 2014-08/txt/msg00140.txt.bz2 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. So forcing non_stop to 1 beforehand like this patch does means a step over might take place that would otherwise not have. In the GDB case forcing non_stop to 1 before target_stop forces GDB to send a SIGSTOP to each LWP. 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. I don't know enough about this to know which is the best solution. Pedro would know, but he's away for the next two weeks. If what is happening currently is correct in both cases then maybe a new target method "target_stop_all" is required to encompass the whole block of code. In the interests of keeping things moving would you be ok for me to commit the following until Pedro is back and able to advise? /* FIXME: This conditional code needs removing, either by setting non_stop in the same place for both cases or by implementing a new client method for this whole block (less the printing code) from "int was_non_stop = non_stop;" to "non_stop = was_non_stop;". */ #ifdef GDBSERVER target_stop (ptid); non_stop = 1; #else non_stop = 1; target_stop (ptid); #endif Thanks, Gary -- http://gbenson.net/