Re: [PATCH 5/5] [gdb/symtab] Fix data race on per_cu->lang

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

On 7/4/22 20:30, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> Both writes are called from the parallel for in dwarf2_build_psymtabs_hard,
> Tom> this one directly:
> Tom> ...
> Tom>     #1 process_psymtab_comp_unit gdb/dwarf2/read.c:6812 (gdb+0x830912)
> Tom>     #2 operator() gdb/dwarf2/read.c:7102 (gdb+0x831902)
> Tom>     #3 operator() gdb/../gdbsupport/parallel-for.h:171 (gdb+0x8723a8)
> Tom> ...
> Tom> and this one when handling cross-CU refs:
> Tom> ...
> Tom>     #1 cooked_indexer::ensure_cu_exists(cutu_reader*, dwarf2_per_objfile*, \
> Tom>     sect_offset, bool, bool) gdb/dwarf2/read.c:17973 (gdb+0x85c522)
> 
> This method tries to ensure that a CU isn't processed twice, using the
> 'scanned' field.  Do you know why this isn't working?
> 

Yes, because for_scanning == false, so this:
...
   /* When scanning, we only want to visit a given CU a single time. 

      Doing this check here avoids self-imports as well.  */
   if (for_scanning)
     {
       bool nope = false;
       if (!per_cu->scanned.compare_exchange_strong (nope, true))
         return nullptr;
     }
...
is not actived.

A quick experiment of setting it to true at the call site:
...
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 23fe5679cbd..f85263564cb 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -18143,7 +18143,7 @@ cooked_indexer::scan_attributes 
(dwarf2_per_cu_data *scanning_
per_cu,
      {
        cutu_reader *new_reader
         = ensure_cu_exists (reader, reader->cu->per_objfile, origin_offset,
-                           origin_is_dwz, false);
+                           origin_is_dwz, true);
        if (new_reader != nullptr)
         {
           const gdb_byte *new_info_ptr = (new_reader->buffer
...
makes thread sanitizer stop complaining, but also makes the test-case fail:
...
FAIL: gdb.dwarf2/inlined_subroutine-inheritance.exp: gdb_breakpoint: set 
breakpoint at bytes_repeat
...

Thanks,
- Tom

> Tom> Fix this by guarding the write with a lock.
> 
> I would rather we avoid locks.  Ideally the existing exclusion mechanism
> should be made to work, but if that fails, perhaps we can use another
> atomic.
> 
> Tom