[PATCH] Mach-O: fix two memory leaks

[PATCH] Mach-O: fix two memory leaks

[shinichiro hamaji]

Hi,

This patch fixes the memory leaks I've noticed while writing my
previous patch. The patch which can be applied to the CVS head:
http://shinh.skr.jp/t/mach-o-leaks.patch

I'd like to add a few descriptions about this although they might be
too obvious for binutils gurus...

For bfd_mach_o_canonicalize_reloc, this leak can be reproduced by something like

% valgrind --leak-check=full objdump -b mach-o-x86-64 -x hello.o

As for bfd_mach_o_canonicalize_dynamic_reloc, I confirmed this patch
fixes the leaks with a dynamic library created by

% gcc -dynamiclib mach/hello.c -mmacosx-version-min=10.4 -o hello.dylib

This patch compiled with --enable-targets=all and "make check" passed.

bfd/
2011-12-13  Shinichiro Hamaji  <shinichiro.hamaji@gmail.com>

	* mach-o.c (bfd_mach_o_canonicalize_reloc): Use bfd_alloc instead
	of bfd_malloc to get rid of memory leaks.
	(bfd_mach_o_canonicalize_dynamic_reloc): Free the malloced
	buffer.

diff --git a/bfd/mach-o.c b/bfd/mach-o.c
index 54edd07..32d6d6c 100644
--- a/bfd/mach-o.c
+++ b/bfd/mach-o.c
@@ -810,7 +810,7 @@ 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));
+  res = bfd_alloc (abfd, asect->reloc_count * sizeof (arelent));
   if (res == NULL)
     return -1;

@@ -881,6 +881,7 @@ bfd_mach_o_canonicalize_dynamic_reloc (bfd *abfd,
arelent **rels,
   for (i = 0; i < dysymtab->nextrel + dysymtab->nlocrel; i++)
     rels[i] = &res[i];
   rels[i] = NULL;
+  free (res);
   return i;
 }

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

[Tristan Gingold]

Hi,


On Dec 13, 2011, at 3:45 PM, shinichiro hamaji wrote:

> Hi,
> 
> This patch fixes the memory leaks I've noticed while writing my
> previous patch. The patch which can be applied to the CVS head:
> http://shinh.skr.jp/t/mach-o-leaks.patch
> 
> I'd like to add a few descriptions about this although they might be
> too obvious for binutils gurus...
> 
> For bfd_mach_o_canonicalize_reloc, this leak can be reproduced by something like
> 
> % valgrind --leak-check=full objdump -b mach-o-x86-64 -x hello.o
> 
> As for bfd_mach_o_canonicalize_dynamic_reloc, I confirmed this patch
> fixes the leaks with a dynamic library created by
> 
> % gcc -dynamiclib mach/hello.c -mmacosx-version-min=10.4 -o hello.dylib
> 
> This patch compiled with --enable-targets=all and "make check" passed.
> 
> bfd/
> 2011-12-13  Shinichiro Hamaji  <shinichiro.hamaji@gmail.com>
> 
> 	* mach-o.c (bfd_mach_o_canonicalize_reloc): Use bfd_alloc instead
> 	of bfd_malloc to get rid of memory leaks.
> 	(bfd_mach_o_canonicalize_dynamic_reloc): Free the malloced
> 	buffer.
> 
> diff --git a/bfd/mach-o.c b/bfd/mach-o.c
> index 54edd07..32d6d6c 100644
> --- a/bfd/mach-o.c
> +++ b/bfd/mach-o.c
> @@ -810,7 +810,7 @@ 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));
> +  res = bfd_alloc (abfd, asect->reloc_count * sizeof (arelent));
>   if (res == NULL)
>     return -1;

This one is correct but not ideal.

