bfd_get_full_section_contents memory leak, plus

H.J. Lu hjl.tools@gmail.com
Sun Oct 21 03:44:00 GMT 2012


On Sat, Oct 20, 2012 at 8:28 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sat, Oct 20, 2012 at 7:45 PM, Alan Modra <amodra@gmail.com> wrote:
>> On Sat, Oct 20, 2012 at 05:30:12PM -0700, H.J. Lu wrote:
>>> On Sat, Oct 20, 2012 at 5:26 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> > On Sat, Oct 20, 2012 at 5:21 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> >> On Sat, Oct 20, 2012 at 3:17 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> >>> It breaks objdump.  I checked in this tectase to show the
>>> >>> error.
>>
>>> diff --git a/bfd/compress.c b/bfd/compress.c
>>> index 294bfd3..0502f80 100644
>>> --- a/bfd/compress.c
>>> +++ b/bfd/compress.c
>>> @@ -166,6 +166,11 @@ bfd_get_full_section_contents (bfd *abfd, sec_ptr
>>> sec, bfd_byte **ptr)
>>>    bfd_byte *compressed_buffer;
>>>  #endif
>>>
>>> +  if ((sec->flags & SEC_IN_MEMORY) != 0
>>> +      && p != NULL
>>> +      && p == sec->contents)
>>> +    return TRUE;
>>> +
>>>    if (abfd->direction != write_direction && sec->rawsize != 0)
>>>      sz = sec->rawsize;
>>>    else
>>
>> I think I prefer to put the extra hack where objdump is twiddling
>> things behind BFD's back.  I also can't see why sec->size needs to be
>> set.  What do you think?
>>
>>         * objdump.c (load_specific_debug_section): Update compress_status
>>         along with setting sec->contents.  No need to set sec->size.
>>
>> Index: binutils/objdump.c
>> ===================================================================
>> RCS file: /cvs/src/src/binutils/objdump.c,v
>> retrieving revision 1.197
>> diff -u -p -r1.197 objdump.c
>> --- binutils/objdump.c  18 Oct 2012 17:42:24 -0000      1.197
>> +++ binutils/objdump.c  21 Oct 2012 02:41:05 -0000
>> @@ -2276,9 +2276,10 @@ load_specific_debug_section (enum dwarf_
>>           decompressed), so we store a pointer to the data in
>>           the bfd_section, and tell it that the contents are
>>           already in memory.  */
>> +      if (sec->compress_status == DECOMPRESS_SECTION_SIZED)
>> +       sec->compress_status = COMPRESS_SECTION_DONE;
>>        sec->contents = section->start;
>>        sec->flags |= SEC_IN_MEMORY;
>> -      sec->size = section->size;
>>
>>        ret = bfd_simple_get_relocated_section_contents (abfd,
>>                                                        sec,
>
> I agree that we don't need to set size in objdump.  On the other hand,
> can we move checking and setting compress_status to compress.c to
> make compressing code more robust?
>
> Thanks.

Something like this.


-- 
H.J.
---
diff --git a/bfd/compress.c b/bfd/compress.c
index 294bfd3..d12cd5e 100644
--- a/bfd/compress.c
+++ b/bfd/compress.c
@@ -176,6 +176,7 @@ bfd_get_full_section_contents (bfd *abfd, sec_ptr sec, bfd_b
yte **ptr)
   switch (sec->compress_status)
     {
     case COMPRESS_SECTION_NONE:
+not_compressed:
       if (p == NULL)
 	{
 	  p = (bfd_byte *) bfd_malloc (sz);
@@ -196,6 +197,9 @@ bfd_get_full_section_contents (bfd *abfd, sec_ptr sec, bfd_b
yte **ptr)
       bfd_set_error (bfd_error_invalid_operation);
       return FALSE;
 #else
+      if (!bfd_is_section_compressed (abfd, sec))
+	goto not_compressed;
+
       /* Read in the full compressed section contents.  */
       compressed_buffer = (bfd_byte *) bfd_malloc (sec->compressed_size);
       if (compressed_buffer == NULL)
@@ -269,11 +273,22 @@ bfd_boolean
 bfd_is_section_compressed (bfd *abfd, sec_ptr sec)
 {
   bfd_byte compressed_buffer [12];
+  unsigned int saved = sec->compress_status;
+  bfd_boolean compressed;
+
+  /* Don't decompress the section.  */
+  sec->compress_status = COMPRESS_SECTION_NONE;

   /* Read the zlib header.  In this case, it should be "ZLIB" followed
      by the uncompressed section size, 8 bytes in big-endian order.  */
-  return (bfd_get_section_contents (abfd, sec, compressed_buffer, 0, 12)
-	  && CONST_STRNEQ ((char*) compressed_buffer, "ZLIB"));
+  compressed = (bfd_get_section_contents (abfd, sec, compressed_buffer, 0,
+					  12)
+		&& CONST_STRNEQ ((char*) compressed_buffer, "ZLIB"));
+
+  /* Restore compress_status.  */
+  sec->compress_status = saved;
+
+  return compressed;
 }

 /*



More information about the Binutils mailing list