Re: Memory leak in 2.12.1

[Jeff Baker <jbaker at qnx dot com>]

This has been fixed now.  If anyone needs closure I've included the 
relevant patch below (originally checked in at 2002-11-12 10:45).

Index: bfd.c
===================================================================
RCS file: /cvs/src/src/bfd/bfd.c,v
retrieving revision 1.40
retrieving revision 1.41
diff -w -u -1 -0 -p -r1.40 -r1.41
--- bfd.c	6 Nov 2002 13:26:27 -0000	1.40
+++ bfd.c	12 Nov 2002 15:44:24 -0000	1.41
@@ -1385,10 +1385,140 @@ bfd_alt_mach_code (abfd, alternative)
 	  return false;
 	}

       elf_elfheader (abfd)->e_machine = code;

       return true;
     }

   return false;
 }
+
+/*
+CODE_FRAGMENT
+
+.struct bfd_preserve
+.{
+.  PTR marker;
+.  PTR tdata;
+.  flagword flags;
+.  const struct bfd_arch_info *arch_info;
+.  struct sec *sections;
+.  struct sec **section_tail;
+.  unsigned int section_count;
+.  struct bfd_hash_table section_htab;
+.};
+.
+*/
+
+/*
+FUNCTION
+	bfd_preserve_save
+
+SYNOPSIS
+	boolean bfd_preserve_save (bfd *, struct bfd_preserve *);
+
+DESCRIPTION
+	When testing an object for compatibility with a particular
+	target back-end, the back-end object_p function needs to set
+	up certain fields in the bfd on successfully recognizing the
+	object.  This typically happens in a piecemeal fashion, with
+	failures possible at many points.  On failure, the bfd is
+	supposed to be restored to its initial state, which is
+	virtually impossible.  However, restoring a subset of the bfd
+	state works in practice.  This function stores the subset and
+	reinitializes the bfd.
+
+*/
+
+boolean
+bfd_preserve_save (abfd, preserve)
+     bfd *abfd;
+     struct bfd_preserve *preserve;
+{
+  preserve->tdata = abfd->tdata.any;
+  preserve->arch_info = abfd->arch_info;
+  preserve->flags = abfd->flags;
+
+  preserve->sections = abfd->sections;
+  preserve->section_tail = abfd->section_tail;
+  preserve->section_count = abfd->section_count;
+  preserve->section_htab = abfd->section_htab;
+
+  if (! bfd_hash_table_init (&abfd->section_htab, 
bfd_section_hash_newfunc))
+    return false;
+
+  abfd->tdata.any = NULL;
+  abfd->arch_info = &bfd_default_arch_struct;
+  abfd->flags = 0;
+
+  abfd->sections = NULL;
+  abfd->section_tail = &abfd->sections;
+  abfd->section_count = 0;
+
+  return true;
+}
+
+/*
+FUNCTION
+	bfd_preserve_restore
+
+SYNOPSIS
+	void bfd_preserve_restore (bfd *, struct bfd_preserve *);
+
+DESCRIPTION
+	This function restores bfd state saved by bfd_preserve_save.
+	If MARKER is non-NULL in struct bfd_preserve then that block
+	and all subsequently bfd_alloc'd memory is freed.
+
+*/
+
+void
+bfd_preserve_restore (abfd, preserve)
+     bfd *abfd;
+     struct bfd_preserve *preserve;
+{
+  bfd_hash_table_free (&abfd->section_htab);
+
+  abfd->tdata.any = preserve->tdata;
+  abfd->arch_info = preserve->arch_info;
+  abfd->flags = preserve->flags;
+
+  abfd->section_htab = preserve->section_htab;
+  abfd->sections = preserve->sections;
+  abfd->section_tail = preserve->section_tail;
+  abfd->section_count = preserve->section_count;
+
+  /* bfd_release frees all memory more recently bfd_alloc'd than
+     its arg, as well as its arg.  */
+  if (preserve->marker != NULL)
+    {
+      bfd_release (abfd, preserve->marker);
+      preserve->marker = NULL;
+    }
+}
+
+/*
+FUNCTION
+	bfd_preserve_finish
+
+SYNOPSIS
+	void bfd_preserve_finish (bfd *, struct bfd_preserve *);
+
+DESCRIPTION
+	This function should be called when the bfd state saved by
+	bfd_preserve_save is no longer needed.  ie. when the back-end
+	object_p function returns with success.
+
+*/
+
+void
+bfd_preserve_finish (abfd, preserve)
+     bfd *abfd ATTRIBUTE_UNUSED;
+     struct bfd_preserve *preserve;
+{
+  /* It would be nice to be able to free more memory here, eg. old
+     tdata, but that's not possible since these blocks are sitting
+     inside bfd_alloc'd memory.  The section hash is on a separate
+     objalloc.  */
+  bfd_hash_table_free (&preserve->section_htab);
+}
Index: elfcode.h
===================================================================
RCS file: /cvs/src/src/bfd/elfcode.h,v
retrieving revision 1.36
retrieving revision 1.37
diff -w -u -1 -0 -p -r1.36 -r1.37
--- elfcode.h	21 Sep 2002 09:59:19 -0000	1.36
+++ elfcode.h	12 Nov 2002 15:44:24 -0000	1.37
@@ -498,30 +498,20 @@ elf_swap_dyn_out (abfd, src, p)
 static INLINE boolean
 elf_file_p (x_ehdrp)
      Elf_External_Ehdr *x_ehdrp;
 {
   return ((x_ehdrp->e_ident[EI_MAG0] == ELFMAG0)
 	  && (x_ehdrp->e_ident[EI_MAG1] == ELFMAG1)
 	  && (x_ehdrp->e_ident[EI_MAG2] == ELFMAG2)
 	  && (x_ehdrp->e_ident[EI_MAG3] == ELFMAG3));
 }

