From patchwork Sat Dec 3 21:13:34 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 61453 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8527A385781F for ; Sat, 3 Dec 2022 21:14:05 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-wm1-f49.google.com (mail-wm1-f49.google.com [209.85.128.49]) by sourceware.org (Postfix) with ESMTPS id 6617A3858422 for ; Sat, 3 Dec 2022 21:13:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6617A3858422 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-wm1-f49.google.com with SMTP id ay27-20020a05600c1e1b00b003d070f4060bso7057250wmb.2 for ; Sat, 03 Dec 2022 13:13:48 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=AzZlfw/YJkzAqhrvTcFM4YDRdfYSmj6nkx9Is7WMQ2Q=; b=BAFS67SHUbxrtdILCv7GLlCbhGcEvrwp0MzAaowZxMgf2Q55Ynb+JDT0hGqUBr/PFU eb1j3jqpTgPoZt7AmYg1tKGk7feL9n9EHsjDi6bTMNZfOFsAY2ABDuGjGcbLUUsb+nzi 6/orNTYs5Us1cvj3zd7jU/ja5g15CIEV5SmV3pAPdYXImO2i9DbEXmhg+cjstB2J865Z 7HPXyCESEFsSWPekZE1xBcJWgrnE5Yk4ZeyDdQVEFeWtX3DOsGbOQM0Wkm2TYXVaKVHJ cUVpFxzgp+YaP1biNdl3oSHwxyjiLvWPMZzUHTjXyCx6ediLucSJ60VKzMejufkZWlno +QoQ== X-Gm-Message-State: ANoB5plXqIRfgQGp1NUT2+LZZNd3LKcUaQxRrXR2Tx0iltBUIhciMEVI 3ug4QNo2YPoG5YhBFEeCGKeg4bxzk6NpeA== X-Google-Smtp-Source: AA0mqf5mDKGyKAe1Nh9BM1iZD5VVACReiQafegRS7j8RfOCw5ZsSb2lUhXHvzlgDNmk1GtpgVG+VFw== X-Received: by 2002:a05:600c:3587:b0:3cf:a9c2:2b04 with SMTP id p7-20020a05600c358700b003cfa9c22b04mr55267253wmq.152.1670102026727; Sat, 03 Dec 2022 13:13:46 -0800 (PST) Received: from localhost ([2001:8a0:f912:6700:afd9:8b6d:223f:6170]) by smtp.gmail.com with ESMTPSA id g13-20020a05600c310d00b003a2f2bb72d5sm20621933wmo.45.2022.12.03.13.13.46 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 03 Dec 2022 13:13:46 -0800 (PST) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH 2/6] Convert previous_inferior_ptid to strong reference to thread_info Date: Sat, 3 Dec 2022 21:13:34 +0000 Message-Id: <20221203211338.2264994-3-pedro@palves.net> X-Mailer: git-send-email 2.36.0 In-Reply-To: <20221203211338.2264994-1-pedro@palves.net> References: <20221203211338.2264994-1-pedro@palves.net> MIME-Version: 1.0 X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, 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 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: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" I originally wrote this patch, because while working on some other patch, I spotted a regression in the gdb.multi/multi-target-no-resumed.exp.exp testcase. Debugging the issue, I realized that the problem was related to how I was using previous_inferior_ptid to look up the thread the user had last selected. The problem is that previous_inferior_ptid alone doesn't tell you which target that ptid is from, and I was just always using the current target, which was incorrect. Two different targets may have threads with the same ptid. I decided to fix this by replacing previous_inferior_ptid with a strong reference to the thread, called previous_thread. I have since found a new motivation for this change -- I would like to tweak "info program" to not rely on get_last_target_status returning a ptid that still exists in the thread list. With both the follow_fork changes later in this series, and the step-over-thread-exit changes, that can happen, as we'll delete threads and not clear the last waitstatus. A new update_previous_thread function is added that can be used to update previous_thread from inferior_ptid. This must be called in several places that really want to get rid of previous_thread thread, and reset the thread id counter, otherwise we get regressions like these: (gdb) info threads -gid Id GId Target Id Frame - * 1 1 Thread 2974541.2974541 "tids-gid-reset" main () at src/gdb/testsuite/gdb.multi/tids-gid-reset.c:21 - (gdb) PASS: gdb.multi/tids-gid-reset.exp: single-inferior: after restart: info threads -gid + * 1 2 Thread 2958361.2958361 "tids-gid-reset" main () at src/gdb/testsuite/gdb.multi/tids-gid-reset.c:21 + (gdb) FAIL: gdb.multi/tids-gid-reset.exp: single-inferior: after restart: info threads -gid and: Core was generated by `build/gdb/testsuite/outputs/gdb.reverse/sigall-precsave/si'. Program terminated with signal SIGTRAP, Trace/breakpoint trap. #0 gen_ABRT () at src/gdb/testsuite/gdb.reverse/sigall-reverse.c:398 398 kill (getpid (), SIGABRT); +[Current thread is 1 (LWP 2662066)] Restored records from core file build/gdb/testsuite/outputs/gdb.reverse/sigall-precsave/sigall.precsave. #0 gen_ABRT () at src/gdb/testsuite/gdb.reverse/sigall-reverse.c:398 398 kill (getpid (), SIGABRT); continue Continuing. -Program received signal SIGABRT, Aborted. +Thread 1 received signal SIGABRT, Aborted. 0x00007ffff7dfd55b in kill () at ../sysdeps/unix/syscall-template.S:78 78 ../sysdeps/unix/syscall-template.S: No such file or directory. -(gdb) PASS: gdb.reverse/sigall-precsave.exp: sig-test-1: get signal ABRT +(gdb) FAIL: gdb.reverse/sigall-precsave.exp: sig-test-1: get signal ABRT I.e., GDB was failing to restart the thread counter back to 1, because the previous_thread thread was being help due to the strong reference. Tested on GNU/Linux native, gdbserver and gdbserver + "maint set target-non-stop on". gdb/ChangeLog: yyyy-mm-dd Pedro Alves * infcmd.c (kill_command, detach_command, disconnect_command): Call update_previous_thread. * infrun.c (previous_inferior_ptid): Delete. (previous_thread): New. (update_previous_thread): New. (proceed, init_wait_for_inferior): Call update_previous_thread. (normal_stop): Adjust to compare previous_thread and inferior_thread. Call update_previous_thread. * infrun.h (update_previous_thread): Declare. * target.c (target_pre_inferior, target_preopen): Call update_previous_thread. Change-Id: I42779a1ee51a996fa1e8f6e1525c6605dbfd42c7 --- gdb/infcmd.c | 5 +++++ gdb/infrun.c | 43 ++++++++++++++++++++++++++++--------------- gdb/infrun.h | 4 ++++ gdb/target.c | 5 +++++ 4 files changed, 42 insertions(+), 15 deletions(-) diff --git a/gdb/infcmd.c b/gdb/infcmd.c index a27d3577b3a..bffe8d554e7 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -2456,6 +2456,8 @@ kill_command (const char *arg, int from_tty) target_kill (); bfd_cache_close_all (); + update_previous_thread (); + if (print_inferior_events) gdb_printf (_("[Inferior %d (%s) killed]\n"), infnum, pid_str.c_str ()); @@ -2816,6 +2818,8 @@ detach_command (const char *args, int from_tty) target_detach (current_inferior (), from_tty); + update_previous_thread (); + /* The current inferior process was just detached successfully. Get rid of breakpoints that no longer make sense. Note we don't do this within target_detach because that is also used when @@ -2854,6 +2858,7 @@ disconnect_command (const char *args, int from_tty) target_disconnect (args, from_tty); no_shared_libraries (nullptr, from_tty); init_thread_list (); + update_previous_thread (); if (deprecated_detach_hook) deprecated_detach_hook (); } diff --git a/gdb/infrun.c b/gdb/infrun.c index c67458b30b6..a936f18fbb1 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -151,8 +151,18 @@ show_step_stop_if_no_debug (struct ui_file *file, int from_tty, /* proceed and normal_stop use this to notify the user when the inferior stopped in a different thread than it had been running in. */ +static thread_info_ref previous_thread; -static ptid_t previous_inferior_ptid; +/* See infrun.h. */ + +void +update_previous_thread () +{ + if (inferior_ptid == null_ptid) + previous_thread = nullptr; + else + previous_thread = thread_info_ref::new_reference (inferior_thread ()); +} /* If set (default for legacy reasons), when following a fork, GDB will detach from one of the fork branches, child or parent. @@ -3150,7 +3160,7 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal) } /* We'll update this if & when we switch to a new thread. */ - previous_inferior_ptid = inferior_ptid; + update_previous_thread (); regcache = get_current_regcache (); gdbarch = regcache->arch (); @@ -3433,7 +3443,7 @@ init_wait_for_inferior (void) nullify_last_target_wait_ptid (); - previous_inferior_ptid = inferior_ptid; + update_previous_thread (); } @@ -8647,21 +8657,24 @@ normal_stop (void) the current thread back to the thread the user had selected right after this event is handled, so we're not really switching, only informing of a stop. */ - if (!non_stop - && previous_inferior_ptid != inferior_ptid - && target_has_execution () - && last.kind () != TARGET_WAITKIND_SIGNALLED - && last.kind () != TARGET_WAITKIND_EXITED - && last.kind () != TARGET_WAITKIND_NO_RESUMED) + if (!non_stop) { - SWITCH_THRU_ALL_UIS () + if ((last.kind () != TARGET_WAITKIND_SIGNALLED + && last.kind () != TARGET_WAITKIND_EXITED + && last.kind () != TARGET_WAITKIND_NO_RESUMED) + && target_has_execution () + && previous_thread != inferior_thread ()) { - target_terminal::ours_for_output (); - gdb_printf (_("[Switching to %s]\n"), - target_pid_to_str (inferior_ptid).c_str ()); - annotate_thread_changed (); + SWITCH_THRU_ALL_UIS () + { + target_terminal::ours_for_output (); + gdb_printf (_("[Switching to %s]\n"), + target_pid_to_str (inferior_ptid).c_str ()); + annotate_thread_changed (); + } } - previous_inferior_ptid = inferior_ptid; + + update_previous_thread (); } if (last.kind () == TARGET_WAITKIND_NO_RESUMED) diff --git a/gdb/infrun.h b/gdb/infrun.h index c711b9b21cc..acaec9b5064 100644 --- a/gdb/infrun.h +++ b/gdb/infrun.h @@ -117,6 +117,10 @@ enum exec_direction_kind /* The current execution direction. */ extern enum exec_direction_kind execution_direction; +/* Call this to point 'previous_thread' at the thread returned by + inferior_thread, or at nullptr, if there's no selected thread. */ +extern void update_previous_thread (); + extern void start_remote (int from_tty); /* Clear out all variables saying what to do when inferior is diff --git a/gdb/target.c b/gdb/target.c index 1dd0f42af7d..f781f7e4f96 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -2502,6 +2502,8 @@ target_pre_inferior (int from_tty) current_inferior ()->highest_thread_num = 0; + update_previous_thread (); + agent_capability_invalidate (); } @@ -2530,6 +2532,9 @@ target_preopen (int from_tty) error (_("Program not killed.")); } + /* Release reference to old previous thread. */ + update_previous_thread (); + /* Calling target_kill may remove the target from the stack. But if it doesn't (which seems like a win for UDI), remove it now. */ /* Leave the exec target, though. The user may be switching from a