From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f43.google.com (mail-wr1-f43.google.com [209.85.221.43]) by sourceware.org (Postfix) with ESMTPS id C63353858D39 for ; Tue, 24 Jan 2023 19:47:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C63353858D39 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-f43.google.com with SMTP id t18so2151001wro.1 for ; Tue, 24 Jan 2023 11:47:55 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:references:to:from:subject :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=kOdajMrxsYpF40c5r+thfvMtIrQty1CxqCf4a5WioUs=; b=iHFusNY672m70Yq8yQTwtpqXEkUErnRW2vcjXvIdym40PHLDTBBK/Q1eSW6yohO6vD DfTh8UUdmil4Xj3bS4rdst3N2sowS2LngvGQfQWYRS0Zb2rfbrfqHlX4RnBmvostLY2x WDa+FGXj2Dz8KutLQFIs5uYw+cBcXXP6jRjNavkVdA7MxjXAASGuE9msV6ck5wEgSOYs fSNXM8JYYWcWphNUk28+9hXf3pJSBzlNqZ1qhElNWem0AFcUIOdzY93FpANFhlYxu04Y nJWhZowCHwJNILbIQpP58O7tX4EOUvuTIeRkwunKVk8exa39gbtQgmundKP4ccM6NYH4 pUwQ== X-Gm-Message-State: AO0yUKX8v+V/I2yI6/FPvI49Eu72hYlgbvwWODriwxrJl7cKAnip78sv Xw77IC0bGiLnXiImvM0ibhgy8W7d/0WN0w== X-Google-Smtp-Source: AK7set8VwBqi4c+ITETbASNL0O1rpmeSJVtMn9j7pjLFXiPzedQqArMAYvk6fKA7iqcjE1XdOjxXGQ== X-Received: by 2002:adf:fcc5:0:b0:2bf:ae3f:640b with SMTP id f5-20020adffcc5000000b002bfae3f640bmr3306559wrs.70.1674589674391; Tue, 24 Jan 2023 11:47:54 -0800 (PST) Received: from ?IPv6:2001:8a0:f92b:9e00::1fe? ([2001:8a0:f92b:9e00::1fe]) by smtp.gmail.com with ESMTPSA id v1-20020a5d4b01000000b002be53aa2260sm2892350wrq.117.2023.01.24.11.47.53 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 24 Jan 2023 11:47:53 -0800 (PST) Subject: Re: [PATCH v3 00/31] Step over thread clone and thread exit From: Pedro Alves To: gdb-patches@sourceware.org References: <20221212203101.1034916-1-pedro@palves.net> Message-ID: <00753cfd-2bb2-95c2-207e-b3660e975ba0@palves.net> Date: Tue, 24 Jan 2023 19:47:53 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <20221212203101.1034916-1-pedro@palves.net> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,KAM_LOTSOFHASH,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: (Slightly fixing subject, I had forgotten to include "v3" there when I posted this.) I had planned to wait until the branch was cut before considering putting this in, and meanwhile it's been a few weeks since that has happened. So I am considering doing that (putting this in). I'll wait a bit more in case someone wants to chime in. Pedro Alves On 2022-12-12 8:30 p.m., Pedro Alves wrote: > Here's v3 of the series I previously posted here: > > https://inbox.sourceware.org/gdb-patches/20220713222433.374898-1-pedro@palves.net/ > > New in v3: > > - Addressed Simon's comments throughout. > > - Addressed Tom de Vries' comments: now builds with > --enable-targets=all, and the testcases should work with no glibc > debug info. > > - There are a few new patches: > > - [PATCH 07/31] enum_flags to_string > > Already approved by Simon. The patches that introduce the new > GDB_THREAD_OPTION_XXX option flags make use of this now to > pretty-print the flags. > > - [PATCH 16/31] Move deleting thread on TARGET_WAITKIND_THREAD_EXITED to core > > This was half done in another patch previously. In v3, it's > been moved to a separate preparatory patch, and in all-stop, we > now delete threads when pending-exits just before we report a > stop to the user. > > - [PATCH 31/31] Cancel execution command on thread exit, when stepping, nexting, etc. > > The testcase was augmented to better cover all scenarios. > > - This series now applies on top of this other series I posted last > week: > > [PATCH 0/6] Eliminate infrun_thread_thread_exit observer > https://inbox.sourceware.org/gdb-patches/20221203211338.2264994-1-pedro@palves.net/T/#t > > For your convenience, I've pushed this to the > users/palves/step-over-thread-exit-v3 branch. > > Here's the series description, updated for v3: > > This is a new series that replaces two different series from last > year. > > The first is this series Simon and I wrote, here: > > [PATCH 00/10] Step over thread exit (PR gdb/27338) > https://sourceware.org/pipermail/gdb-patches/2021-July/180567.html > > The other is a series that coincidentally, back then, Andrew posted at > about the same time, and that addressed problems in kind of the mirror > scenario. His patch series was about stepping over clone (creating > new threads), instead of stepping over thread exit: > > [PATCH 0/3] Stepping over clone syscall > https://sourceware.org/pipermail/gdb-patches/2021-June/180517.html > > My & Simon's solution back then involved adding a new contract between > GDB and GDBserver -- if a thread is single stepping, and it exits, the > server was supposed to report back the thread's exit to GDB. One of > the motivations for this approach was to be able to control the > enablement of thread exit events per thread, to avoid creating > thread-exit event traffic unnecessarily, as done by > target_thread_events()/QThreadEvents. > > Andrew's solution involves using the QThreadEvents mechanism, which > tells the server to report thread create and thread exit events for > all threads. This would conflict with the desire to avoid unnecessary > traffic in the step over thread exit series. > > The step over clone fixes back then also weren't yet fully complete, > as Andrew's series only addressed inline step overs. Fixing displaced > stepping over clone syscall would still remain broken. > > This new series fixes all of stepping over thread exit and clone, for > both of displaced stepping and inline step overs. It: > > - Merges both Andrew's and my/Simon's series, and then reworks both > parts in different ways. > > - Introduces clone events at the GDB core and remote protocol level. > > - Gets rid of the idea of "reporting thread exit if thread is > single-stepping", replaces it by a new mechanism GDB can use to > explicitly enable thread clone and/or thread exit events, and other > events in the future. The old mechanism also only worked when the > remote server supported hardware single-stepping. This new approach > has an advantage of also working on software single-step targets. > > - Uses the new clone events to fix displaced stepping over clone > syscalls too. > > - Addresses an issue that Andrew alluded to in his series, and that > coincidentally, we/AMD also ran into with AMDGPU debugging -- > currently, with "set scheduler-locking on", if you step over a > function that spawns a thread, that thread runs free, for a bit at > least, and then may stop or not, basically in an unspecified manner. > > - Addresses Simon's review comments on the original "Step over thread > exit" series referenced above. > > - Centralizes "[Thread ...exited]" notifications in core code. > > - Cancels next/step/until/etc. commands on thread exit event, like so: > > (gdb) n > [Thread 0x7ffff7d89700 (LWP 3961883) exited] > Command aborted, thread exited. > (gdb) > > There are documentation changes in the following patches: > > [PATCH 23/31] Don't resume new threads if scheduler-locking is in effect > [PATCH 28/31] Document remote clone events, and QThreadOptions packet > > ... and they have both already been approved by Eli, in v2. (They > haven't changed in v3). > > Tested on x86-64 Ubuntu 20.04, native and gdbserver. > > Andrew Burgess (1): > Add test for stepping over clone syscall > > Pedro Alves (29): > displaced step: pass down target_waitstatus instead of gdb_signal > linux-nat: introduce pending_status_str > gdb/linux: Delete all other LWPs immediately on ptrace exec event > Step over clone syscall w/ breakpoint, TARGET_WAITKIND_THREAD_CLONED > Support clone events in the remote protocol > Avoid duplicate QThreadEvents packets > enum_flags to_string > Thread options & clone events (core + remote) > Thread options & clone events (native Linux) > Thread options & clone events (Linux GDBserver) > gdbserver: Hide and don't detach pending clone children > Remove gdb/19675 kfails (displaced stepping + clone) > all-stop/synchronous RSP support thread-exit events > gdbserver/linux-low.cc: Ignore event_ptid if TARGET_WAITKIND_IGNORE > Move deleting thread on TARGET_WAITKIND_THREAD_EXITED to core > Introduce GDB_THREAD_OPTION_EXIT thread option, fix > step-over-thread-exit > Implement GDB_THREAD_OPTION_EXIT support for Linux GDBserver > Implement GDB_THREAD_OPTION_EXIT support for native Linux > gdb: clear step over information on thread exit (PR gdb/27338) > stop_all_threads: (re-)enable async before waiting for stops > gdbserver: Queue no-resumed event after thread exit > Don't resume new threads if scheduler-locking is in effect > Report thread exit event for leader if reporting thread exit events > Ignore failure to read PC when resuming > gdb/testsuite/lib/my-syscalls.S: Refactor new SYSCALL macro > Document remote clone events, and QThreadOptions packet > inferior::clear_thread_list always silent > Centralize "[Thread ...exited]" notifications > Cancel execution command on thread exit, when stepping, nexting, etc. > > Simon Marchi (1): > Testcases for stepping over thread exit syscall (PR gdb/27338) > > gdb/NEWS | 27 + > gdb/annotate.c | 4 +- > gdb/breakpoint.c | 4 +- > gdb/displaced-stepping.c | 18 +- > gdb/displaced-stepping.h | 2 +- > gdb/doc/gdb.texinfo | 131 +++- > gdb/fbsd-nat.c | 3 - > gdb/gdbarch-components.py | 6 +- > gdb/gdbarch-gen.h | 10 +- > gdb/gdbarch.c | 4 +- > gdb/gdbthread.h | 38 +- > gdb/inferior.c | 12 +- > gdb/inferior.h | 7 +- > gdb/infrun.c | 615 +++++++++++++++--- > gdb/linux-nat.c | 362 ++++++----- > gdb/linux-nat.h | 4 + > gdb/linux-tdep.c | 5 +- > gdb/linux-tdep.h | 2 +- > gdb/mi/mi-interp.c | 8 +- > gdb/netbsd-nat.c | 5 - > gdb/observable.h | 11 +- > gdb/procfs.c | 6 - > gdb/python/py-inferior.c | 4 +- > gdb/remote.c | 264 +++++++- > gdb/rs6000-tdep.c | 4 +- > gdb/target-debug.h | 2 + > gdb/target-delegates.c | 52 ++ > gdb/target.c | 16 + > gdb/target.h | 10 + > gdb/target/target.c | 12 + > gdb/target/target.h | 20 + > gdb/target/waitstatus.c | 1 + > gdb/target/waitstatus.h | 31 +- > gdb/testsuite/gdb.base/step-over-syscall.exp | 44 +- > .../gdb.threads/schedlock-new-thread.c | 54 ++ > .../gdb.threads/schedlock-new-thread.exp | 67 ++ > gdb/testsuite/gdb.threads/step-over-exec.exp | 6 + > ...-over-thread-exit-while-stop-all-threads.c | 77 +++ > ...ver-thread-exit-while-stop-all-threads.exp | 69 ++ > .../gdb.threads/step-over-thread-exit.c | 52 ++ > .../gdb.threads/step-over-thread-exit.exp | 151 +++++ > gdb/testsuite/gdb.threads/stepi-over-clone.c | 90 +++ > .../gdb.threads/stepi-over-clone.exp | 392 +++++++++++ > gdb/testsuite/lib/my-syscalls.S | 54 +- > gdb/testsuite/lib/my-syscalls.h | 5 + > gdb/thread.c | 71 +- > gdb/unittests/enum-flags-selftests.c | 69 +- > gdb/windows-nat.c | 16 +- > gdbserver/gdbthread.h | 3 + > gdbserver/linux-low.cc | 399 +++++++----- > gdbserver/linux-low.h | 56 +- > gdbserver/remote-utils.cc | 26 +- > gdbserver/server.cc | 158 ++++- > gdbserver/target.cc | 15 +- > gdbserver/target.h | 27 +- > gdbsupport/enum-flags.h | 66 ++ > 56 files changed, 3043 insertions(+), 624 deletions(-) > create mode 100644 gdb/testsuite/gdb.threads/schedlock-new-thread.c > create mode 100644 gdb/testsuite/gdb.threads/schedlock-new-thread.exp > create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit-while-stop-all-threads.c > create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit-while-stop-all-threads.exp > create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit.c > create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit.exp > create mode 100644 gdb/testsuite/gdb.threads/stepi-over-clone.c > create mode 100644 gdb/testsuite/gdb.threads/stepi-over-clone.exp > > > base-commit: fb699bafb5f23c2fd43d7f20495171b16903b20f > prerequisite-patch-id: 75b36824a7ee067b57a8d8db6016cb57e4d7f620 > prerequisite-patch-id: d5bf8b612deed2abf58a48c553178e080347f34d > prerequisite-patch-id: 3f00d1a451bed27d1b1c27174e15eb604d109a44 > prerequisite-patch-id: 5c458d627f349446e40114cb5424b42af08c0824 > prerequisite-patch-id: d81a20c336a23c79dafd4da3d780033052b3ac28 > prerequisite-patch-id: 8cf0c661a601ff3eacfcd26f11e26bcd11cc867d >