From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14017 invoked by alias); 25 Nov 2007 18:47:49 -0000 Received: (qmail 14009 invoked by uid 22791); 25 Nov 2007 18:47:48 -0000 X-Spam-Check-By: sourceware.org Received: from nf-out-0910.google.com (HELO nf-out-0910.google.com) (64.233.182.191) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sun, 25 Nov 2007 18:47:32 +0000 Received: by nf-out-0910.google.com with SMTP id b11so433278nfh for ; Sun, 25 Nov 2007 10:47:28 -0800 (PST) Received: by 10.78.147.6 with SMTP id u6mr1893853hud.1196016447857; Sun, 25 Nov 2007 10:47:27 -0800 (PST) Received: from ?78.130.98.201? ( [78.130.98.201]) by mx.google.com with ESMTPS id h6sm3166212nfh.2007.11.25.10.47.24 (version=TLSv1/SSLv3 cipher=RC4-MD5); Sun, 25 Nov 2007 10:47:26 -0800 (PST) Message-ID: <4749C306.7050003@portugalmail.pt> Date: Sun, 25 Nov 2007 18:47:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; pt-BR; rv:1.8.1.9) Gecko/20071031 Thunderbird/2.0.0.9 Mnenhy/0.7.5.0 MIME-Version: 1.0 To: Lerele CC: gdb-patches@sourceware.org Subject: Re: [gdbserver/win32] (4/11) New interrupting method References: <4737B553.2040302@portugalmail.pt> <47498E1A.5070101@champenstudios.com> In-Reply-To: <47498E1A.5070101@champenstudios.com> 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/msg00469.txt.bz2 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 -- Pedro Alves