From: Andrew Burgess <aburgess@redhat.com>
Date: Thu, 5 Jun 2025 13:50:41 +0000 (+0100)
Subject: gdb/guile: fix memory leak in gdbscm_parse_command_name
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=41cb00ce25900aadbf089f80bb2830494bf2a780;p=thirdparty%2Fbinutils-gdb.git

gdb/guile: fix memory leak in gdbscm_parse_command_name

For reference see the previous commit.

Fix a memory leak in gdbscm_parse_command_name when a guile exception
is thrown.  To reveal the memory leak I placed the following content
into a file 'leak.scm':

  (use-modules (gdb))
  (register-command!
   (make-command
    "break cmd"
    #:command-class COMMAND_OBSCURE
    #:invoke (lambda (self arg from-tty)
               (display "Hello World"))))

Then in GDB:

  (gdb) source leak.scm
  ERROR: In procedure register-command!:
  In procedure gdbscm_register_command_x: Out of range: 'break' is not a prefix command in position 1: "break cmd"
  Error while executing Scheme code.

Running this under valgrind reveals a memory leak for 'result' and
'prefix_text' from gdbscm_parse_command_name.

Another leak can be revealed with this input script:

  (use-modules (gdb))
  (register-command!
   (make-command
    "unknown-prefix cmd"
    #:command-class COMMAND_OBSCURE
    #:invoke (lambda (self arg from-tty)
               (display "Hello World"))))

This one occurs earlier in gdbscm_parse_command_name, and now only
'result' leaks.

The problem is that, when guile throws an exception then a longjmp is
performed from the function that raise the exception back to the guile
run-time.  A consequence of this is that no function local destructors
will be run.

In gdbscm_parse_command_name, this means that the two function locals
`result` and `prefix_text` will not have their destructors run, and
any memory managed by these objects will be leaked.

Fix this by assigning nullptr to these two function locals before
throwing an exception.  This will cause the managed memory to be
deallocated.

I could have implemented a fix that made use of Guile's dynwind
mechanism to register a cleanup callback, however, this felt like
overkill.  At the point the exception is being thrown we know that we
no longer need the managed memory, so we might as well just free the
memory at that point.

With this fix in place, the two leaks are now fixed in the valgrind
output.

Approved-By: Tom Tromey <tom@tromey.com>
---

diff --git a/gdb/guile/scm-cmd.c b/gdb/guile/scm-cmd.c
index 46825f96b9e..19fb742f7f1 100644
--- a/gdb/guile/scm-cmd.c
+++ b/gdb/guile/scm-cmd.c
@@ -520,6 +520,10 @@ gdbscm_parse_command_name (const char *name,
 			prefix_text.get ()).release ();
       scm_dynwind_begin ((scm_t_dynwind_flags) 0);
       gdbscm_dynwind_xfree (msg);
+      /* Release memory now as the destructors will not be run when the
+	 guile exception is thrown.  */
+      result = nullptr;
+      prefix_text = nullptr;
       gdbscm_out_of_range_error (func_name, arg_pos,
 				 gdbscm_scm_from_c_string (name), msg);
     }
@@ -536,6 +540,10 @@ gdbscm_parse_command_name (const char *name,
 		    prefix_text.get ()).release ();
   scm_dynwind_begin ((scm_t_dynwind_flags) 0);
   gdbscm_dynwind_xfree (msg);
+  /* Release memory now as the destructors will not be run when the guile
+     exception is thrown.  */
+  result = nullptr;
+  prefix_text = nullptr;
   gdbscm_out_of_range_error (func_name, arg_pos,
 			     gdbscm_scm_from_c_string (name), msg);
   /* NOTREACHED */