Re: [PATCH v2 2/2] [gdb/symtab] Fix data race in try_open_dwop_file

[Tom de Vries <tdevries@suse.de>]

On 3/4/26 9:29 PM, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> The call to bfd_check_format in try_open_dwop_file:
> Tom> ...
> Tom>     /* The operations below are not thread-safe, use a lock to synchronize
> Tom>        concurrent accesses.  */
> Tom>     static gdb::mutex mutex;
> Tom>     gdb::lock_guard<gdb::mutex> lock (mutex);
> 
> Tom>     if (!bfd_check_format (sym_bfd.get (), bfd_object))
> Tom>       return NULL;
> Tom> ...
> Tom> accesses the sym_bfd.get () BFD, so it should be guarded by the global BFD
> Tom> lock.
> 
> Doesn't bfd_check_format do this itself?

Hi Tom,

thanks for the review, and sorry for the delay in replying, I seem to 
have dropped the ball on this.

AFAIU, bfd_check_format does locking, but it's only used to [1]:
...
lock the internal global data structures of BFD (like its global cache 
or some global counter), but not data in individual `bfd *`instances.  ...

So while it does do locking, it's not sufficient.

> Otherwise, don't we need to wrap all calls to bfd_check_format?
> 

Agreed, and likewise for bfd_check_format_matches.

I've submitted a v3 that adds this ( 
https://sourceware.org/pipermail/gdb-patches/2026-May/227652.html ).

Thanks,
- Tom

> Tom> The local lock also guarded a call to gdb_bfd_record_inclusion, which doesn't
> Tom> do any locking, so likewise use the global BFD lock in
> Tom> gdb_bfd_record_inclusion.
> 
> This part seems unobjectionable to me.
> 
> Tom

[1] https://sourceware.org/pipermail/gdb-patches/2026-January/224291.html