Re: [PATCH] Mach-O: fix two memory leaks

[Tristan Gingold <gingold at adacore dot com>]

> 
> Thanks again for your quick response. Here is the updated patch:
> http://shinh.skr.jp/t/mach-o-leaks-3.patch

Ok.

Should I commit it ?

Tristan.

> 
> bfd/
> 2011-12-14  Shinichiro Hamaji  <shinichiro.hamaji@gmail.com>
> 
> 	* mach-o.c (bfd_mach_o_canonicalize_reloc): Update relocation
> 	table only when there isn't the cahce.
> 	(bfd_mach_o_get_dynamic_reloc_upper_bound): Need one more space
> 	for a pointer for the watchdog.
> 	(bfd_mach_o_canonicalize_dynamic_reloc): Utilize cache like
> 	bfd_mach_o_canonicalize_reloc.
> 	(bfd_mach_o_close_and_cleanup): Call bfd_mach_o_free_cached_info.
> 	(bfd_mach_o_free_cached_info): Free up cache data.
> 	* mach-o.h (reloc_cache): A place to store cache of dynamic relocs.
> 	(bfd_mach_o_free_cached_info): Add declaration.
> 
> diff --git a/bfd/mach-o.c b/bfd/mach-o.c
> index c768689..e5da70b 100644
> --- a/bfd/mach-o.c
> +++ b/bfd/mach-o.c
> @@ -1024,21 +1024,25 @@ bfd_mach_o_canonicalize_reloc (bfd *abfd,
> asection *asect,
>   if (bed->_bfd_mach_o_swap_reloc_in == NULL)
>     return 0;
> 
> -  res = bfd_malloc (asect->reloc_count * sizeof (arelent));
> -  if (res == NULL)
> -    return -1;
> -
> -  if (bfd_mach_o_canonicalize_relocs (abfd, asect->rel_filepos,
> -                                      asect->reloc_count, res, syms) < 0)
> +  if (asect->relocation == NULL)
>     {
> -      free (res);
> -      return -1;
> +      res = bfd_malloc (asect->reloc_count * sizeof (arelent));
> +      if (res == NULL)
> +        return -1;
> +
> +      if (bfd_mach_o_canonicalize_relocs (abfd, asect->rel_filepos,
> +                                          asect->reloc_count, res, syms) < 0)
> +        {
> +          free (res);
> +          return -1;
> +        }
> +      asect->relocation = res;
>     }
> 
> +  res = asect->relocation;
>   for (i = 0; i < asect->reloc_count; i++)
>     rels[i] = &res[i];
>   rels[i] = NULL;
> -  asect->relocation = res;
> 
>   return i;
> }
> @@ -1050,7 +1054,7 @@ bfd_mach_o_get_dynamic_reloc_upper_bound (bfd *abfd)
> 
>   if (mdata->dysymtab == NULL)
>     return 1;
> -  return (mdata->dysymtab->nextrel + mdata->dysymtab->nlocrel)
> +  return (mdata->dysymtab->nextrel + mdata->dysymtab->nlocrel + 1)
>     * sizeof (arelent *);
> }
> 
> @@ -1073,25 +1077,32 @@ bfd_mach_o_canonicalize_dynamic_reloc (bfd
> *abfd, arelent **rels,
>   if (bed->_bfd_mach_o_swap_reloc_in == NULL)
>     return 0;
> 
> -  res = bfd_malloc ((dysymtab->nextrel + dysymtab->nlocrel) * sizeof
> (arelent));
> -  if (res == NULL)
> -    return -1;
> -
> -  if (bfd_mach_o_canonicalize_relocs (abfd, dysymtab->extreloff,
> -                                      dysymtab->nextrel, res, syms) < 0)
> +  if (mdata->dyn_reloc_cache == NULL)
>     {
> -      free (res);
> -      return -1;
> -    }
> +      res = bfd_malloc ((dysymtab->nextrel + dysymtab->nlocrel)
> +                        * sizeof (arelent));
> +      if (res == NULL)
> +        return -1;
> 
> -  if (bfd_mach_o_canonicalize_relocs (abfd, dysymtab->locreloff,
> -                                      dysymtab->nlocrel,
> -                                      res + dysymtab->nextrel, syms) < 0)
> -    {
> -      free (res);
> -      return -1;
> +      if (bfd_mach_o_canonicalize_relocs (abfd, dysymtab->extreloff,
> +                                          dysymtab->nextrel, res, syms) < 0)
> +        {
> +          free (res);
> +          return -1;
> +        }
> +
> +      if (bfd_mach_o_canonicalize_relocs (abfd, dysymtab->locreloff,
> +                                          dysymtab->nlocrel,
> +                                          res + dysymtab->nextrel, syms) < 0)
> +        {
> +          free (res);
> +          return -1;
> +        }
> +
> +      mdata->dyn_reloc_cache = res;
>     }
> 
> +  res = mdata->dyn_reloc_cache;
>   for (i = 0; i < dysymtab->nextrel + dysymtab->nlocrel; i++)
>     rels[i] = &res[i];
>   rels[i] = NULL;
> @@ -3740,9 +3751,26 @@ bfd_mach_o_close_and_cleanup (bfd *abfd)
>   if (bfd_get_format (abfd) == bfd_object && mdata != NULL)
>     _bfd_dwarf2_cleanup_debug_info (abfd, &mdata->dwarf2_find_line_info);
> 
> +  bfd_mach_o_free_cached_info (abfd);
> +
>   return _bfd_generic_close_and_cleanup (abfd);
> }
> 
> +bfd_boolean bfd_mach_o_free_cached_info (bfd *abfd)
> +{
> +  bfd_mach_o_data_struct *mdata = bfd_mach_o_get_data (abfd);
> +  asection *asect;
> +  free (mdata->dyn_reloc_cache);
> +  mdata->dyn_reloc_cache = NULL;
> +  for (asect = abfd->sections; asect != NULL; asect = asect->next)
> +    {
> +      free (asect->relocation);
> +      asect->relocation = NULL;
> +    }
> +
> +  return TRUE;
> +}
> +
> #define bfd_mach_o_bfd_reloc_type_lookup _bfd_norelocs_bfd_reloc_type_lookup
> #define bfd_mach_o_bfd_reloc_name_lookup _bfd_norelocs_bfd_reloc_name_lookup
> 
> diff --git a/bfd/mach-o.h b/bfd/mach-o.h
> index 0c6f4fd..07c6935 100644
> --- a/bfd/mach-o.h
> +++ b/bfd/mach-o.h
> @@ -519,6 +519,9 @@ typedef struct mach_o_data_struct
> 
>   /* A place to stash dwarf2 info for this bfd.  */
>   void *dwarf2_find_line_info;
> +
> +  /* Cache of dynamic relocs. */
> +  arelent *dyn_reloc_cache;
> }
> bfd_mach_o_data_struct;
> 
> @@ -589,6 +592,7 @@ bfd_boolean bfd_mach_o_find_nearest_line (bfd *,
> asection *, asymbol **,
>                                           bfd_vma, const char **,
>                                           const char **, unsigned int *);
> bfd_boolean bfd_mach_o_close_and_cleanup (bfd *);
> +bfd_boolean bfd_mach_o_free_cached_info (bfd *);
> 
> unsigned int bfd_mach_o_section_get_nbr_indirect (bfd *, bfd_mach_o_section *);
> unsigned int bfd_mach_o_section_get_entry_size (bfd *, bfd_mach_o_section *);