> @@ -881,6 +881,7 @@ bfd_mach_o_canonicalize_dynamic_reloc (bfd *abfd,
> arelent **rels,
>   for (i = 0; i < dysymtab->nextrel + dysymtab->nlocrel; i++)
>     rels[i] = &res[i];
>   rels[i] = NULL;
> +  free (res);
>   return i;

This one is wrong, as res is still referenced by rels[].  I think you should use bfd_alloc here too.

The issue with bfd_alloc is that you can't free just the relocs, which might be an issue in the context of gdb.

Why not keep using bfd_malloc and freeing relocs in close_and_cleanup and in free_cached_info ?

Tristan.

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

[shinichiro hamaji]

Hi,

Thanks for the comments!

On Wed, Dec 14, 2011 at 12:03 AM, Tristan Gingold <gingold@adacore.com> wrote:
> Hi,
>
>
> On Dec 13, 2011, at 3:45 PM, shinichiro hamaji wrote:
>
>> Hi,
>>
>> This patch fixes the memory leaks I've noticed while writing my
>> previous patch. The patch which can be applied to the CVS head:
>> http://shinh.skr.jp/t/mach-o-leaks.patch
>>
>> I'd like to add a few descriptions about this although they might be
>> too obvious for binutils gurus...
>>
>> For bfd_mach_o_canonicalize_reloc, this leak can be reproduced by something like
>>
>> % valgrind --leak-check=full objdump -b mach-o-x86-64 -x hello.o
>>
>> As for bfd_mach_o_canonicalize_dynamic_reloc, I confirmed this patch
>> fixes the leaks with a dynamic library created by
>>
>> % gcc -dynamiclib mach/hello.c -mmacosx-version-min=10.4 -o hello.dylib
>>
>> This patch compiled with --enable-targets=all and "make check" passed.
>>
>> bfd/
>> 2011-12-13  Shinichiro Hamaji  <shinichiro.hamaji@gmail.com>
>>
>>       * mach-o.c (bfd_mach_o_canonicalize_reloc): Use bfd_alloc instead
>>       of bfd_malloc to get rid of memory leaks.
>>       (bfd_mach_o_canonicalize_dynamic_reloc): Free the malloced
>>       buffer.
>>
>> diff --git a/bfd/mach-o.c b/bfd/mach-o.c
>> index 54edd07..32d6d6c 100644
>> --- a/bfd/mach-o.c
>> +++ b/bfd/mach-o.c
>> @@ -810,7 +810,7 @@ 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));
>> +  res = bfd_alloc (abfd, asect->reloc_count * sizeof (arelent));
>>   if (res == NULL)
>>     return -1;
>
> This one is correct but not ideal.
>
>> @@ -881,6 +881,7 @@ bfd_mach_o_canonicalize_dynamic_reloc (bfd *abfd,
>> arelent **rels,
>>   for (i = 0; i < dysymtab->nextrel + dysymtab->nlocrel; i++)
>>     rels[i] = &res[i];
>>   rels[i] = NULL;
>> +  free (res);
>>   return i;
>
> This one is wrong, as res is still referenced by rels[].  I think you should use bfd_alloc here too.

Oops, I might somehow miss & in "rels[i] = &res[i];"...

>
> The issue with bfd_alloc is that you can't free just the relocs, which might be an issue in the context of gdb.
>
> Why not keep using bfd_malloc and freeing relocs in close_and_cleanup and in free_cached_info ?

I see. How about this? http://shinh.skr.jp/t/mach-o-leaks-2.patch

Now, it doesn't allocate relocs again if there is the cache, like
elf_slurp_reloc_table in elfcode.h. I modified
bfd_mach_o_get_dynamic_reloc_upper_bound as well because it seems to
need one more space for a NULL pointer.

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..1c3505c 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->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->reloc_cache = res;
     }

+  res = mdata->reloc_cache;
   for (i = 0; i < dysymtab->nextrel + dysymtab->nlocrel; i++)
     rels[i] = &res[i];
   rels[i] = NULL;
