From patchwork Sat Jan 19 17:28:02 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philippe Waroquiers X-Patchwork-Id: 31129 Received: (qmail 11638 invoked by alias); 19 Jan 2019 17:28: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 11608 invoked by uid 89); 19 Jan 2019 17:28:17 -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= 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; Sat, 19 Jan 2019 17:28:12 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=skynet.be; i=@skynet.be; q=dns/txt; s=securemail; t=1547918892; x=1579454892; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=Z8oPcfsASQGV7DqC0IQ6zIm/fR/4bODK1+bKaE0BW3o=; b=SQdLbk8X+sUYhm3V/XE9VlqOrg5u9B7mYDMpPjogj2+LlhWWVIR5XxZx DZ5Ri3fSFInlMzni6LyIMtNqyIKvqQ==; Received: from 184.205-67-87.adsl-dyn.isp.belgacom.be (HELO md.ops.cfmu.eurocontrol.be) ([87.67.205.184]) by relay.skynet.be with ESMTP/TLS/DHE-RSA-AES128-GCM-SHA256; 19 Jan 2019 18:28:09 +0100 From: Philippe Waroquiers To: gdb-patches@sourceware.org Cc: Philippe Waroquiers Subject: [RFAv2] Fix leaks in macro definitions. Date: Sat, 19 Jan 2019 18:28:02 +0100 Message-Id: <20190119172802.25220-1-philippe.waroquiers@skynet.be> MIME-Version: 1.0 X-IsSubscribed: yes Valgrind detects leaks like the following (gdb.base/macscp.exp). This patch fixes 2 of the 3 leaks. The remaining leak is to be fixed in splay_tree_remove in libiberty. Tested on debian/amd64, natively and under valgrind. ==22285== 64 (48 direct, 16 indirect) bytes in 1 blocks are definitely lost in loss record 737 of 3,377 ==22285== at 0x4C2BE6D: malloc (vg_replace_malloc.c:309) ==22285== by 0x4049E7: xmalloc (common-utils.c:44) ==22285== by 0x533A20: new_macro_key(macro_table*, char const*, macro_source_file*, int) (macrotab.c:355) ==22285== by 0x53438B: macro_define_function(macro_source_file*, int, char const*, int, char const**, char const*) (macrotab.c:822) ==22285== by 0x52F945: macro_define_command(char const*, int) (macrocmd.c:409) ... ==22285== 128 (96 direct, 32 indirect) bytes in 2 blocks are definitely lost in loss record 1,083 of 3,377 ==22285== at 0x4C2BE6D: malloc (vg_replace_malloc.c:309) ==22285== by 0x4049E7: xmalloc (common-utils.c:44) ==22285== by 0x533A20: new_macro_key(macro_table*, char const*, macro_source_file*, int) (macrotab.c:355) ==22285== by 0x534277: macro_define_object_internal(macro_source_file*, int, char const*, char const*, macro_special_kind) (macrotab.c:776) ==22285== by 0x52F7E0: macro_define_command(char const*, int) (macrocmd.c:414) ... ==22285== 177 bytes in 19 blocks are definitely lost in loss record 1,193 of 3,377 ==22285== at 0x4C2BE6D: malloc (vg_replace_malloc.c:309) ==22285== by 0x4049E7: xmalloc (common-utils.c:44) ==22285== by 0x52F5BD: extract_identifier(char const**, int) (macrocmd.c:316) ==22285== by 0x52F77D: macro_define_command(char const*, int) (macrocmd.c:355) This is the second version of the patch. Compared to first version, the changes are: * Handled the comment of Simon to have extract_identifier returning a unique_ptr. * Handled the comment of Tom that suggested rather to fix one of the leaks in splay-tree.c (which is a libiberty patch). gdb/ChangeLog 2019-01-19 Philippe Waroquiers * macrocmd.c (extract_identifier): Return a gdb::unique_xmalloc_ptr instead of a char *, and update callers. * macrotab.c (macro_define_internal): New function that factorizes macro_define_object_internal and macro_define_function code. If the newly inserted node has replaced a existing node, call macro_tree_delete_key to free the unused just allocated key. (macro_define_object_internal): Use macro_define_internal. (macro_define_function): Likewise. --- gdb/macrocmd.c | 21 ++++++++------- gdb/macrotab.c | 69 ++++++++++++++++++++++++++++---------------------- 2 files changed, 49 insertions(+), 41 deletions(-) diff --git a/gdb/macrocmd.c b/gdb/macrocmd.c index 706a8353e2..7913c9871c 100644 --- a/gdb/macrocmd.c +++ b/gdb/macrocmd.c @@ -288,7 +288,7 @@ skip_ws (const char **expp) function will also allow "..." forms as used in varargs macro parameters. */ -static char * +static gdb::unique_xmalloc_ptr extract_identifier (const char **expp, int is_parameter) { char *result; @@ -317,7 +317,7 @@ extract_identifier (const char **expp, int is_parameter) memcpy (result, *expp, len); result[len] = '\0'; *expp += len; - return result; + return gdb::unique_xmalloc_ptr (result); } struct temporary_macro_definition : public macro_definition @@ -346,14 +346,13 @@ static void macro_define_command (const char *exp, int from_tty) { temporary_macro_definition new_macro; - char *name = NULL; if (!exp) error (_("usage: macro define NAME[(ARGUMENT-LIST)] [REPLACEMENT-LIST]")); skip_ws (&exp); - name = extract_identifier (&exp, 0); - if (! name) + gdb::unique_xmalloc_ptr name = extract_identifier (&exp, 0); + if (name == NULL) error (_("Invalid macro name.")); if (*exp == '(') { @@ -380,7 +379,7 @@ macro_define_command (const char *exp, int from_tty) /* Must update new_macro as well... */ new_macro.argv = (const char * const *) argv; } - argv[new_macro.argc] = extract_identifier (&exp, 1); + argv[new_macro.argc] = extract_identifier (&exp, 1).release (); if (! argv[new_macro.argc]) error (_("Macro is missing an argument.")); ++new_macro.argc; @@ -404,14 +403,15 @@ macro_define_command (const char *exp, int from_tty) ++exp; skip_ws (&exp); - macro_define_function (macro_main (macro_user_macros), -1, name, + macro_define_function (macro_main (macro_user_macros), -1, name.get (), new_macro.argc, (const char **) new_macro.argv, exp); } else { skip_ws (&exp); - macro_define_object (macro_main (macro_user_macros), -1, name, exp); + macro_define_object (macro_main (macro_user_macros), -1, name.get (), + exp); } } @@ -419,7 +419,7 @@ macro_define_command (const char *exp, int from_tty) static void macro_undef_command (const char *exp, int from_tty) { - char *name; + gdb::unique_xmalloc_ptr name; if (!exp) error (_("usage: macro undef NAME")); @@ -428,8 +428,7 @@ macro_undef_command (const char *exp, int from_tty) name = extract_identifier (&exp, 0); if (! name) error (_("Invalid macro name.")); - macro_undef (macro_main (macro_user_macros), -1, name); - xfree (name); + macro_undef (macro_main (macro_user_macros), -1, name.get ()); } diff --git a/gdb/macrotab.c b/gdb/macrotab.c index 7fcab4691b..99e2231089 100644 --- a/gdb/macrotab.c +++ b/gdb/macrotab.c @@ -743,21 +743,27 @@ check_for_redefinition (struct macro_source_file *source, int line, return 0; } -/* A helper function to define a new object-like macro. */ +/* A helper function to define a new object-like or function-like macro + according to KIND. When KIND is macro_object_like, + the macro_special_kind must be provided as ARGC, and ARGV must be NULL. + When KIND is macro_function_like, ARGC and ARGV are giving the function + arguments. */ static void -macro_define_object_internal (struct macro_source_file *source, int line, - const char *name, const char *replacement, - enum macro_special_kind kind) +macro_define_internal (struct macro_source_file *source, int line, + const char *name, enum macro_kind kind, + int argc, const char **argv, + const char *replacement) { struct macro_table *t = source->table; struct macro_key *k = NULL; struct macro_definition *d; + splay_tree_node s; if (! t->redef_ok) - k = check_for_redefinition (source, line, - name, macro_object_like, - 0, 0, + k = check_for_redefinition (source, line, + name, kind, + argc, argv, replacement); /* If we're redefining a symbol, and the existing key would be @@ -774,8 +780,28 @@ macro_define_object_internal (struct macro_source_file *source, int line, return; k = new_macro_key (t, name, source, line); - d = new_macro_definition (t, macro_object_like, kind, 0, replacement); - splay_tree_insert (t->definitions, (splay_tree_key) k, (splay_tree_value) d); + d = new_macro_definition (t, kind, argc, argv, replacement); + s = splay_tree_insert (t->definitions, (splay_tree_key) k, (splay_tree_value) d); + if ((struct macro_key *) s->key != k) + { + /* If the node inserted is not using k, it means we have a redefinition. + We must free the newly allocated k, as it will not be stored in + the splay tree. */ + macro_tree_delete_key (k); + } +} + +/* A helper function to define a new object-like macro. */ + +static void +macro_define_object_internal (struct macro_source_file *source, int line, + const char *name, const char *replacement, + enum macro_special_kind special_kind) +{ + macro_define_internal (source, line, + name, macro_object_like, + special_kind, 0, + replacement); } void @@ -802,29 +828,12 @@ macro_define_function (struct macro_source_file *source, int line, const char *name, int argc, const char **argv, const char *replacement) { - struct macro_table *t = source->table; - struct macro_key *k = NULL; - struct macro_definition *d; - - if (! t->redef_ok) - k = check_for_redefinition (source, line, - name, macro_function_like, - argc, argv, - replacement); - - /* See comments about duplicate keys in macro_define_object. */ - if (k && ! key_compare (k, name, source, line)) - return; - - /* We should also check here that all the argument names in ARGV are - distinct. */ - - k = new_macro_key (t, name, source, line); - d = new_macro_definition (t, macro_function_like, argc, argv, replacement); - splay_tree_insert (t->definitions, (splay_tree_key) k, (splay_tree_value) d); + macro_define_internal (source, line, + name, macro_function_like, + argc, argv, + replacement); } - void macro_undef (struct macro_source_file *source, int line, const char *name)