-struct bfd_preserve
-{
-  const struct bfd_arch_info *arch_info;
-  struct elf_obj_tdata *tdata;
-  struct bfd_hash_table section_htab;
-  struct sec *sections;
-  struct sec **section_tail;
-  unsigned int section_count;
-};
-
 /* Check to see if the file associated with ABFD matches the target vector
    that ABFD points to.

    Note that we may be called several times with the same ABFD, but 
different
    target vectors, most of which will not match.  We have to avoid leaving
    any side effects in ABFD, or any data it points to (like tdata), if the
    file does not match the target vector.  */

 const bfd_target *
 elf_object_p (abfd)
@@ -529,25 +519,24 @@ elf_object_p (abfd)
 {
   Elf_External_Ehdr x_ehdr;	/* Elf file header, external form */
   Elf_Internal_Ehdr *i_ehdrp;	/* Elf file header, internal form */
   Elf_External_Shdr x_shdr;	/* Section header table entry, external 
form */
   Elf_Internal_Shdr i_shdr;
   Elf_Internal_Shdr *i_shdrp;	/* Section header table, internal form */
   unsigned int shindex;
   char *shstrtab;		/* Internal copy of section header stringtab */
   struct elf_backend_data *ebd;
   struct bfd_preserve preserve;
-  struct elf_obj_tdata *new_tdata = NULL;
   asection *s;
   bfd_size_type amt;

-  preserve.arch_info = abfd->arch_info;
+  preserve.marker = NULL;

   /* Read in the ELF header in external format.  */

   if (bfd_bread ((PTR) & x_ehdr, (bfd_size_type) sizeof (x_ehdr), abfd)
       != sizeof (x_ehdr))
     {
       if (bfd_get_error () != bfd_error_system_call)
 	goto got_wrong_format_error;
       else
 	goto got_no_match;
@@ -577,38 +566,28 @@ elf_object_p (abfd)
       break;
     case ELFDATANONE:		/* No data encoding specified */
     default:			/* Unknown data encoding specified */
       goto got_wrong_format_error;
     }

   /* Allocate an instance of the elf_obj_tdata structure and hook it up to
      the tdata pointer in the bfd.  */

   amt = sizeof (struct elf_obj_tdata);
-  new_tdata = (struct elf_obj_tdata *) bfd_zalloc (abfd, amt);
-  if (new_tdata == NULL)
+  preserve.marker = bfd_zalloc (abfd, amt);
+  if (preserve.marker == NULL)
     goto got_no_match;
-  preserve.tdata = elf_tdata (abfd);
-  elf_tdata (abfd) = new_tdata;
-
-  /* Clear section information, since there might be a recognized bfd that
-     we now check if we can replace, and we don't want to append to it.  */
-  preserve.sections = abfd->sections;
-  preserve.section_tail = abfd->section_tail;
-  preserve.section_count = abfd->section_count;
-  preserve.section_htab = abfd->section_htab;
-  abfd->sections = NULL;
-  abfd->section_tail = &abfd->sections;
-  abfd->section_count = 0;
-  if (!bfd_hash_table_init (&abfd->section_htab, bfd_section_hash_newfunc))
+  if (!bfd_preserve_save (abfd, &preserve))
     goto got_no_match;

+  elf_tdata (abfd) = preserve.marker;
+
   /* Now that we know the byte order, swap in the rest of the header */
   i_ehdrp = elf_elfheader (abfd);
   elf_swap_ehdr_in (abfd, &x_ehdr, i_ehdrp);
 #if DEBUG & 1
   elf_debug_file (i_ehdrp);
 #endif

   /* Reject ET_CORE (header indicates core file, not object file) */
   if (i_ehdrp->e_type == ET_CORE)
     goto got_wrong_format_error;
@@ -626,22 +605,24 @@ elf_object_p (abfd)

   /* Further sanity check.  */
   if (i_ehdrp->e_shoff == 0 && i_ehdrp->e_shnum != 0)
     goto got_wrong_format_error;

   ebd = get_elf_backend_data (abfd);

   /* Check that the ELF e_machine field matches what this particular
      BFD format expects.  */
   if (ebd->elf_machine_code != i_ehdrp->e_machine
-      && (ebd->elf_machine_alt1 == 0 || i_ehdrp->e_machine != 
ebd->elf_machine_alt1)
-      && (ebd->elf_machine_alt2 == 0 || i_ehdrp->e_machine != 
ebd->elf_machine_alt2))
+      && (ebd->elf_machine_alt1 == 0
+	  || i_ehdrp->e_machine != ebd->elf_machine_alt1)
+      && (ebd->elf_machine_alt2 == 0
+	  || i_ehdrp->e_machine != ebd->elf_machine_alt2))
     {
       const bfd_target * const *target_ptr;

       if (ebd->elf_machine_code != EM_NONE)
 	goto got_wrong_format_error;

       /* This is the generic ELF target.  Let it match any ELF target
 	 for which we do not have a specific backend.  */
       for (target_ptr = bfd_target_vector; *target_ptr != NULL; 
target_ptr++)
 	{
@@ -837,51 +818,39 @@ elf_object_p (abfd)
 	  asection *targ_sec;

 	  targ_index = elf_section_data (s)->this_hdr.sh_info;
 	  targ_sec = bfd_section_from_elf_index (abfd, targ_index);
 	  if (targ_sec != NULL
 	      && (targ_sec->flags & SEC_DEBUGGING) != 0)
 	    s->flags |= SEC_DEBUGGING;
 	}
     }

