[binutils-gdb] Fix splay tree KEY leak detected in GDB test gdb.base/macscp.exp

[Tom Tromey <tromey at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e20773049fde7b9a123fda4485259fc06249b22f

commit e20773049fde7b9a123fda4485259fc06249b22f
Author: tromey <tromey@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Tue Feb 12 13:02:48 2019 +0000

    Fix splay tree KEY leak detected in GDB test gdb.base/macscp.exp
    
    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.
    
    2019-02-11  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
    
    	* splay-tree.h (splay_tree_delete_key_fn): Update comment.
    	(splay_tree_delete_value_fn): Likewise.
    
    libiberty/ChangeLog
    2019-02-11  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
    
    	* 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.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@268793 138bc75d-0d04-0410-961f-82ee72b054a4

Diff:
---
 include/ChangeLog      |  5 +++++
 include/splay-tree.h   | 11 +++++++++--
 libiberty/ChangeLog    |  6 ++++++
 libiberty/splay-tree.c | 13 ++++++++++---
 4 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/include/ChangeLog b/include/ChangeLog
index a88efc2..2a71eee 100644
--- a/include/ChangeLog
+++ b/include/ChangeLog
@@ -1,3 +1,8 @@
+2019-02-11  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
+
+	* splay-tree.h (splay_tree_delete_key_fn): Update comment.
+	(splay_tree_delete_value_fn): Likewise.
+
 2019-01-31  Andreas Krebbel  <krebbel@linux.ibm.com>
 
 	* opcode/s390.h (enum s390_opcode_cpu_val): Add
diff --git a/include/splay-tree.h b/include/splay-tree.h
index 0d26272..da533de 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/ChangeLog b/libiberty/ChangeLog
index 496d76d..c9ff317 100644
--- a/libiberty/ChangeLog
+++ b/libiberty/ChangeLog
@@ -1,3 +1,9 @@
+2019-02-11  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
+
+	* 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.
+
 2019-01-21  Tom Tromey  <tom@tromey.com>
 
 	* splay-tree.c (splay_tree_remove): Delete the key if necessary.
diff --git a/libiberty/splay-tree.c b/libiberty/splay-tree.c
index 21d23c3..4bbb39a 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