@@ -3740,9 +3751,24 @@ 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;
+#define BFCI_FREE(x) if (x != NULL) { free (x); x = NULL; }
+  BFCI_FREE (mdata->reloc_cache);
+  for (asect = abfd->sections; asect != NULL; asect = asect->next)
+    BFCI_FREE (asect->relocation);
+#undef BFCI_FREE
+
+  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..dadf442 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 *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 *);

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

[Tristan Gingold]

Hi,


On Dec 14, 2011, at 2:12 PM, shinichiro hamaji wrote:

> Hi,
> 
> Thanks for the comments!

[…]

> I see. How about this? http://shinh.skr.jp/t/mach-o-leaks-2.patch

Great.  Just a few minor comments.

> Now, it doesn't allocate relocs again if there is the cache, like
> elf_slurp_reloc_table in elfcode.h. I modified
> bfd_mach_o_get_dynamic_reloc_upper_bound as well because it seems to
> need one more space for a NULL pointer.

Indeed, good catch.

> 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..1c3505c 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->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->reloc_cache = res;
>     }
> 
> +  res = mdata->reloc_cache;
>   for (i = 0; i < dysymtab->nextrel + dysymtab->nlocrel; i++)
>     rels[i] = &res[i];
>   rels[i] = NULL;
> @@ -3740,9 +3751,24 @@ 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;
> +#define BFCI_FREE(x) if (x != NULL) { free (x); x = NULL; }
> +  BFCI_FREE (mdata->reloc_cache);
> +  for (asect = abfd->sections; asect != NULL; asect = asect->next)
> +    BFCI_FREE (asect->relocation);
> +#undef BFCI_FREE

Honestly I don't like this style.  You don't need to test if x is NULL before calling free, as free(0) is in fact a no-op.
I prefer you directly write free (…); … = 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..dadf442 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 *reloc_cache;

Can you just rename reloc_cache to dyn_reloc_cache, just to make its purpose clearer.

Ok with these changes.

Thank you for improving Mach-O,
Tristan.

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

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

[shinichiro hamaji]

On Wed, Dec 14, 2011 at 10:35 PM, Tristan Gingold <gingold@adacore.com> wrote:
> Hi,
>
>
> On Dec 14, 2011, at 2:12 PM, shinichiro hamaji wrote:
>
>> Hi,
>>
>> Thanks for the comments!
>
> […]
>
>> I see. How about this? http://shinh.skr.jp/t/mach-o-leaks-2.patch
>
> Great.  Just a few minor comments.
>
>> Now, it doesn't allocate relocs again if there is the cache, like
>> elf_slurp_reloc_table in elfcode.h. I modified
>> bfd_mach_o_get_dynamic_reloc_upper_bound as well because it seems to
>> need one more space for a NULL pointer.
>
> Indeed, good catch.
>
>> 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..1c3505c 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->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->reloc_cache = res;
>>     }
>>
>> +  res = mdata->reloc_cache;
>>   for (i = 0; i < dysymtab->nextrel + dysymtab->nlocrel; i++)
>>     rels[i] = &res[i];
>>   rels[i] = NULL;
>> @@ -3740,9 +3751,24 @@ 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;
>> +#define BFCI_FREE(x) if (x != NULL) { free (x); x = NULL; }
>> +  BFCI_FREE (mdata->reloc_cache);
>> +  for (asect = abfd->sections; asect != NULL; asect = asect->next)
>> +    BFCI_FREE (asect->relocation);
>> +#undef BFCI_FREE
>
> Honestly I don't like this style.  You don't need to test if x is NULL before calling free, as free(0) is in fact a no-op.
> I prefer you directly write free (…); … = NULL;

OK, I just borrowed this macro from aoutx.h and I'm not a big fun of
this either.

>
>> +
>> +  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..dadf442 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 *reloc_cache;
>
> Can you just rename reloc_cache to dyn_reloc_cache, just to make its purpose clearer.

Done.

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

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

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

[Tristan Gingold]

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

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

[shinichiro hamaji]

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

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

[Tristan Gingold]

On Dec 14, 2011, at 3:23 PM, shinichiro hamaji wrote:

> 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!

