Fix double free in tui_source_element
Commit Message
To reproduce, cycle a few times between these layouts: no tui, tui
one-window, tui two-windows (including some layout that shows
disassembly).
tui_set_source_content() expands win_info->content, and has to move
tui_source_element items to the new vector storage, destroying the
items in the old storage, and ~tui_source_element() calls xfree on
'line'. Due to a missing copy ctor, items in the new storage have the
old 'line' pointer which eventually gets freed again. Patch is
attached, I added DISABLE_COPY_AND_ASSIGN() in a few more tui classes
to check for more similar issues.
Valgrind output:
==27971== Invalid free() / delete / delete[] / realloc()
void xfree<char>(char*) (common-utils.h:60)
tui_set_source_content(...) (tui-source.c:198)
tui_update_source_window_as_is(...) (tui-winsource.c:95)
tui_show_symtab_source(...) (tui-source.c:221)
tui_update_source_windows_with_addr(...) (tui-winsource.c:152)
tui_set_layout(tui_layout_type) (tui-layout.c:198)
==27971== Address 0x... is 0 bytes inside a block of size 63 free'd
void xfree<char> (common-utils.h:60)
tui_source_element::~tui_source_element() (tui-data.h:184)
void std::_Destroy<tui_source_element,...) (stl_construct.h:206)
std::vector<tui_source_element,...>::_M_default_append(...)
std::vector<tui_source_element, ...>::resize(...) (stl_vector.h:692)
tui_alloc_source_buffer(...) (tui-winsource.c:693)
tui_set_source_content(...) (tui-source.c:138)
Comments
>>>>> "Bogdan" == Bogdan Harjoc <harjoc@gmail.com> writes:
Bogdan> To reproduce, cycle a few times between these layouts: no tui, tui
Bogdan> one-window, tui two-windows (including some layout that shows
Bogdan> disassembly).
Bogdan> tui_set_source_content() expands win_info->content, and has to move
Bogdan> tui_source_element items to the new vector storage, destroying the
Bogdan> items in the old storage, and ~tui_source_element() calls xfree on
Bogdan> 'line'. Due to a missing copy ctor, items in the new storage have the
Bogdan> old 'line' pointer which eventually gets freed again. Patch is
Bogdan> attached, I added DISABLE_COPY_AND_ASSIGN() in a few more tui classes
Bogdan> to check for more similar issues.
Thanks. I already have something like this on my big TUI refactoring
branch, but your patch is better.
It needs a ChangeLog entry.
Also, do you have a copyright assignment in place?
Tom
@@ -39,6 +39,8 @@ struct tui_point
/* Generic window information. */
struct tui_gen_win_info
{
+ DISABLE_COPY_AND_ASSIGN(tui_gen_win_info);
+
protected:
explicit tui_gen_win_info (enum tui_win_type t)
@@ -183,6 +185,19 @@ struct tui_source_element
xfree (line);
}
+ tui_source_element (tui_source_element && o):
+ line(o.line),
+ line_or_addr(o.line_or_addr),
+ is_exec_point(o.is_exec_point),
+ break_mode(o.break_mode)
+ {
+ o.line = nullptr;
+ }
+
+ tui_source_element (const tui_source_element & o) = delete;
+ tui_source_element & operator=(tui_source_element && o) = delete;
+ tui_source_element & operator=(const tui_source_element & o) = delete;
+
char *line = nullptr;
struct tui_line_or_address line_or_addr;
bool is_exec_point = false;
@@ -208,6 +223,8 @@ typedef char tui_exec_info_content[TUI_EXECINFO_SIZE];
struct tui_locator_window : public tui_gen_win_info
{
+ DISABLE_COPY_AND_ASSIGN(tui_locator_window);
+
tui_locator_window ()
: tui_gen_win_info (LOCATOR_WIN)
{
@@ -25,6 +25,8 @@
class tui_file : public stdio_file
{
+ DISABLE_COPY_AND_ASSIGN(tui_file);
+
public:
explicit tui_file (FILE *stream);
@@ -26,6 +26,8 @@
window instead of printing the line in the console window. */
class tui_ui_out : public cli_ui_out
{
+ DISABLE_COPY_AND_ASSIGN(tui_ui_out);
+
public:
explicit tui_ui_out (ui_file *stream);
@@ -28,6 +28,8 @@
struct tui_data_item_window : public tui_gen_win_info
{
+ DISABLE_COPY_AND_ASSIGN(tui_data_item_window);
+
tui_data_item_window ()
: tui_gen_win_info (DATA_ITEM_WIN)
{
@@ -28,6 +28,8 @@
struct tui_exec_info_window : public tui_gen_win_info
{
+ DISABLE_COPY_AND_ASSIGN(tui_exec_info_window);
+
tui_exec_info_window ()
: tui_gen_win_info (EXEC_INFO_WIN)
{