From patchwork Tue Jan 15 05:56:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philippe Waroquiers X-Patchwork-Id: 31061 Received: (qmail 47648 invoked by alias); 15 Jan 2019 05:56:25 -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 47620 invoked by uid 89); 15 Jan 2019 05:56:24 -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=newly, debian, from_tty, redefining X-HELO: mailsec104.isp.belgacom.be Received: from mailsec104.isp.belgacom.be (HELO mailsec104.isp.belgacom.be) (195.238.20.100) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 15 Jan 2019 05:56:20 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=skynet.be; i=@skynet.be; q=dns/txt; s=securemail; t=1547531780; x=1579067780; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=UU3UrTSZIaI+hjkyox3ZyJwocJrgEw3VL7ryKxYxq2I=; b=cRIgULdThPhK8k8uksyIPxbw2Nzzq2Dfy53hTchGoMS+dVHBxSvDZxEb Ul6ottwbNvm6fTJud+kV1gkJdn/Ipg==; 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; 15 Jan 2019 06:56:17 +0100 From: Philippe Waroquiers To: gdb-patches@sourceware.org Cc: Philippe Waroquiers Subject: [RFA] Fix leaks in macro definitions. Date: Tue, 15 Jan 2019 06:56:11 +0100 Message-Id: <20190115055611.17967-1-philippe.waroquiers@skynet.be> MIME-Version: 1.0 X-IsSubscribed: yes Valgrind detects leaks like the following (gdb.base/macsp.exp). This patch fixes the 3 leaks. 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) gdb/ChangeLog 2019-01-15 Philippe Waroquiers * macrocmd.c (macro_define_command): Use a unique_xmalloc_ptr to maintain name ownership. * 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. (macro_undef): Free the key of the removed node. --- gdb/macrocmd.c | 10 +++---- gdb/macrotab.c | 75 +++++++++++++++++++++++++++++--------------------- 2 files changed, 48 insertions(+), 37 deletions(-) diff --git a/gdb/macrocmd.c b/gdb/macrocmd.c index 706a8353e2..c1c42fa910 100644 --- a/gdb/macrocmd.c +++ b/gdb/macrocmd.c @@ -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.get () == NULL) error (_("Invalid macro name.")); if (*exp == '(') { @@ -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); } } diff --git a/gdb/macrotab.c b/gdb/macrotab.c index 7fcab4691b..b2343c8c29 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) @@ -841,8 +850,10 @@ macro_undef (struct macro_source_file *source, int line, arguments like '-DFOO -UFOO -DFOO=2'. */ if (source == key->start_file && line == key->start_line) - splay_tree_remove (source->table->definitions, n->key); - + { + splay_tree_remove (source->table->definitions, n->key); + macro_tree_delete_key (key); + } else { /* This function is the only place a macro's end-of-scope