[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.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index edfeec934fa0906cfaf42851001d2c402e08ccf7..57d8e7b51e89cb379a4c16b890d088a753f88485 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2013-06-18  Joel Brobecker  <brobecker@adacore.com>
+
+       * 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  <brobecker@adacore.com>
 
        * dwarf2read.c (dwarf2_per_objfile): Replace uses of

diff --git a/gdb/machoread.c b/gdb/machoread.c
index 7a4f0c37a0bedf68dac644f26da52c6c4302e15e..8d45f6f3b989f32fe585f64cee37c23b5dac41b1 100644 (file)
--- 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 *