Multi-threading and _bfd_section_id

[Alan Modra]

On Mon, Oct 28, 2024 at 05:13:20PM -0600, Tom Tromey wrote:
> >> However, bfd_preserve_save and friends save and restore the global.  My
> >> inclination would be to just remove this code, since it doesn't seem
> >> like a big deal to waste a few IDs.
> 
> Alan> Wasting a few ids isn't the issue.  The problem is that bfd ld uses
> Alan> section id on a number of targets to create symbols, and then does
> Alan> hash table traversals over those symbols (for eg. stub layout).
> 
> Ok, I see.  I missed that section::id and section::section_id are both
> set from _bfd_section_id but used differently.
> 
> Alan> I was going to say you could probably ignore section->id for gdb, but
> Alan> I see at least x86 and aarch64 use it in a local sym hash, which will
> Alan> be used by their respective relocate_section functions.
> 
> It's specifically the way the global is managed that is a problem for
> gdb.  So one (I guess ugly) option would be to only apply the
> bfd_preserve_save global-resetting logic when BFD knows it is
> single-threaded.  That is, allow gaps in the numbering when called from
> gdb.

Yes, and make _bfd_section_id atomic.

> Another option would be for BFD to acquire the BFD lock around
> bfd_check_format_matches.  This would ensure that all such uses are
> serialized.  However, it seems unfortunate to require this.  Also it
> seems to me that locking in bfd_check_format_matches would require the
> BFD lock to be a recursive lock.

Right, because check_format_matches calls functions that acquire the
lock.

>  This isn't a problem for gdb, though
> (in fact it already uses one for its own reasons).
> 
> I'm happy to implement whichever one you think is best.

You're not getting away with that one!  The choice is yours.  ;-)

> thanks,
> Tom

-- 
Alan Modra