Re: [PATCH 3/5] remove deleted BFDs from the archive cache

[Tom Tromey <tromey@redhat.com>]

HJ> Your checkin destroys binutils:
HJ> http://sourceware.org/bugzilla/show_bug.cgi?id=14475
HJ> Can you fix it?

Sorry about that.  I see now that I will need a more clever plan to
destroy binutils.

I debugged it.  The immediate problem is that
_bfd_compute_and_write_armap calls bfd_free_cached_info
(aka _bfd_free_cached_info).  This then does:

      objalloc_free ((struct objalloc *) abfd->memory);

Whoops, this also frees the areltdata.

So, I have two possible fixes for it.

I've appended the first possible fix.  It changes _bfd_free_cached_info
not to free all the memory attached to the BFD.

A couple notes here.

First, it seems very wrong to me to clear usrdata in this function.  I
didn't touch this; since presumably clients may be relying on this
clearing in a subtle way (if they allocate the usrdata on the BFD
objalloc, which is perhaps the only sensible approach anyhow).  But, I
think that if the appended patch goes in then this line should be
removed in a follow-up.

Second, the check in _bfd_delete_bfd is perhaps ugly.  Maybe
bfd_hash_table_free should do this check instead.  Let me know what you
think.

I rebuilt ld and binutils with this patch.  Additionally, I hacked the
Makefiles to link all the programs with -lmcheck.  Then I ran the ld and
binutils test suites.  There were no regressions.  I also examined one
particular case from ar.exp using valgrind -- I could reproduce the
problem before the patch, but not after.


Another possible fix for this bug would be to allocate the areltdata
using malloc.  That way it would be immune to the objalloc_free call.
This would require a few more tweaks, like properly freeing it in
_bfd_delete_bfd, etc.

I'm happy to make and test this change if you think it would be better.

Tom

2012-08-16  Tom Tromey  <tromey@redhat.com>

	* opncls.c (_bfd_delete_bfd): Check to see if section htab is
	already freed.
	(_bfd_free_cached_info): Don't free the objalloc.

Index: opncls.c
===================================================================
RCS file: /cvs/src/src/bfd/opncls.c,v
retrieving revision 1.72
diff -u -r1.72 opncls.c
--- opncls.c	9 Aug 2012 06:25:53 -0000	1.72
+++ opncls.c	16 Aug 2012 16:57:36 -0000
@@ -132,14 +132,15 @@
 {
   if (abfd->memory)
     {
-      bfd_hash_table_free (&abfd->section_htab);
+      if (abfd->section_htab.memory != NULL)
+	bfd_hash_table_free (&abfd->section_htab);
       objalloc_free ((struct objalloc *) abfd->memory);
     }
 
   free (abfd);
 }
 
-/* Free objalloc memory.  */
+/* Free some information cached in the BFD.  */
 
 bfd_boolean
 _bfd_free_cached_info (bfd *abfd)
@@ -147,14 +148,12 @@
   if (abfd->memory)
     {
       bfd_hash_table_free (&abfd->section_htab);
-      objalloc_free ((struct objalloc *) abfd->memory);
 
       abfd->sections = NULL;
       abfd->section_last = NULL;
       abfd->outsymbols = NULL;
       abfd->tdata.any = NULL;
       abfd->usrdata = NULL;
-      abfd->memory = NULL;
     }
 
   return TRUE;