From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11323 invoked by alias); 25 Nov 2007 14:44:44 -0000 Received: (qmail 11314 invoked by uid 22791); 25 Nov 2007 14:44:43 -0000 X-Spam-Check-By: sourceware.org Received: from gw.sprintaddict.net (HELO champenstudios.com) (80.91.89.73) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sun, 25 Nov 2007 14:44:37 +0000 Received: from [192.168.1.5] (164.Red-80-36-45.staticIP.rima-tde.net [80.36.45.164]) (authenticated bits=0) by champenstudios.com (8.13.8/8.13.8) with ESMTP id lAPEawuQ032195 (version=TLSv1/SSLv3 cipher=DHE-DSS-AES256-SHA bits=256 verify=NO); Sun, 25 Nov 2007 15:36:59 +0100 Message-ID: <47498A4F.3050101@champenstudios.com> Date: Sun, 25 Nov 2007 14:44:00 -0000 From: Lerele User-Agent: Thunderbird 2.0.0.6 (Windows/20070728) MIME-Version: 1.0 To: Lerele , gdb-patches@sourceware.org Subject: Re: [win32] Fix suspend count handling References: <000001c82c31$4a57b220$df071660$@u-strasbg.fr> <4053daab0711210543w4b241e1ek2371e887f3c4f7d2@mail.gmail.com> <000401c82c48$a450df10$ecf29d30$@u-strasbg.fr> <4053daab0711210708o607018b9n8b63147a8498a207@mail.gmail.com> <4053daab0711211019r15f3a862g677080b65b4d8e71@mail.gmail.com> <4744BCCE.60705@portugalmail.pt> <20071123010744.GA31180@ednor.casa.cgf.cx> <4746A922.30404@champenstudios.com> <20071124053341.GA4214@ednor.casa.cgf.cx> <474832C2.7030307@champenstudios.com> <20071124204828.GB4928@ednor.casa.cgf.cx> In-Reply-To: <20071124204828.GB4928@ednor.casa.cgf.cx> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 8bit X-IsSubscribed: yes 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 X-SW-Source: 2007-11/txt/msg00463.txt.bz2 Christopher Faylor escribió: > On Sat, Nov 24, 2007 at 03:18:42PM +0100, Lerele wrote: > >>>> Christopher Faylor escribi?: >>>> >>>>> I have to ask, however, if the SuspendThread's are even needed at all. >>>>> When I was writing this code, I wasn't entirely sure that gdb needed to >>>>> do anything like this but I erred on the side of caution. So, I'm >>>>> wondering if things would still work ok if the >>>>> SuspendThread/ResumeThread stuff was gone. >>>>> >>>>> >>>> I think they are needed. They were anyway with the new gdbserver based >>>> (vs. Win32 API based) interrupt code I sent several days ago, and that >>>> so very kindly Pedro prepared for commitment, but that I still haven't >>>> found the time to sit down and look at them (however I'm absolutely >>>> sure they're just fine), I guess his changes must be similar to what I >>>> sent in the first place. >>>> >>>> >>> *Why* did you think you needed them in gdbserver? Did you actually try >>> not using them or did you just copy my code from win32-nat.c? >>> >> I thought it natural to implement win32 functionality into gdbserver not >> starting from scratch, but from an already working implementation, such as >> win32-nat.c, however *truly* one drawback of this is that original code >> bugs got inherited into new implementation (such as the subject in >> question), and the porting process however was not really as >> straightforward as copy/paste; certain amount of work was involved (a lot >> of debugging gdbserver and gdb themselves), however I did not stop to think >> that SuspendThread was not actually needed, as it was already there. >> > > So, in answer to my question about whether something was needed or not, > you asserted that it wasn't needed because you copied the code that I > wrote which was the source of my question. Sorry but that doesn't really > answer my question. > > Sure I did, however the situation seems a bit more complicated. I'll explain below. > If Windows already does the SuspendThread on all threads on debug event > then there is no reason to also do it in gdb. I was asking if anyone > had actually checked if that was the case since it has been quite some > time since I did any experiments. > > What I said basically is that it is safer to leave the SuspendThread there as, besides being safer, it may be required for any future enhancement. Also, upon reception of a Win32 debug event, calling SuspendThread should not do any harm, as the threads are already paused. It is also the basis of the new interrupt method, which is SuspendThread based. In my previous message, in the paragraph I wrote: "Apart from this, there's also the case where (at least for gdbserver) socket data is received asynchronously while the child is running. This socket data could indicate gdbserver to set/enable/disable a breakpoint, read thread registers, etc., and this kind of things may require to stop the child using SuspendThread. Right?" I was refering a specific gdbserver functionality that for some reason is not currently working for current cvs gdbserver. In fact right now I do not actually know for sure if this functionality ever worked for cvs gdb and got broken, or it is based on local modifications I have made to gdbserver. Please let me clarify: I have a gdbserver.exe version here that lets you set breakpoints into the child while the child is running (without having to interrupt the child --if you can, set breakpoint and continue the child). For this functionality to work socket data availability check must be done somewhere in the WaitForDebugEvent loop/call trace. I think however that what I did was have get_child_debug_event return a "spurious" signal periodically to have gdbserver check for this socket data and act accordingly to this received data. The really important thing is that in this case, no Win32 debug event was received (and the child was still running), so when gdbserver wanted to access the thread, the thread_rec function was ultimately called, which itself calls SuspendThread. So in this case calling SuspendThread *is* necessary. Now as to the reason why this is not working, I think it's pointless to figure why I have a local gdbserver that asynchrounously lets you set breakpoints, or try figure out if some older official gdbserver version ever had this support. It is not currently working, and I think it is a very nice feature, not only because it actually is useful, but also because people used to other debugging systems (such as msvcmon) are used to this. I think it's best to have it working upstream, and for this, SuspendThread is mandatory. What do you think? Leo.