From 8b89a20adba7694255492a9aaa4a4fe000bb9ad3 Mon Sep 17 00:00:00 2001 From: Joel Brobecker Date: Tue, 18 Jun 2013 23:35:37 +0000 Subject: [PATCH] [Darwin] Fix cleanup leak in machoread.c:macho_symfile_read This patch fixes a cleanup leak in macho_symfile_read (symbol_table): symbol_table = (asymbol **) xmalloc (storage_needed); make_cleanup (xfree, symbol_table); Unfortunately, fixing the leak alone triggers a crash which occurs while loading the symbols from an executable: % gdb (gdb) file g_exe [SIGSEGV] The crash is caused by the fact that performing the cleanup right after the call to macho_symtab_read, as currently done, is too early. Indeed, references to this symbol_table get saved in the oso_vector global during the call to macho_symtab_read via calls to macho_register_oso, and those references then get accessed later on, when processing all the OSOs that got pushed (see call to macho_symfile_read_all_oso). This patch prevents this by using one single cleanup queue for the entire function, rather than having additional separate cleanup queues (Eg: for the handling of the minimal symbols), thus preventing the premature free'ing of the minimal_symbols array. Secondly, this patch takes this opportunity for avoiding the use of the oso_vector global, thus making it simpler to track its lifetime. gdb/ChangeLog: * machoread.c (oso_vector): Delete this global. (macho_register_oso): Add new parameter "oso_vector_ptr". Use it instead of the "oso_vector" global. (macho_symtab_read, macho_symfile_read_all_oso): Likewise. (macho_symfile_read): Use a local oso_vector, to be free'ed at the end of this function, in place of the old "oso_vector" global. Update various function calls accordingly. Use one single cleanup chain for the entire function. --- gdb/ChangeLog | 11 +++++++++++ gdb/machoread.c | 50 ++++++++++++++++++++++++------------------------- 2 files changed, 35 insertions(+), 26 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index edfeec934f..57d8e7b51e 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,14 @@ +2013-06-18 Joel Brobecker + + * machoread.c (oso_vector): Delete this global. + (macho_register_oso): Add new parameter "oso_vector_ptr". + Use it instead of the "oso_vector" global. + (macho_symtab_read, macho_symfile_read_all_oso): Likewise. + (macho_symfile_read): Use a local oso_vector, to be free'ed + at the end of this function, in place of the old "oso_vector" + global. Update various function calls accordingly. Use one + single cleanup chain for the entire function. + 2013-06-18 Joel Brobecker * dwarf2read.c (dwarf2_per_objfile): Replace uses of diff --git a/gdb/machoread.c b/gdb/machoread.c index 7a4f0c37a0..8d45f6f3b9 100644 --- a/gdb/machoread.c +++ b/gdb/machoread.c @@ -64,10 +64,8 @@ typedef struct oso_el } oso_el; -/* Vector of object files to be read after the executable. This is one - global variable but it's life-time is the one of macho_symfile_read. */ +/* Vector of object files to be read after the executable. */ DEF_VEC_O (oso_el); -static VEC (oso_el) *oso_vector; static void macho_new_init (struct objfile *objfile) @@ -83,7 +81,8 @@ macho_symfile_init (struct objfile *objfile) /* Add a new OSO to the vector of OSO to load. */ static void -macho_register_oso (struct objfile *objfile, +macho_register_oso (VEC (oso_el) **oso_vector_ptr, + struct objfile *objfile, asymbol **oso_sym, asymbol **end_sym, unsigned int nbr_syms) { @@ -94,7 +93,7 @@ macho_register_oso (struct objfile *objfile, el.oso_sym = oso_sym; el.end_sym = end_sym; el.nbr_syms = nbr_syms; - VEC_safe_push (oso_el, oso_vector, &el); + VEC_safe_push (oso_el, *oso_vector_ptr, &el); } /* Add symbol SYM to the minimal symbol table of OBJFILE. */ @@ -175,7 +174,8 @@ macho_symtab_add_minsym (struct objfile *objfile, const asymbol *sym) static void macho_symtab_read (struct objfile *objfile, - long number_of_symbols, asymbol **symbol_table) + long number_of_symbols, asymbol **symbol_table, + VEC (oso_el) **oso_vector_ptr) { long i; const asymbol *dir_so = NULL; @@ -299,7 +299,8 @@ macho_symtab_read (struct objfile *objfile, { /* End of file. */ if (state == S_DWARF_FILE) - macho_register_oso (objfile, oso_file, symbol_table + i, + macho_register_oso (oso_vector_ptr, objfile, + oso_file, symbol_table + i, nbr_syms); state = S_NO_SO; } @@ -642,19 +643,20 @@ macho_add_oso_symfile (oso_el *oso, bfd *abfd, do_cleanups (cleanup); } -/* Read symbols from the vector of oso files. */ +/* Read symbols from the vector of oso files. + + Note that this function sorts OSO_VECTOR_PTR. */ static void -macho_symfile_read_all_oso (struct objfile *main_objfile, int symfile_flags) +macho_symfile_read_all_oso (VEC (oso_el) **oso_vector_ptr, + struct objfile *main_objfile, + int symfile_flags) { int ix; - VEC (oso_el) *vec; + VEC (oso_el) *vec = *oso_vector_ptr; oso_el *oso; struct cleanup *cleanup = make_cleanup (null_cleanup, NULL); - vec = oso_vector; - oso_vector = NULL; - /* Sort oso by name so that files from libraries are gathered. */ qsort (VEC_address (oso_el, vec), VEC_length (oso_el, vec), sizeof (oso_el), oso_el_compare_name); @@ -773,7 +775,6 @@ macho_symfile_read_all_oso (struct objfile *main_objfile, int symfile_flags) } } - VEC_free (oso_el, vec); do_cleanups (cleanup); } @@ -850,6 +851,8 @@ macho_symfile_read (struct objfile *objfile, int symfile_flags) CORE_ADDR offset; long storage_needed; bfd *dsym_bfd; + VEC (oso_el) *oso_vector = NULL; + struct cleanup *old_chain = make_cleanup (VEC_cleanup (oso_el), &oso_vector); /* Get symbols from the symbol table only if the file is an executable. The symbol table of object files is not relocated and is expected to @@ -867,13 +870,12 @@ macho_symfile_read (struct objfile *objfile, int symfile_flags) { asymbol **symbol_table; long symcount; - struct cleanup *back_to; symbol_table = (asymbol **) xmalloc (storage_needed); make_cleanup (xfree, symbol_table); init_minimal_symbol_collection (); - back_to = make_cleanup_discard_minimal_symbols (); + make_cleanup_discard_minimal_symbols (); symcount = bfd_canonicalize_symtab (objfile->obfd, symbol_table); @@ -882,10 +884,9 @@ macho_symfile_read (struct objfile *objfile, int symfile_flags) bfd_get_filename (objfile->obfd), bfd_errmsg (bfd_get_error ())); - macho_symtab_read (objfile, symcount, symbol_table); + macho_symtab_read (objfile, symcount, symbol_table, &oso_vector); install_minimal_symbols (objfile); - do_cleanups (back_to); } /* Try to read .eh_frame / .debug_frame. */ @@ -901,15 +902,10 @@ macho_symfile_read (struct objfile *objfile, int symfile_flags) int ix; oso_el *oso; struct bfd_section *asect, *dsect; - struct cleanup *cleanup; if (mach_o_debug_level > 0) printf_unfiltered (_("dsym file found\n")); - /* Remove oso. They won't be used. */ - VEC_free (oso_el, oso_vector); - oso_vector = NULL; - /* Set dsym section size. */ for (asect = objfile->obfd->sections, dsect = dsym_bfd->sections; asect && dsect; @@ -922,11 +918,11 @@ macho_symfile_read (struct objfile *objfile, int symfile_flags) } /* Add the dsym file as a separate file. */ - cleanup = make_cleanup_bfd_unref (dsym_bfd); + make_cleanup_bfd_unref (dsym_bfd); symbol_file_add_separate (dsym_bfd, symfile_flags, objfile); - do_cleanups (cleanup); /* Don't try to read dwarf2 from main file or shared libraries. */ + do_cleanups (old_chain); return; } } @@ -939,7 +935,9 @@ macho_symfile_read (struct objfile *objfile, int symfile_flags) /* Then the oso. */ if (oso_vector != NULL) - macho_symfile_read_all_oso (objfile, symfile_flags); + macho_symfile_read_all_oso (&oso_vector, objfile, symfile_flags); + + do_cleanups (old_chain); } static bfd_byte * -- 2.34.1