From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id C02FB3858D35 for ; Wed, 1 Feb 2023 14:51:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C02FB3858D35 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1675263117; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=9jtWCLQUrq09NPy9veLWtZFxmFMfN3J24FEFpVjRQ8g=; b=OaALaIXyagFmslIaDtNfydsxnlGvJIYS0r4UTJpBct7eRtiOPCHYZhci2OUNesOaGXIPRP FJyYebpUoOsWQ4p+Pj+QrhsARW32y0c7sudnUqCK7fERaZSCkVrP0MAoYTC7IdcI6elfQu FKS15IOJWKse0s+xm4huc4QGbQiHZU4= Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-164-xQ2bcNVJMde9-iT35XQQAQ-1; Wed, 01 Feb 2023 09:51:56 -0500 X-MC-Unique: xQ2bcNVJMde9-iT35XQQAQ-1 Received: by mail-qv1-f70.google.com with SMTP id ly4-20020a0562145c0400b0054d2629a759so2047589qvb.16 for ; Wed, 01 Feb 2023 06:51:56 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=9jtWCLQUrq09NPy9veLWtZFxmFMfN3J24FEFpVjRQ8g=; b=AeqGde0ltBllyRRLr/gnFeg6iRyVVv51eZ+C1nu+RL9MKRZZ0Y2ov0UZtnMw4HgKUM 7Sl9oUiHb7fk63+DDzNlObRKcizyyAos9EorSUNWVW0AT5/fglFZ/7vPtMyTQ9DXYTgQ I9aVQx5TZ9iCh0se81SmAuU0NfkztFWt0DofSqYB9jFjwSV9AeZq6GzgrP09hJN8cd6y zCxWhgoc+GRXXTz9IXnZtEcJOOj2SlnYM5wblVLknmX49AIYev5n6dkAPzbZtyPJTzD4 KgA0aqjTKBT9cGzbGAtJAYba4yb10Zro2rVBSTXv+UsbTJiGwYlSDDWRq64QfJO1+LB7 bGbQ== X-Gm-Message-State: AO0yUKXBCM6KlX7FvzEhQzvT6MDJNAFj9lHA2eAgzwTZaggo+SmwVzFi y6gbDfGy64pTiYmn7PXDNMP9PsDE6MY0I4NbYOcu/hfeyZjlgz3bj+6DpIW0Iyej5LBBtviW14m LXOWL84BIJWtmm8OUOT0uVk5NU8YC5V4P0g0dDrwRkHbksfCr4TKO2f5FhGrkwYIfhPDQ6TEHkA == X-Received: by 2002:ad4:5f8d:0:b0:537:7061:89d8 with SMTP id jp13-20020ad45f8d000000b00537706189d8mr6323007qvb.15.1675263115835; Wed, 01 Feb 2023 06:51:55 -0800 (PST) X-Google-Smtp-Source: AK7set8QEvLnk7s+1KmSEXcAX+QAxC1uFtMQvGAt2MAGYtdizqqdkaQwbuJmoYzn/7tqusZ+3AFnFw== X-Received: by 2002:ad4:5f8d:0:b0:537:7061:89d8 with SMTP id jp13-20020ad45f8d000000b00537706189d8mr6322952qvb.15.1675263115343; Wed, 01 Feb 2023 06:51:55 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id b2-20020a05620a0f8200b006f9f3c0c63csm3076243qkn.32.2023.02.01.06.51.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Feb 2023 06:51:54 -0800 (PST) From: Andrew Burgess To: Thiago Jung Bauermann via Gdb-patches , gdb-patches@sourceware.org Cc: Thiago Jung Bauermann , Simon Marchi Subject: Re: [PATCH v3 5/8] gdbserver: Transmit target description ID in thread list and stop reply In-Reply-To: <20230130044518.3322695-6-thiago.bauermann@linaro.org> References: <20230130044518.3322695-1-thiago.bauermann@linaro.org> <20230130044518.3322695-6-thiago.bauermann@linaro.org> Date: Wed, 01 Feb 2023 14:51:52 +0000 Message-ID: <87bkmdtp4n.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,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: Thiago Jung Bauermann via Gdb-patches writes: > Now that an inferior thread can have a different target description than > its process, there needs to be a way to communicate this target > description to GDB. So add the concept of a target description ID to the > remote protocol, which is used to reference them and allows them to be > transferred only once over the wire. > > The ID is an unsigned integer, and is sent in the 'T AA n1:r1;n2:r2;...' > stop reply packet as a new 'n:r' pair, where n is "tdesc" and "r" is an > unsigned integer containing the ID. > > It is also sent in the threads list XML in the response of a > qXfer:threads:read request. The ID is sent as a new "tdesc" attribute of > the node. > > To request the target description XML of a given ID, GDB sends the > qXfer:features:read request with "target-id-%u.xml" as the annex, where %u > is the target description ID. > > Suggested-By: Simon Marchi > --- > gdb/doc/gdb.texinfo | 27 ++++++++++++++++--- > gdbserver/remote-utils.cc | 57 +++++++++++++++++++++++++++++++++++++++ > gdbserver/remote-utils.h | 9 +++++++ > gdbserver/server.cc | 17 ++++++++++-- > gdbserver/server.h | 4 +++ > 5 files changed, 109 insertions(+), 5 deletions(-) > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 9c0018ea5c14..fbf7e59853b5 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -42030,6 +42030,10 @@ the stopped thread, as specified in @ref{thread-id syntax}. > If @var{n} is @samp{core}, then @var{r} is the hexadecimal number of > the core on which the stop event was detected. > > +@item > +If @var{n} is @samp{tdesc}, then @var{r} is the hexadecimal number of > +the target description ID (@pxref{Target Description ID}). > + > @item > If @var{n} is a recognized @dfn{stop reason}, it describes a more > specific event that stopped the target. The currently defined stop > @@ -46546,7 +46550,7 @@ the following structure: > @smallexample > > > - > + > ... description ... > > > @@ -46556,8 +46560,10 @@ Each @samp{thread} element must have the @samp{id} attribute that > identifies the thread (@pxref{thread-id syntax}). The > @samp{core} attribute, if present, specifies which processor core > the thread was last executing on. The @samp{name} attribute, if > -present, specifies the human-readable name of the thread. The content > -of the of @samp{thread} element is interpreted as human-readable > +present, specifies the human-readable name of the thread. The > +@samp{tdesc} attribute, if present, specifies the target description > +ID of the thread (@pxref{Target Description ID}). The content of > +the @samp{thread} element is interpreted as human-readable > auxiliary information. The @samp{handle} attribute, if present, > is a hex encoded representation of the thread handle. > > @@ -46767,6 +46773,8 @@ descriptions are accurate, and that @value{GDBN} understands them. > target descriptions. @xref{Expat}. > > @menu > +* Target Description ID:: Referencing different descriptions in the > + remote protocol. > * Retrieving Descriptions:: How descriptions are fetched from a target. > * Target Description Format:: The contents of a target description. > * Predefined Target Types:: Standard types available for target > @@ -46775,6 +46783,14 @@ target descriptions. @xref{Expat}. > * Standard Target Features:: Features @value{GDBN} knows about. > @end menu > > +@node Target Description ID > +@section Target Description ID > + > +In cases where a remote target supports threads having different > +target descriptions than their parent process, the remote protocol > +assigns a non-negative integer to each target description to reference > +it in the communication between the host and the target. Having read some of the later patches, I have some additional thoughts here: I think we should make it explicit here that IDs are connection wide, not per-process. We should also make it clear that GDB might[1] cache target descriptions per remote connection, and so a remote target should not reuse a target description ID except where the target description is identical. [1] I say "GDB might" here because if we say "GDB will" then this would imply each target description will only be asked for once. And I figure, why be overly restrictive. Thanks, Andrew > + > @node Retrieving Descriptions > @section Retrieving Descriptions > > @@ -46787,6 +46803,11 @@ qXfer}). The @var{annex} in the @samp{qXfer} packet will be > XML document, of the form described in @ref{Target Description > Format}. > > +If target description IDs are being used (@pxref{Target Description ID}), > +@value{GDBN} can retrieve a target description with a given ID by using > +@samp{target-id-ID.xml} as the @var{annex}, where @var{ID} is the > +non-negative integer identifier of the desired target description. > + > Alternatively, you can specify a file to read for the target description. > If a file is set, the target will not be queried. The commands to > specify a file are: > diff --git a/gdbserver/remote-utils.cc b/gdbserver/remote-utils.cc > index 80310bc2c709..baff899307cc 100644 > --- a/gdbserver/remote-utils.cc > +++ b/gdbserver/remote-utils.cc > @@ -1049,6 +1049,53 @@ outreg (struct regcache *regcache, int regno, char *buf) > return buf; > } > > +/* See remote-utils.h. */ > + > +unsigned int > +get_tdesc_rsp_id (const target_desc *tdesc) > +{ > + client_state &cs = get_client_state (); > + unsigned int i; > + > + for (i = 0; i < cs.tdescs.size (); i++) > + if (cs.tdescs[i] == tdesc) > + return i; > + > + cs.tdescs.push_back (tdesc); > + > + return i; > +} > + > +/* See remote-utils.h. */ > + > +const target_desc * > +get_tdesc_from_rsp_id (unsigned int id) > +{ > + client_state &cs = get_client_state (); > + > + if (id >= cs.tdescs.size ()) > + return nullptr; > + > + return cs.tdescs[id]; > +} > + > +/* Return the ID as used in the remote protocol for the target descriptor of the > + given PTID. */ > + > +static unsigned int > +get_tdesc_rsp_id (ptid_t ptid) > +{ > + const thread_info *thread = find_thread_ptid (ptid); > + const target_desc *tdesc; > + > + if (thread == nullptr) > + tdesc = find_process_pid (ptid.pid ())->tdesc; > + else > + tdesc = get_thread_target_desc (thread); > + > + return get_tdesc_rsp_id (tdesc); > +} > + > void > prepare_resume_reply (char *buf, ptid_t ptid, const target_waitstatus &status) > { > @@ -1241,6 +1288,16 @@ prepare_resume_reply (char *buf, ptid_t ptid, const target_waitstatus &status) > buf += strlen (buf); > current_process ()->dlls_changed = false; > } > + > + if (current_thread->tdesc != nullptr > + && current_thread->tdesc != current_process ()->tdesc) > + { > + sprintf (buf, "tdesc:"); > + buf += strlen (buf); > + sprintf (buf, "%x", get_tdesc_rsp_id (ptid)); > + strcat (buf, ";"); > + buf += strlen (buf); > + } > } > break; > case TARGET_WAITKIND_EXITED: > diff --git a/gdbserver/remote-utils.h b/gdbserver/remote-utils.h > index cb2d6c346c99..61ef80b4dad7 100644 > --- a/gdbserver/remote-utils.h > +++ b/gdbserver/remote-utils.h > @@ -75,4 +75,13 @@ int relocate_instruction (CORE_ADDR *to, CORE_ADDR oldloc); > > void monitor_output (const char *msg); > > +/* Return the ID as used in the remote protocol for the given target > + descriptor. */ > + > +unsigned int get_tdesc_rsp_id (const target_desc *tdesc); > + > +/* Return the target description corresponding to the remote protocol ID. */ > + > +const target_desc *get_tdesc_from_rsp_id (unsigned int id); > + > #endif /* GDBSERVER_REMOTE_UTILS_H */ > diff --git a/gdbserver/server.cc b/gdbserver/server.cc > index 21fb51a45d16..2d1062f98468 100644 > --- a/gdbserver/server.cc > +++ b/gdbserver/server.cc > @@ -976,7 +976,15 @@ handle_general_set (char *own_buf) > static const char * > get_features_xml (const char *annex) > { > - const struct target_desc *desc = current_target_desc (); > + const struct target_desc *desc; > + unsigned int id; > + > + if (strcmp (annex, "target.xml") == 0) > + desc = current_target_desc (); > + else if (sscanf (annex, "target-id-%u.xml", &id) == 1) > + desc = get_tdesc_from_rsp_id (id); > + else > + desc = nullptr; > > /* `desc->xmltarget' defines what to return when looking for the > "target.xml" file. Its contents can either be verbatim XML code > @@ -986,7 +994,7 @@ get_features_xml (const char *annex) > This variable is set up from the auto-generated > init_registers_... routine for the current target. */ > > - if (strcmp (annex, "target.xml") == 0) > + if (desc != nullptr) > { > const char *ret = tdesc_get_features_xml (desc); > > @@ -1664,6 +1672,11 @@ handle_qxfer_threads_worker (thread_info *thread, struct buffer *buffer) > if (name != NULL) > buffer_xml_printf (buffer, " name=\"%s\"", name); > > + if (thread->tdesc != nullptr > + && thread->tdesc != get_thread_process (thread)->tdesc) > + buffer_xml_printf (buffer, " tdesc=\"%u\"", > + get_tdesc_rsp_id (thread->tdesc)); > + > if (handle_status) > { > char *handle_s = (char *) alloca (handle_len * 2 + 1); > diff --git a/gdbserver/server.h b/gdbserver/server.h > index 7997d1a32e6e..58be5027795b 100644 > --- a/gdbserver/server.h > +++ b/gdbserver/server.h > @@ -193,6 +193,10 @@ struct client_state > /* If true, memory tagging features are supported. */ > bool memory_tagging_feature = false; > > + /* The target descriptions that have been communicated to the client. The > + index of a target description in this vector is the ID used to reference it > + in the remote protocol. */ > + std::vector tdescs; > }; > > client_state &get_client_state ();