-  /* It would be nice to be able to free more memory here, eg. old
-     elf_elfsections, old tdata, but that's not possible since these
-     blocks are sitting inside obj_alloc'd memory.  */
-  bfd_hash_table_free (&preserve.section_htab);
-  return (abfd->xvec);
+  bfd_preserve_finish (abfd, &preserve);
+  return abfd->xvec;

  got_wrong_format_error:
   /* There is way too much undoing of half-known state here.  The caller,
      bfd_check_format_matches, really shouldn't iterate on live bfd's to
      check match/no-match like it does.  We have to rely on that a call to
      bfd_default_set_arch_mach with the previously known mach, undoes what
      was done by the first bfd_default_set_arch_mach (with mach 0) here.
      For this to work, only elf-data and the mach may be changed by the
      target-specific elf_backend_object_p function.  Note that saving the
      whole bfd here and restoring it would be even worse; the first thing
      you notice is that the cached bfd file position gets out of sync.  */
   bfd_set_error (bfd_error_wrong_format);

  got_no_match:
   abfd->arch_info = preserve.arch_info;
-  if (new_tdata != NULL)
-    {
-      /* bfd_release frees all memory more recently bfd_alloc'd than
-	 its arg, as well as its arg.  */
-      bfd_release (abfd, new_tdata);
-      elf_tdata (abfd) = preserve.tdata;
-      abfd->section_htab = preserve.section_htab;
-      abfd->sections = preserve.sections;
-      abfd->section_tail = preserve.section_tail;
-      abfd->section_count = preserve.section_count;
-    }
+  if (preserve.marker != NULL)
+    bfd_preserve_restore (abfd, &preserve);
   return NULL;
 }
 
 /* ELF .o/exec file writing */

 /* Write out the relocs.  */

 void
 elf_write_relocs (abfd, sec, data)
      bfd *abfd;



