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.129.124]) by sourceware.org (Postfix) with ESMTPS id D118D3856DC7 for ; Thu, 21 Apr 2022 10:25:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D118D3856DC7 Received: from mail-ed1-f69.google.com (mail-ed1-f69.google.com [209.85.208.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-85-AL_ewjR5M---k36eV0-mkw-1; Thu, 21 Apr 2022 06:25:08 -0400 X-MC-Unique: AL_ewjR5M---k36eV0-mkw-1 Received: by mail-ed1-f69.google.com with SMTP id r26-20020a50aada000000b00425afa72622so182075edc.19 for ; Thu, 21 Apr 2022 03:25:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:in-reply-to:references:date :message-id:mime-version; bh=Ryxi8HfHUVMLhXYW1ibh+WcAepUER1UrPIBDzwN/Usw=; b=B7ZgOTIPQNhh0h//zWyL3A6kXTLPbBKhmar3exv46KCnfvYzZmF9+A09rLGFMPosyn zyCEjjF0bML0P1kK0TapGvFskvUKrFPRWNbzinJ3CBgSNDQGfnApCpYN7G7u1cYJD7wU TGm3ylRW9pKpIgRAkhp77BDeu/NKj9N8+4JRfQRS/j7az2jP0dcNCihGBPgwob25VGXf nwOSPUFCCH9pDfyNLwk4c8XJSjlDgxZv40bia8ju3wVkPR7BwaAhof0m2pnnLF8RMBOj 7MYOuya7XzMEolFzfUQniQrBFNzSZ/2SmZVZCVnCkvs60s2leEwfCObroQbluSpdhsvp 4rMw== X-Gm-Message-State: AOAM533WSurA0nfm7+P7Lm6sehzLpQxn5heBz9W6z6GANW5iwI2ZrxWO mp5+mg52nrSSe7Qw5P9uMkQUlgU/N42VPvpp7reVVWhy9CPlMuH4Lmp66yV/xIx69RrG8+yfuHu 9TN8GcMi3LwPXYh+T1KQOOw== X-Received: by 2002:a17:906:6a13:b0:6f0:26f3:fd3e with SMTP id qw19-20020a1709066a1300b006f026f3fd3emr967467ejc.190.1650536706924; Thu, 21 Apr 2022 03:25:06 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwxYXQDxykONxQE8KlnTms39+YyPFtFnXXFfAqAAPwvXc1dYFV+HUU1Yl9ZDi2MnX6wwhkjwQ== X-Received: by 2002:a17:906:6a13:b0:6f0:26f3:fd3e with SMTP id qw19-20020a1709066a1300b006f026f3fd3emr967458ejc.190.1650536706692; Thu, 21 Apr 2022 03:25:06 -0700 (PDT) Received: from localhost (92.40.168.187.threembb.co.uk. [92.40.168.187]) by smtp.gmail.com with ESMTPSA id kx5-20020a170907774500b006e1382b8192sm7800694ejc.147.2022.04.21.03.25.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Apr 2022 03:25:06 -0700 (PDT) From: Andrew Burgess To: Pedro Alves , Christina Schimpe , gdb-patches@sourceware.org Subject: Re: "show remote foo-packet" regression (Re: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array) In-Reply-To: <7c319441-067e-6f5c-cf80-84d696c9236e@palves.net> References: <20220329131158.3970228-1-christina.schimpe@intel.com> <20220329131158.3970228-2-christina.schimpe@intel.com> <08fd8bbf-c44e-7313-d7b3-7b0770c2c7d4@palves.net> <7c319441-067e-6f5c-cf80-84d696c9236e@palves.net> Date: Thu, 21 Apr 2022 11:25:04 +0100 Message-ID: <87mtgerawv.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.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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, 21 Apr 2022 10:25:11 -0000 Pedro Alves writes: > On 2022-04-18 20:01, Pedro Alves wrote: >> >> (gdb) show remote X-packet >> Support for the 'p' packet on newly created remote targets is "disabled". >> >> and of course: >> >> (gdb) set remote X-packet disabled >> "on", "off" or "auto" expected. >> >> >> Also, BTW, err, why do we talk about the 'p' packet if the command is about >> the X packet? That's busted. Seems like a preexisting issue in current master. > > I bisected this, and found out it was a regression introduced by this commit: > > 8579fd136a614985bd27f20539c7bb7c5a51287d is the first bad commit > commit 8579fd136a614985bd27f20539c7bb7c5a51287d > Author: Andrew Burgess > Date: Mon Nov 8 14:58:46 2021 +0000 > > gdb/gdbsupport: make xstrprintf and xstrvprintf return a unique_ptr > > The motivation is to reduce the number of places where unmanaged > pointers are returned from allocation type routines. All of the > callers are updated. > > There should be no user visible changes after this commit. > > > This commit is present in the GDB 12 branch and not in GDB 11, so it's a regression there. Sorry for the regression. I took a look at the code and I honestly have no idea why I thought the code in the above commit would work :-/ Below is a patch that should fix this issue. I probably should write a test to go along with it, but thought I'd share this for feedback first. Let me know what you think, Thanks, Andrew --- commit c4be5cb70bc9b67f7d465eb14a62a0692ed6ad94 Author: Andrew Burgess Date: Thu Apr 21 11:16:18 2022 +0100 gdb: fix 'remote show FOO-packet' aliases The following behaviour was observed in GDB: (gdb) show remote X-packet Support for the `p' packet is auto-detected, currently unknown. Note the message mentions the 'p' packet. This is a regression since this commit: commit 8579fd136a614985bd27f20539c7bb7c5a51287d Date: Mon Nov 8 14:58:46 2021 +0000 gdb/gdbsupport: make xstrprintf and xstrvprintf return a unique_ptr Before this commit the behaviour was: (gdb) show remote X-packet Support for the `X' packet is auto-detected, currently unknown. The problem was caused by a failed attempt to ensure that some allocated strings were deleted when GDB exits. The code in the above commit attempted to make use of 'static' to solve this problem, however, the solution was just wrong. In this new commit I instead allocate a static vector into which all the allocated strings are stored, this ensures the strings are released when GDB exits (which makes output from tools like valgrind cleaner), but each string within the vector can be unique, which fixes the regression. diff --git a/gdb/remote.c b/gdb/remote.c index 75d6bf3919d..562cc586f2b 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -1968,15 +1968,17 @@ add_packet_config_cmd (struct packet_config *config, const char *name, /* set/show remote NAME-packet {auto,on,off} -- legacy. */ if (legacy) { - /* It's not clear who should take ownership of this string, so, for - now, make it static, and give copies to each of the add_alias_cmd - calls below. */ - static gdb::unique_xmalloc_ptr legacy_name + /* It's not clear who should take ownership of the LEGACY_NAME string + created below, so, for now, place the string into a static vector + which ensures the strings is released when GDB exits. */ + static std::vector> legacy_names; + gdb::unique_xmalloc_ptr legacy_name = xstrprintf ("%s-packet", name); add_alias_cmd (legacy_name.get (), cmds.set, class_obscure, 0, &remote_set_cmdlist); add_alias_cmd (legacy_name.get (), cmds.show, class_obscure, 0, &remote_show_cmdlist); + legacy_names.emplace_back (std::move (legacy_name)); } }