From patchwork Fri Dec 28 11:26:44 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philippe Waroquiers X-Patchwork-Id: 30891 Received: (qmail 79855 invoked by alias); 28 Dec 2018 11:26:55 -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 79825 invoked by uid 89); 28 Dec 2018 11:26:55 -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 autolearn=ham version=3.3.2 spammy=non, informational, history, HContent-Transfer-Encoding:8bit X-HELO: mailsec101.isp.belgacom.be Received: from mailsec101.isp.belgacom.be (HELO mailsec101.isp.belgacom.be) (195.238.20.97) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 28 Dec 2018 11:26:52 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=skynet.be; i=@skynet.be; q=dns/txt; s=securemail; t=1545996412; x=1577532412; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=UtUUfctOlyXLgHJ2Zb0/1NFx0dQVd12W+wrvdzgbnpw=; b=DCxfE7Pzr8TdTm1oMxkWAbV73SU0f1HC1EpG1JMDjrsG45FWwK9cAJlL +4ZmZSB6Xj2uBSRt2m1HMIw+U59vGg==; Received: from 184.205-67-87.adsl-dyn.isp.belgacom.be (HELO md.home) ([87.67.205.184]) by relay.skynet.be with ESMTP/TLS/DHE-RSA-AES128-GCM-SHA256; 28 Dec 2018 12:26:50 +0100 From: Philippe Waroquiers To: gdb-patches@sourceware.org Cc: Philippe Waroquiers Subject: [RFA] Fix leak of set/show verbose doc, avoid xfree of static string Date: Fri, 28 Dec 2018 12:26:44 +0100 Message-Id: <20181228112644.28560-1-philippe.waroquiers@skynet.be> MIME-Version: 1.0 X-IsSubscribed: yes In the tests py-pp-registration/gdb.log default/gdb.log foll-fork/gdb.log setshow/gdb.log break-interp/gdb.log Valgrind detects a leak of the doc strings for the set and show verbose cmd. Here is the stacktrace of the leaked set doc: ==25548== 15 bytes in 1 blocks are definitely lost in loss record 101 of 3,120 ==25548== at 0x4C2BE6D: malloc (vg_replace_malloc.c:309) ==25548== by 0x409C27: xmalloc (common-utils.c:44) ==25548== by 0x778AF9: xstrdup (xstrdup.c:34) ==25548== by 0x3F860F: add_setshow_cmd_full(char const*, command_class, var_types, void*, char const*, char const*, char const*, void (*)(char const*, int, cmd_list_element*), void (*)(ui_file*, int, cmd_list_element*, char const*), cmd_list_element**, cmd_list_element**, cmd_list_element**, cmd_list_element**) [clone .constprop.10] (cli-decode.c:495) ==25548== by 0x3F8ADB: add_setshow_boolean_cmd(char const*, command_class, int*, char const*, char const*, char const*, void (*)(char const*, int, cmd_list_element*), void (*)(ui_file*, int, cmd_list_element*, char const*), cmd_list_element**, cmd_list_element**) (cli-decode.c:593) ==25548== by 0x3F7442: _initialize_cli_cmds() (cli-cmds.c:1768) ==25548== by 0x69EED3: initialize_all_files() (init.c:365) ==25548== by 0x658A84: gdb_init(char*) (top.c:2163) ==25548== by 0x5403E1: captured_main_1 (main.c:863) ==25548== by 0x5403E1: captured_main (main.c:1167) ==25548== by 0x5403E1: gdb_main(captured_main_args*) (main.c:1193) ==25548== by 0x289CA7: main (gdb.c:32) The leak is created by top.c set_verbose 'elaborate joke': the doc string is changed according to the verbosity: (gdb) help set verbose Set verbosity. (gdb) set verbose on (gdb) help set verbose Set verbose printing of informational messages. (gdb) set_verbose creates the leak as it replaces the string allocated in the above stacktrace by a static (non translated) string: ... if (info_verbose) { c->doc = "Set verbose printing of informational messages."; ... Also, this can possibly trigger a call to 'free' of a static string, as c->doc_allocated is kept true, while the string is not allocated anymore. This patch: * fixes the leak by freeing the previous docs if doc_allocated. * internationalize the messages. * properly sets doc_allocated to 0 once doc strings are static. gdb/ChangeLog 2018-12-28 Philippe Waroquiers * top.c (set_verbose): Free previous docs if doc_allocated. Internationalize messages. Set doc_allocated to 0. --- gdb/top.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/gdb/top.c b/gdb/top.c index ac74cb3a22..4884888bec 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -1820,7 +1820,7 @@ show_history (const char *args, int from_tty) int info_verbose = 0; /* Default verbose msgs off. */ -/* Called by do_setshow_command. An elaborate joke. */ +/* Called by do_set_command. An elaborate joke. */ void set_verbose (const char *args, int from_tty, struct cmd_list_element *c) { @@ -1830,16 +1830,22 @@ set_verbose (const char *args, int from_tty, struct cmd_list_element *c) showcmd = lookup_cmd_1 (&cmdname, showlist, NULL, 1); gdb_assert (showcmd != NULL && showcmd != CMD_LIST_AMBIGUOUS); + if (c->doc && c->doc_allocated) + xfree ((char *) c->doc); + if (showcmd->doc && showcmd->doc_allocated) + xfree ((char *) showcmd->doc); if (info_verbose) { - c->doc = "Set verbose printing of informational messages."; - showcmd->doc = "Show verbose printing of informational messages."; + c->doc = _("Set verbose printing of informational messages."); + showcmd->doc = _("Show verbose printing of informational messages."); } else { - c->doc = "Set verbosity."; - showcmd->doc = "Show verbosity."; + c->doc = _("Set verbosity."); + showcmd->doc = _("Show verbosity."); } + c->doc_allocated = 0; + showcmd->doc_allocated = 0; } /* Init the history buffer. Note that we are called after the init file(s)