From patchwork Sun Nov 26 20:55:59 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 24541 Received: (qmail 31620 invoked by alias); 26 Nov 2017 20:56:18 -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 31610 invoked by uid 89); 26 Nov 2017 20:56:18 -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=H*r:8.14.7, IDE 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; Sun, 26 Nov 2017 20:56:07 +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 vAQKu0ZM015351 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sun, 26 Nov 2017 15:56:05 -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 7C91B1E02D; Sun, 26 Nov 2017 15:56:00 -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> From: Simon Marchi Message-ID: <46c74cbe-39a0-bcf0-e1b0-fcb9148c5303@polymtl.ca> Date: Sun, 26 Nov 2017 15:55:59 -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: <87y3msvp9r.fsf@tromey.com> X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Sun, 26 Nov 2017 20:56:00 +0000 X-IsSubscribed: yes On 2017-11-26 02:00 PM, Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi writes: > > Simon> While playing with valgrind, I noticed that the progname variable in > Simon> do_start_initialization is not being freed (concat returns a malloc'ed > Simon> string). Use a unique_xmalloc_ptr to manage it. > > I don't think this is quite correct. > On the Python 2 branch, the code does this: > > Py_SetProgramName (progname); > > The docs for Py_SetProgramName say: > > The argument should point to a > zero-terminated character string in static storage whose contents > will not change for the duration of the program's execution. Ahh, you are completely right, I always forget about Python 2. And I relied too much on my IDE to find usages of progname, it didn't find the usage in the disabled #if branch. At least it would have failed at compile time with Python 2, not runtime. > I think a comment referring to the Python docs would be better. > And, freeing progname on the Python 3 branch is ok. > Maybe the Python 2 branch could pass "progname.release ()". There's already a comment in that regard: /* Note that Py_SetProgramName expects the string it is passed to remain alive for the duration of the program's execution, so it is not freed after this call. */ When I read it, I thought it referred only to progname_copy, but it also applies to progname for Python 2. I integrated your suggestion, wdyt? From 468fe44d21dfed10f547d790d233547ab0da2cf5 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Sat, 25 Nov 2017 22:51:02 -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/python/python.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/gdb/python/python.c b/gdb/python/python.c index 5f152611e8..6b05f97b39 100644 --- a/gdb/python/python.c +++ b/gdb/python/python.c @@ -1658,7 +1658,7 @@ finalize_python (void *ignore) static bool do_start_initialization () { - char *progname; + gdb::unique_xmalloc_ptr progname; #ifdef IS_PY3K int i; size_t progsize, count; @@ -1672,19 +1672,19 @@ 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); + progname.reset (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