bfd_get_full_section_contents memory leak, plus

[Alan Modra]

On Sat, Oct 20, 2012 at 08:43:58PM -0700, H.J. Lu wrote:
> 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:
> >> 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?

Yes, we could move the above to compress.c.

> Something like this.

Huh?  What you posted is some error checking code..

I committed the following.

bfd/
	* compress.c (bfd_cache_section_contents): New function.
	* bfd-in2.h: Regenerate.
binutils/
	* objdump.c (load_specific_debug_section): Use
	bfd_cache_section_contents.

Index: bfd/compress.c
===================================================================
RCS file: /cvs/src/src/bfd/compress.c,v
retrieving revision 1.16
diff -u -p -r1.16 compress.c
--- bfd/compress.c	20 Oct 2012 08:27:13 -0000	1.16
+++ bfd/compress.c	21 Oct 2012 08:42:47 -0000
@@ -255,6 +255,29 @@ bfd_get_full_section_contents (bfd *abfd
 
 /*
 FUNCTION
+	bfd_cache_section_contents
+
+SYNOPSIS
+	void bfd_cache_section_contents
+	  (asection *sec, void *contents);
+
+DESCRIPTION
+	Stash @var(contents) so any following reads of @var(sec) do
+	not need to decompress again.
+*/
+
+void
+bfd_cache_section_contents (asection *sec, void *contents)
+{
+  if (sec->compress_status == DECOMPRESS_SECTION_SIZED)
+    sec->compress_status = COMPRESS_SECTION_DONE;
+  sec->contents = contents;
+  sec->flags |= SEC_IN_MEMORY;
+}
+
+
+/*
+FUNCTION
 	bfd_is_section_compressed
 
 SYNOPSIS
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 08:42:49 -0000
@@ -2272,13 +2272,7 @@ load_specific_debug_section (enum dwarf_
 
   if (is_relocatable && debug_displays [debug].relocate)
     {
-      /* We want to relocate the data we've already read (and
-         decompressed), so we store a pointer to the data in
-         the bfd_section, and tell it that the contents are
-         already in memory.  */
-      sec->contents = section->start;
-      sec->flags |= SEC_IN_MEMORY;
-      sec->size = section->size;
+      bfd_cache_section_contents (sec, section->start);
 
       ret = bfd_simple_get_relocated_section_contents (abfd,
 						       sec,

-- 
Alan Modra
Australia Development Lab, IBM