From patchwork Sat Jan 26 22:34:35 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philippe Waroquiers X-Patchwork-Id: 31221 Received: (qmail 16406 invoked by alias); 26 Jan 2019 22:34:46 -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 16397 invoked by uid 89); 26 Jan 2019 22:34:46 -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=indicated X-HELO: mailsec106.isp.belgacom.be Received: from mailsec106.isp.belgacom.be (HELO mailsec106.isp.belgacom.be) (195.238.20.102) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 26 Jan 2019 22:34:44 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=skynet.be; i=@skynet.be; q=dns/txt; s=securemail; t=1548542084; x=1580078084; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=lAaEzlOp6WhHIidgvA35fD55cLpkRWjwJEqmfiZign4=; b=R5OusvP4Q6MDS7W2gZzyP1163mxXA8MR3SzZy36iW0T6DowFJVd9ftWx UeJz9crgtkxwgdoNSR4yN5P8Ee+BtQ==; Received: from 30.202-67-87.adsl-dyn.isp.belgacom.be (HELO md.home) ([87.67.202.30]) by relay.skynet.be with ESMTP/TLS/DHE-RSA-AES128-GCM-SHA256; 26 Jan 2019 23:34:42 +0100 From: Philippe Waroquiers To: gdb-patches@sourceware.org Cc: Philippe Waroquiers Subject: [RFA] Fix splay tree KEY leak detected in GDB test gdb.base/macscp.exp Date: Sat, 26 Jan 2019 23:34:35 +0100 Message-Id: <20190126223435.9411-1-philippe.waroquiers@skynet.be> MIME-Version: 1.0 X-IsSubscribed: yes When a node is removed from a splay tree, the splay tree was not using the function splay_tree_delete_key_fn to release the key. This was causing a leak, fixed by Tom Tromey. This patch fixes another key leak, that happens when a key equal to a key already present is inserted. In such a case, we have to release the old KEY. Note that this is based on the assumption that the caller always allocates a new KEY when doing an insert. Also, clarify the documentation about when the release functions are called. include/ChangeLog 2019-01-26 Philippe Waroquiers * splay-tree.h (splay_tree_delete_key_fn): Update comment. (splay_tree_delete_value_fn): Likewise. libiberty/ChangeLog 2019-01-26 Philippe Waroquiers * splay-tree.c (splay_tree_insert): Also release old KEY in case of insertion of a key equal to an already present key. (splay_tree_new_typed_alloc): Update comment. --- include/splay-tree.h | 11 +++++++++-- libiberty/splay-tree.c | 13 ++++++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/include/splay-tree.h b/include/splay-tree.h index 0d26272943..da533dec18 100644 --- a/include/splay-tree.h +++ b/include/splay-tree.h @@ -58,11 +58,18 @@ typedef struct splay_tree_node_s *splay_tree_node; typedef int (*splay_tree_compare_fn) (splay_tree_key, splay_tree_key); /* The type of a function used to deallocate any resources associated - with the key. */ + with the key. If you provide this function, the splay tree + will take the ownership of the memory of the splay_tree_key arg + of splay_tree_insert. This function is called to release the keys + present in the tree when calling splay_tree_delete or splay_tree_remove. + If splay_tree_insert is called with a key equal to a key already + present in the tree, the old key and old value will be released. */ typedef void (*splay_tree_delete_key_fn) (splay_tree_key); /* The type of a function used to deallocate any resources associated - with the value. */ + with the value. If you provide this function, the memory of the + splay_tree_value arg of splay_tree_insert is managed similarly to + the splay_tree_key memory: see splay_tree_delete_key_fn. */ typedef void (*splay_tree_delete_value_fn) (splay_tree_value); /* The type of a function used to iterate over the tree. */ diff --git a/libiberty/splay-tree.c b/libiberty/splay-tree.c index 21d23c38df..4bbb39a62c 100644 --- a/libiberty/splay-tree.c +++ b/libiberty/splay-tree.c @@ -318,7 +318,11 @@ different types need to be allocated with different allocators. The splay tree will use @var{compare_fn} to compare nodes, @var{delete_key_fn} to deallocate keys, and @var{delete_value_fn} to -deallocate values. +deallocate values. Keys and values will be deallocated when the +tree is deleted using splay_tree_delete or when a node is removed +using splay_tree_remove. splay_tree_insert will release the previously +inserted key and value using @var{delete_key_fn} and @var{delete_value_fn} +if the inserted key is already found in the tree. @end deftypefn @@ -372,10 +376,13 @@ splay_tree_insert (splay_tree sp, splay_tree_key key, splay_tree_value value) if (sp->root && comparison == 0) { - /* If the root of the tree already has the indicated KEY, just - replace the value with VALUE. */ + /* If the root of the tree already has the indicated KEY, delete + the old key and old value, and replace them with KEY and VALUE. */ + if (sp->delete_key) + (*sp->delete_key) (sp->root->key); if (sp->delete_value) (*sp->delete_value)(sp->root->value); + sp->root->key = key; sp->root->value = value; } else