From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23544 invoked by alias); 25 Nov 2007 20:39:26 -0000 Received: (qmail 23536 invoked by uid 22791); 25 Nov 2007 20:39:25 -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 20:39:16 +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 lAPKVbY7001789 (version=TLSv1/SSLv3 cipher=DHE-DSS-AES256-SHA bits=256 verify=NO); Sun, 25 Nov 2007 21:31:38 +0100 Message-ID: <4749DD6E.5090407@champenstudios.com> Date: Sun, 25 Nov 2007 20:39: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: [gdbserver/win32] (4/11) New interrupting method References: <4737B553.2040302@portugalmail.pt> <47498E1A.5070101@champenstudios.com> <4749C306.7050003@portugalmail.pt> In-Reply-To: <4749C306.7050003@portugalmail.pt> 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/msg00473.txt.bz2 My mistake, I did not read the entire patch chain. Pedro Alves escribió: > Lerele wrote: >> I think this patch breaks interrupt functionality. >> The function suspend_one_thread in the new patch does not seem to >> retreive the thread context and sets the thread suspend count to 1, >> so the next call to thread_rec that should happen in gdbserver will >> get incorrect thread context. >> >> My interrupt patch I sent initially used thread_rec to pause the >> child threads, so get context should work here. >> I think it's more correct anyway to use thread_rec everywhere so that >> SuspendThread is centralized in one unique function. >> >> Is this correct, or is there something I may have slipped reading >> your patch? >> > > You slipped reading the previous patch in the series :-) > > The previous patch in the series [1] has this hunk: > > @@ -105,10 +105,19 @@ thread_rec (DWORD id, int get_context) > return NULL; > > th = inferior_target_data (thread); > - if (!th->suspend_count && get_context) > + if (get_context && th->context.ContextFlags == 0) > { > - if (id != current_event.dwThreadId) > - th->suspend_count = SuspendThread (th->h) + 1; > + if (!th->suspended) > + { > + if (SuspendThread (th->h) == (DWORD) -1) > + { > + DWORD err = GetLastError (); > + OUTMSG (("warning: SuspendThread failed in thread_rec, " > + "(error %d): %s\n", (int) err, strwinerror > (err))); > + } > + else > + th->suspended = 1; > + } > > (*the_low_target.get_thread_context) (th, ¤t_event); > } > > Which means that thread_rec will fetch the register context > if it hasn't already (th->context.ContextFlags == 0), > not if the thread wasn't suspended, which makes more sense: > "If you want the registers, and you don't have them already", > as opposed to: > "If you want the registers and the thread is not suspended". > > [1] - [gdbserver/win32] (3/11) Fix suspend count handling > http://sourceware.org/ml/gdb-patches/2007-11/msg00216.html >