Bug 30261 - [gdb] segfaults in gdb.base/index-cache.exp
Summary: [gdb] segfaults in gdb.base/index-cache.exp
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: symtab (show other bugs)
Version: HEAD
: P2 normal
Target Milestone: 14.1
Assignee: Tom Tromey
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-03-22 12:21 UTC by Tom de Vries
Modified: 2023-03-31 14:55 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
gdb.log with thread sanitizer (12.99 KB, text/x-log)
2023-03-23 15:35 UTC, Tom de Vries
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tom de Vries 2023-03-22 12:21:15 UTC
With test-case gdb.base/index-cache.exp and target board native-extended-gdbserver I run into:
...
Reading symbols from /data/vries/gdb/leap-15-4/build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache...^M
^M
^M
Fatal signal: Segmentation fault^M
  ...
0x479798 _ZN4type10set_fieldsEP5field^M
        /data/vries/gdb/src/gdb/gdbtypes.h:975^M
...

Not 100% reproducible, also can happen at a different location.
Comment 1 Tom de Vries 2023-03-22 12:29:18 UTC
After clean rebuild, reproduces as a hang instead.
Comment 2 Tom de Vries 2023-03-22 12:35:14 UTC
Now, again as a segfault, but different location:
...
(gdb) maintenance wait-for-index-cache^M
0x595ac6 gdb_internal_backtrace_1^M
        /data/vries/gdb/src/gdb/bt-utils.c:122^M
  ...
0x41feea _ZNK4type4codeEv^M
        /data/vries/gdb/src/gdb/gdbtypes.h:912^M
...
Comment 3 Tom de Vries 2023-03-22 12:52:30 UTC
Hmm, the test already doesn't work with "maint set worker-threads 0" on native.
Comment 4 Tom de Vries 2023-03-22 13:54:26 UTC
(In reply to Tom de Vries from comment #3)
> Hmm, the test already doesn't work with "maint set worker-threads 0" on
> native.

Likewise in test-case gdb.dwarf2/per-bfd-sharing.exp, there we run into:
...
FAIL: gdb.dwarf2/per-bfd-sharing.exp: couldn't remove files in temporary cache dir
...
Comment 5 Tom de Vries 2023-03-23 15:19:56 UTC
At commit f0c3dcc1ca7 ("Ensure index cache entry written in test"):
...
$ for n in $(seq 1 10); do ./test.sh -native-extended-gdbserver | grep '^#' | sort -u; done
# of expected passes            23
# of unresolved testcases       1
# of expected passes            24
# of expected passes            24
# of expected passes            24
# of expected passes            24
# of expected passes            22
# of unexpected core files      1
# of unresolved testcases       2
# of expected passes            24
# of expected passes            24
# of expected passes            24
# of expected passes            24
...

At commit 52e5e48e533 ("Write the DWARF index in the background"):
...
$ for n in $(seq 1 10); do ./test.sh -native-extended-gdbserver | grep '^#' | sort -u; done
# of expected passes            19
# of expected passes            19
# of expected passes            19
# of expected passes            17
# of unexpected core files      1
# of unexpected failures        3
# of expected passes            17
# of unexpected core files      1
# of unexpected failures        2
# of unresolved testcases       1
# of expected passes            16
# of unexpected core files      1
# of unexpected failures        4
# of expected passes            19
# of expected passes            17
# of unexpected core files      1
# of unresolved testcases       2
# of expected passes            17
# of unexpected core files      1
# of unexpected failures        3
# of expected passes            19
...

At commit 52e5e48e533^:
...
$ for n in $(seq 1 10); do ./test.sh -native-extended-gdbserver | grep '^#' | sort -u; done
# of expected passes            19
# of expected passes            19
# of expected passes            19
# of expected passes            19
# of expected passes            19
# of expected passes            19
# of expected passes            19
# of expected passes            19
# of expected passes            19
# of expected passes            19
...

So, this looks like a regression by commit 52e5e48e533 ("Write the DWARF index in the background") that hasn't been fixed by commit f0c3dcc1ca7 ("Ensure index cache entry written in test").
Comment 6 Tom de Vries 2023-03-23 15:35:26 UTC
Created attachment 14775 [details]
gdb.log with thread sanitizer

$ grep ThreadS gdb.log 
WARNING: ThreadSanitizer: data race (pid=22270)
SUMMARY: ThreadSanitizer: data race /data/vries/gdb/src/gdb/dwarf2/index-cache.c:76 in index_cache::enable()
WARNING: ThreadSanitizer: data race (pid=22359)
SUMMARY: ThreadSanitizer: data race /usr/include/c++/7/bits/unique_ptr.h:147 in std::__uniq_ptr_impl<dwarf_scanner_base, std::default_delete<dwarf_scanner_base> >::_M_ptr() const
WARNING: ThreadSanitizer: data race (pid=22359)
SUMMARY: ThreadSanitizer: data race /data/vries/gdb/src/gdb/dwarf2/index-write.c:1229 in write_gdbindex
WARNING: ThreadSanitizer: data race (pid=22404)
SUMMARY: ThreadSanitizer: data race /data/vries/gdb/src/bfd/cache.c:585 in bfd_open_file
WARNING: ThreadSanitizer: data race (pid=22404)
SUMMARY: ThreadSanitizer: data race /usr/include/c++/7/bits/unique_ptr.h:147 in std::__uniq_ptr_impl<dwarf_scanner_base, std::default_delete<dwarf_scanner_base> >::_M_ptr() const
WARNING: ThreadSanitizer: data race (pid=22404)
SUMMARY: ThreadSanitizer: data race /data/vries/gdb/src/gdb/dwarf2/index-write.c:1229 in write_gdbindex
...
Comment 7 Tom Tromey 2023-03-23 19:22:10 UTC
I'll investigate.
Comment 8 Tom Tromey 2023-03-24 20:57:15 UTC
Ok, I found the problem.
This would also be fixed by my WIP to move more reading to the background,
but unfortunately that series isn't quite ready yet.
Comment 10 Sourceware Commits 2023-03-31 14:54:52 UTC
The master branch has been updated by Tom Tromey <tromey@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=6f214d0f399847b13f979651c3b46befcbb42140

commit 6f214d0f399847b13f979651c3b46befcbb42140
Author: Tom Tromey <tom@tromey.com>
Date:   Fri Mar 24 15:53:22 2023 -0600

    Fix race in background index-cache writing
    
    Tom de Vries pointed out a bug in the index-cache background writer --
    sometimes it will fail.  He also noted that it fails when the number
    of worker threads is set to zero.  These turn out to be the same
    problem -- the cache can't be written to until the per-BFD's
    "index_table" member is set.
    
    This patch avoids the race by rearranging the code slightly, to ensure
    the cache cannot possibly be written before the member is set.
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30261
Comment 11 Tom Tromey 2023-03-31 14:55:33 UTC
Fixed.