[PATCH] memory leak in coffgen

[PATCH] memory leak in coffgen

[msnyder]

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

This one's mine (not Coverity's).  I noticed that this malloc buffer
was not getting freed even when it was not requested to be cached.


[-- Attachment #2: coffgen2.txt --]
[-- Type: text/plain, Size: 1527 bytes --]

2007-07-25  Michael Snyder  <msnyder@svkmacdonelllnx>

	* coffgen.c (_bfd_coff_read_internal_relocs): If internal_relocs
	are not to be cached, free the temporary buffer.

Index: coffgen.c
===================================================================
RCS file: /cvs/src/src/bfd/coffgen.c,v
retrieving revision 1.59
diff -p -r1.59 coffgen.c
*** coffgen.c	12 Jul 2007 07:16:40 -0000	1.59
--- coffgen.c	25 Jul 2007 23:41:50 -0000
*************** _bfd_coff_read_internal_relocs (bfd *abf
*** 457,473 ****
        free_external = NULL;
      }
  
!   if (cache && free_internal != NULL)
      {
!       if (coff_section_data (abfd, sec) == NULL)
  	{
! 	  amt = sizeof (struct coff_section_tdata);
! 	  sec->used_by_bfd = bfd_zalloc (abfd, amt);
! 	  if (sec->used_by_bfd == NULL)
! 	    goto error_return;
! 	  coff_section_data (abfd, sec)->contents = NULL;
  	}
-       coff_section_data (abfd, sec)->relocs = free_internal;
      }
  
    return internal_relocs;
--- 457,478 ----
        free_external = NULL;
      }
  
!   if (free_internal != NULL)
      {
!       if (cache == FALSE)
! 	free (free_internal);
!       else
  	{
! 	  if (coff_section_data (abfd, sec) == NULL)
! 	    {
! 	      amt = sizeof (struct coff_section_tdata);
! 	      sec->used_by_bfd = bfd_zalloc (abfd, amt);
! 	      if (sec->used_by_bfd == NULL)
! 		goto error_return;
! 	      coff_section_data (abfd, sec)->contents = NULL;
! 	    }
! 	  coff_section_data (abfd, sec)->relocs = free_internal;
  	}
      }
  
    return internal_relocs;

Re: [PATCH] memory leak in coffgen

[Alan Modra]

On Wed, Jul 25, 2007 at 04:45:10PM -0700, msnyder@sonic.net wrote:
> 	* coffgen.c (_bfd_coff_read_internal_relocs): If internal_relocs
> 	are not to be cached, free the temporary buffer.

OK.

> !       if (cache == FALSE)

if (!cache)

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] memory leak in coffgen

[msnyder]

> On Wed, Jul 25, 2007 at 04:45:10PM -0700, msnyder@sonic.net wrote:
>> 	* coffgen.c (_bfd_coff_read_internal_relocs): If internal_relocs
>> 	are not to be cached, free the temporary buffer.
>
> OK.
>
>> !       if (cache == FALSE)
>
> if (!cache)

Committed, thanks.

Re: [PATCH] memory leak in coffgen

[msnyder]

> On Wed, Jul 25, 2007 at 04:45:10PM -0700, msnyder@sonic.net wrote:
>> 	* coffgen.c (_bfd_coff_read_internal_relocs): If internal_relocs
>> 	are not to be cached, free the temporary buffer.
>
> OK.

Alan, unfortunately, I think I was wrong about this one.

We can't free "free_internal" at this point, because its aliased
by internal_relocs, and we're going to return that.

OK to back out this change?

Michael

Re: [PATCH] memory leak in coffgen

[Alan Modra]

On Mon, Jul 30, 2007 at 11:36:52AM -0700, msnyder@sonic.net wrote:
> > On Wed, Jul 25, 2007 at 04:45:10PM -0700, msnyder@sonic.net wrote:
> >> 	* coffgen.c (_bfd_coff_read_internal_relocs): If internal_relocs
> >> 	are not to be cached, free the temporary buffer.
> >
> > OK.
> 
> Alan, unfortunately, I think I was wrong about this one.
> 
> We can't free "free_internal" at this point, because its aliased
> by internal_relocs, and we're going to return that.
> 
> OK to back out this change?

Yes of course.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] memory leak in coffgen

[msnyder]

> On Mon, Jul 30, 2007 at 11:36:52AM -0700, msnyder@sonic.net wrote:
>> > On Wed, Jul 25, 2007 at 04:45:10PM -0700, msnyder@sonic.net wrote:
>> >> 	* coffgen.c (_bfd_coff_read_internal_relocs): If internal_relocs
>> >> 	are not to be cached, free the temporary buffer.
>> >
>> > OK.
>>
>> Alan, unfortunately, I think I was wrong about this one.
>>
>> We can't free "free_internal" at this point, because its aliased
>> by internal_relocs, and we're going to return that.
>>
>> OK to back out this change?
>
> Yes of course.

Thanks, change reverted.