diff mbox series

[v2] Fix leak with internal functions

Message ID 20240426155625.104269-1-tromey@adacore.com
State New
Headers
Series [v2] Fix leak with internal functions |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm fail Testing failed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 fail Testing failed

Commit Message

Tom Tromey April 26, 2024, 3:56 p.m. UTC 
  Valgrind reported a memory leak on exit with internal functions.

To fix this, I added a new case to clear_internalvar, and then
arranged to call clear_internalvar when an internalvar is deleted.
Also, because an internalvar can reference a value, it seemed prudent
to clear 'internalvars' from the final cleanup in value.c.

Regression tested on x86-64 Fedora 38.  I also did a bit of testing by
hand with valgrind and ASAN.

This version includes the DISABLE_COPY_AND_ASSIGN requested in review.
The move constructor and assignment operator are needed by the
std::map.
---
 gdb/testsuite/gdb.base/gdbvars.exp |  8 ++++++++
 gdb/value.c                        | 31 ++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+)
diff mbox series

Patch

diff --git a/gdb/testsuite/gdb.base/gdbvars.exp b/gdb/testsuite/gdb.base/gdbvars.exp
index 7ec7df36b9e..32fd4c4da4e 100644
--- a/gdb/testsuite/gdb.base/gdbvars.exp
+++ b/gdb/testsuite/gdb.base/gdbvars.exp
@@ -102,6 +102,14 @@  proc test_convenience_functions {} {
 
     gdb_test "print \$_isvoid (foo_int ())" " = 0" \
 	"Check whether non-void function is void"
+
+    gdb_test "print \$isvoid_copy = \$_isvoid" \
+	" = <internal function _isvoid>"
+    gdb_test "print \$isvoid_copy = 0" " = 0"
+
+    # Can't reset the canonical name.
+    gdb_test "print \$_isvoid = 0" \
+	"Cannot overwrite convenience function _isvoid"
 }
 
 proc test_value_history {} {
diff --git a/gdb/value.c b/gdb/value.c
index e71f38b0ce4..47e7a616e48 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1883,6 +1883,28 @@  struct internalvar
     : name (std::move (name))
   {}
 
+  ~internalvar ()
+  {
+    clear_internalvar (this);
+  }
+
+  internalvar (internalvar &&other)
+    : kind (other.kind),
+      u (other.u)
+  {
+    other.kind = INTERNALVAR_VOID;
+  }
+
+  internalvar &operator= (internalvar &&other)
+  {
+    kind = other.kind;
+    u = other.u;
+    other.kind = INTERNALVAR_VOID;
+    return *this;
+  }
+
+  DISABLE_COPY_AND_ASSIGN (internalvar);
+
   std::string name;
 
   /* We support various different kinds of content of an internal variable.
@@ -2320,6 +2342,14 @@  clear_internalvar (struct internalvar *var)
       xfree (var->u.string);
       break;
 
+    case INTERNALVAR_FUNCTION:
+      if (var->u.fn.canonical)
+	{
+	  xfree (var->u.fn.function->name);
+	  xfree (var->u.fn.function);
+	}
+      break;
+
     default:
       break;
     }
@@ -4514,5 +4544,6 @@  and exceeds this limit will cause an error."),
   add_final_cleanup ([] ()
     {
       all_values.clear ();
+      internalvars.clear ();
     });
 }