* [PATCH] gdbsupport: fix memory leak in create_file_handler when re-using file handler
@ 2021-12-02 16:51 Simon Marchi
2021-12-02 17:31 ` Tom Tromey
0 siblings, 1 reply; 5+ messages in thread
From: Simon Marchi @ 2021-12-02 16:51 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
From: Simon Marchi <simon.marchi@efficios.com>
ASan made me notice a memory leak, where the memory tied to the file
handle name string wasn't freed. When register a file handler with an
fd that is already registered, we re-use the file_handler object, so we
ended up creating a new std::string object and overwriting the
file_handler::name pointer, without free-ing the old std::string.
Change it so that if re-using a file_handler object, we assign the
name to the existing std::string object.
Change-Id: Ie304cc78ab5ae5dfad9a1366e9890c09de651f43
---
gdbsupport/event-loop.cc | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/gdbsupport/event-loop.cc b/gdbsupport/event-loop.cc
index 98d1ada52cd8..48e5e0031919 100644
--- a/gdbsupport/event-loop.cc
+++ b/gdbsupport/event-loop.cc
@@ -319,6 +319,7 @@ create_file_handler (int fd, int mask, handler_func * proc,
file_ptr->fd = fd;
file_ptr->ready_mask = 0;
file_ptr->next_file = gdb_notifier.first_file_handler;
+ file_ptr->name = new std::string (std::move (name));
gdb_notifier.first_file_handler = file_ptr;
if (use_poll)
@@ -362,11 +363,16 @@ create_file_handler (int fd, int mask, handler_func * proc,
gdb_notifier.num_fds = fd + 1;
}
}
+ else
+ {
+ /* Re-using a file_handler object, re-use previous name object. */
+ *file_ptr->name = std::move (name);
+ }
+
file_ptr->proc = proc;
file_ptr->client_data = client_data;
file_ptr->mask = mask;
- file_ptr->name = new std::string (std::move (name));
file_ptr->is_ui = is_ui;
}
--
2.33.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gdbsupport: fix memory leak in create_file_handler when re-using file handler
2021-12-02 16:51 [PATCH] gdbsupport: fix memory leak in create_file_handler when re-using file handler Simon Marchi
@ 2021-12-02 17:31 ` Tom Tromey
2021-12-02 19:10 ` Simon Marchi
0 siblings, 1 reply; 5+ messages in thread
From: Tom Tromey @ 2021-12-02 17:31 UTC (permalink / raw)
To: Simon Marchi via Gdb-patches; +Cc: Simon Marchi, Simon Marchi
>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
Simon> Change it so that if re-using a file_handler object, we assign the
Simon> name to the existing std::string object.
This seems fine, but it seems to me that this would also be fixed if
file handlers were allocated with new and could hold smart pointers.
Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gdbsupport: fix memory leak in create_file_handler when re-using file handler
2021-12-02 17:31 ` Tom Tromey
@ 2021-12-02 19:10 ` Simon Marchi
2021-12-04 18:18 ` Tom Tromey
0 siblings, 1 reply; 5+ messages in thread
From: Simon Marchi @ 2021-12-02 19:10 UTC (permalink / raw)
To: Tom Tromey, Simon Marchi via Gdb-patches
On 2021-12-02 12:31, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Simon> Change it so that if re-using a file_handler object, we assign the
> Simon> name to the existing std::string object.
>
> This seems fine, but it seems to me that this would also be fixed if
> file handlers were allocated with new and could hold smart pointers.
>
> Tom
I thought it would be more difficult, but not really. Here's the
updated patch.
From 3c588d67961bca5ccf8b8824917e381c79dca505 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Thu, 2 Dec 2021 14:04:18 -0500
Subject: [PATCH] gdbsupport: fix memory leak in create_file_handler when
re-using file handler
ASan made me notice a memory leak, where the memory tied to the file
handle name string wasn't freed. When register a file handler with an
fd that is already registered, we re-use the file_handler object, so we
ended up creating a new std::string object and overwriting the
file_handler::name pointer, without free-ing the old std::string.
Fix this by allocating file_handler with new, deleting it with
delete, and making file_handler::name not a pointer.
Change-Id: Ie304cc78ab5ae5dfad9a1366e9890c09de651f43
---
gdbsupport/event-loop.cc | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/gdbsupport/event-loop.cc b/gdbsupport/event-loop.cc
index 98d1ada52cd8..51cfaec4c7ea 100644
--- a/gdbsupport/event-loop.cc
+++ b/gdbsupport/event-loop.cc
@@ -65,8 +65,8 @@ struct file_handler
/* Argument to pass to proc. */
gdb_client_data client_data;
- /* User-friendly name of this handler. Heap-allocated, owned by this.*/
- std::string *name;
+ /* User-friendly name of this handler. */
+ std::string name;
/* If set, this file descriptor is used for a user interface. */
bool is_ui;
@@ -315,7 +315,7 @@ create_file_handler (int fd, int mask, handler_func * proc,
change the data associated with it. */
if (file_ptr == NULL)
{
- file_ptr = XNEW (file_handler);
+ file_ptr = new file_handler;
file_ptr->fd = fd;
file_ptr->ready_mask = 0;
file_ptr->next_file = gdb_notifier.first_file_handler;
@@ -366,7 +366,7 @@ create_file_handler (int fd, int mask, handler_func * proc,
file_ptr->proc = proc;
file_ptr->client_data = client_data;
file_ptr->mask = mask;
- file_ptr->name = new std::string (std::move (name));
+ file_ptr->name = std::move (name);
file_ptr->is_ui = is_ui;
}
@@ -500,8 +500,7 @@ delete_file_handler (int fd)
prev_ptr->next_file = file_ptr->next_file;
}
- delete file_ptr->name;
- xfree (file_ptr);
+ delete file_ptr;
}
/* Handle the given event by calling the procedure associated to the
@@ -571,7 +570,7 @@ handle_file_event (file_handler *file_ptr, int ready_mask)
{
event_loop_ui_debug_printf (file_ptr->is_ui,
"invoking fd file handler `%s`",
- file_ptr->name->c_str ());
+ file_ptr->name.c_str ());
file_ptr->proc (file_ptr->error, file_ptr->client_data);
}
}
--
2.33.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gdbsupport: fix memory leak in create_file_handler when re-using file handler
2021-12-02 19:10 ` Simon Marchi
@ 2021-12-04 18:18 ` Tom Tromey
2021-12-05 2:47 ` Simon Marchi
0 siblings, 1 reply; 5+ messages in thread
From: Tom Tromey @ 2021-12-04 18:18 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, Simon Marchi via Gdb-patches, Simon Marchi
>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:
Simon> I thought it would be more difficult, but not really. Here's the
Simon> updated patch.
Looks great, thank you.
Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gdbsupport: fix memory leak in create_file_handler when re-using file handler
2021-12-04 18:18 ` Tom Tromey
@ 2021-12-05 2:47 ` Simon Marchi
0 siblings, 0 replies; 5+ messages in thread
From: Simon Marchi @ 2021-12-05 2:47 UTC (permalink / raw)
To: Tom Tromey; +Cc: Simon Marchi via Gdb-patches, Simon Marchi
On 2021-12-04 13:18, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:
>
> Simon> I thought it would be more difficult, but not really. Here's the
> Simon> updated patch.
>
> Looks great, thank you.
>
> Tom
>
Thanks, pushed.
Simon
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-12-05 2:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-02 16:51 [PATCH] gdbsupport: fix memory leak in create_file_handler when re-using file handler Simon Marchi
2021-12-02 17:31 ` Tom Tromey
2021-12-02 19:10 ` Simon Marchi
2021-12-04 18:18 ` Tom Tromey
2021-12-05 2:47 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).