This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [gold commit] PR gold/14860: Fix race condition.
- From: Cary Coutant <ccoutant at google dot com>
- To: Ian Lance Taylor <iant at google dot com>
- Cc: Binutils <binutils at sourceware dot org>
- Date: Thu, 14 Nov 2013 13:12:07 -0800
- Subject: Re: [gold commit] PR gold/14860: Fix race condition.
- Authentication-results: sourceware.org; auth=none
- References: <20131114184538 dot C96BC1121182 at ccoutant-macbookpro2 dot roam dot corp dot google dot com> <CAKOQZ8wHoqz+txW=_8giVJ6skfuOfva7AGORHSunzAYBGV7uCw at mail dot gmail dot com>
>> This patch fixes a race condition in Eh_frame_hdr::record_fde. When
>> running multi-threaded, many Write_sections tasks may call record_fde_,
>> which pushes an entry onto a shared vector. We need to hold a lock
>> while modifying the vector.
>
> I don't see how this could happen. I think there is only one Eh_frame
> section, created in Layout::make_eh_frame_section. And I think the
> only place that an FDE is written out is when the Eh_frame section is
> written out, and I think that only happens once. What am I missing?
Ugh, you're right. I was thinking Fde::write was called while writing
out input sections, but indeed the .eh_frame sections are all handled
as part of the one output section, in Write_sections_task.
>> + Hold_lock(*this->lock_);
>
> I don't think this works. I think you need to write
> Hold_lock hl(*this->lock_);
Ugh, again. I have no idea what I was doing when I wrote that. I was
copying an idiom used in several other places, so I should have gotten
that right.
Anyway, it seems there is no real race condition here, so the drop in
frequency of asserts must have been an anomaly (or the patch caused a
sufficient perturbation to hide it better). I'll revert this patch.
Sorry!
Back to the drawing board...
-cary