From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x33.google.com (mail-oa1-x33.google.com [IPv6:2001:4860:4864:20::33]) by sourceware.org (Postfix) with ESMTPS id 28A8A3858416 for ; Sun, 5 Feb 2023 00:06:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 28A8A3858416 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-oa1-x33.google.com with SMTP id 586e51a60fabf-163bd802238so11190939fac.1 for ; Sat, 04 Feb 2023 16:06:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=content-transfer-encoding:mime-version:message-id:date:in-reply-to :subject:cc:to:from:user-agent:references:from:to:cc:subject:date :message-id:reply-to; bh=5V0sY5ehDX18k+Ci6LiUw055H4Q/iUWUFOX76vWnEqY=; b=F7gmGHdC8HxHtqehvgbDISwy5pt8F+0O5BAkNZN76lzaG6CrpW7cm0Hermi1pS52sa X6701xW8LiwIyAQ1K61khHPYJpAsMOi/LNoz7xDRitShZp/hzrtoG9peFDGjEawOGjWk K9yydOere0U6FePe7AbARFLsVWUGbj8l/+7oGUe53iEdG7XVm+qhjBCVqQxvEjj+mzOV Akm11KT2ldfU2gsaaVbtX8fWR9y8A/RJiqCf0qgD9KDwj2kf9QJiixgHL/zEn96mrOa6 FDxf4UlhzcSe3+712fuiGUBHUPVBr2sSN6mPd9wH9V8Ufj2/4BF+Eu/smhy+kYab0NAA ewUw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:in-reply-to :subject:cc:to:from:user-agent:references:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=5V0sY5ehDX18k+Ci6LiUw055H4Q/iUWUFOX76vWnEqY=; b=J/T2xowhpmSLbpqff7AHK2j/iQ8khi5RM84gVMQSSfuLW2IysVV+Z2Mba2CIczQpqI +ldTMhWFMNmpRkfYK5Kgf8phcWzPPrx5hAjdUG3Z3gTl3SktjgdriSLW0qbrxg9hJE8H zfYbHILERWcc2nK/rYiTINjfiVuN59ub8hQJYx23Guv/C0/v0VK975riu1ssDbNE6etA sNc/ShGEyzotR575umTtSjjyv4tSZVji/cwjiuWIEwKTvt8XN8zHwBru9IIZegFTfxTf nEtMDzP1BEJsjn8SeCU2tCSDCudEnFXERuVcWDh4VaecAcg38eXRhdC2qn3zkc9GI5YB EhSg== X-Gm-Message-State: AO0yUKVzTq6jz21fpj4yHS61GCp54GN6Aal9bLm5jtRN17eGmafCSvHn AJ43Dsn7wFFSSmBLGdaeSm2fvw== X-Google-Smtp-Source: AK7set/7+IJVt7zyazXOnZxMNVZTbYZDWsDHjsHK5KWDtX3j05McrmHMfuNBMpyh9UwYgXjR3qIWkg== X-Received: by 2002:a05:6870:8a10:b0:163:db4a:c40d with SMTP id p16-20020a0568708a1000b00163db4ac40dmr8195392oaq.11.1675555617333; Sat, 04 Feb 2023 16:06:57 -0800 (PST) Received: from localhost ([189.100.172.221]) by smtp.gmail.com with ESMTPSA id a18-20020a0568300b9200b0068bd6cf405dsm2911614otv.1.2023.02.04.16.06.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 04 Feb 2023 16:06:56 -0800 (PST) References: <20230130044518.3322695-1-thiago.bauermann@linaro.org> <20230130044518.3322695-7-thiago.bauermann@linaro.org> <249be3dc-668e-9aa3-d3cc-5fc9fea3f99a@arm.com> User-agent: mu4e 1.8.13; emacs 28.2 From: Thiago Jung Bauermann To: Luis Machado Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v3 6/8] gdb/remote: Parse tdesc field in stop reply and threads list XML In-reply-to: <249be3dc-668e-9aa3-d3cc-5fc9fea3f99a@arm.com> Date: Sun, 05 Feb 2023 00:06:53 +0000 Message-ID: <871qn5m0v6.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-9.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_BARRACUDACENTRAL,RCVD_IN_DNSWL_NONE,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: Luis Machado writes: > On 1/30/23 04:45, Thiago Jung Bauermann via Gdb-patches wrote: >> --- a/gdb/remote.c >> +++ b/gdb/remote.c >> @@ -80,6 +80,7 @@ >> #include >> #include "async-event.h" >> #include "gdbsupport/selftest.h" >> +#include "xml-tdesc.h" >> /* The remote target. */ >> @@ -238,6 +239,16 @@ class remote_state >> /* Get the remote arch state for GDBARCH. */ >> struct remote_arch_state *get_remote_arch_state (struct gdbarch *gdb= arch); >> + /* Add new ID to the target description list. The corresponding XM= L will be >> + requested soon. */ > > Will it be requested soon or can gdb just ignore it if the user > doesn't switch to that thread? In this patch it would be requested soon, right after the threads list XML and the stop reply packet were parsed. But in my local branch that will become v4 I implemented Andrew's suggestion of getting the target descriptions on demand in remote_state::get_tdesc so now GDB may indeed ignore it. > If gdb can ignore it, then it might be nice to mention it here that > gdb can chose to request it at any point in time, but may opt not to > do it at all. As a consequence of Andrew's suggestion, the add_tdesc_id method isn't necessary anymore, so this comment isn't present anymore. >> + void add_tdesc_id (ULONGEST id); >> + >> + /* Get the target description corresponding to remote protocol ID. */ > > s/remote protocol/remote target description? I meant that in the sense of =E2=80=9CID that is used in the remote protoco= l=E2=80=9D, but I agree it's more confusing than helpful. I changed it to: /* Get the target description corresponding to the given remote target description ID. */ WDYT? >> @@ -3814,6 +3844,13 @@ start_thread (struct gdb_xml_parser *parser, >> attr =3D xml_find_attribute (attributes, "handle"); >> if (attr !=3D NULL) >> item.thread_handle =3D hex2bin ((const char *) attr->value.get ()); >> + >> + attr =3D xml_find_attribute (attributes, "tdesc"); >> + if (attr !=3D NULL) > > s/NULL/nullptr Fixed. >> diff --git a/gdb/xml-tdesc.h b/gdb/xml-tdesc.h >> index 0fbfc7e043e9..c7cc97c5dfc0 100644 >> --- a/gdb/xml-tdesc.h >> +++ b/gdb/xml-tdesc.h >> @@ -38,6 +38,12 @@ const struct target_desc *file_read_description_xml (= const char *filename); >> const struct target_desc *target_read_description_xml (struct target= _ops *); >> +/* Read an XML target description with the given ID using OPS. Parse= it, and >> + return the parsed description. */ >> + >> +const struct target_desc *target_read_description_xml (struct target_op= s *ops, >> + ULONGEST id); >> + >> /* Fetches an XML target description using OPS, processing includes, >> but not parsing it. Used to dump whole tdesc as a single XML file. >> Returns the description on success, and a disengaged optional > > I noticed we're dealing with the target description id as ULONGEST on > gdb's side, but as unsigned int on gdbserver's side. > > Should we make them the same, if possible? My thinking was that since it's gdbserver that defines what the ID will be, it can use a simpler type (2=C2=B3=C2=B2 target descriptions should be = enough for anybody) since it knows what kind of IDs it will issue. But GDB doesn't know what the remote stub will want to do, so it should use a big type. But with Andrew's idea of passing a value during target negotiation to decide whether the ID will be a number or a hash, then we can use unsigned int for both types. --=20 Thiago