From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f44.google.com (mail-wr1-f44.google.com [209.85.221.44]) by sourceware.org (Postfix) with ESMTPS id 4CF583858C31 for ; Wed, 20 Sep 2023 17:13:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4CF583858C31 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wr1-f44.google.com with SMTP id ffacd0b85a97d-31f7638be6eso35461f8f.3 for ; Wed, 20 Sep 2023 10:13:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695230006; x=1695834806; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=eot4CJaE2DLZRG/sXZcAHdp6+DXOjEIdxkwYdd7/IV4=; b=oziSytwzXgUQnvQ/QOtIkwFTxalQE/OhPs/6jpvikfQXUUNxhrL0iO4f9Frp5Qg5lc 8exvL8rHA9azr1f1ljUuXj9KmiH1rjKNKyubH8aS5X2WNZOfZ4K2ZM4WvSQk2qvFH/oX yxPazHdsZbAlf8BTV7clHIUgW99LZ248ZQkY1TzQ0YVmIQJw639wAa2CNFrwyVBtqD4n 2m07xX9n/nLz2rjE23oi+tXtcZLiD3mkYPeYOMTLWg8jaLUcRCStyjLiSo5hOYeUVrh+ ro6wnCPNqT/2Yr0P1ralcKkmcc23m3ZKOLYCAZCCbDrJiND+ze2PpgVfiF7FVoKjE+iF HU5g== X-Gm-Message-State: AOJu0YyfDaTMt932Dm0s7gIRkENncJlxUSwPn0J/U555RWoHFajMuHEd yUY9rfhNGJPdvCWfkfZflPbkifxeO2VBVA== X-Google-Smtp-Source: AGHT+IF/ryE+2W4uJg4j8GmzUHU1GYmt/UKZxu3yc68VsPt0y6u0ioyT+Qv0erIQJQA3F5+hpBSITA== X-Received: by 2002:adf:fd0e:0:b0:31f:97e2:a924 with SMTP id e14-20020adffd0e000000b0031f97e2a924mr2989033wrr.14.1695230005774; Wed, 20 Sep 2023 10:13:25 -0700 (PDT) Received: from ?IPV6:2001:8a0:f939:d200:f66d:9e3b:9c38:4aec? ([2001:8a0:f939:d200:f66d:9e3b:9c38:4aec]) by smtp.gmail.com with ESMTPSA id j5-20020a5d4645000000b003214fc12a30sm9744472wrs.106.2023.09.20.10.13.24 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 20 Sep 2023 10:13:25 -0700 (PDT) Message-ID: Date: Wed, 20 Sep 2023 18:13:15 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [PATCH v2 1/2] gdb: keep record of thread numbers for reverse execution targets Content-Language: en-US To: Magne Hov , gdb-patches@sourceware.org References: <20230629083651.3145268-1-mhov@undo.io> <20230707162451.3605544-1-mhov@undo.io> <20230707162451.3605544-2-mhov@undo.io> From: Pedro Alves In-Reply-To: <20230707162451.3605544-2-mhov@undo.io> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 2023-07-07 17:24, Magne Hov via Gdb-patches wrote: > --- a/gdb/thread.c > +++ b/gdb/thread.c > @@ -290,6 +290,15 @@ add_thread_silent (process_stratum_target *targ, ptid_t ptid) > inf->num, ptid.to_string ().c_str (), > targ->shortname ()); > > + /* Targets that support reverse execution can see the same thread > + being added multiple times. If the state for an exited thread is > + still present in the inferior's thread list it must be cleaned up > + now before we add a new non-exited entry for the same thread. > + Targets without reverse execution are not affected by this because > + they do not reuse thread numbers. */ > + if (target_can_execute_reverse ()) > + delete_exited_threads (); What if the exited thread that we are re-adding is the current thread? delete_exited_threads won't delete it, because you can't delete the current thread. (See thread_info::deletable().) > + > /* We may have an old thread with the same id in the thread list. > If we do, it must be dead, otherwise we wouldn't be adding a new > thread with the same id. The OS is reusing this id --- delete > @@ -332,8 +341,33 @@ thread_info::thread_info (struct inferior *inf_, ptid_t ptid_) > { > gdb_assert (inf_ != NULL); > > - this->global_num = ++highest_thread_num; > - this->per_inf_num = ++inf_->highest_thread_num; > + /* Targets that support reverse execution may see the same thread be > + created multiple times so a historical record must be maintained > + and queried. For targets without reverse execution we don't look > + up historical thread numbers because it leaves us vulnerable to > + collisions between thread identifiers that have been recycled by > + the target operating system. */ I'm worried a little about the assumption that only targets that don't support reverse execution need to handle the case of thread identifiers being recycled. If you record gdb.threads/tid-reuse.exp you should hit the tid reuse case, for instance. Wouldn't it be better to distinguish adding a thread when executing forward (in which case we treat the thread id as potentially recycled by the OS), vs executing backward or replaying forward (in which case we reuse the gdb thread id from the previous time.) Pedro Alves