From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31889 invoked by alias); 7 Dec 2008 19:04:52 -0000 Received: (qmail 31880 invoked by uid 22791); 7 Dec 2008 19:04:51 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 07 Dec 2008 19:04:10 +0000 Received: (qmail 6459 invoked from network); 7 Dec 2008 19:04:08 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 7 Dec 2008 19:04:08 -0000 From: Pedro Alves To: "Ulrich Weigand" Subject: Re: [rfc] [2/7] infrun cleanup: miscellaneous cleanups Date: Sun, 07 Dec 2008 19:04:00 -0000 User-Agent: KMail/1.9.10 Cc: gdb-patches@sourceware.org References: <200812071823.mB7IN6TL005725@d12av02.megacenter.de.ibm.com> In-Reply-To: <200812071823.mB7IN6TL005725@d12av02.megacenter.de.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200812071904.14370.pedro@codesourcery.com> 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: 2008-12/txt/msg00135.txt.bz2 On Sunday 07 December 2008 18:23:06, Ulrich Weigand wrote: > Well, I was concerned about the cases where today handle_inferior_event > *modifies* the ecs->ptid it gets as input (e.g. thread-hopping or switching > back to the single-stepped thread). I'm not sure if that can today actually > ever cause it switch to a different *inferior*, but it appears it could. > Using simply the caller's ptid means we will miss that switch. Using > inferior_ptid should catch it, however, as handle_inferior_event will > have done a switch_to_thread in addition to changing ecs->ptid. Do those cases cause handle_inferior_event to return 0? Notice that normal_stop uses the last target event (get_last_target_status) to check if this was an inferior exit, because inferior_ptid will usually be set to null_ptid on that case, at that point. But I do see what you mean. There are several cases where we don't switch inferior_ptid to the event ptid, which are wrong in multiprocess. I just haven't been afected by them yet (e.g., TARGET_WAITKIND_LOADED, TARGET_WAITKIND_SYSCALL_ENTRY, new_thread_event), so I didn't address it. The issues I was mentioning here in particular were related to inferior exits. 1) we are not switching inferior_ptid to the event ptid on an inferior exit (before calling target_mourn_inferior). This is of course bad in multi-process, as we mourn the wrong inferior. Furthermore, some targets return pid_to_ptid (-1) instead of the pid of the process that exited, when returning a TARGET_WAITKING_EXITED / TARGET_WAITKING_SIGNALLED. 2) On an inferior exit, the remote target is deleting the inferior from gdb's list even before target_mourn_inferior is called. Calling current_inferior here will assert. 3) most targets' target_mourn_inferior calls generic_mourn_inferior, which deletes the dead inferior from gdb's list, and sets inferior_ptid to null_ptid. This is done inside handle_inferior_event, so, on an inferior exit, inferior_ptid is most of the times null_ptid in the hunk we're talking about in fetch_inferior_event. I'm going to post patches that address #1 and #2. Changing #3 involves further design decisions, so best to leave that out of this series, IMO. The 3 points above mean that unconditionally calling current_inferior in fetch_inferior_event like you were doing asserts. We could make fetch_inferior_event call normal_stop like: if (ecs->ws.kind == TARGET_WAITKIND_EXITED || ecs->ws.kind == TARGET_WAITKIND_SIGNALLED || current_inferior ()->stop_soon == NO_STOP_QUIETLY) normal_stop (); Maybe also add a: || ptid_equal (inferior_ptid, null_ptid)) ... but that will still not be 100% correct due to the missing inferior_ptid switches... -- Pedro Alves