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

[shinichiro hamaji <shinichiro dot hamaji at gmail dot com>]

On Wed, Dec 14, 2011 at 11:03 PM, Tristan Gingold <gingold@adacore.com> wrote:
>>
>> 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 ?

Yes, please? Thanks!

>
> 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 *);
>