From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 034D3385DC1B for ; Wed, 15 Apr 2020 15:42:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 034D3385DC1B Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark@simark.ca Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 6BFA21E5F9; Wed, 15 Apr 2020 11:42:44 -0400 (EDT) Subject: Re: [PATCH 00/28] Decouple inferior_ptid/inferior_thread(); dup ptids in thread list (PR/25412) To: Pedro Alves , gdb-patches@sourceware.org References: <20200414175434.8047-1-palves@redhat.com> <89e862fa-678d-7b64-6776-c9179fce6ba6@simark.ca> <0b87f757-dfa8-6737-1e28-f6534d44bf11@redhat.com> From: Simon Marchi Message-ID: Date: Wed, 15 Apr 2020 11:42:44 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <0b87f757-dfa8-6737-1e28-f6534d44bf11@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: fr Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Apr 2020 15:42:46 -0000 On 2020-04-15 11:33 a.m., Pedro Alves wrote: > On 4/15/20 3:46 PM, Simon Marchi wrote: >> On 2020-04-14 1:54 p.m., Pedro Alves via Gdb-patches wrote: >>> In PR/25412, Simon noticed that after the multi-target series, the >>> tid-reuse.exp testcase manages to create a duplicate thread in the >>> thread list. Or rather, two threads with the same PTID. >>> >>> This in turn exposes a design problem in GDB. The inferior_thread() >>> function looks up the current thread based on inferior_ptid: >>> >>> struct thread_info* >>> inferior_thread (void) >>> { >>> struct thread_info *tp = find_thread_ptid (current_inferior (), inferior_ptid); >>> gdb_assert (tp); >>> return tp; >>> } >>> >>> But if there can be more than one thread in the thread list with the >>> same ptid_t, inferior_thread() may well return the wrong thread. >> >> I don't quite understand this part of the explanation. Thread lists are >> per-inferior, and an inferior is specific to one target, and ptids are >> unique per-target. So it's not possible at the moment (or at least not >> expected) to have two threads with the same ptid in one specific list. > > The tid-reuse.exp testcase is about: > > # Test running a program that spawns enough threads that the tid of an > # exited thread is reused. GDB should not crash when this happens. > > I.e., the testcase spawns enough short-lived threads that we end up > in add_thread_silent adding a new thread with the same ptid as > a preexisting thread, because the kernel tids eventually wrap around. > In this scenario, you know the preexisting thread with that ptid must > be gone, since otherwise the kernel would not be reusing the thread id. > > But if that thread that is gone was the currently selected > thread, or somewhere throughout gdb something holds a reference > to it, then we can't delete that exiting thread immediately. That's > how you end up with more than one thread with the same ptid in the > thread list. Only one of those threads will be live, all other > "dups" will be exited threads. > > So given that currently inferior_thread() does a look up by > ptid, it can happen that GDB meant to have the live thread > selected temporarily, say, but inferior_thread() returns the > exited thread. Like, this could fail: > > switch_to_thread (exited_thread); > { > scoped_restore_current_thread restore_thread; > switch_to_thread (live_thread); > gdb_assert (inferior_thread () == live_thread); > } > > Note this happens with a single inferior. > > Thanks, > Pedro Alves > Ok thanks, that clarifies it. Simon