Committed.

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

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

[Iain Sandoe]

[-- Attachment #1: Type: text/plain, Size: 1524 bytes --]


On 15 Dec 2011, at 11:01, Tristan Gingold wrote:
> On Dec 14, 2011, at 3:23 PM, shinichiro hamaji wrote:
>
>> 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!
>
> Committed.

I suspect that the following was intended -
-  otherwise we try to deallocate mach-o data when the input bfd is  
the archive header...

cheers
Iain

Index: bfd/mach-o.c
===================================================================
RCS file: /cvs/src/src/bfd/mach-o.c,v
retrieving revision 1.78
diff -u -p -r1.78 mach-o.c
--- bfd/mach-o.c	15 Dec 2011 11:01:14 -0000	1.78
+++ bfd/mach-o.c	15 Dec 2011 13:25:19 -0000
@@ -3202,6 +3202,7 @@ bfd_mach_o_mkobject_init (bfd *abfd)
    mdata->commands = NULL;
    mdata->nsects = 0;
    mdata->sections = NULL;
+  mdata->dyn_reloc_cache = NULL;

    return TRUE;
  }
@@ -3765,9 +3766,10 @@ bfd_mach_o_close_and_cleanup (bfd *abfd)
  {
    bfd_mach_o_data_struct *mdata = bfd_mach_o_get_data (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);
+    {
+      _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);
  }



[-- Attachment #2: 11121513-cleanup-diff.txt --]
[-- Type: text/plain, Size: 956 bytes --]

Index: bfd/mach-o.c
===================================================================
RCS file: /cvs/src/src/bfd/mach-o.c,v
retrieving revision 1.78
diff -u -p -r1.78 mach-o.c
--- bfd/mach-o.c	15 Dec 2011 11:01:14 -0000	1.78
+++ bfd/mach-o.c	15 Dec 2011 13:25:19 -0000
@@ -3202,6 +3202,7 @@ bfd_mach_o_mkobject_init (bfd *abfd)
   mdata->commands = NULL;
   mdata->nsects = 0;
   mdata->sections = NULL;
+  mdata->dyn_reloc_cache = NULL;
 
   return TRUE;
 }
@@ -3765,9 +3766,10 @@ bfd_mach_o_close_and_cleanup (bfd *abfd)
 {
   bfd_mach_o_data_struct *mdata = bfd_mach_o_get_data (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);
+    {
+      _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);
 }

[-- Attachment #3: Type: text/plain, Size: 3 bytes --]

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

[Tristan Gingold]

On Dec 15, 2011, at 2:28 PM, Iain Sandoe wrote:

> 
> On 15 Dec 2011, at 11:01, Tristan Gingold wrote:
>> On Dec 14, 2011, at 3:23 PM, shinichiro hamaji wrote:
>> 
>>> 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!
>> 
>> Committed.
> 
> I suspect that the following was intended -
> -  otherwise we try to deallocate mach-o data when the input bfd is the archive header…

Well spotted.  Thanks.

I can commit it, but you forgot to write the ChangeLog entry :-)

Tristan.

> 
> cheers
> Iain
> 
> Index: bfd/mach-o.c
> ===================================================================
> RCS file: /cvs/src/src/bfd/mach-o.c,v
> retrieving revision 1.78
> diff -u -p -r1.78 mach-o.c
> --- bfd/mach-o.c	15 Dec 2011 11:01:14 -0000	1.78
> +++ bfd/mach-o.c	15 Dec 2011 13:25:19 -0000
> @@ -3202,6 +3202,7 @@ bfd_mach_o_mkobject_init (bfd *abfd)
>   mdata->commands = NULL;
>   mdata->nsects = 0;
>   mdata->sections = NULL;
> +  mdata->dyn_reloc_cache = NULL;
> 
>   return TRUE;
> }
> @@ -3765,9 +3766,10 @@ bfd_mach_o_close_and_cleanup (bfd *abfd)
> {
>   bfd_mach_o_data_struct *mdata = bfd_mach_o_get_data (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);
> +    {
> +      _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);
> }
> 
> 
> <11121513-cleanup-diff.txt>
> 
> 

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

