From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x12b.google.com (mail-lf1-x12b.google.com [IPv6:2a00:1450:4864:20::12b]) by sourceware.org (Postfix) with ESMTPS id 6E86A3858D37 for ; Thu, 29 Jun 2023 08:36:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6E86A3858D37 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=undo.io Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=undo.io Received: by mail-lf1-x12b.google.com with SMTP id 2adb3069b0e04-4faaaa476a9so640453e87.2 for ; Thu, 29 Jun 2023 01:36:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=undo.io; s=google2; t=1688027817; x=1690619817; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=WOukgUA1s42KZSFj+68ND9WGXsGg0BWHKqSJCRefNz4=; b=O4PTKY/HHM9lCuFlsCxXbYd8LfvZtDnjGEei9GtdDZ4SY26cKPmD43sLwcNQJjdqjm ooayR5dr/dq64OWGAPRmRqJq+QVmZ2iQVwJ1ZGVUcyVD0cD2GdOav/PXvG5Iqu8s89Pu AMV75VgEo/ZwW7/xTOnQQq+gK1aY2xLnwKwFdziuAiMT5k1QN9rOu2Xwyxf7KJc9HW5X gMzCHxAQcPqOXJJ+WHGnz89cpymHFqxZjqvTSRywpdHOKSvjLBi0O4+69OitX3pPTZNN dxkGl3u2Z1J+K8E2EChtIa9wHM68ePa5qNSt1/3HA/gH9SyT2QgKiSMJu8QLidNtXODH Gesg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1688027817; x=1690619817; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=WOukgUA1s42KZSFj+68ND9WGXsGg0BWHKqSJCRefNz4=; b=APxODekvxqRZJEDWbGmzBk6JJHf2kQHLctbf0S97PGj4XDboTqp56OWXKzOd3hITfl 2MUZ0wLjsnH2mi1fv1uRGvLhD47aCATwAD7/myFuxApqXtZ+X3Oxt4eDMSu0xtt6WENw 5VXrYH0hwWqNZz0nRchJrJrhf3GLrUDdsv7Lr6jRBx4qg6uRSJfT+PNnQKUbbWpBwNU2 kWgFEgQKIuwKiC64v96yx4T+3WG6WP7WZ949HK0pxGC58J6I5iTV9TrV1oIOWffFd3l4 gM2GaNLli5CISIfy6L6wUbeASAenCGvpinvjJfHB3O1bNCIMEz6by39ESJiYN1ggysIW OK4w== X-Gm-Message-State: AC+VfDwPWQYiUgkaAb40koF7T08iMduGJk3jwW5vP9TO/2VBvx4/g7fy 0NUGE8d13Zye4Cn9rZYzWajTQIxrVSbucRFijDU9emJFIfQWvvhBR+GIaozH/cle52pPELcMCA4 5ssiVhtgpvWFA2Cu5j52kQgIyZL5rNlY4H4Cahw01tHswNO+sY48rhTJPp3IN1fP3RrGaAOQ= X-Google-Smtp-Source: ACHHUZ6O8tA7ZGpXhQ7NHXOyc408ESjhhQC+RU82ADum44Hgxoip16MdBBG02fjUPxgsCfPi+epNxA== X-Received: by 2002:a19:4f1a:0:b0:4f8:4512:c846 with SMTP id d26-20020a194f1a000000b004f84512c846mr20935896lfb.49.1688027816725; Thu, 29 Jun 2023 01:36:56 -0700 (PDT) Received: from sbrinz-thinkpad.undoers.io (ip-185-104-136-31.ptr.icomera.net. [185.104.136.31]) by smtp.gmail.com with ESMTPSA id g11-20020a7bc4cb000000b003fbab76165asm5195747wmk.48.2023.06.29.01.36.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 29 Jun 2023 01:36:56 -0700 (PDT) From: Magne Hov To: gdb-patches@sourceware.org Cc: Magne Hov Subject: [PATCH 1/2] gdb: keep record of thread numbers for reverse execution targets Date: Thu, 29 Jun 2023 09:36:50 +0100 Message-Id: <20230629083651.3145268-2-mhov@undo.io> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20230629083651.3145268-1-mhov@undo.io> References: <20230629083651.3145268-1-mhov@undo.io> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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: Targets that support reverse execution may report threads that have previously been seen to exit. Currently, GDB assigns a new thread number every time it sees the same thread (re)appearing, but this is problematic because it makes the output of `info threads` inconsistent, and because thread-specific breakpoints depend on stable thread-numbers in order to stop on the right thread. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23454 --- gdb/inferior.c | 1 + gdb/inferior.h | 7 +++++++ gdb/thread.c | 38 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/gdb/inferior.c b/gdb/inferior.c index eee4785fbf7..b6323110e69 100644 --- a/gdb/inferior.c +++ b/gdb/inferior.c @@ -258,6 +258,7 @@ inferior::clear_thread_list (bool silent) delete thr; }); ptid_thread_map.clear (); + ptid_thread_num_map.clear (); } /* Notify interpreters and observers that inferior INF was removed. */ diff --git a/gdb/inferior.h b/gdb/inferior.h index caa8e4d494a..33eb857645e 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -569,6 +569,13 @@ class inferior : public refcounted_object, /* The highest thread number this inferior ever had. */ int highest_thread_num = 0; + /* A map of ptid_t to global thread number and per-inferior thread + number, used for targets that support reverse execution. These + mappings are maintained for the lifetime of the inferior so that + thread numbers can be reused for threads that reappear after + having exited. */ + std::unordered_map, hash_ptid> ptid_thread_num_map; + /* State of GDB control of inferior process execution. See `struct inferior_control_state'. */ inferior_control_state control; diff --git a/gdb/thread.c b/gdb/thread.c index 7f7f035b5ab..31897bddbb1 100644 --- 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 (); + /* 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. */ + if (target_can_execute_reverse ()) + { + auto pair = inf_->ptid_thread_num_map.find (ptid_); + if (pair != inf_->ptid_thread_num_map.end ()) + { + this->global_num = pair->second.first; + this->per_inf_num = pair->second.second; + } + else { + this->global_num = ++highest_thread_num; + this->per_inf_num = ++inf_->highest_thread_num; + if (target_can_execute_reverse ()) + inf_->ptid_thread_num_map[ptid_] = std::make_pair (this->global_num, + this->per_inf_num); + } + } + else + { + this->global_num = ++highest_thread_num; + this->per_inf_num = ++inf_->highest_thread_num; + } /* Nothing to follow yet. */ this->pending_follow.set_spurious (); -- 2.25.1