From 4c8aa72d0eb714a91ca2e47b816d0b4a0cb27843 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Thu, 17 Aug 2017 22:53:53 +0100 Subject: [PATCH] Plug line_header leaks This plugs a couple leaks introduced by commit fff8551cf549 ("dwarf2read.c: Some C++fycation, use std::vector, std::unique_ptr"). The first problem is that nothing owns the temporary line_header that handle_DW_AT_stmt_list creates in some cases. Before the commit mentioned above, the temporary line_header case used to have: make_cleanup (free_cu_line_header, cu); and that cleanup was assumed to be run by process_die, after handle_DW_AT_stmt_list returns and before child DIEs were processed. The second problem is found in setup_type_unit_groups: that also used to have a similar make_cleanup call, and ended up with a similar leak after the commit mentioned above. Fix both cases by recording in dwarf2_cu whether a line header is owned by the cu/die, and have process_die explicitly free the line_header if so, making use of a new RAII object that also replaces the reset_die_in_process cleanup, while at it. Thanks to Philippe Waroquiers for noticing the leak and pointing in the right direction. gdb/ChangeLog: 2017-08-17 Pedro Alves * dwarf2read.c (struct dwarf2_cu) : New field. (reset_die_in_process): Delete, replaced by ... (process_die_scope): ... this new class. Make it responsible for freeing cu->line_header too. (process_die): Use process_die_scope. (handle_DW_AT_stmt_list): Record the line header's owner CU/DIE in cu->line_header_die_owner. Don't release the line header if it's owned by the CU. (setup_type_unit_groups): Make the CU/DIE own the line header. Don't release the line header here. --- gdb/ChangeLog | 14 ++++++++ gdb/dwarf2read.c | 89 ++++++++++++++++++++++++++++++------------------ 2 files changed, 70 insertions(+), 33 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index d2c194eff3b..50b723743fd 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,17 @@ +2017-08-17 Pedro Alves + + * dwarf2read.c (struct dwarf2_cu) : New + field. + (reset_die_in_process): Delete, replaced by ... + (process_die_scope): ... this new class. Make it responsible for + freeing cu->line_header too. + (process_die): Use process_die_scope. + (handle_DW_AT_stmt_list): Record the line header's owner CU/DIE in + cu->line_header_die_owner. Don't release the line header if it's + owned by the CU. + (setup_type_unit_groups): Make the CU/DIE own the line header. + Don't release the line header here. + 2017-08-17 Alex Lindsay (tiny change) * elfread.c (elf_read_minimal_symbols): xfree synthsyms. diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 4f2fdce8de1..0e281446da9 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -546,6 +546,12 @@ struct dwarf2_cu /* Header data from the line table, during full symbol processing. */ struct line_header *line_header; + /* Non-NULL if LINE_HEADER is owned by this DWARF_CU. Otherwise, + it's owned by dwarf2_per_objfile::line_header_hash. If non-NULL, + this is the DW_TAG_compile_unit die for this CU. We'll hold on + to the line header as long as this DIE is being processed. See + process_die_scope. */ + die_info *line_header_die_owner; /* A list of methods which need to have physnames computed after all type information has been read. */ @@ -8471,28 +8477,44 @@ process_imported_unit_die (struct die_info *die, struct dwarf2_cu *cu) } } -/* Reset the in_process bit of a die. */ - -static void -reset_die_in_process (void *arg) +/* RAII object that represents a process_die scope: i.e., + starts/finishes processing a DIE. */ +class process_die_scope { - struct die_info *die = (struct die_info *) arg; +public: + process_die_scope (die_info *die, dwarf2_cu *cu) + : m_die (die), m_cu (cu) + { + /* We should only be processing DIEs not already in process. */ + gdb_assert (!m_die->in_process); + m_die->in_process = true; + } - die->in_process = 0; -} + ~process_die_scope () + { + m_die->in_process = false; + + /* If we're done processing the DIE for the CU that owns the line + header, we don't need the line header anymore. */ + if (m_cu->line_header_die_owner == m_die) + { + delete m_cu->line_header; + m_cu->line_header = NULL; + m_cu->line_header_die_owner = NULL; + } + } + +private: + die_info *m_die; + dwarf2_cu *m_cu; +}; /* Process a die and its children. */ static void process_die (struct die_info *die, struct dwarf2_cu *cu) { - struct cleanup *in_process; - - /* We should only be processing those not already in process. */ - gdb_assert (!die->in_process); - - die->in_process = 1; - in_process = make_cleanup (reset_die_in_process,die); + process_die_scope scope (die, cu); switch (die->tag) { @@ -8583,8 +8605,6 @@ process_die (struct die_info *die, struct dwarf2_cu *cu) new_symbol (die, NULL, cu); break; } - - do_cleanups (in_process); } /* DWARF name computation. */ @@ -9362,7 +9382,9 @@ handle_DW_AT_stmt_list (struct die_info *die, struct dwarf2_cu *cu, line_header_up lh = dwarf_decode_line_header (line_offset, cu); if (lh == NULL) return; - cu->line_header = lh.get (); + + cu->line_header = lh.release (); + cu->line_header_die_owner = die; if (dwarf2_per_objfile->line_header_hash == NULL) slot = NULL; @@ -9378,6 +9400,7 @@ handle_DW_AT_stmt_list (struct die_info *die, struct dwarf2_cu *cu, /* This newly decoded line number information unit will be owned by line_header_hash hash table. */ *slot = cu->line_header; + cu->line_header_die_owner = NULL; } else { @@ -9392,7 +9415,6 @@ handle_DW_AT_stmt_list (struct die_info *die, struct dwarf2_cu *cu, dwarf_decode_lines (cu->line_header, comp_dir, cu, NULL, lowpc, decode_mapping); - lh.release (); } /* Process DW_TAG_compile_unit or DW_TAG_partial_unit. */ @@ -9530,7 +9552,8 @@ setup_type_unit_groups (struct die_info *die, struct dwarf2_cu *cu) return; } - cu->line_header = lh.get (); + cu->line_header = lh.release (); + cu->line_header_die_owner = die; if (first_time) { @@ -9541,21 +9564,23 @@ setup_type_unit_groups (struct die_info *die, struct dwarf2_cu *cu) process_full_type_unit still needs to know if this is the first time. */ - tu_group->num_symtabs = lh->file_names.size (); - tu_group->symtabs = XNEWVEC (struct symtab *, lh->file_names.size ()); + tu_group->num_symtabs = cu->line_header->file_names.size (); + tu_group->symtabs = XNEWVEC (struct symtab *, + cu->line_header->file_names.size ()); - for (i = 0; i < lh->file_names.size (); ++i) + for (i = 0; i < cu->line_header->file_names.size (); ++i) { - file_entry &fe = lh->file_names[i]; + file_entry &fe = cu->line_header->file_names[i]; - dwarf2_start_subfile (fe.name, fe.include_dir (lh.get ())); + dwarf2_start_subfile (fe.name, fe.include_dir (cu->line_header)); if (current_subfile->symtab == NULL) { - /* NOTE: start_subfile will recognize when it's been passed - a file it has already seen. So we can't assume there's a - simple mapping from lh->file_names to subfiles, plus - lh->file_names may contain dups. */ + /* NOTE: start_subfile will recognize when it's been + passed a file it has already seen. So we can't + assume there's a simple mapping from + cu->line_header->file_names to subfiles, plus + cu->line_header->file_names may contain dups. */ current_subfile->symtab = allocate_symtab (cust, current_subfile->name); } @@ -9568,16 +9593,14 @@ setup_type_unit_groups (struct die_info *die, struct dwarf2_cu *cu) { restart_symtab (tu_group->compunit_symtab, "", 0); - for (i = 0; i < lh->file_names.size (); ++i) + for (i = 0; i < cu->line_header->file_names.size (); ++i) { - struct file_entry *fe = &lh->file_names[i]; + file_entry &fe = cu->line_header->file_names[i]; - fe->symtab = tu_group->symtabs[i]; + fe.symtab = tu_group->symtabs[i]; } } - lh.release (); - /* The main symtab is allocated last. Type units don't have DW_AT_name so they don't have a "real" (so to speak) symtab anyway. There is later code that will assign the main symtab to all symbols -- 2.30.2