From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 59349 invoked by alias); 9 Aug 2017 11:40:33 -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 59340 invoked by uid 89); 9 Aug 2017 11:40:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.4 required=5.0 tests=BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-io0-f177.google.com Received: from mail-io0-f177.google.com (HELO mail-io0-f177.google.com) (209.85.223.177) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 09 Aug 2017 11:40:31 +0000 Received: by mail-io0-f177.google.com with SMTP id o9so24068660iod.1 for ; Wed, 09 Aug 2017 04:40:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=9AGa4woiE/HQ8yNMF9K1d1nZ0+/C1/iNJXfrVFTuVTw=; b=I3jsR4X9QlvWjyN4sBKbTPuxGL6Ily12Xfu+Oatnb+mUYPwi/1kbgHI9SgaYJDaxmp 9wzMB7WP78CeeGFchILHrpFDXH5OPQCtV2ERjV/abGhiwmo7WoQ21UavTdbGw/E78j0x LTrXQCTCR5KC1RuXv0SzxHtfgAUpWgE+pTSGpfagkyq2/aNPHRgSY49cKjA6TB7kH6+Z eMaKbTApjxPGFqvyB/MXRbgY05oowL88SLqb8LsOcaVhGhKVyf4dIFOrID+h1PkU7YGP Jmt6vzRQqR1lDEruCOEKISTcEjUZkhcWSlynsaZeYdhYUgG8JznfnzIw3Z1dfBqEKXyh /jgQ== X-Gm-Message-State: AHYfb5g6UDIC3dNnK9puxFDdpO+aOW9YwlxFkJ6T4gLuXPrMumxDLMiy hMvDjuvPgXWEjP4L X-Received: by 10.107.15.32 with SMTP id x32mr6270103ioi.168.1502278829528; Wed, 09 Aug 2017 04:40:29 -0700 (PDT) Received: from E107787-LIN (static.42.136.251.148.clients.your-server.de. [148.251.136.42]) by smtp.gmail.com with ESMTPSA id 201sm1843495iof.21.2017.08.09.04.40.28 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Wed, 09 Aug 2017 04:40:28 -0700 (PDT) From: Yao Qi To: Alex Lindsay Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Fix memory leak in cp-support.c (cp_canonicalize_string) References: <20170807201821.25207-1-alexlindsay239@gmail.com> Date: Wed, 09 Aug 2017 11:40:00 -0000 In-Reply-To: <20170807201821.25207-1-alexlindsay239@gmail.com> (Alex Lindsay's message of "Mon, 7 Aug 2017 15:18:21 -0500") Message-ID: <86k22dyno6.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2017-08/txt/msg00166.txt.bz2 Alex Lindsay writes: > Formerly, in cp_canonicalize_string in cp-support.c, the return value of > cp_comp_to_string was never freed, creating a sizable memory leak detecta= ble > with valgrind. This patch fixes the leak. However, a longer term solution > would be to change the return type of cp_comp_to_string to > gdb::unique_xmalloc_ptr. Hi Alex, Thanks a lot for the investigation and the patch. I revise it a little to use gdb::unique_xmalloc_ptr, and fix another leak somewhere else. Patch below is pushed in. --=20 Yao (=E9=BD=90=E5=B0=A7) =46rom e88e8651cf3415ba440ee17eb1b22b7d2e8368be Mon Sep 17 00:00:00 2001 From: Yao Qi Date: Wed, 9 Aug 2017 12:39:16 +0100 Subject: [PATCH] Fix memory leak in cp-support.c The return value of cp_comp_to_string was never freed, creating a sizable memory leak detectable with valgrind. =3D=3D21225=3D=3D 8 bytes in 1 blocks are definitely lost in loss record 4,= 599 of 10,949^M =3D=3D21225=3D=3D at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_m= emcheck-amd64-linux.so)^M =3D=3D21225=3D=3D by 0x4C2FDEF: realloc (in /usr/lib/valgrind/vgpreload_= memcheck-amd64-linux.so)^M =3D=3D21225=3D=3D by 0x76CB31: d_growable_string_resize (cp-demangle.c:3= 963)^M =3D=3D21225=3D=3D by 0x76CB31: d_growable_string_init (cp-demangle.c:394= 2)^M =3D=3D21225=3D=3D by 0x76CB31: cplus_demangle_print (cp-demangle.c:4308)= ^M =3D=3D21225=3D=3D by 0x4C9535: cp_comp_to_string(demangle_component*, in= t) (cp-name-parser.y:1972)^M =3D=3D21225=3D=3D by 0x53E1D4: cp_canonicalize_string_full[abi:cxx11](ch= ar const*, char const* (*)(type*, void*), void*) (cp-support.c:530)^M =3D=3D21225=3D=3D by 0x53E360: cp_canonicalize_string_no_typedefs[abi:cx= x11](char const*) (cp-support.c:548)^M =3D=3D21225=3D=3D by 0x5D51D2: find_linespec_symbols(linespec_state*, VE= C_symtab_ptr*, char const*, VEC_symbolp**, VEC_bound_minimal_symbol_d**) (l= inespec.c:4030)^M =3D=3D21225=3D=3D by 0x5D6CF6: linespec_parse_basic (linespec.c:1907) =3D=3D21279=3D=3D 32 bytes in 1 blocks are definitely lost in loss record 6= ,066 of 10,947^M =3D=3D21279=3D=3D at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_m= emcheck-amd64-linux.so)^M =3D=3D21279=3D=3D by 0x4C2FDEF: realloc (in /usr/lib/valgrind/vgpreload_= memcheck-amd64-linux.so)^M =3D=3D21279=3D=3D by 0x76CB31: d_growable_string_resize (cp-demangle.c:3= 963)^M =3D=3D21279=3D=3D by 0x76CB31: d_growable_string_init (cp-demangle.c:394= 2)^M =3D=3D21279=3D=3D by 0x76CB31: cplus_demangle_print (cp-demangle.c:4308)= ^M =3D=3D21279=3D=3D by 0x4C9535: cp_comp_to_string(demangle_component*, in= t) (cp-name-parser.y:1972)^M =3D=3D21279=3D=3D by 0x53EF14: cp_canonicalize_string[abi:cxx11](char co= nst*) (cp-support.c:569)^M =3D=3D21279=3D=3D by 0x561B75: dwarf2_canonicalize_name(char const*, dwa= rf2_cu*, obstack*) [clone .isra.210] (dwarf2read.c:20159) This patch fixes the leak. It is a regression by 2f408ecb. gdb: 2017-08-09 Alex Lindsay Yao Qi * cp-support.c (cp_canonicalize_string_full): Use gdb::unique_xmalloc_ptr. (cp_canonicalize_string): Likewise. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 97c39d7..209d0b6 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2017-08-09 Alex Lindsay + Yao Qi + + * cp-support.c (cp_canonicalize_string_full): Use + gdb::unique_xmalloc_ptr. + (cp_canonicalize_string): Likewise. + 2017-08-09 Yao Qi =20 * features/Makefile (WHICH): Remove i386/ non-linux stuff. diff --git a/gdb/cp-support.c b/gdb/cp-support.c index df9a563..f6557ab 100644 --- a/gdb/cp-support.c +++ b/gdb/cp-support.c @@ -527,9 +527,11 @@ cp_canonicalize_string_full (const char *string, replace_typedefs (info.get (), info->tree, finder, data); =20 /* Convert the tree back into a string. */ - ret =3D cp_comp_to_string (info->tree, estimated_len); - gdb_assert (!ret.empty ()); + gdb::unique_xmalloc_ptr us (cp_comp_to_string (info->tree, + estimated_len)); + gdb_assert (us); =20 + ret =3D us.get (); /* Finally, compare the original string with the computed name, returning NULL if they are the same. */ if (ret =3D=3D string) @@ -566,15 +568,18 @@ cp_canonicalize_string (const char *string) return std::string (); =20 estimated_len =3D strlen (string) * 2; - std::string ret =3D cp_comp_to_string (info->tree, estimated_len); + gdb::unique_xmalloc_ptr us (cp_comp_to_string (info->tree, + estimated_len)); =20 - if (ret.empty ()) + if (!us) { warning (_("internal error: string \"%s\" failed to be canonicalized= "), string); return std::string (); } =20 + std::string ret (us.get ()); + if (ret =3D=3D string) return std::string (); =20