Multi-threading and _bfd_section_id

[Alan Modra]

On Sun, Oct 27, 2024 at 11:13:58AM -0600, Tom Tromey wrote:
> I found out today about the _bfd_section_id global and how its use
> introduces data races when trying to use BFD from multiple threads.
> This was discovered in
> 
>     https://sourceware.org/bugzilla/show_bug.cgi?id=31713
> 
> _bfd_section_id seems to primarily be used to assign a globally unique
> ID to each section.  This by itself is no big deal -- fixing the races
> here could be done either by acquiring the existing BFD global lock when
> using the global, or by making it atomic.  (I did only find one use of
> section_id in the tree, so removing it seems tempting, but I don't know
> how difficult that would be.)
> 
> 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.

Wasting a few ids isn't the issue.  The problem is that bfd ld uses
section id on a number of targets to create symbols, and then does
hash table traversals over those symbols (for eg. stub layout).
Changing the id thus changes the symbols and the layout.  It's bad
enough for the testsuite when section id changes for a target due to
that target itself creating extra sections, but not saving the section
id when trying target matches may result in reproducability problems
depending on the total set of targets supported by a binutils build.
For example, you might find the linker produces different output for
aarch64-linux with the default on an aarch64-linux host
vs. configuring on that same host with --enable-targets=all.

I was going to say you could probably ignore section->id for gdb, but
I see at least x86 and aarch64 use it in a local sym hash, which will
be used by their respective relocate_section functions.

> But, when looking  into this, I found this code in elf64-ppc.c:
> 
>     int
>     ppc64_elf_setup_section_lists (struct bfd_link_info *info)
>     {
>       unsigned int id;
>       size_t amt;
>       struct ppc_link_hash_table *htab = ppc_hash_table (info);
> 
>       if (htab == NULL)
>         return -1;
> 
>       htab->sec_info_arr_size = _bfd_section_id;
> 
> ... and I was wondering whether having holes in the section ID space
> would cause problems here.
> 
> I'd appreciate some advice here, as I don't know what this code is for.
> Like, would there be some other way to count the number of sections?  Or
> would having unused slots be harmless?

Unused slots are harmless here, and this function will only by used by
ld.  Yes, you could iterate over all linker bfds and their sections to
find the max section id.

> thanks,
> Tom

-- 
Alan Modra