From patchwork Mon Nov 27 00:35:56 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 24543 Received: (qmail 52989 invoked by alias); 27 Nov 2017 00:36:07 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 52979 invoked by uid 89); 27 Nov 2017 00:36:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KB_WAM_FROM_NAME_SINGLEWORD, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 27 Nov 2017 00:36:05 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id vAR0ZvIu018077 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sun, 26 Nov 2017 19:36:02 -0500 Received: from [10.0.0.11] (192-222-251-162.qc.cable.ebox.net [192.222.251.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 591D41E02D; Sun, 26 Nov 2017 19:35:57 -0500 (EST) Subject: Re: [PATCH] python: Fix memleak in do_start_initialization To: Tom Tromey Cc: gdb-patches@sourceware.org References: <20171126035308.12274-1-simon.marchi@polymtl.ca> <87y3msvp9r.fsf@tromey.com> <46c74cbe-39a0-bcf0-e1b0-fcb9148c5303@polymtl.ca> <87po84vh77.fsf@tromey.com> From: Simon Marchi Message-ID: Date: Sun, 26 Nov 2017 19:35:56 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <87po84vh77.fsf@tromey.com> X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Mon, 27 Nov 2017 00:35:57 +0000 X-IsSubscribed: yes On 2017-11-26 04:54 PM, Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi writes: > > Simon> There's already a comment in that regard: > > Hah, I totally missed this. > > Simon> When I read it, I thought it referred only to progname_copy, but it > Simon> also applies to progname for Python 2. I integrated your suggestion, > Simon> wdyt? > > Simon> - char *progname; > Simon> + gdb::unique_xmalloc_ptr progname; > > I think the patch looks good, but would be even clearer if this > declaration were moved into the #if, so that.. > > Simon> - progname = concat (ldirname (python_libdir).c_str (), SLASH_STRING, "bin", > Simon> - SLASH_STRING, "python", (char *) NULL); > Simon> + progname.reset (concat (ldirname (python_libdir).c_str (), SLASH_STRING, > Simon> + "bin", SLASH_STRING, "python", (char *) NULL)); > > ...this reset could be replaced with plain initialization. > > thanks, > Tom > Good idea, here's what I pushed. Thanks for the comments! From e8e7d10c39955e7ff99ff42f6f16d6befe2fa12e Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Sun, 26 Nov 2017 19:32:47 -0500 Subject: [PATCH] python: Fix memleak in do_start_initialization While playing with valgrind, I noticed that with Python 3, the progname variable in do_start_initialization is not being freed (concat returns a malloc'ed string). This patch uses unique_xmalloc_ptr to manage it. With Python 2, we pass progname it directly to Py_SetProgramName, so it should not be freed. We therefore release it before passing it. gdb/ChangeLog: * python/python.c (do_start_initialization): Change progname type to gdb::unique_xmalloc_ptr. Release the pointer when using Python 2. --- gdb/ChangeLog | 6 ++++++ gdb/python/python.c | 12 ++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index a901409228..b3032ed7d8 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2017-11-26 Simon Marchi + + * python/python.c (do_start_initialization): Change progname + type to gdb::unique_xmalloc_ptr. Release the pointer when using + Python 2. + 2017-11-26 Tom Tromey * common/format.h: Add include guards. diff --git a/gdb/python/python.c b/gdb/python/python.c index 5f152611e8..9ed9b6b1ca 100644 --- a/gdb/python/python.c +++ b/gdb/python/python.c @@ -1658,7 +1658,6 @@ finalize_python (void *ignore) static bool do_start_initialization () { - char *progname; #ifdef IS_PY3K int i; size_t progsize, count; @@ -1672,19 +1671,20 @@ do_start_initialization () /foo/bin/python /foo/lib/pythonX.Y/... This must be done before calling Py_Initialize. */ - progname = concat (ldirname (python_libdir).c_str (), SLASH_STRING, "bin", - SLASH_STRING, "python", (char *) NULL); + gdb::unique_xmalloc_ptr progname + (concat (ldirname (python_libdir).c_str (), SLASH_STRING, "bin", + SLASH_STRING, "python", (char *) NULL)); #ifdef IS_PY3K std::string oldloc = setlocale (LC_ALL, NULL); setlocale (LC_ALL, ""); - progsize = strlen (progname); + progsize = strlen (progname.get ()); progname_copy = (wchar_t *) PyMem_Malloc ((progsize + 1) * sizeof (wchar_t)); if (!progname_copy) { fprintf (stderr, "out of memory\n"); return false; } - count = mbstowcs (progname_copy, progname, progsize + 1); + count = mbstowcs (progname_copy, progname.get (), progsize + 1); if (count == (size_t) -1) { fprintf (stderr, "Could not convert python path to string\n"); @@ -1697,7 +1697,7 @@ do_start_initialization () it is not freed after this call. */ Py_SetProgramName (progname_copy); #else - Py_SetProgramName (progname); + Py_SetProgramName (progname.release ()); #endif #endif