From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 126426 invoked by alias); 6 Dec 2017 18:58:09 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 126411 invoked by uid 89); 6 Dec 2017 18:58:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=Completion X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 06 Dec 2017 18:58:07 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3A697C0587DC; Wed, 6 Dec 2017 18:58:06 +0000 (UTC) Received: from localhost (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0562E78416; Wed, 6 Dec 2017 18:58:05 +0000 (UTC) From: Sergio Durigan Junior To: Simon Marchi Cc: Tom Tromey , Subject: Re: [RFA 3/3] Remove cleanups from break-catch-syscall.c References: <20171018040645.7212-1-tom@tromey.com> <20171018040645.7212-3-tom@tromey.com> <280a7994-38a6-2138-b199-8902ee2651b0@ericsson.com> Date: Wed, 06 Dec 2017 18:58:00 -0000 In-Reply-To: <280a7994-38a6-2138-b199-8902ee2651b0@ericsson.com> (Simon Marchi's message of "Fri, 20 Oct 2017 16:26:15 -0400") Message-ID: <87efo7elaa.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2017-12/txt/msg00125.txt.bz2 On Friday, October 20 2017, Simon Marchi wrote: > On 2017-10-18 12:06 AM, Tom Tromey wrote: >> This removes the remaining cleanups from break-catch-syscall.c by >> storing temporary strings in a vector. >> >> ChangeLog >> 2017-10-17 Tom Tromey >> >> * break-catch-syscall.c (catch_syscall_completer): Use >> std::string, gdb::unique_xmalloc_ptr. >> --- >> gdb/ChangeLog | 5 +++++ >> gdb/break-catch-syscall.c | 35 ++++++++++++++++++----------------- >> 2 files changed, 23 insertions(+), 17 deletions(-) >> >> diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c >> index 01e761ce37..2bbfee0ac4 100644 >> --- a/gdb/break-catch-syscall.c >> +++ b/gdb/break-catch-syscall.c >> @@ -560,9 +560,7 @@ catch_syscall_completer (struct cmd_list_element *cmd, >> const char *text, const char *word) >> { >> struct gdbarch *gdbarch = get_current_arch (); >> - struct cleanup *cleanups = make_cleanup (null_cleanup, NULL); >> - const char **group_list = NULL; >> - const char **syscall_list = NULL; >> + gdb::unique_xmalloc_ptr group_list; >> const char *prefix; >> int i; >> >> @@ -575,34 +573,37 @@ catch_syscall_completer (struct cmd_list_element *cmd, >> if (startswith (prefix, "g:") || startswith (prefix, "group:")) >> { >> /* Perform completion inside 'group:' namespace only. */ >> - group_list = get_syscall_group_names (gdbarch); >> + group_list.reset (get_syscall_group_names (gdbarch)); >> if (group_list != NULL) >> - complete_on_enum (tracker, group_list, word, word); >> + complete_on_enum (tracker, group_list.get (), word, word); >> } >> else >> { >> /* Complete with both, syscall names and groups. */ >> - syscall_list = get_syscall_names (gdbarch); >> - group_list = get_syscall_group_names (gdbarch); >> + gdb::unique_xmalloc_ptr syscall_list >> + (get_syscall_names (gdbarch)); >> + group_list.reset (get_syscall_group_names (gdbarch)); >> + >> + const char **group_ptr = group_list.get (); >> + >> + /* Hold on to strings while we're using them. */ >> + std::vector holders; >> >> /* Append "group:" prefix to syscall groups. */ >> - for (i = 0; group_list[i] != NULL; i++) >> + for (i = 0; group_ptr[i] != NULL; i++) >> { >> - char *prefixed_group = xstrprintf ("group:%s", group_list[i]); >> + std::string prefixed_group = string_printf ("group:%s", >> + group_ptr[i]); >> >> - group_list[i] = prefixed_group; >> - make_cleanup (xfree, prefixed_group); >> + group_ptr[i] = prefixed_group.c_str (); >> + holders.push_back (prefixed_group); >> } > > Err, I think there's something that doesn't make sense here actually. We > record in group_ptr[i] a pointer to the buffer of the temporary std::string, > that gets deleted when we go out of scope (end of the iteration). That > causes this fail: > > Running /home/emaisin/src/binutils-gdb/gdb/testsuite/gdb.base/catch-syscall.exp ... > FAIL: gdb.base/catch-syscall.exp: complete catch syscall group suggests 'group:' prefix (pattern 2) Hey guys, Any news on this? I was about to report this regression, but it seems you were already dealing with it here. Thanks, > By hand, you can do > > (gdb) catch syscall g > > There should be many entries starting with group:, in the failing case there's only > one. Presumably because in group_ptr all the pointers point to the same location, > that contains the last group added. The completion mechanism then removes duplicates. > > It is not enough to assign holders.back ().c_str () (after having pushed the string in > the vector), because when the vector gets reallocated it can now point to stale memory. > I think we have to do it in two pass, prepare the vector of std::string, and then get > pointers to the strings. Something like this: > > > commit f9cab673480425a130b73394c3a63d256eadf314 > Author: Simon Marchi > Date: Fri Oct 20 16:22:40 2017 -0400 > > Fix syscall group completion > > diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c > index 82d3e36..095284c 100644 > --- a/gdb/break-catch-syscall.c > +++ b/gdb/break-catch-syscall.c > @@ -562,7 +562,6 @@ catch_syscall_completer (struct cmd_list_element *cmd, > struct gdbarch *gdbarch = get_current_arch (); > gdb::unique_xmalloc_ptr group_list; > const char *prefix; > - int i; > > /* Completion considers ':' to be a word separator, so we use this to > verify whether the previous word was a group prefix. If so, we > @@ -590,14 +589,11 @@ catch_syscall_completer (struct cmd_list_element *cmd, > std::vector holders; > > /* Append "group:" prefix to syscall groups. */ > - for (i = 0; group_ptr[i] != NULL; i++) > - { > - std::string prefixed_group = string_printf ("group:%s", > - group_ptr[i]); > + for (int i = 0; group_ptr[i] != NULL; i++) > + holders.push_back (string_printf ("group:%s", group_ptr[i])); > > - group_ptr[i] = prefixed_group.c_str (); > - holders.push_back (std::move (prefixed_group)); > - } > + for (int i = 0; group_ptr[i] != NULL; i++) > + group_ptr[i] = holders[i].c_str (); > > if (syscall_list != NULL) > complete_on_enum (tracker, syscall_list.get (), word, word); -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/