From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x341.google.com (mail-wm1-x341.google.com [IPv6:2a00:1450:4864:20::341]) by sourceware.org (Postfix) with ESMTPS id E7C793951C9F for ; Thu, 11 Jun 2020 14:05:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E7C793951C9F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wm1-x341.google.com with SMTP id l17so5112672wmj.0 for ; Thu, 11 Jun 2020 07:05:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=GG9b/k0koMWVGqQSsZjgTF2C/jGeqmKfR1QLEsk/IN4=; b=X9JVhUNWt3frV2zuDVE7p33Az9gJo41fmEzhWWNjNUk3SyybMiRyD7rNnn/Tt+7yjN V8RgZAohDFmoARPnNZphK/Vzmo0eIg6QbDNLmmYXHQLjH7sd4txz19UqrKUM4S+WyYil 78TafuQplpAPvnvaJOAPjC5cfnYPmB2ngVSwL0rJ+vSpo+EhKFSguV6FyekjXz3e7Qu/ mqdiXhgzCGt5iX4La9+wFucRVvCzHGTdH6xWmP7FJMXgYzUahnktLPYbpVAiJHe3aP+J CaiCWrMfVjgyQfNWaBtlcpNP3qBkzxB1+w/P07TucfL9GF32hW0kocD5tAe6DTP4BZ19 8d8Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=GG9b/k0koMWVGqQSsZjgTF2C/jGeqmKfR1QLEsk/IN4=; b=l+1PDZ0nOjF6JU/XMOFRlKXs4zPFtVZ2yZzCaIPjh3t1IHhhaRuyDnDPoWtj0XogXJ PKOIr7kO6EeHuAycu4tVkbuiqL9pWzDTtMguhdYHVy3DtQNr1nVnfKCD0cOMlQjY7Joe KJHkMxNrWZ7A04rGisRElHYC53g5yNbPJgi9iJ8bpMJjPYSCor0nPX9Xf7cMxufVG+EY 6NxG9FTboUfIuCauyuL7IZ9Ypl4O/Yuz9lhSurJtqZKMjySoc3Wfrrbiv5JsfdmQe09Q 7T1fH1Og0Y3HQn4mzfyGFLsjcAIhE675XZZjScq5xhCo4KwHCTBwfydCdlhcjUw2zaQd EAkg== X-Gm-Message-State: AOAM530hmNpCqc2SK0QyKZ85iXVVe3ZiXkxF8XwpkXq8HnqFAzlxqLZZ GXwQEPu3cNNpchSjM1QoTjd1UA== X-Google-Smtp-Source: ABdhPJyMJNka+ajjzTfgVDncUmazTQO1kcJRPRO+/fVH91NjnnI3mzO7WcO1nFFleMeVH3d1VrciHA== X-Received: by 2002:a1c:ab07:: with SMTP id u7mr8375060wme.130.1591884356834; Thu, 11 Jun 2020 07:05:56 -0700 (PDT) Received: from localhost (host86-128-12-16.range86-128.btcentralplus.com. [86.128.12.16]) by smtp.gmail.com with ESMTPSA id f11sm4920691wrm.13.2020.06.11.07.05.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 11 Jun 2020 07:05:54 -0700 (PDT) Date: Thu, 11 Jun 2020 15:05:53 +0100 From: Andrew Burgess To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 1/2] gdb: Allow target description to be dumped even when it is remote Message-ID: <20200611140553.GK2737@embecosm.com> References: <3dc13658-f2ab-a76b-ee67-e1b9f343eef1@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3dc13658-f2ab-a76b-ee67-e1b9f343eef1@redhat.com> X-Operating-System: Linux/5.6.15-200.fc31.x86_64 (x86_64) X-Uptime: 15:03:05 up 3 days, 4:10, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-10.3 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.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: , X-List-Received-Date: Thu, 11 Jun 2020 14:06:01 -0000 * Pedro Alves [2020-06-11 12:33:08 +0100]: > On 6/11/20 11:41 AM, Andrew Burgess wrote: > > The maintenance command 'maintenance print c-tdesc' can only print the > > target description if it was loaded from a local file, or if the local > > filename is passed to the maintenance command as an argument. > > > > Sometimes it would be nice to know what target description GDB was > > given by the remote, however, if I connect to a remote target and try > > this command I see this: > > > > (gdb) maintenance print c-tdesc > > The current target description did not come from an XML file. > > (gdb) > > > > Which is not very helpful. > > > > This commit changes things so that if the description came from the > > remote end then GDB will use the fake filename 'target.xml' as the > > filename for the description, GDB will then create the C description > > of the target as if it was in a file 'target.xml'. > > > > I originally added this functionality so I could inspect the > > description passed to GDB by the remote target. After using this for > > a while I realised that actually having GDB recreate the XML would be > > even better, so a later commit will add that functionality too. > > > > Still, given how small this patch is I thought it might be nice to > > include this in GDB anyway. > > > > While I was working on this anyway I've added filename command > > completion to this command. > > > > gdb/ChangeLog: > > > > * target-descriptions.c (maint_print_c_tdesc_cmd): Use fake > > filename for target descriptions that came from the target. > > (_initialize_target_descriptions): Add filename command completion > > for 'maint print c-tdesc'. > > --- > > gdb/ChangeLog | 7 +++++++ > > gdb/target-descriptions.c | 14 ++++++++++---- > > 2 files changed, 17 insertions(+), 4 deletions(-) > > > > diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c > > index 20a3a640f4f..55ea416d69a 100644 > > --- a/gdb/target-descriptions.c > > +++ b/gdb/target-descriptions.c > > @@ -1680,7 +1680,12 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty) > > error (_("There is no target description to print.")); > > > > if (filename == NULL) > > - error (_("The current target description did not come from an XML file.")); > > + { > > + printf_unfiltered (_("The current target description was fetched " > > + "from the target, using\n'target.xml' as a fake " > > + "filename.\n\n")); > > That "," makes it read a little bit ambiguously. Try reading the sentence > without the "," to see what I mean: > > The current target description was fetched from the target using > 'target.xml' as a fake filename. > > This can be read as GDB having read the remote fake 'target.xml' filename, > like: > fetch_available_features_from_target ("target.xml", ops); > which is what it always does anyway... > > I'd suggest a hard period (and line break after the period) instead: > > The current target description was fetched from the target. > Using 'target.xml' as a fake filename. > > But, why do we need to provide a fake name at all? Isn't the only use of > that filename to print it in the comment, here: > > /* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro: > Original: target.xml */ > > How about just doing this: No, the filename is used in a couple of other places too, object names are created based on the filename as well. Still I took your idea and went with it. The patch below uses 'fetched from target' as the fake filename instead. I needed one small extra change so that 'fetched from target' could become 'fetched_from_target' when creating C++ variable names, but otherwise this seem fine to me. How's this? Thanks, Andrew --- commit 7426b6d447cc764cfe5a23486ad8af95ec46d2fc Author: Andrew Burgess Date: Tue Jun 9 19:00:55 2020 +0100 gdb: Allow target description to be dumped even when it is remote The maintenance command 'maintenance print c-tdesc' can only print the target description if it was loaded from a local file, or if the local filename is passed to the maintenance command as an argument. Sometimes it would be nice to know what target description GDB was given by the remote, however, if I connect to a remote target and try this command I see this: (gdb) maintenance print c-tdesc The current target description did not come from an XML file. (gdb) Which is not very helpful. This commit changes things so that if the description came from the remote end then GDB will use a fake filename 'fetched from target' as the filename for the description, GDB will then create the C description of the target as though it came from this file. Example output would look like this (I snipped the feature creation from the middle as that hasn't changed): (gdb) maintenance print c-tdesc /* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro: Original: fetched from target */ #include "defs.h" #include "osabi.h" #include "target-descriptions.h" struct target_desc *tdesc_fetched_from_target; static void initialize_tdesc_fetched_from_target (void) { struct target_desc *result = allocate_target_description (); struct tdesc_feature *feature; /* ... features created here ... */ tdesc_fetched_from_target = result; } (gdb) I originally added this functionality so I could inspect the description passed to GDB by the remote target. After using this for a while I realised that actually having GDB recreate the XML would be even better, so a later commit will add that functionality too. Still, given how small this patch is I thought it might be nice to include this in GDB anyway. While I was working on this anyway I've added filename command completion to this command. gdb/ChangeLog: * target-descriptions.c (print_c_tdesc::print_c_tdesc): Change whitespace to underscore. (maint_print_c_tdesc_cmd): Use fake filename for target descriptions that came from the target. (_initialize_target_descriptions): Add filename command completion for 'maint print c-tdesc'. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 26310c429b3..d358ff4ace0 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,12 @@ +2020-06-10 Andrew Burgess + + * target-descriptions.c (print_c_tdesc::print_c_tdesc): Change + whitespace to underscore. + (maint_print_c_tdesc_cmd): Use fake filename for target + descriptions that came from the target. + (_initialize_target_descriptions): Add filename command completion + for 'maint print c-tdesc'. + 2020-06-08 Simon Marchi * gdbtypes.h (TYPE_FIELD_TYPE): Remove. Change all call sites diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c index 20a3a640f4f..da6cb76404c 100644 --- a/gdb/target-descriptions.c +++ b/gdb/target-descriptions.c @@ -1270,6 +1270,8 @@ class print_c_tdesc : public tdesc_element_visitor break; else if (*inp == '-') *outp++ = '_'; + else if (*inp == ' ') + *outp++ = '_'; else *outp++ = *inp; *outp = '\0'; @@ -1680,7 +1682,7 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty) error (_("There is no target description to print.")); if (filename == NULL) - error (_("The current target description did not come from an XML file.")); + filename = "fetched from target"; std::string filename_after_features (filename); auto loc = filename_after_features.rfind ("/features/"); @@ -1811,6 +1813,8 @@ void _initialize_target_descriptions (); void _initialize_target_descriptions () { + cmd_list_element *cmd; + tdesc_data = gdbarch_data_register_pre_init (tdesc_data_init); add_basic_prefix_cmd ("tdesc", class_maintenance, _("\ @@ -1842,11 +1846,10 @@ Unset the file to read for an XML target description.\n\ When unset, GDB will read the description from the target."), &tdesc_unset_cmdlist); - add_cmd ("c-tdesc", class_maintenance, maint_print_c_tdesc_cmd, _("\ + cmd = add_cmd ("c-tdesc", class_maintenance, maint_print_c_tdesc_cmd, _("\ Print the current target description as a C source file."), &maintenanceprintlist); - - cmd_list_element *cmd; + set_cmd_completer (cmd, filename_completer); cmd = add_cmd ("xml-descriptions", class_maintenance, maintenance_check_xml_descriptions, _("\