This is the mail archive of the
mailing list for the binutils project.
Re: [PATCH] Call _close_and_cleanup in bfd_close_all_done
- From: Alan Modra <amodra at gmail dot com>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>
- Cc: Binutils <binutils at sourceware dot org>
- Date: Thu, 31 Aug 2017 08:49:21 +0930
- Subject: Re: [PATCH] Call _close_and_cleanup in bfd_close_all_done
- Authentication-results: sourceware.org; auth=none
- References: <20170829190531.GA2862@intel.com> <20170830064928.GI3368@bubble.grove.modra.org> <CAMe9rOqhzx9Sn6qB_W5vWA9hfUnsZbppvf=VpMzb8MujEXAFHA@mail.gmail.com>
On Wed, Aug 30, 2017 at 05:46:14AM -0700, H.J. Lu wrote:
> On Tue, Aug 29, 2017 at 11:49 PM, Alan Modra <email@example.com> wrote:
> > On Tue, Aug 29, 2017 at 12:05:31PM -0700, H.J. Lu wrote:
> >> OK for master?
> >> H.J.
> >> ---
> >> PR binutils/22032
> >> * opncls.c (bfd_close_all_done): Call _close_and_cleanup.
> > Please have a look at archive.c:_bfd_archive_close_and_cleanup and
> > convince yourself that this change is safe. archive_close_worker
> > calls bfd_close_all_done. I think it is OK, but I'd like to make sure
> > you have put eyes on that code too.
> Since an archive member can never be the archive which it is in,
> it is safe.
Yeah, I thought it was good too, but --enable-targets=all objdump -i
now dies with a segfault. HJ, testing only x86 is just not good
The problem appears to be due to elf64_vms_close_and_cleanup calling
bfd_get_size, which calls iovec->bstat. cache_bstat ends up adding
the bfd to the cache lru list, negating the bfd_cache_close call in
bfd_close_all_done. So there is a dangling pointer into the freed and
then reused bfd.
I'm testing the following fix.
* opncls.c (bfd_close_all_done): Don't call bfd_cache_close
before _close_and_cleanup. Call iovec->bclose after.
(bfd_close): Remove code common to, and call bfd_close_all_done.
diff --git a/bfd/opncls.c b/bfd/opncls.c
index b99ae72..fa54986 100644
@@ -726,25 +726,13 @@ RETURNS
bfd_close (bfd *abfd)
- bfd_boolean ret;
if (bfd_write_p (abfd))
if (! BFD_SEND_FMT (abfd, _bfd_write_contents, (abfd)))
- if (! BFD_SEND (abfd, _close_and_cleanup, (abfd)))
- return FALSE;
- ret = abfd->iovec->bclose (abfd) == 0;
- if (ret)
- _maybe_make_executable (abfd);
- _bfd_delete_bfd (abfd);
- return ret;
+ return bfd_close_all_done (abfd);
@@ -774,11 +762,11 @@ bfd_close_all_done (bfd *abfd)
- ret = bfd_cache_close (abfd);
if (! BFD_SEND (abfd, _close_and_cleanup, (abfd)))
+ ret = abfd->iovec->bclose (abfd) == 0;
Australia Development Lab, IBM