From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17336 invoked by alias); 4 Nov 2007 12:40:29 -0000 Received: (qmail 17283 invoked by uid 22791); 4 Nov 2007 12:40:28 -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, 04 Nov 2007 12:40:25 +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 lA4CXIce026919 (version=TLSv1/SSLv3 cipher=DHE-DSS-AES256-SHA bits=256 verify=NO); Sun, 4 Nov 2007 13:33:20 +0100 Message-ID: <472DBDB3.1040602@champenstudios.com> Date: Sun, 04 Nov 2007 12:40:00 -0000 From: Lerele User-Agent: Thunderbird 2.0.0.6 (Windows/20070728) MIME-Version: 1.0 To: Pedro Alves CC: gdb-patches@sourceware.org Subject: Re: [RFC] New gdbserver Win32 interrupt code References: <472D2035.5070004@champenstudios.com> <472D2E1B.2020203@portugalmail.pt> In-Reply-To: <472D2E1B.2020203@portugalmail.pt> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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/msg00038.txt.bz2 > Welcome back! > Thanks! > I also implemented the synthetic suspending here, because soon > after that thread, I had a need for it in WinCE. Unfortunatelly, I got > distracted with other things and never submitted. I hate it when > work gets duplicated :( Yep, me too. :( There's one benefit though, which is that things that slip through one's mind, may not through the other's. Merging may give us the super-patch! ;) > > At a quick glance, I think your patch should be at least split > in 3. You mean three different patches? Submitted individually? I didn't submit thread pause counting separately, as it was a really simple fix (one line). I'll write that down for next time. > > 1 - the synthetic suspending > > On WinCE, the synthetic suspending is also needed when > attaching, since in opposition to 9xNT, the OS doesn't stop > the inferior at all (neither when attaching nor running). > If you're OK with it, I'll clean up my patch a bit, and merge > with your bits so 1 can go in first. It will be easier for me > this way, since I've already taken care of the function > reusing and ifdefing minimising. I'll then volunteer to > clean up the rest of your patch for you, as a reward for > you patience ;) > Sure, no problem with that. When you do so, please note that in the patch main pausing function synthetic_child_interrupt, which just pauses or resumes the child process, every single line is important (at least for Win32) and is there for a reason, as is also the order of calls, according to the conversation in the old thread. I'd like to know what you are referring to when you say clean up the rest of my patch. I took care of properly formatting code, full stops, and even fixed some previous erroneous code indentation and formatting. I'd like to know, what is it I missed that you are talking about? I'd also like to know what ifdefing in the patch I submitted that you don't like. The NEW_INT macro is the only one I sent, and I left it there just in case this new interrupt code isn't suitable for all, but can be removed easily. Also, note that I did not include this synthetic_child_interrupt into separate architecture files (i386/arm) because of one of the benefits of this pausing method, which is portability among Windows versions, and this new code is compatible from Win95 and newer, however I don't know about WinCE. > 2 - the suspend count handling > > This is also a problem in native debugging > (gdb/win32-nat.c). I also saw this when doing my version Yes, actually when I wrote win32 gdbserver port, I started from modifying gdb/win32-nat.c, and the thread_rec function if I remember right is the same one, apart from the arch (i386/arm) modifications you made, so truly the problem must be there in native gdb too. By the way, maybe this interrupt method could be of interest for native Win32 gdb. If so, should we handle the native win32 patch, or should that be done by their respective maintainers? Also note, that this problem is not a problem I have seen to fail in practice, so maybe it isn't a problem at all (for some reason). This fix is just something I noticed when I wrote the new interrupt function, which ultimately relies on thread_rec function. > of 1. I got a chance to look at the logs of the native > WinCE debugger, and could infer that it also takes care of > this correctly. The way they do it, is to read the current > suspend count by always doing a SuspendThread + ResumeThread > sequence on a debug event (ResumeThread return the current > suspend count). But isn't this wrong too, according to my explanation in my previous message? The problem actually is that when resuming a thread in win32-nat.c, in function continue_one_thread, which resumes a thread suspend_count times, which itself was being set to the number of suspends of the thread as reported by Windows, so for example if the client app had a thread, say suspended 2 times, and then gdbserver suspends the thread again, the internal gdbserver count would be 3, and when resuming it, gdbserver would wake it up three times, which is wrong because the child wanted it to be suspended 2 times. > > 3 - the elevation > What do you mean by this? Regards, Leo.