Jeff Baker wrote:
> Thanks.
> That was the plan if no-one happened to remember.
>
> Nick Clifton wrote:
>
>
> > Hi Jeff,
> >
> >
> >
> > > If it helps refresh anyones memory, it was fixed somewhere between 
> > > 2.13.2 and 2.14.
> >
> >
> > Sorry it does not ring any bells. :-(
> >
> >
> >
> >
> > > > We build one set of binutils (minus ld and gas, which do not show
> >
> > this 
> >
> >
> > > > problem) that target x86, but also have powerpc, mips, arm and sh 
> > > > enabled.  If you attempt to use any of the secondary targetting (arm,
> >
> >
> > > > sh, ppc, mips) utils on a static library it will very quickly exhaust
> >
> >
> > > > your system memory.  It doesn't matter in which order you specify the
> >
> >
> > > > targets.  If you configure with '--target=powerpc-nto-qnx 
> > > > --enable-targets="mips-nto-qnx sh-nto-qnx i386-nto-qnx arm-nto-qnx"',
> >
> >
> > > > then powerpc is fine and the rest show this behaviour.
> >
> >
> >
> > My best suggestion would be to use a binary-chop method to locate the 
> > patch that fixed the bug.  It is quite an involved process but I have 
> > used it successfully before.  The steps are as follows:
> >
> >   0.  Create a local copy of the CVS repository.  (The sourceware 
> > repository supports rsync).
> >
> >   1.  Create a script to do this following:
> >
> >      A.  Pick a start date, eg the 2.13.2 release date, and check out
>
> a
>
> > copy of the sources using your local archive.  It also helps if you
>
> can 
>
> > do this on a ramdisk to speed things up.
> >
> >      B.  Test this checked out copy for the bug.  Since you are 
> > concerned with running out of memory you will probably want to use 
> > something like ulimit to make sure that the build process does not
>
> bring
>
> > your machine to a halt.
> >
> >   2.  The first time through the test should fail.  Now pick a known 
> > good date, eg the 2.14 release, and repeat step 1.  The test should 
> > pass.  Now you have two dates and you can the binary chop algorithm to
>
>
> > narrow down to the exact day when a patch was checked in to fix the
> > problem.
> >
> > Cheers
> >   Nick