[Iain Sandoe]

On 15 Dec 2011, at 13:34, Tristan Gingold wrote:

>
> On Dec 15, 2011, at 2:28 PM, Iain Sandoe wrote:
>
>>
>> On 15 Dec 2011, at 11:01, Tristan Gingold wrote:
>>> On Dec 14, 2011, at 3:23 PM, shinichiro hamaji wrote:
>>>
>>>> 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!
>>>
>>> Committed.
>>
>> I suspect that the following was intended -
>> -  otherwise we try to deallocate mach-o data when the input bfd is  
>> the archive header…
>
> Well spotted.  Thanks.
>
> I can commit it, but you forgot to write the ChangeLog entry :-)

so I did :-)

11-12-xx  Tristan Gingold <...
	  Iain Sandoe <...

bfd:

	* mach-o.c (bfd_mach_o_mkobject_init): Initialize dyn_reloc_cache.
	(bfd_mach_o_close_and_cleanup): Only cleanup Mach-O private data
	for object files.

....
cheers
Iain


>
> Tristan.
>
>>
>> cheers
>> Iain
>>
>> Index: bfd/mach-o.c
>> ===================================================================
>> RCS file: /cvs/src/src/bfd/mach-o.c,v
>> retrieving revision 1.78
>> diff -u -p -r1.78 mach-o.c
>> --- bfd/mach-o.c	15 Dec 2011 11:01:14 -0000	1.78
>> +++ bfd/mach-o.c	15 Dec 2011 13:25:19 -0000
>> @@ -3202,6 +3202,7 @@ bfd_mach_o_mkobject_init (bfd *abfd)
>>  mdata->commands = NULL;
>>  mdata->nsects = 0;
>>  mdata->sections = NULL;
>> +  mdata->dyn_reloc_cache = NULL;
>>
>>  return TRUE;
>> }
>> @@ -3765,9 +3766,10 @@ bfd_mach_o_close_and_cleanup (bfd *abfd)
>> {
>>  bfd_mach_o_data_struct *mdata = bfd_mach_o_get_data (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);
>> +    {
>> +      _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);
>> }
>>
>>
>> <11121513-cleanup-diff.txt>
>>
>>
>

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

[Tristan Gingold]

On Dec 15, 2011, at 2:40 PM, Iain Sandoe wrote:

> 
> On 15 Dec 2011, at 13:34, Tristan Gingold wrote:
> 
>> 
>> On Dec 15, 2011, at 2:28 PM, Iain Sandoe wrote:
>> 
>>> 
>>> On 15 Dec 2011, at 11:01, Tristan Gingold wrote:
>>>> On Dec 14, 2011, at 3:23 PM, shinichiro hamaji wrote:
>>>> 
>>>>> 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!
>>>> 
>>>> Committed.
>>> 
>>> I suspect that the following was intended -
>>> -  otherwise we try to deallocate mach-o data when the input bfd is the archive header…
>> 
>> Well spotted.  Thanks.
>> 
>> I can commit it, but you forgot to write the ChangeLog entry :-)
> 
> so I did :-)

Thanks, committed.

> 11-12-xx  Tristan Gingold <...
> 	  Iain Sandoe <…

Please, don't forget your email address, so that C&P works well!

Tristan.

