From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24907 invoked by alias); 25 Nov 2018 22:25:17 -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 24421 invoked by uid 89); 25 Nov 2018 22:24:41 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-27.6 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_LOW,SPF_PASS,TIME_LIMIT_EXCEEDED autolearn=unavailable version=3.3.2 spammy=thrown, Valgrind X-HELO: mailsec110.isp.belgacom.be Received: from mailsec110.isp.belgacom.be (HELO mailsec110.isp.belgacom.be) (195.238.20.106) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 25 Nov 2018 22:24:22 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=skynet.be; i=@skynet.be; q=dns/txt; s=securemail; t=1543184650; x=1574720650; h=message-id:subject:from:to:date:in-reply-to:references: mime-version:content-transfer-encoding; bh=006oWNLwmkSwj4ERKX/hMdU4CeVnWgm1eiYoYuZ28IY=; b=iP4k+MQZql1uMcq2bBUjEimjlHkvt2Lz8J1yo4NEZ8NPW+bnTws79MTL zmofBTgksM/ugkzhqdnaEuCAgNZbuQ==; Received: from 184.205-67-87.adsl-dyn.isp.belgacom.be (HELO md) ([87.67.205.184]) by relay.skynet.be with ESMTP/TLS/AES256-GCM-SHA384; 25 Nov 2018 23:24:08 +0100 Message-ID: <1543184648.10222.1.camel@skynet.be> Subject: Re: [RFA] Fix leak in linespec parser. From: Philippe Waroquiers To: gdb-patches@sourceware.org Date: Sun, 25 Nov 2018 22:25:00 -0000 In-Reply-To: <20181125220637.4362-1-philippe.waroquiers@skynet.be> References: <20181125220637.4362-1-philippe.waroquiers@skynet.be> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-11/txt/msg00428.txt.bz2 On Sun, 2018-11-25 at 23:06 +0100, Philippe Waroquiers wrote: > Valgrind reports the below leak. > linespec_parser_new allocates a std::vector at line 2756, > and stores the pointer to this vector in PARSER_RESULT (parser)->file_symtabs. > At 3 different places in linespec.c, another std::vector is assigned to > a linespec->file_symtabs, without first deleting the current value. > > The leak is fixed by delete-ing linespec->file_symtabs before > assigning a new value to it. > > Tested on debian/amd64, + a bunch of tests re-run under valgrind. Groinch, a few more tests under valgrind shows the fix is not ok: in case an exception is thrown, the delete-d value is then accessed in the cleanup. So, I will further investigate ... Philippe > > Ok to push ? (so, not ok). > > gdb/ChangeLog > 2018-11-25 Philippe Waroquiers > > * linespec.c (create_sals_line_offset): Fix leak by deleting > previous value before assigning new value. > (convert_explicit_location_to_linespec): Likewise. > (parse_linespec): Likewise. > > ==798== VALGRIND_GDB_ERROR_BEGIN > ==798== 32 (24 direct, 8 indirect) bytes in 1 blocks are definitely lost in loss record 447 of 3,143 > ==798== at 0x4C2C48C: operator new(unsigned long) (vg_replace_malloc.c:334) > ==798== by 0x51D401: linespec_parser_new(ls_parser*, int, language_defn const*, program_space*, symtab*, int, linespec_result*) (linespec.c:2756) > ==798== by 0x524BF7: decode_line_full(event_location const*, int, program_space*, symtab*, int, linespec_result*, char const*, char const*) (linespec.c:3271) > ==798== by 0x3E8893: parse_breakpoint_sals(event_location const*, linespec_result*) (breakpoint.c:9067) > ==798== by 0x3E4E7F: create_breakpoint(gdbarch*, event_location const*, char const*, int, char const*, int, int, bptype, int, auto_boolean, breakpoint_ops const*, int, int, int, unsigned int) (breakpoint.c:9248) > ==798== by 0x3E55F5: break_command_1(char const*, int, int) (breakpoint.c:9434) > ==798== by 0x40BA68: cmd_func(cmd_list_element*, char const*, int) (cli-decode.c:1888) > ==798== by 0x665300: execute_command(char const*, int) (top.c:630) > ... > --- > gdb/linespec.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/gdb/linespec.c b/gdb/linespec.c > index 00f59f9c28..9eff779eba 100644 > --- a/gdb/linespec.c > +++ b/gdb/linespec.c > @@ -2117,8 +2117,9 @@ create_sals_line_offset (struct linespec_state *self, > set_default_source_symtab_and_line (); > initialize_defaults (&self->default_symtab, &self->default_line); > fullname = symtab_to_fullname (self->default_symtab); > - symtab_vector_up r = > - collect_symtabs_from_filename (fullname, self->search_pspace); > + symtab_vector_up r > + = collect_symtabs_from_filename (fullname, self->search_pspace); > + delete ls->file_symtabs; > ls->file_symtabs = r.release (); > use_default = 1; > } > @@ -2401,6 +2402,7 @@ convert_explicit_location_to_linespec (struct linespec_state *self, > { > TRY > { > + delete result->file_symtabs; > result->file_symtabs > = symtabs_from_filename (source_filename, > self->search_pspace).release (); > @@ -2630,6 +2632,7 @@ parse_linespec (linespec_parser *parser, const char *arg, > symtab_vector_up r > = symtabs_from_filename (user_filename.get (), > PARSER_STATE (parser)->search_pspace); > + delete PARSER_RESULT (parser)->file_symtabs; > PARSER_RESULT (parser)->file_symtabs = r.release (); > } > CATCH (ex, RETURN_MASK_ERROR)