From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id OCsbCJdb6mIRxB8AWB0awg (envelope-from ) for ; Wed, 03 Aug 2022 07:27:19 -0400 Received: by simark.ca (Postfix, from userid 112) id 13DD51EA05; Wed, 3 Aug 2022 07:27:19 -0400 (EDT) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=viGXlgqt; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 602151E9ED for ; Wed, 3 Aug 2022 07:27:18 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 850D03856DF5 for ; Wed, 3 Aug 2022 11:27:16 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 850D03856DF5 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1659526036; bh=4PIW6FfU0F/E2PqtIEIMAzdfR/clvhDgyTm/tDIEQMU=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=viGXlgqthDNiAehQ/pRwhmHarzwaFETXi72A/bA0KbuO1q5xOfc1mlR4wzcOiZHUJ /pOwpsFjmh4ioiApoxNBynyaNJkxUWwUMf9pf1EQul8ncyfZgWaU7zlKmyrh56XTwo br/MuridPMWD+zMr0s6U/poMUkYQil6/oy3AxscM= Received: from mail-qv1-xf31.google.com (mail-qv1-xf31.google.com [IPv6:2607:f8b0:4864:20::f31]) by sourceware.org (Postfix) with ESMTPS id 1A160385840A for ; Wed, 3 Aug 2022 11:26:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1A160385840A Received: by mail-qv1-xf31.google.com with SMTP id j11so12657453qvt.10 for ; Wed, 03 Aug 2022 04:26:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-disposition; bh=4PIW6FfU0F/E2PqtIEIMAzdfR/clvhDgyTm/tDIEQMU=; b=ajpe2DVpYdtUHujrY/54xIttwUe7QsN5Vs/ULZZ8W7W+XNviSW79PKpixgBvq/UW8n a6AAH46w0bF3PWVOCLbCwC0xmGueviRJ00+rfBw0FpiACxaf0AWDvV3+mB84RHxyHTSH y4oLCuFHtXGkwY6aMwDwimvnTmthssNtqfiTyIwLM0HGqKFzxbl8KPyEXn8c4EpXfAz0 gciV6a0fQsTQtsP+cYrF9oQJJCnmlBlvvBZOD8RZnkL+8F/BlbZrGy/Q/oFokYsiJY1N 2jl2pnJJzcBZHfY7PvXxjwzcTsRAyMd3Q5Ix3qS4l6dr0t2PMMluAyNfoTgia6bupFiE /upA== X-Gm-Message-State: ACgBeo0fuMx/l4TzjDRXkrw652Q/DeiB72Fd2v7i6iIKpPJN9J81ETVz iJP7Sf5d+QvSLp9Zy2BGT+ZHz4QQbzSzbw== X-Google-Smtp-Source: AA6agR6xriXlUDMkPuUcRyAqrCsYutbkgNoWLjH4FOOp2jV4XXRXlguzTadn4E6Fy9Wfu+eWMu9o4w== X-Received: by 2002:a05:6214:e62:b0:477:274f:3e24 with SMTP id jz2-20020a0562140e6200b00477274f3e24mr593478qvb.129.1659526013207; Wed, 03 Aug 2022 04:26:53 -0700 (PDT) Received: from juliacomputing.com ([2601:184:4700:2f2:58cc:29ff:2314:69d8]) by smtp.gmail.com with ESMTPSA id x22-20020a05622a001600b0033d1ac2517esm1775238qtw.64.2022.08.03.04.26.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 03 Aug 2022 04:26:52 -0700 (PDT) Date: Wed, 3 Aug 2022 07:26:51 -0400 To: gdb-patches@sourceware.org Subject: [RFC][PATCH] Extend qAttached for shared program spaces Message-ID: <20220803112651.GA3091455@juliacomputing.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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: , From: Keno Fischer via Gdb-patches Reply-To: Keno Fischer Cc: khuey@pernos.co, robert@ocallahan.org, tim@juliacomputing.com, gabriel.baraldi@juliacomputing.com Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" Currently, when connecting to an existing extended-remote target and the target reports a stop in a previously-unseen process, GDB will create a new inferior and assign it a fresh pspace/aspace. However, there is no particular reason why this is necessarily the correct answer. The newly connected process might easily be in a vfork, or worse, be a long running process that happened to get cloned of using CLONE_VM (some GC and instrumentation tooling works this way). Without properly accounting for the shared pspace/aspace, GDB will ignore e.g. watchpoint events in the shared address space, giving a worse user experience as well as needing to perform redundant work. This proposes an extension to the remote protocol to let a gdb server inform the gdb client of which processes a newly attached process shares a pspace with. This is accomplished through a new `v thread-id-list` response option for `qAttached`. If support for this option is indicated, the server may use it to indicate that the attached inferior shares a pspace with the given list of other threads in the system, which the client can then use to find an existing pspace (if any). Signed-off-by: Keno Fischer --- This is a quick-and-dirty implementation of the above proposal, mostly as a starting point for discussion. I would appreciate feedback on the remote protocol semantics, as well as if there are implementation issues that I need to consider, I'm not particularly well versed in this code. That said, I did implement the other side of this protocol in a branch of https://github.com/rr-debugger/rr and verified that watchpoints etc were working well between recorded processes that shared an address space. Comments appreciated! gdb/doc/gdb.texinfo | 3 +++ gdb/remote.c | 59 ++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 382df00ee7d..05069eb6abf 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -43690,6 +43690,9 @@ the @code{quit} command. Reply: @table @samp +@item v @var{thread-id-list} +The remote server attached to an existing process that shares its address space +with the threads in @var{thread-id-list} (e.g. through a previous vfork). @item 1 The remote server attached to an existing process. @item 0 diff --git a/gdb/remote.c b/gdb/remote.c index 49d26c2039e..158883ebc91 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -784,7 +784,7 @@ class remote_target : public process_stratum_target void remote_interrupt_ns (); char *remote_get_noisy_reply (); - int remote_query_attached (int pid); + int remote_query_attached (int pid, std::vector &threads_sharing_as); inferior *remote_add_inferior (bool fake_pid_p, int pid, int attached, int try_open_exec); @@ -2220,6 +2220,9 @@ enum { packets and the tag violation stop replies. */ PACKET_memory_tagging_feature, + /* Support for `v` reply in qAttached. */ + PACKET_qattached_v, + PACKET_MAX }; @@ -2442,7 +2445,7 @@ static const ptid_t any_thread_ptid (42000, 0, 1); detach instead of killing it when bailing out). */ int -remote_target::remote_query_attached (int pid) +remote_target::remote_query_attached (int pid, std::vector &threads_sharing_pspace) { struct remote_state *rs = get_remote_state (); size_t size = get_remote_packet_size (); @@ -2457,11 +2460,22 @@ remote_target::remote_query_attached (int pid) putpkt (rs->buf); getpkt (&rs->buf, 0); + const char *bufp = rs->buf.data(); switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_qAttached])) { case PACKET_OK: + if (bufp[0] == 'v' && bufp[1] == ' ') { + bufp += 2; // Skip "v " + do + { + ptid_t ptid = read_ptid (bufp, &bufp); + threads_sharing_pspace.emplace_back (ptid); + } + while (*bufp++ == ','); /* comma-separated list */ + return 1; + } if (strcmp (rs->buf.data (), "1") == 0) return 1; break; @@ -2491,12 +2505,13 @@ remote_target::remote_add_inferior (bool fake_pid_p, int pid, int attached, int try_open_exec) { struct inferior *inf; + std::vector threads_sharing_pspace; /* Check whether this process we're learning about is to be considered attached, or if is to be considered to have been spawned by the stub. */ if (attached == -1) - attached = remote_query_attached (pid); + attached = remote_query_attached (pid, threads_sharing_pspace); if (gdbarch_has_global_solist (target_gdbarch ())) { @@ -2516,6 +2531,8 @@ remote_target::remote_add_inferior (bool fake_pid_p, int pid, int attached, between program/address spaces. We simply bind the inferior to the program space's address space. */ inf = current_inferior (); + address_space *aspace = nullptr; + program_space *pspace = nullptr; /* However, if the current inferior is already bound to a process, find some other empty inferior. */ @@ -2523,17 +2540,39 @@ remote_target::remote_add_inferior (bool fake_pid_p, int pid, int attached, { inf = nullptr; for (inferior *it : all_inferiors ()) - if (it->pid == 0) + { + if (!inf && it->pid == 0) { inf = it; - break; + if (pspace || threads_sharing_pspace.empty()) + break; } + if (!pspace) + for (size_t i = 0; i < threads_sharing_pspace.size (); i++) + if (threads_sharing_pspace[i].pid () == it->pid) + { + pspace = it->pspace; + aspace = it->aspace; + break; + } + if (inf && pspace) + break; + } } if (inf == nullptr) { /* Since all inferiors were already bound to a process, add a new inferior. */ - inf = add_inferior_with_spaces (); + if (!pspace) + inf = add_inferior_with_spaces (); + else + { + inf = add_inferior (0); + inf->pspace = pspace; + inf->aspace = aspace; + gdbarch_info info; + inf->gdbarch = gdbarch_find_by_info (info); + } } switch_to_inferior_no_thread (inf); inf->push_target (this); @@ -5427,6 +5466,7 @@ static const struct protocol_feature remote_protocol_features[] = { { "no-resumed", PACKET_DISABLE, remote_supported_packet, PACKET_no_resumed }, { "memory-tagging", PACKET_DISABLE, remote_supported_packet, PACKET_memory_tagging_feature }, + { "qAttachedV", PACKET_DISABLE, remote_supported_packet, PACKET_qattached_v } }; static char *remote_support_xml; @@ -5525,6 +5565,10 @@ remote_target::remote_query_supported () != AUTO_BOOLEAN_FALSE) remote_query_supported_append (&q, "memory-tagging+"); + if (packet_set_cmd_state (PACKET_qattached_v) + != AUTO_BOOLEAN_FALSE) + remote_query_supported_append (&q, "qAttachedV+"); + /* Keep this one last to work around a gdbserver <= 7.10 bug in the qSupported:xmlRegisters=i386 handling. */ if (remote_support_xml != NULL @@ -15347,6 +15391,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL, add_packet_config_cmd (&remote_protocol_packets[PACKET_memory_tagging_feature], "memory-tagging-feature", "memory-tagging-feature", 0); + add_packet_config_cmd (&remote_protocol_packets[PACKET_qattached_v], + "qattached-v", "qattached-v", 0); + /* Assert that we've registered "set remote foo-packet" commands for all packet configs. */ { -- 2.25.1