> 
> bfd:
> 
> 	* mach-o.c (bfd_mach_o_mkobject_init): Initialize dyn_reloc_cache.
> 	(bfd_mach_o_close_and_cleanup): Only cleanup Mach-O private data
> 	for object files.
> 
> ....
> cheers
> Iain
> 
> 
>> 
>> Tristan.
>> 
>>> 
>>> cheers
>>> Iain
>>> 
>>> Index: bfd/mach-o.c
>>> ===================================================================
>>> RCS file: /cvs/src/src/bfd/mach-o.c,v
>>> retrieving revision 1.78
>>> diff -u -p -r1.78 mach-o.c
>>> --- bfd/mach-o.c	15 Dec 2011 11:01:14 -0000	1.78
>>> +++ bfd/mach-o.c	15 Dec 2011 13:25:19 -0000
>>> @@ -3202,6 +3202,7 @@ bfd_mach_o_mkobject_init (bfd *abfd)
>>> mdata->commands = NULL;
>>> mdata->nsects = 0;
>>> mdata->sections = NULL;
>>> +  mdata->dyn_reloc_cache = NULL;
>>> 
>>> return TRUE;
>>> }
>>> @@ -3765,9 +3766,10 @@ bfd_mach_o_close_and_cleanup (bfd *abfd)
>>> {
>>> bfd_mach_o_data_struct *mdata = bfd_mach_o_get_data (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);
>>> +    {
>>> +      _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);
>>> }
>>> 
>>> 
>>> <11121513-cleanup-diff.txt>
>>> 
>>> 
>> 
> 

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

[shinichiro hamaji]

On Thu, Dec 15, 2011 at 11:05 PM, Tristan Gingold <gingold@adacore.com> wrote:
>
> On Dec 15, 2011, at 2:40 PM, Iain Sandoe wrote:
>
>>
>> On 15 Dec 2011, at 13:34, Tristan Gingold wrote:
>>
>>>
>>> On Dec 15, 2011, at 2:28 PM, Iain Sandoe wrote:
>>>
>>>>
>>>> On 15 Dec 2011, at 11:01, Tristan Gingold wrote:
>>>>> On Dec 14, 2011, at 3:23 PM, shinichiro hamaji wrote:
>>>>>
>>>>>> 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!
>>>>>
>>>>> Committed.
>>>>
>>>> I suspect that the following was intended -
>>>> -  otherwise we try to deallocate mach-o data when the input bfd is the archive header…
>>>
>>> Well spotted.  Thanks.
>>>
>>> I can commit it, but you forgot to write the ChangeLog entry :-)
>>
>> so I did :-)
>
> Thanks, committed.

Oops... Thanks Iain for the fix and Tristan for the commit!

>
>> 11-12-xx  Tristan Gingold <...
>>         Iain Sandoe <…
>
> Please, don't forget your email address, so that C&P works well!
>
> Tristan.
>
>>
>> bfd:
>>
>>       * mach-o.c (bfd_mach_o_mkobject_init): Initialize dyn_reloc_cache.
>>       (bfd_mach_o_close_and_cleanup): Only cleanup Mach-O private data
>>       for object files.
>>
>> ....
>> cheers
>> Iain
>>
>>
>>>
>>> Tristan.
>>>
>>>>
>>>> cheers
>>>> Iain
>>>>
>>>> Index: bfd/mach-o.c
>>>> ===================================================================
>>>> RCS file: /cvs/src/src/bfd/mach-o.c,v
>>>> retrieving revision 1.78
>>>> diff -u -p -r1.78 mach-o.c
>>>> --- bfd/mach-o.c    15 Dec 2011 11:01:14 -0000      1.78
>>>> +++ bfd/mach-o.c    15 Dec 2011 13:25:19 -0000
>>>> @@ -3202,6 +3202,7 @@ bfd_mach_o_mkobject_init (bfd *abfd)
>>>> mdata->commands = NULL;
>>>> mdata->nsects = 0;
>>>> mdata->sections = NULL;
>>>> +  mdata->dyn_reloc_cache = NULL;
>>>>
>>>> return TRUE;
>>>> }
>>>> @@ -3765,9 +3766,10 @@ bfd_mach_o_close_and_cleanup (bfd *abfd)
>>>> {
>>>> bfd_mach_o_data_struct *mdata = bfd_mach_o_get_data (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);
>>>> +    {
>>>> +      _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);
>>>> }
>>>>
>>>>
>>>> <11121513-cleanup-diff.txt>
>>>>
>>>>
>>>
>>
>