Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disassembler: Rounding mode on widening instructions #2

Closed
wants to merge 2 commits into from

Conversation

a4lg
Copy link
Owner

@a4lg a4lg commented Jul 2, 2022

a4lg added 2 commits July 2, 2022 08:39
This commit reorganizes and adds testcases to invalid rounding mode
operand test.  It also fixes a typo in the filename.

gas/ChangeLog:

	* testsuite/gas/riscv/rounding-fail-invalid.d: Rename from
	rouding-fail. Add some testcases.
	* testsuite/gas/riscv/rounding-fail-invalid.s: Likewise.
	* testsuite/gas/riscv/rounding-fail-invalid.l: Likewise.
This commit adds support for rounding modes on widening instructions to
the assembler/disassembler.  For now, optional rounding mode is
displayed when `no-aliases' option is given to the disassembler and
specifying such rounding modes in assembly is prohibited unless we have
supported in the past.

gas/ChangeLog:

	* config/tc-riscv.c (validate_riscv_insn): Add rounding mode
	support to widening instructions.
	(riscv_ip): Likewise.
	* testsuite/gas/riscv/rounding-dis-widening.d: New test.
	* testsuite/gas/riscv/rounding-dis-widening.s: Likewise.
	* testsuite/gas/riscv/rounding-dis-widening-noalias.d: Likewise.
	* testsuite/gas/riscv/rounding-fail-widening.d: New test.
	* testsuite/gas/riscv/rounding-fail-widening.l: Likewise.
	* testsuite/gas/riscv/rounding-fail-widening.s: Likewise.
	* testsuite/gas/riscv/rounding-fcvt.q.l.d: New test.
	* testsuite/gas/riscv/rounding-fcvt.q.l.l: Likewise.
	* testsuite/gas/riscv/rounding-fcvt.q.l.s: Likewise.
	* testsuite/gas/riscv/rounding-fcvt.q.l-noalias.d: Likewise.

opcodes/ChangeLog:

	* riscv-dis.c (print_insn_args): Add rounding mode support to
	widening instructions.
	* riscv-opc.c (riscv_opcodes): Use new operand types.

Signed-off-by: Tsukasa OI <[email protected]>
Idea-by: S Pawan Kumar <[email protected]>
a4lg pushed a commit that referenced this pull request Jul 2, 2022
When building gdb with -fsanitize=thread and gcc 12, and running test-case
gdb.dwarf2/dwz.exp, we run into a data race between thread T2 and the main
thread in the same write:
...
Write of size 1 at 0x7b200000300c:^M
    #0 cutu_reader::cutu_reader(dwarf2_per_cu_data*, dwarf2_per_objfile*, \
    abbrev_table*, dwarf2_cu*, bool, abbrev_cache*) gdb/dwarf2/read.c:6252 \
    (gdb+0x82f3b3)^M
...
which is here:
...
         this_cu->dwarf_version = cu->header.version;
...

Both writes are called from the parallel for in dwarf2_build_psymtabs_hard,
this one directly:
...
    #1 process_psymtab_comp_unit gdb/dwarf2/read.c:6774 (gdb+0x8304d7)^M
    #2 operator() gdb/dwarf2/read.c:7098 (gdb+0x8317be)^M
    #3 operator() gdbsupport/parallel-for.h:163 (gdb+0x872380)^M
...
and this via the PU import:
...
    #1 cooked_indexer::ensure_cu_exists(cutu_reader*, dwarf2_per_objfile*, \
    sect_offset, bool,  bool) gdb/dwarf2/read.c:17964 (gdb+0x85c43b)^M
    #2 cooked_indexer::index_imported_unit(cutu_reader*, unsigned char const*, \
    abbrev_info const*) gdb/dwarf2/read.c:18248 (gdb+0x85d8ff)^M
    #3 cooked_indexer::index_dies(cutu_reader*, unsigned char const*, \
    cooked_index_entry const*, bool) gdb/dwarf2/read.c:18302 (gdb+0x85dcdb)^M
    #4 cooked_indexer::make_index(cutu_reader*) gdb/dwarf2/read.c:18443 \
    (gdb+0x85e68a)^M
    #5 process_psymtab_comp_unit gdb/dwarf2/read.c:6812 (gdb+0x830879)^M
    #6 operator() gdb/dwarf2/read.c:7098 (gdb+0x8317be)^M
    #7 operator() gdbsupport/parallel-for.h:171 (gdb+0x8723e2)^M
...

Fix this by setting the field earlier, in read_comp_units_from_section.

The write in cutu_reader::cutu_reader() is still needed, in case
read_comp_units_from_section is not used (run the test-case with say, target
board cc-with-gdb-index).

Make the write conditional, such that it doesn't trigger if the field is
already set by read_comp_units_from_section.  Instead, verify that the
field already has the value that we're trying to set it to.

Move this logic into into a member function set_version (in analogy to the
already present member function version) to make sure it's used consistenly,
and make the field private in order to enforce access through the member
functions, and rename it to m_dwarf_version.

While we're at it, make sure that the version is set before read, to avoid
say returning true for "per_cu.version () < 5" if "per_cu.version () == 0".

Tested on x86_64-linux.
@a4lg a4lg closed this Jul 7, 2022
@a4lg a4lg deleted the riscv-dis-float-rounding-widening branch July 7, 2022 04:02
a4lg pushed a commit that referenced this pull request Jul 13, 2022
After loading a core file, you're supposed to be able to use "detach"
to unload the core file.  That unfortunately regressed starting with
GDB 11, with these commits:

 1192f12 - gdb: generalize commit_resume, avoid commit-resuming when threads have pending statuses
 408f668 - detach in all-stop with threads running

resulting in a GDB crash:

 ...
 Thread 1 "gdb" received signal SIGSEGV, Segmentation fault.
 0x0000555555e842bf in maybe_set_commit_resumed_all_targets () at ../../src/gdb/infrun.c:2899
 2899          if (proc_target->commit_resumed_state)
 (top-gdb) bt
 #0  0x0000555555e842bf in maybe_set_commit_resumed_all_targets () at ../../src/gdb/infrun.c:2899
 #1  0x0000555555e848bf in scoped_disable_commit_resumed::reset (this=0x7fffffffd440) at ../../src/gdb/infrun.c:3023
 #2  0x0000555555e84a0c in scoped_disable_commit_resumed::reset_and_commit (this=0x7fffffffd440) at ../../src/gdb/infrun.c:3049
 #3  0x0000555555e739cd in detach_command (args=0x0, from_tty=1) at ../../src/gdb/infcmd.c:2791
 #4  0x0000555555c0ba46 in do_simple_func (args=0x0, from_tty=1, c=0x55555662a600) at ../../src/gdb/cli/cli-decode.c:95
 #5  0x0000555555c112b0 in cmd_func (cmd=0x55555662a600, args=0x0, from_tty=1) at ../../src/gdb/cli/cli-decode.c:2514
 #6  0x0000555556173b1f in execute_command (p=0x5555565c5916 "", from_tty=1) at ../../src/gdb/top.c:699

The code that crashes looks like:

 static void
 maybe_set_commit_resumed_all_targets ()
 {
   scoped_restore_current_thread restore_thread;

   for (inferior *inf : all_non_exited_inferiors ())
     {
       process_stratum_target *proc_target = inf->process_target ();

       if (proc_target->commit_resumed_state)
           ^^^^^^^^^^^

With 'proc_target' above being null.  all_non_exited_inferiors filters
out inferiors that have pid==0.  We get here at the end of
detach_command, after core_target::detach has already run, at which
point the inferior _should_ have pid==0 and no process target.  It is
clear it no longer has a process target, but, it still has a pid!=0
somehow.

The reason the inferior still has pid!=0, is that core_target::detach
just unpushes, and relies on core_target::close to actually do the
getting rid of the core and exiting the inferior.  The problem with
that is that detach_command grabs an extra strong reference to the
process stratum target, so the unpush_target inside
core_target::detach doesn't actually result in a call to
core_target::close.

Fix this my moving the cleaning up the core inferior to a shared
routine called by both core_target::close and core_target::detach.  We
still need to cleanup the inferior from within core_file::close
because there are paths to it that want to get rid of the core without
going through detach.  E.g., "core-file" -> "run".

This commit includes a new test added to gdb.base/corefile.exp to
cover the "core-file core" -> "detach" scenario.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29275

Change-Id: Ic42bdd03182166b19f598428b0dbc2ce6f67c893
a4lg pushed a commit that referenced this pull request Jul 14, 2022
When doing:
...
$ gdb ./outputs/gdb.ada/char_enum_unicode/foo -batch -ex "break foo.adb:26"
...
with a gdb build with -fsanitize=thread I run into a data race:
...
WARNING: ThreadSanitizer: data race (pid=30917)
  Write of size 8 at 0x7b0400004070 by main thread:
    #0 free <null> (libtsan.so.2+0x4c5e2)
    #1 xfree<char> gdbsupport/gdb-xfree.h:37 (gdb+0x650f17)
    #2 charset_vector::clear() gdb/charset.c:703 (gdb+0x651354)
    #3 charset_vector::~charset_vector() gdb/charset.c:697 (gdb+0x6512d3)
    #4 <null> <null> (libtsan.so.2+0x32643)
    #5 captured_main_1 gdb/main.c:1310 (gdb+0xa3975a)
...

The problem is that we're freeing the charset_vector elements in the destructor,
which may still be used by a worker thread.

Fix this by not freeing the charset_vector elements in the destructor.

Tested on x86_64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29311
a4lg pushed a commit that referenced this pull request Jul 15, 2022
…matching

When building gdb with -fsanitize-threads and running test-case
gdb.ada/char_enum_unicode.exp, I run into:
...
WARNING: ThreadSanitizer: data race (pid=21301)^M
  Write of size 8 at 0x7b2000008080 by main thread:^M
    #0 free <null> (libtsan.so.2+0x4c5e2)^M
    #1 _dl_close_worker <null> (ld-linux-x86-64.so.2+0x4b7b)^M
    #2 convert_between_encodings() charset.c:584^M
  ...
    #21 cooked_index_functions::expand_symtabs_matching() read.c:18606
...

This is fixed by making cooked_index_functions::expand_symtabs_matching wait
for the cooked index finalization to be done.

Tested on x86_64-linux.

https://sourceware.org/bugzilla/show_bug.cgi?id=29311
https://sourceware.org/bugzilla/show_bug.cgi?id=29286
a4lg pushed a commit that referenced this pull request Jul 26, 2022
When adding libopcodes disassembler styling support for AArch64, it
feels like the results would be improved by having a new sub-mnemonic
style.  This will be used in cases like:

  add    w16, w7, w1, uxtb #2
                      ^^^^----- Here

And:

  cinc   w0, w1, ne
                 ^^----- Here

This commit just adds the new style, and prepares objdump to handle
the style.  A later commit will add AArch64 styling, and will actually
make use of the style.

As this style is currently unused, there should be no user visible
changes after this commit.
a4lg pushed a commit that referenced this pull request Aug 20, 2022
Bug 29374 shows this crash:

    $ ./gdb -nx --data-directory=data-directory -q -batch -ex "catch throw" -ex r -ex bt a.out
    ...
    /home/simark/src/binutils-gdb/gdb/../gdbsupport/array-view.h:217: internal-error: copy: Assertion `dest.size () == src.size ()' failed.

The backtrace is:

    #0  internal_error (file=0x5555606504c0 "/home/simark/src/binutils-gdb/gdb/../gdbsupport/array-view.h", line=217, fmt=0x55556064b700 "%s: Assertion `%s' failed.") at /home/simark/src/binutils-gdb/gdbsupport/errors.cc:51
    #1  0x000055555d41c0bb in gdb::copy<unsigned char const, unsigned char> (src=..., dest=...) at /home/simark/src/binutils-gdb/gdb/../gdbsupport/array-view.h:217
    #2  0x000055555deef28c in dwarf_expr_context::fetch_result (this=0x7fffffffb830, type=0x621007a86830, subobj_type=0x621007a86830, subobj_offset=0, as_lval=false) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1040
    #3  0x000055555def0015 in dwarf_expr_context::evaluate (this=0x7fffffffb830, addr=0x62f00004313e "0", len=1, as_lval=false, per_cu=0x60b000069550, frame=0x621007c9e910, addr_info=0x0, type=0x621007a86830, subobj_type=0x621007a86830, subobj_offset=0) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1091
    #4  0x000055555e084327 in dwarf2_evaluate_loc_desc_full (type=0x621007a86830, frame=0x621007c9e910, data=0x62f00004313e "0", size=1, per_cu=0x60b000069550, per_objfile=0x613000006080, subobj_type=0x621007a86830, subobj_byte_offset=0, as_lval=false) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1485
    #5  0x000055555e0849e2 in dwarf2_evaluate_loc_desc (type=0x621007a86830, frame=0x621007c9e910, data=0x62f00004313e "0", size=1, per_cu=0x60b000069550, per_objfile=0x613000006080, as_lval=false) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1529
    #6  0x000055555e0828c6 in dwarf_entry_parameter_to_value (parameter=0x621007a96e58, deref_size=0x0, type=0x621007a86830, caller_frame=0x621007c9e910, per_cu=0x60b000069550, per_objfile=0x613000006080) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1235
    #7  0x000055555e082f55 in value_of_dwarf_reg_entry (type=0x621007a86890, frame=0x621007acc510, kind=CALL_SITE_PARAMETER_DWARF_REG, kind_u=...) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1332
    #8  0x000055555e083449 in value_of_dwarf_block_entry (type=0x621007a86890, frame=0x621007acc510, block=0x61e000033568 "T\004\205\001\240\004\004\243\001T\237\004\240\004\261\004\001T\004\261\004\304\005\004\243\001T\237\004\304\005\310\005\001T\004\310\005\311\005\004\243\001T\237", block_len=1) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1365
    #9  0x000055555e094d40 in loclist_read_variable_at_entry (symbol=0x621007a99bd0, frame=0x621007acc510) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:3889
    #10 0x000055555f5192e0 in read_frame_arg (fp_opts=..., sym=0x621007a99bd0, frame=0x621007acc510, argp=0x7fffffffbf20, entryargp=0x7fffffffbf60) at /home/simark/src/binutils-gdb/gdb/stack.c:559
    #11 0x000055555f51c352 in print_frame_args (fp_opts=..., func=0x621007a99ad0, frame=0x621007acc510, num=-1, stream=0x6030000bad90) at /home/simark/src/binutils-gdb/gdb/stack.c:887
    #12 0x000055555f521919 in print_frame (fp_opts=..., frame=0x621007acc510, print_level=1, print_what=LOCATION, print_args=1, sal=...) at /home/simark/src/binutils-gdb/gdb/stack.c:1390
    #13 0x000055555f51f22e in print_frame_info (fp_opts=..., frame=0x621007acc510, print_level=1, print_what=LOCATION, print_args=1, set_current_sal=0) at /home/simark/src/binutils-gdb/gdb/stack.c:1116
    #14 0x000055555f526c6d in backtrace_command_1 (fp_opts=..., bt_opts=..., count_exp=0x0, from_tty=0) at /home/simark/src/binutils-gdb/gdb/stack.c:2079
    #15 0x000055555f527ae5 in backtrace_command (arg=0x0, from_tty=0) at /home/simark/src/binutils-gdb/gdb/stack.c:2198

The problem is that the type that gets passed down to
dwarf_expr_context::fetch_result (the type of a variable of which we're
trying to read the entry value) is a typedef whose size has never been
computed yet (check_typedef has never been called on it).  As we get in
the DWARF_VALUE_STACK case (line 1028 of dwarf2/expr.c), the `len`
variable is therefore set to 0, instead of the actual type length.  We
then call allocate_value on subobj_type, which does call check_typedef,
so the length of the typedef gets filled in at that point.  We end up
passing to the copy function a source array view of length 0 and a
target array view of length 4, and the assertion fails.

Fix this by calling check_typedef on both type and subobj_type at the
beginning of fetch_result.

I tried writing a test for this using the DWARF assembler, but I haven't
succeeded.  It's possible that we need to get into this specific code
path (value_of_dwarf_reg_entry and all) to manage to get to
dwarf_expr_context::fetch_result with a typedef type that has never been
resolved.  In all my attempts, the typedef would always be resolved
already, so the bug wouldn't show up.

As a fallback, I made a gdb.dwarf2 test with compiler-generated .S
files.  I don't particularly like those, but I think it's better than no
test.  The .cpp source code is the smallest reproducer I am able to make
from the reproducer given in the bug (thanks to Pedro for suggestions on
how to minimize it further than I had).  Since I tested on both amd64
and aarch64, I added versions of the test for these two architectures.

Change-Id: I182733ad08e34df40d8bcc47af72c482fabf4900
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29374
a4lg pushed a commit that referenced this pull request Sep 22, 2022
When running gdb.base/skip-solib.exp, the backtrace tests could fail with
compilers that associated epilogue instructions with the last statement
line of the function, instead of associating it with the closing brace,
despite the feature being fully functional.  As an example, when testing
skipping the function square, the testsuite would show

Breakpoint 1, main () at (...)/binutils-gdb/gdb/testsuite/gdb.base/skip-solib-main.c:5
5         return square(0);
(gdb) step
0x00007ffff7cef560 in __libc_start_call_main () from /lib64/libc.so.6
(gdb) PASS: gdb.base/skip-solib.exp: ignoring solib file: step
bt
 #0  0x00007ffff7cef560 in __libc_start_call_main () from /lib64/libc.so.6
 #1  0x00007ffff7cef60c in __libc_start_main_impl () from /lib64/libc.so.6
 #2  0x0000000000401065 in _start ()
(gdb) FAIL: gdb.base/skip-solib.exp: ignoring solib file: bt

Which means that the feature is working, the testsuite is just
mis-identifying it.  To avoid this problem, the skipped function calls
have been sent to a line before `return`, so epilogues won't factor in.
a4lg pushed a commit that referenced this pull request Oct 8, 2022
On ubuntu 22.04 with the libc6-dbg package installed, I have the
following failure:

    where
    #0  print_philosopher (n=3, left=33 '!', right=33 '!') at .../gdb/testsuite/gdb.threads/linux-dp.c:105
    #1  0x000055555555576a in philosopher (data=0x55555555937c) at .../gdb/testsuite/gdb.threads/linux-dp.c:148
    #2  0x00007ffff7e11b43 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:442
    #3  0x00007ffff7ea3a00 in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
    (gdb) FAIL: gdb.threads/linux-dp.exp: first thread-specific breakpoint hit

The regex for this test accounts for different situations (with /
without debug symbol) but assumes that if debug info is present the
backtrace shows execution under pthread_create.  However, for the
implementation under test, we are under start_thread.

Update the regex to accept start_thread.

Tested on Ubuntu-22.04 x86_64 with and without libc6-dbg debug symbols
available.

Change-Id: I1e1536279890bca2cd07f038e026b41e46af44e0
a4lg pushed a commit that referenced this pull request Oct 11, 2022
There's a flaw in the interaction of the auxv caching and the fact that
target_auxv_search allows reading auxv from an arbitrary target_ops
(passed in as a parameter).  This has consequences as explained in this
thread:

  https://inbox.sourceware.org/gdb-patches/[email protected]/

In summary, when loading an AArch64 core file with MTE support by
passing the executable and core file names directly to GDB, we see the
MTE info:

    $ ./gdb -nx --data-directory=data-directory -q aarch64-mte-gcore aarch64-mte-gcore.core
    ...
    Program terminated with signal SIGSEGV, Segmentation fault
    Memory tag violation while accessing address 0x0000ffff8ef5e000
    Allocation tag 0x1
    Logical tag 0x0.
    #0  0x0000aaaade3d0b4c in ?? ()
    (gdb)

But if we do it as two separate commands (file and core) we don't:

    $ ./gdb -nx --data-directory=data-directory -q -ex "file aarch64-mte-gcore" -ex "core aarch64-mte-gcore.core"
    ...
    Program terminated with signal SIGSEGV, Segmentation fault.
    #0  0x0000aaaade3d0b4c in ?? ()
    (gdb)

The problem with the latter is that auxv data gets improperly cached
between the two commands.  When executing the file command, auxv gets
first queried here, when loading the executable:

    #0  target_auxv_search (ops=0x55555b842400 <exec_ops>, match=0x9, valp=0x7fffffffc5d0) at /home/simark/src/binutils-gdb/gdb/auxv.c:383
    #1  0x0000555557e576f2 in svr4_exec_displacement (displacementp=0x7fffffffc8c0) at /home/simark/src/binutils-gdb/gdb/solib-svr4.c:2482
    #2  0x0000555557e594d1 in svr4_relocate_main_executable () at /home/simark/src/binutils-gdb/gdb/solib-svr4.c:2878
    #3  0x0000555557e5989e in svr4_solib_create_inferior_hook (from_tty=1) at /home/simark/src/binutils-gdb/gdb/solib-svr4.c:2933
    #4  0x0000555557e6e49f in solib_create_inferior_hook (from_tty=1) at /home/simark/src/binutils-gdb/gdb/solib.c:1253
    #5  0x0000555557f33e29 in symbol_file_command (args=0x7fffffffe01c "aarch64-mte-gcore", from_tty=1) at /home/simark/src/binutils-gdb/gdb/symfile.c:1655
    #6  0x00005555573319c3 in file_command (arg=0x7fffffffe01c "aarch64-mte-gcore", from_tty=1) at /home/simark/src/binutils-gdb/gdb/exec.c:555
    #7  0x0000555556e47185 in do_simple_func (args=0x7fffffffe01c "aarch64-mte-gcore", from_tty=1, c=0x612000047740) at /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:95
    #8  0x0000555556e551c9 in cmd_func (cmd=0x612000047740, args=0x7fffffffe01c "aarch64-mte-gcore", from_tty=1) at /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:2543
    #9  0x00005555580e63fd in execute_command (p=0x7fffffffe02c "e", from_tty=1) at /home/simark/src/binutils-gdb/gdb/top.c:692
    #10 0x0000555557771913 in catch_command_errors (command=0x5555580e55ad <execute_command(char const*, int)>, arg=0x7fffffffe017 "file aarch64-mte-gcore", from_tty=1, do_bp_actions=true) at /home/simark/src/binutils-gdb/gdb/main.c:513
    #11 0x0000555557771fba in execute_cmdargs (cmdarg_vec=0x7fffffffd570, file_type=CMDARG_FILE, cmd_type=CMDARG_COMMAND, ret=0x7fffffffd230) at /home/simark/src/binutils-gdb/gdb/main.c:608
    #12 0x00005555577755ac in captured_main_1 (context=0x7fffffffda10) at /home/simark/src/binutils-gdb/gdb/main.c:1299
    #13 0x0000555557775c2d in captured_main (data=0x7fffffffda10) at /home/simark/src/binutils-gdb/gdb/main.c:1320
    #14 0x0000555557775cc2 in gdb_main (args=0x7fffffffda10) at /home/simark/src/binutils-gdb/gdb/main.c:1345
    #15 0x00005555568bdcbe in main (argc=10, argv=0x7fffffffdba8) at /home/simark/src/binutils-gdb/gdb/gdb.c:32

Here, target_auxv_search is called on the inferior's target stack.  The
target stack only contains the exec target, so the query returns empty
auxv data.  This gets cached for that inferior in `auxv_inferior_data`.

In its constructor (before it is pushed to the inferior's target stack),
the core_target needs to identify the right target description from the
core, and for that asks the gdbarch to read a target description from
the core file.  Because some implementations of
gdbarch_core_read_description (such as AArch64's) need to read auxv data
from the core in order to determine the right target description, the
core_target passes a pointer to itself, allowing implementations to call
target_auxv_search it.  However, because we have previously cached
(empty) auxv data for that inferior, target_auxv_search searched that
cached (empty) auxv data, not auxv data read from the core.  Remember
that this data was obtained by reading auxv on the inferior's target
stack, which only contained an exec target.

The problem I see is that while target_auxv_search offers the
flexibility of reading from an arbitrary (passed as an argument) target,
the caching doesn't do the distinction of which target is being queried,
and where the cached data came from.  So, you could read auxv from a
target A, it gets cached, then you try to read auxv from a target B, and
it returns the cached data from target A.  That sounds wrong.  In our
case, we expect to read different auxv data from the core target than
what we have read from the target stack earlier, so it doesn't make
sense to hit the cache in this case.

To fix this, I propose splitting the code paths that read auxv data from
an inferior's target stack and those that read from a passed-in target.
The code path that reads from the target stack will keep caching,
whereas the one that reads from a passed-in target won't.  And since,
searching in auxv data is independent from where this data came from,
split the "read" part from the "search" part.

From what I understand, auxv caching was introduced mostly to reduce
latency on remote connections, when doing many queries.  With the change
I propose, only the queries done while constructing the core_target
end up not using cached auxv data.  This is fine, because there are just
a handful of queries max, done at this point, and reading core files is
local.

The changes to auxv functions are:

 - Introduce 2 target_read_auxv functions.  One reads from an explicit
   target_ops and doesn't do caching (to be used in
   gdbarch_core_read_description context).  The other takes no argument,
   reads from the current inferior's target stack (it looks just like a
   standard target function wrapper) and does caching.

   The first target_read_auxv actually replaces get_auxv_inferior_data,
   since it became a trivial wrapper around it.

 - Change the existing target_auxv_search to not read auxv data from the
   target, but to accept it as a parameter (a gdb::byte_vector).  This
   function doesn't care where the data came from, it just searches in
   it.  It still needs to take a target_ops and gdbarch to know how to
   parse auxv entries.

 - Add a convenience target_auxv_search overload that reads auxv
   data from the inferior's target stack and searches in it.  This
   overload is useful to replace the exist target_auxv_search calls that
   passed the `current_inferior ()->top_target ()` target and keep the
   call sites short.

 - Modify parse_auxv to accept a target_ops and gdbarch to use for
   parsing entries.  Not strictly related to the rest of this change,
   but it seems like a good change in the context.

Changes in architecture-specific files (tdep and nat):

 - In linux-tdep, linux_get_hwcap and linux_get_hwcap2 get split in two,
   similar to target_auxv_search.  One version receives auxv data,
   target and arch as parameters.  The other gets everything from the
   current inferior.  The latter is for convenience, to avoid making
   call sites too ugly.

 - Call sites of linux_get_hwcap and linux_get_hwcap2 are adjusted to
   use either of the new versions.  The call sites in
   gdbarch_core_read_description context explicitly read auxv data from
   the passed-in target and call the linux_get_hwcap{,2} function with
   parameters.  Other call sites use the versions without parameters.

 - Same idea for arm_fbsd_read_description_auxv.

 - Call sites of target_auxv_search that passed
   `current_inferior ()->top_target ()` are changed to use the
   target_auxv_search overload that works in the current inferior.

Reviewed-By: John Baldwin <[email protected]>
Reviewed-By: Luis Machado <[email protected]>
Change-Id: Ib775a220cf1e76443fb7da2fdff8fc631128fe66
a4lg pushed a commit that referenced this pull request Oct 26, 2022
In the lockup state the PC value of the the outer frame is irreversibly
lost. The other registers are intact so LR likely contains
PC of some frame next to the outer one, but we cannot analyze
the nearest outer frame without knowing its PC
therefore we do not know SP fixup for this frame.

The frame unwinder possibly gets mad due to the wrong SP value.
To prevent problems terminate unwinding if PC contains the magic
value of the lockup state.

Example session wihtout this change,
Cortex-M33 CPU in lockup, gdb 13.0.50.20221016-git:
----------------
  (gdb) c
  Continuing.

  Program received signal SIGINT, Interrupt.
  0xeffffffe in ?? ()
  (gdb) bt
  #0  0xeffffffe in ?? ()
  #1  0x0c000a9c in HardFault_Handler ()
      at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/stm32l5xx_it.c:99
  #2  0x2002ffd8 in ?? ()
  Backtrace stopped: previous frame identical to this frame (corrupt stack?)
  (gdb)
----------------
The frame #1 is at correct PC taken from LR, #2 is a total nonsense.

With the change:
----------------
  (gdb) c
  Continuing.

  Program received signal SIGINT, Interrupt.
  warning: ARM M in lockup state, stack unwinding terminated.
  <signal handler called>
  (gdb) bt
  #0  <signal handler called>
  (gdb)
----------------

There is a visible drawback of emitting a warning in a cache buildnig routine
as introduced in Torbjörn SVENSSON's
[PATCH v4] gdb/arm: Stop unwinding on error, but do not assert
The warning is printed just once and not repeated on each backtrace command.

Signed-off-by: Tomas Vanek <[email protected]>
a4lg pushed a commit that referenced this pull request Oct 26, 2022
Arm v8-M Architecture Reference Manual,
D1.2.95 EXC_RETURN, Exception Return Payload
describes ES bit:

"ES, bit [0]
     Exception Secure. The security domain the exception was taken to.
     The possible values of this bit are:
       0 Non-secure.
       1 Secure"

arm-tdep.c:3443, arm_m_exception_cache () function tests this bit:

  exception_domain_is_secure = (bit (lr, 0) == 0);

The test is negated!

Later on line 3553, the condition evaluates if an additional state
context is stacked:

  /* With the Security extension, the hardware saves R4..R11 too.  */
  if (tdep->have_sec_ext && secure_stack_used
      && (!default_callee_register_stacking || exception_domain_is_secure))

RM, B3.19 Exception entry, context stacking
reads:
RPLHM "In a PE with the Security Extension, on taking an exception,
the PE hardware:
  ...
  2. If exception entry requires a transition from Secure state to
     Non-secure state, the PE hardware extends the stack frame and also
     saves additional state context."

So we should test for !exception_domain_is_secure instead of non-negated
value!
These two bugs compensate each other so unstacking works correctly.

But another test of exception_domain_is_secure (negated due to the
first bug) prevents arm_unwind_secure_frames to work as expected:

  /* Unwinding from non-secure to secure can trip security
     measures.  In order to avoid the debugger being
     intrusive, rely on the user to configure the requested
     mode.  */
  if (secure_stack_used && !exception_domain_is_secure
      && !arm_unwind_secure_frames)

Test with GNU gdb (GDB) 13.0.50.20221016-git.
Stopped in a non-secure handler:

 (gdb) set arm unwind-secure-frames 0
 (gdb) bt
 #0  HAL_SYSTICK_Callback () at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/NonSecure/Src/nsmain.c:490
 #1  0x0804081c in SysTick_Handler ()
     at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/NonSecure/Src/nsstm32l5xx_it.c:134
 #2  <signal handler called>
 #3  HAL_GPIO_ReadPin (GPIOx=0x52020800, GPIO_Pin=8192)
     at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Drivers/STM32L5xx_HAL_Driver/Src/stm32l5xx_hal_gpio.c:386
 #4  0x0c000338 in SECURE_Mode () at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/main.c:86
 #5  0x080403f2 in main () at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/NonSecure/Src/nsmain.c:278
 Backtrace stopped: previous frame inner to this frame (corrupt stack?)

The frames #3 and #4 are secure. backtrace should stop before #3.

Stopped in a secure handler:

 (gdb) bt
 #0  HAL_SYSTICK_Callback () at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/main.c:425
 #1  0x0c000b6a in SysTick_Handler ()
     at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/stm32l5xx_it.c:234
 warning: Non-secure to secure stack unwinding disabled.
 #2  <signal handler called>

The exception from secure to secure erroneously stops unwinding. It should
continue as far as the security unlimited backtrace:

 (gdb) set arm unwind-secure-frames 1
 (gdb) si <-- used to rebuild frame cache after change of unwind-secure-frames
 0x0c0008e6      425       if (SecureTimingDelay != 0U)
 (gdb) bt
 #0  0x0c0008e6 in HAL_SYSTICK_Callback () at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/main.c:425
 #1  0x0c000b6a in SysTick_Handler ()
     at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/stm32l5xx_it.c:234
 #2  <signal handler called>
 #3  0x0c000328 in SECURE_Mode () at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/Secure/Src/main.c:88
 #4  0x080403f2 in main () at C:/dvl/stm32l5trustzone/GPIO_IOToggle_TrustZone/NonSecure/Src/nsmain.c:278

 Backtrace stopped: previous frame inner to this frame (corrupt stack?)

Set exception_domain_is_secure to the value expected by its name.
Fix exception_domain_is_secure usage in the additional state context
stacking condition.

Signed-off-by: Tomas Vanek <[email protected]>
a4lg pushed a commit that referenced this pull request Nov 9, 2022
Commit be6276e "Allow debugging of runtime loader / dynamic linker"
introduced a small regression when stepping into the runtime loader /
dynamic linker from function we do not have debug information for.  This
is reported in PR/29747.

This can be shown by the following example (given by Simon Marchi in
buzilla bug report):

    $ cat test.c
    #include <stdio.h>

    int main()
    {
      printf("Hi\n");
      return 0;
    }
    $ gcc test.c -O0 -o test
    $ ./gdb -q -nx --data-directory=data-directory test -ex start -ex s
    Reading symbols from test...
    (No debugging symbols found in test)
    Temporary breakpoint 1 at 0x1151
    Starting program: .../binutils-gdb/gdb/test
    [Thread debugging using libthread_db enabled]
    Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

    Temporary breakpoint 1, 0x0000555555555151 in main ()
    Single stepping until exit from function main,
    which has no line number information.
    /home/smarchi/src/binutils-gdb/gdb/infrun.c:6960:64: runtime error: member call on null pointer of type 'struct symbol'

    The crash happens here:

    #0  __sanitizer::Die () at ../../../../src/libsanitizer/sanitizer_common/sanitizer_termination.cpp:50
    #1  0x00007ffff5dd7128 in __ubsan::__ubsan_handle_type_mismatch_v1_abort (Data=<optimized out>, Pointer=<optimized out>) at ../../../../src/libsanitizer/ubsan/ubsan_handlers.cpp:148
    #2  0x000055556183e1a7 in process_event_stop_test (ecs=0x7fffffffccd0) at .../binutils-gdb/gdb/infrun.c:6960
    #3  0x0000555561838ea4 in handle_signal_stop (ecs=0x7fffffffccd0) at .../binutils-gdb/gdb/infrun.c:6615
    #4  0x000055556182f77b in handle_inferior_event (ecs=0x7fffffffccd0) at .../binutils-gdb/gdb/infrun.c:5866

When evaluating:

    6956   if (execution_direction != EXEC_REVERSE
    6957       && ecs->event_thread->control.step_over_calls == STEP_OVER_UNDEBUGGABLE
    6958       && in_solib_dynsym_resolve_code (ecs->event_thread->stop_pc ())
    6959       && !in_solib_dynsym_resolve_code (
    6961          ecs->event_thread->control.step_start_function->value_block ()
    6962              ->entry_pc ()))

we dereference, ecs->event_thread->control.step_start_function which is
nullptr.

This patch changes this condition so it evaluates to true if
ecs->event_thread->control.step_start_function is nullptr since this
matches the behaviour before be6276e.

Tested on ubuntu-22.04 x86_64.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29747
Reviewed-By: Bruno Larsen <[email protected]>
Approved-By: Kevin Buettner <[email protected]>
a4lg pushed a commit that referenced this pull request Nov 17, 2022
On x86_64-windows, since 04e2ac7, we observe this internal error:

  [...]/gdbsupport/intrusive_list.h:458: internal-error: erase_element:
  Assertion `elem_node->prev != INTRUSIVE_LIST_UNLINKED_VALUE' failed.

Breaking in the destructors for intrusive_list and frame_info_ptr shows that
in this configuration, the destructors for frame.c's statically-stored
objects are run before frame-info.c's:

  Thread 1 hit Breakpoint 7, intrusive_list<frame_info_ptr,
  intrusive_base_node<frame_info_ptr> >::~intrusive_list (this=0x7ff69c418c90
  <frame_info_ptr::frame_list>, __in_chrg=<optimized out>)
  [...]/../gdbsupport/intrusive_list.h:250
  250	    clear ();
  (gdb) bt
  #0  intrusive_list<frame_info_ptr, intrusive_base_node<frame_info_ptr> >
      ::~intrusive_list (this=0x7ff69c418c90 <frame_info_ptr::frame_list>,
      __in_chrg=<optimized out>) [...]/../gdbsupport/intrusive_list.h:250
  #1  0x00007ff69b78edba in __tcf_1 () [...]/frame-info.c:27
  #2  0x00007ff9c457aa9f in msvcrt!_initterm_e ()
      from C:\Windows\System32\msvcrt.dll
  #3  0x00007ff69b8246a6 in captured_main_1 (context=0x5ffe00)
      [...]/main.c:1111
  #4  0x00007ff69b825149 in captured_main (data=0x5ffe00) [...]/main.c:1320
  #5  0x00007ff69b8251b1 in gdb_main (args=0x5ffe00) [...]/main.c:1345
  #6  0x00007ff69b5d1730 in main (argc=2, argv=0x751730) [...]/gdb.c:32
  (gdb) c
  Continuing.

  Thread 1 hit Breakpoint 8, frame_info_ptr::~frame_info_ptr
  (this=0x7ff69c418e20 <selected_frame>, __in_chrg=<optimized out>)
  [...]/frame-info.h:74
  74	    if (is_linked ())
  (gdb) bt
  #0  frame_info_ptr::~frame_info_ptr (this=0x7ff69c418e20 <selected_frame>,
      __in_chrg=<optimized out>) [...]/frame-info.h:74
  #1  0x00007ff69b79a643 in __tcf_1 () [...]/frame.c:1675
  #2  0x00007ff9c457aa9f in msvcrt!_initterm_e () from
      C:\Windows\System32\msvcrt.dll
  #3  0x00007ff69b8246a6 in captured_main_1 (context=0x5ffe00)
      [...]/main.c:1111
  #4  0x00007ff69b825149 in captured_main (data=0x5ffe00) [...]/main.c:1320
  #5  0x00007ff69b8251b1 in gdb_main (args=0x5ffe00) [...]/main.c:1345
  #6  0x00007ff69b5d1730 in main (argc=2, argv=0x751730) [...]/gdb.c:32

Approved-By: Simon Marchi <[email protected]>
a4lg pushed a commit that referenced this pull request Nov 28, 2022
This commit addresses one of the issues identified in PR gdb/28275.

Bug gdb/28275 identifies a number of situations in which this assert:

  Assertion `!proc_target->commit_resumed_state' failed.

could be triggered.  There's actually a number of similar places where
this assert is found in GDB, the two of interest in gdb/28275 are in
target_wait and target_stop.

In one of the comments:

  https://sourceware.org/bugzilla/show_bug.cgi?id=28275#c1

steps to trigger the assertion within target_stop were identified when
using a modified version of the gdb.threads/detach-step-over.exp test
script.

In the gdb.threads/detach-step-over.exp test, we attach to a
multi-threaded inferior, and continue the inferior in asynchronous
(background) mode.  Each thread is continuously hitting a conditional
breakpoint where the condition is always false.  While the inferior is
running we detach.  The goal is that we detach while GDB is performing a
step-over for the conditional breakpoint in at least one thread.

While detaching, if a step-over is in progress, then GDB has to
complete the step over before we can detach.  This involves calling
target_stop and target_wait (see prepare_for_detach).

As far as gdb/28275 is concerned, the interesting part here, is the
the process_stratum_target::commit_resumed_state variable must be
false when target_stop and target_wait are called.

This is currently ensured because, in detach_command (infrun.c), we
create an instance of scoped_disable_commit_resumed, this ensures that
when target_detach is called, ::commit_resumed_state will be false.

The modification to the test that I propose here, and which exposed
the bug, is that, instead of using "detach" to detach from the
inferior, we instead use "quit".  Quitting GDB after attaching to an
inferior will cause GDB to first detach, and then exit.

When we quit GDB we end up calling target_detach via a different code
path, the stack looks like:

  #0 target_detach
  #1 kill_or_detach
  #2 quit_force
  #3 quit_command

Along this path there is no scoped_disable_commit_resumed created.
::commit_resumed_state can be true when we reach prepare_for_detach,
which calls target_wait and target_stop, so the assertion will trigger.

In this commit, I propose fixing this by adding the creation of a
scoped_disable_commit_resumed into target_detach.  This will ensure
that ::commit_resumed_state is false when calling prepare_for_detach
from within target_detach.

I did consider placing the scoped_disable_commit_resumed in
prepare_for_detach, however, placing it in target_detach ensures that
the target's commit_resumed_state flag is left to false if the detached
inferior was the last one for that target.  It's the same rationale as
for patch "gdb: disable commit resumed in target_kill" that comes later
in this series, but for detach instead of kill.

detach_command still includes a scoped_disable_commit_resumed too, but I
think it is still relevant to cover the resumption at the end of the
function.

Co-Authored-By: Simon Marchi <[email protected]>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28275
Change-Id: Ie128f7aba6ef0e018859275eca372e6ea738e96f
a4lg pushed a commit that referenced this pull request Nov 28, 2022
New in this version: add a dedicated test.

When I do this:

    $ ./gdb -nx --data-directory=data-directory -q \
        /bin/sleep \
	-ex "maint set target-non-stop on" \
	-ex "tar ext :1234" \
	-ex "set remote exec-file /bin/sleep" \
	-ex "run 1231 &" \
	-ex add-inferior \
	-ex "inferior 2"
    Reading symbols from /bin/sleep...
    (No debugging symbols found in /bin/sleep)
    Remote debugging using :1234
    Starting program: /bin/sleep 1231
    Reading /lib64/ld-linux-x86-64.so.2 from remote target...
    warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
    Reading /lib64/ld-linux-x86-64.so.2 from remote target...
    Reading /usr/lib/debug/.build-id/a6/7a1408f18db3576757eea210d07ba3fc560dff.debug from remote target...
    [New inferior 2]
    Added inferior 2 on connection 1 (extended-remote :1234)
    [Switching to inferior 2 [<null>] (<noexec>)]
    (gdb) Reading /lib/x86_64-linux-gnu/libc.so.6 from remote target...
    attach 3659848
    Attaching to process 3659848
    /home/smarchi/src/binutils-gdb/gdb/thread.c:85: internal-error: inferior_thread: Assertion `current_thread_ != nullptr' failed.

Note the "attach" command just above.  When doing it on the command-line
with a -ex switch, the bug doesn't trigger.

The internal error of GDB is actually caused by GDBserver crashing, and
the error recovery of GDB is not on point.  This patch aims to fix just
the GDBserver crash, not the GDB problem.

GDBserver crashes with a segfault here:

    (gdb) bt
    #0  0x00005555557fb3f4 in find_one_thread (ptid=...) at /home/smarchi/src/binutils-gdb/gdbserver/thread-db.cc:177
    #1  0x00005555557fd5cf in thread_db_thread_handle (ptid=<error reading variable: Cannot access memory at address 0xffffffffffffffa0>, handle=0x7fffffffc400, handle_len=0x7fffffffc3f0)
        at /home/smarchi/src/binutils-gdb/gdbserver/thread-db.cc:461
    #2  0x000055555578a0b6 in linux_process_target::thread_handle (this=0x5555558a64c0 <the_x86_target>, ptid=<error reading variable: Cannot access memory at address 0xffffffffffffffa0>, handle=0x7fffffffc400,
        handle_len=0x7fffffffc3f0) at /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:6905
    #3  0x00005555556dfcc6 in handle_qxfer_threads_worker (thread=0x60b000000510, buffer=0x7fffffffc8a0) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:1645
    #4  0x00005555556e00e6 in operator() (__closure=0x7fffffffc5e0, thread=0x60b000000510) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:1696
    #5  0x00005555556f54be in for_each_thread<handle_qxfer_threads_proper(buffer*)::<lambda(thread_info*)> >(struct {...}) (func=...) at /home/smarchi/src/binutils-gdb/gdbserver/gdbthread.h:159
    #6  0x00005555556e0242 in handle_qxfer_threads_proper (buffer=0x7fffffffc8a0) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:1694
    #7  0x00005555556e04ba in handle_qxfer_threads (annex=0x629000000213 "", readbuf=0x621000019100 '\276' <repeats 200 times>..., writebuf=0x0, offset=0, len=4097)
        at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:1732
    #8  0x00005555556e1989 in handle_qxfer (own_buf=0x629000000200 "qXfer:threads", packet_len=26, new_packet_len_p=0x7fffffffd630) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:2045
    #9  0x00005555556e720a in handle_query (own_buf=0x629000000200 "qXfer:threads", packet_len=26, new_packet_len_p=0x7fffffffd630) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:2685
    #10 0x00005555556f1a01 in process_serial_event () at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:4176
    #11 0x00005555556f4457 in handle_serial_event (err=0, client_data=0x0) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:4514
    #12 0x0000555555820f56 in handle_file_event (file_ptr=0x607000000250, ready_mask=1) at /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:573
    #13 0x0000555555821895 in gdb_wait_for_event (block=1) at /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:694
    #14 0x000055555581f533 in gdb_do_one_event (mstimeout=-1) at /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:264
    #15 0x00005555556ec9fb in start_event_loop () at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:3512
    #16 0x00005555556f0769 in captured_main (argc=4, argv=0x7fffffffe0d8) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:3992
    #17 0x00005555556f0e3f in main (argc=4, argv=0x7fffffffe0d8) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:4078

The reason is a wrong current process when find_one_thread is called.
The current process is the 2nd one, which was just attached.  It does
not yet have thread_db data (proc->priv->thread_db is nullptr).  As we
iterate on all threads of all process to fulfull the qxfer:threads:read
request, we get to a thread of process 1 for which we haven't read
thread_db information yet (lwp_info::thread_known is false), so we get
into find_one_thread.  find_one_thread uses
`current_process ()->priv->thread_db`, assuming the current process
matches the ptid passed as a parameter, which is wrong.  A segfault
happens when trying to dereference that thread_db pointer.

Fix this by making find_one_thread not assume what the current process /
current thread is.  If it needs to call into libthread_db, which we know
will try to read memory from the current process, then temporarily set
the current process.

In the case where the thread is already know and we return early, we
don't need to switch process.

Add a test to reproduce this specific situation.

Change-Id: I09b00883e8b73b7e5f89d0f47cb4e9c0f3d6caaa
Approved-By: Andrew Burgess <[email protected]>
a4lg pushed a commit that referenced this pull request Nov 28, 2022
New in this version:

 - Better comment in target_kill
 - Uncomment line in test to avoid hanging when exiting, when testing on
   native-extended-gdbserver

PR 28275 shows that doing a sequence of:

 - Run inferior in background (run &)
 - kill that inferior
 - Run again

We get into this assertion:

    /home/smarchi/src/binutils-gdb/gdb/target.c:2590: internal-error: target_wait: Assertion `!proc_target->commit_resumed_state' failed.

    #0  internal_error_loc (file=0x5606b344e740 "/home/smarchi/src/binutils-gdb/gdb/target.c", line=2590, fmt=0x5606b344d6a0 "%s: Assertion `%s' failed.") at /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:54
    #1  0x00005606b6296475 in target_wait (ptid=..., status=0x7fffb9390630, options=...) at /home/smarchi/src/binutils-gdb/gdb/target.c:2590
    #2  0x00005606b5767a98 in startup_inferior (proc_target=0x5606bfccb2a0 <the_amd64_linux_nat_target>, pid=3884857, ntraps=1, last_waitstatus=0x0, last_ptid=0x0) at /home/smarchi/src/binutils-gdb/gdb/nat/fork-inferior.c:482
    #3  0x00005606b4e6c9c5 in gdb_startup_inferior (pid=3884857, num_traps=1) at /home/smarchi/src/binutils-gdb/gdb/fork-child.c:132
    #4  0x00005606b50f14a5 in inf_ptrace_target::create_inferior (this=0x5606bfccb2a0 <the_amd64_linux_nat_target>, exec_file=0x604000039f50 "/home/smarchi/build/binutils-gdb/gdb/test", allargs="", env=0x61500000a580, from_tty=1)
        at /home/smarchi/src/binutils-gdb/gdb/inf-ptrace.c:105
    #5  0x00005606b53b6d23 in linux_nat_target::create_inferior (this=0x5606bfccb2a0 <the_amd64_linux_nat_target>, exec_file=0x604000039f50 "/home/smarchi/build/binutils-gdb/gdb/test", allargs="", env=0x61500000a580, from_tty=1)
        at /home/smarchi/src/binutils-gdb/gdb/linux-nat.c:978
    #6  0x00005606b512b79b in run_command_1 (args=0x0, from_tty=1, run_how=RUN_NORMAL) at /home/smarchi/src/binutils-gdb/gdb/infcmd.c:468
    #7  0x00005606b512c236 in run_command (args=0x0, from_tty=1) at /home/smarchi/src/binutils-gdb/gdb/infcmd.c:526

When running the kill command, commit_resumed_state for the
process_stratum_target (linux-nat, here) is true.  After the kill, when
there are no more threads, commit_resumed_state is still true, as
nothing touches this flag during the kill operation.  During the
subsequent run command, run_command_1 does:

    scoped_disable_commit_resumed disable_commit_resumed ("running");

We would think that this would clear the commit_resumed_state flag of
our native target, but that's not the case, because
scoped_disable_commit_resumed iterates on non-exited inferiors in order
to find active process targets.  And after the kill, the inferior is
exited, and the native target was unpushed from it anyway.  So
scoped_disable_commit_resumed doesn't touch the commit_resumed_state
flag of the native target, it stays true.  When reaching target_wait, in
startup_inferior (to consume the initial expect stop events while the
inferior is starting up and working its way through the shell),
commit_resumed_state is true, breaking the contract saying that
commit_resumed_state is always false when calling the targets' wait
method.

(note: to be correct, I think that startup_inferior should toggle
commit_resumed between the target_wait and target_resume calls, but I'll
ignore that for now)

I can see multiple ways to fix this.  In the end, we need
commit_resumed_state to be cleared by the time we get to that
target_wait.  It could be done at the end of the kill command, or at the
beginning of the run command.

To keep things in a coherent state, I'd like to make it so that after
the kill command, when the target is left with no threads, its
commit_resumed_state flag is left to false.  This way, we can keep
working with the assumption that a target with no threads (and therefore
no running threads) has commit_resumed_state == false.

Do this by adding a scoped_disable_commit_resumed in target_kill.  It
clears the target's commit_resumed_state on entry, and leaves it false
if the target does not have any resumed thread on exit.  That means,
even if the target has another inferior with stopped threads,
commit_resumed_state will be left to false, which makes sense.

Add a test that tries to cover various combinations of actions done
while an inferior is running (and therefore while commit_resumed_state
is true on the process target).

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28275
Change-Id: I8e6fe6dc1f475055921520e58cab68024039a1e9
Approved-By: Andrew Burgess <[email protected]>
a4lg pushed a commit that referenced this pull request Nov 29, 2022
Bug gdb/29712 identifies a problem with the Python disassembler API.
In some cases GDB will try to throw an exception through the
libopcodes disassembler code, however, not all targets include
exception unwind information when compiling C code, for targets that
don't include this information GDB will terminate when trying to pass
the exception through libopcodes.

To explain what GDB is trying to do, consider the following trivial
use of the Python disassembler API:

  class ExampleDisassembler(gdb.disassembler.Disassembler):

      class MyInfo(gdb.disassembler.DisassembleInfo):
          def __init__(self, info):
              super().__init__(info)

          def read_memory(self, length, offset):
              return super().read_memory(length, offset)

      def __init__(self):
          super().__init__("ExampleDisassembler")

      def __call__(self, info):
          info = self.MyInfo(info)
          return gdb.disassembler.builtin_disassemble(info)

This disassembler doesn't add any value, it defers back to GDB to do
all the actual work, but it serves to allow us to discuss the problem.

The problem occurs when a Python exception is raised by the
MyInfo.read_memory method.  The MyInfo.read_memory method is called
from the C++ function gdbpy_disassembler::read_memory_func.  The C++
stack at the point this function is called looks like this:

  #0  gdbpy_disassembler::read_memory_func (memaddr=4198805, buff=0x7fff9ab9d2a8 "\220ӹ\232\377\177", len=1, info=0x7fff9ab9d558) at ../../src/gdb/python/py-disasm.c:510
  #1  0x000000000104ba06 in fetch_data (info=0x7fff9ab9d558, addr=0x7fff9ab9d2a9 "ӹ\232\377\177") at ../../src/opcodes/i386-dis.c:305
  #2  0x000000000104badb in ckprefix (ins=0x7fff9ab9d100) at ../../src/opcodes/i386-dis.c:8571
  #3  0x000000000104e28e in print_insn (pc=4198805, info=0x7fff9ab9d558, intel_syntax=-1) at ../../src/opcodes/i386-dis.c:9548
  #4  0x000000000104f4d4 in print_insn_i386 (pc=4198805, info=0x7fff9ab9d558) at ../../src/opcodes/i386-dis.c:9949
  #5  0x00000000004fa7ea in default_print_insn (memaddr=4198805, info=0x7fff9ab9d558) at ../../src/gdb/arch-utils.c:1033
  #6  0x000000000094fe5e in i386_print_insn (pc=4198805, info=0x7fff9ab9d558) at ../../src/gdb/i386-tdep.c:4072
  #7  0x0000000000503d49 in gdbarch_print_insn (gdbarch=0x5335560, vma=4198805, info=0x7fff9ab9d558) at ../../src/gdb/gdbarch.c:3351
  #8  0x0000000000bcc8c6 in disasmpy_builtin_disassemble (self=0x7f2ab07f54d0, args=0x7f2ab0789790, kw=0x0) at ../../src/gdb/python/py-disasm.c:324

  ### ... snip lots of frames as we pass through Python itself ...

  #22 0x0000000000bcd860 in gdbpy_print_insn (gdbarch=0x5335560, memaddr=0x401195, info=0x7fff9ab9e3c8) at ../../src/gdb/python/py-disasm.c:783
  #23 0x00000000008995a5 in ext_lang_print_insn (gdbarch=0x5335560, address=0x401195, info=0x7fff9ab9e3c8) at ../../src/gdb/extension.c:939
  #24 0x0000000000741aaa in gdb_print_insn_1 (gdbarch=0x5335560, vma=0x401195, info=0x7fff9ab9e3c8) at ../../src/gdb/disasm.c:1078
  #25 0x0000000000741bab in gdb_disassembler::print_insn (this=0x7fff9ab9e3c0, memaddr=0x401195, branch_delay_insns=0x0) at ../../src/gdb/disasm.c:1101

So gdbpy_disassembler::read_memory_func is called from the libopcodes
disassembler to read memory, this C++ function then calls into user
supplied Python code to do the work.

If the user supplied Python code raises an gdb.MemoryError exception
indicating the memory read failed, this is fine.  The C++ code
converts this exception back into a return value that libopcodes can
understand, and returns to libopcodes.

However, if the user supplied Python code raises some other exception,
what we want is for this exception to propagate through GDB and appear
as if raised by the call to gdb.disassembler.builtin_disassemble.  To
achieve this, when gdbpy_disassembler::read_memory_func spots an
unknown Python exception, we must pass the information about this
exception from frame #0 to frame #8 in the above backtrace.  Frame #8
is the C++ implementation of gdb.disassembler.builtin_disassemble, and
so it is this function that we want to re-raise the unknown Python
exception, so the user can, if they want, catch the exception in their
code.

The previous mechanism by which the exception was passed was to pack
the details of the Python exception into a C++ exception, then throw
the exception from frame #0, and catch the exception in frame #8,
unpack the details of the Python exception, and re-raise it.

However, this relies on the exception passing through frames #1 to #7,
some of which are in libopcodes, which is C code, and so, might not be
compiled with exception support.

This commit proposes an alternative solution that does not rely on
throwing a C++ exception.

When we spot an unhandled Python exception in frame #0, we will store
the details of this exception within the gdbpy_disassembler object
currently in use.  Then we return to libopcodes a value indicating
that the memory_read failed.

libopcodes will now continue to disassemble as though that memory read
failed (with one special case described below), then, when we
eventually return to disasmpy_builtin_disassemble we check to see if
there is an exception stored in the gdbpy_disassembler object.  If
there is then this exception can immediately be installed, and then we
return back to Python, when the user will be able to catch the
exception.

There is one extra change in gdbpy_disassembler::read_memory_func.
After the first call that results in an exception being stored on the
gdbpy_disassembler object, any future calls to the ::read_memory_func
function will immediately return as if the read failed.  This avoids
any additional calls into user supplied Python code.

My thinking here is that should the first call fail with some unknown
error, GDB should not keep trying with any additional calls.  This
maintains the illusion that the exception raised from
MyInfo.read_memory is immediately raised by
gdb.disassembler.builtin_disassemble.  I have no tests for this change
though - to trigger this issue would rely on a libopcodes disassembler
that will try to read further memory even after the first failed
read.  I'm not aware of any such disassembler that currently does
this, but that doesn't mean such a disassembler couldn't exist in the
future.

With this change in place the gdb.python/py-disasm.exp test should now
pass on AArch64.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29712

Approved-By: Simon Marchi <[email protected]>
a4lg pushed a commit that referenced this pull request Dec 18, 2022
There are two places where unaligned loads were seen on aarch64:
  - #1. access to the SFrame FRE stack offsets in the in-memory
    representation/abstraction provided by libsframe.
  - #2. access to the SFrame FRE start address in the on-disk representation
    of the frame row entry.

For #1, we can fix this by reordering the struct members of
sframe_frame_row_entry in libsframe/sframe-api.h.

For #2, we need to default to using memcpy instead, and copy out the bytes
to a location for output.

SFrame format is an unaligned on-disk format. As such, there are other blobs
of memory in the on-disk SFrame FRE that are on not on their natural
boundaries.  But that does not pose further problems yet, because the users
are provided access to the on-disk SFrame FRE data via libsframe's
sframe_frame_row_entry, the latter has its' struct members aligned on their
respective natural boundaries (and initialized using memcpy).

PR 29856 libsframe asan: load misaligned at sframe.c:516

ChangeLog:

	PR libsframe/29856
	* bfd/elf64-x86-64.c: Adjust as the struct members have been
	reordered.
	* libsframe/sframe.c (sframe_decode_fre_start_address): Use
	memcpy to perform 16-bit/32-bit reads.
	* libsframe/testsuite/libsframe.encode/encode-1.c: Adjust as the
	struct members have been reordered.

include/ChangeLog:

	PR libsframe/29856
	* sframe-api.h: Reorder fre_offsets for natural alignment.
a4lg pushed a commit that referenced this pull request Dec 31, 2022
On Ubuntu 22.04.1 x86_64 (with glibc 2.35), I run into:
...
(gdb) PASS: gdb.base/corefile.exp: $_exitcode is void
bt^M
 #0  __pthread_kill_implementation (...) at ./nptl/pthread_kill.c:44^M
 #1  __pthread_kill_internal (...) at ./nptl/pthread_kill.c:78^M
 #2  __GI___pthread_kill (...) at ./nptl/pthread_kill.c:89^M
 #3  0x00007f4985e1a476 in __GI_raise (...) at ../sysdeps/posix/raise.c:26^M
 #4  0x00007f4985e007f3 in __GI_abort () at ./stdlib/abort.c:79^M
 #5  0x0000556b4ea4b504 in func2 () at gdb.base/coremaker.c:153^M
 #6  0x0000556b4ea4b516 in func1 () at gdb.base/coremaker.c:159^M
 #7  0x0000556b4ea4b578 in main (...) at gdb.base/coremaker.c:171^M
(gdb) PASS: gdb.base/corefile.exp: backtrace
up^M
 #1  __pthread_kill_internal (...) at ./nptl/pthread_kill.c:78^M
 78      in ./nptl/pthread_kill.c^M
(gdb) FAIL: gdb.base/corefile.exp: up
...

The problem is that the regexp used here:
...
gdb_test "up" "#\[0-9\]* *\[0-9xa-fH'\]* in .* \\(.*\\).*" "up"
...
does not fit the __pthread_kill_internal line which lacks the instruction
address due to inlining.

Fix this by making the regexp less strict.

Tested on x86_64-linux.
a4lg pushed a commit that referenced this pull request Jan 23, 2023
I would like to improve frame_info_ptr to automatically grab the
information needed to reinflate a frame, and automatically reinflate it
as needed.  One thing that is in the way is the fact that some frames
can be created out of thin air by the create_new_frame function.  These
frames are not the fruit of unwinding from the target's current frame.
These frames are created by the "select-frame view" command.

These frames are not correctly handled by the frame save/restore
functions, save_selected_frame, restore_selected_frame and
lookup_selected_frame.  This can be observed here, using the test
included in this patch:

    $ ./gdb --data-directory=data-directory -nx -q testsuite/outputs/gdb.base/frame-view/frame-view
    Reading symbols from testsuite/outputs/gdb.base/frame-view/frame-view...
    (gdb) break thread_func
    Breakpoint 1 at 0x11a2: file /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c, line 42.
    (gdb) run
    Starting program: /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/frame-view/frame-view

    [Thread debugging using libthread_db enabled]
    Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1".
    [New Thread 0x7ffff7cc46c0 (LWP 4171134)]
    [Switching to Thread 0x7ffff7cc46c0 (LWP 4171134)]

    Thread 2 "frame-view" hit Breakpoint 1, thread_func (p=0x0) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c:42
    42        foo (11);
    (gdb) info frame
    Stack level 0, frame at 0x7ffff7cc3ee0:
     rip = 0x5555555551a2 in thread_func (/home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c:42); saved rip = 0x7ffff7d4e8fd
     called by frame at 0x7ffff7cc3f80
     source language c.
     Arglist at 0x7ffff7cc3ed0, args: p=0x0
     Locals at 0x7ffff7cc3ed0, Previous frame's sp is 0x7ffff7cc3ee0
     Saved registers:
      rbp at 0x7ffff7cc3ed0, rip at 0x7ffff7cc3ed8
    (gdb) thread 1
    [Switching to thread 1 (Thread 0x7ffff7cc5740 (LWP 4171122))]
    #0  0x00007ffff7d4b4b6 in ?? () from /usr/lib/libc.so.6

Here, we create a custom frame for thread 1 (using the stack from thread
2, for convenience):

    (gdb) select-frame view 0x7ffff7cc3f80 0x5555555551a2

The first calls to "frame" looks good:

    (gdb) frame
    #0  thread_func (p=0x7ffff7d4e630) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c:42
    42        foo (11);

But not the second one:

    (gdb) frame
    #0  0x00007ffff7d4b4b6 in ?? () from /usr/lib/libc.so.6

This second "frame" command shows the current target frame instead of
the user-created frame.

It's not totally clear how the "select-frame view" feature is expected
to behave, especially since it's not tested.  I heard accounts that it
used to be possible to select a frame like this and do "up" and "down"
to navigate the backtrace starting from that frame.  The fact that
create_new_frame calls frame_unwind_find_by_frame to install the right
unwinder suggest that it used to be possible.  But that doesn't work
today:

    (gdb) select-frame view 0x7ffff7cc3f80 0x5555555551a2
    (gdb) up
    Initial frame selected; you cannot go up.
    (gdb) down
    Bottom (innermost) frame selected; you cannot go down.

and "backtrace" always shows the actual thread's backtrace, it ignores
the user-created frame:

    (gdb) bt
    #0  0x00007ffff7d4b4b6 in ?? () from /usr/lib/libc.so.6
    #1  0x00007ffff7d50403 in ?? () from /usr/lib/libc.so.6
    #2  0x000055555555521a in main () at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c:56

I don't want to address all the `select-frame view` issues , but I think
we can agree that the "frame" command changing the selected frame, as
shown above, is a bug.  I would expect that command to show the
currently selected frame and not change it.

This happens because of the scoped_restore_selected_frame object in
print_frame_args.  The frame information is saved in the constructor
(the backtrace below), and restored in the destructor.

    #0  save_selected_frame (frame_id=0x7ffdc0020ad0, frame_level=0x7ffdc0020af0) at /home/simark/src/binutils-gdb/gdb/frame.c:1682
    #1  0x00005631390242f0 in scoped_restore_selected_frame::scoped_restore_selected_frame (this=0x7ffdc0020ad0) at /home/simark/src/binutils-gdb/gdb/frame.c:324
    #2  0x000056313993581e in print_frame_args (fp_opts=..., func=0x62100023bde0, frame=..., num=-1, stream=0x60b000000300) at /home/simark/src/binutils-gdb/gdb/stack.c:755
    #3  0x000056313993ad49 in print_frame (fp_opts=..., frame=..., print_level=1, print_what=SRC_AND_LOC, print_args=1, sal=...) at /home/simark/src/binutils-gdb/gdb/stack.c:1401
    #4  0x000056313993835d in print_frame_info (fp_opts=..., frame=..., print_level=1, print_what=SRC_AND_LOC, print_args=1, set_current_sal=1) at /home/simark/src/binutils-gdb/gdb/stack.c:1126
    #5  0x0000563139932e0b in print_stack_frame (frame=..., print_level=1, print_what=SRC_AND_LOC, set_current_sal=1) at /home/simark/src/binutils-gdb/gdb/stack.c:368
    #6  0x0000563139932bbe in print_stack_frame_to_uiout (uiout=0x611000016840, frame=..., print_level=1, print_what=SRC_AND_LOC, set_current_sal=1) at /home/simark/src/binutils-gdb/gdb/stack.c:346
    #7  0x0000563139b0641e in print_selected_thread_frame (uiout=0x611000016840, selection=...) at /home/simark/src/binutils-gdb/gdb/thread.c:1993
    #8  0x0000563139940b7f in frame_command_core (fi=..., ignored=true) at /home/simark/src/binutils-gdb/gdb/stack.c:1871
    #9  0x000056313994db9e in frame_command_helper<frame_command_core>::base_command (arg=0x0, from_tty=1) at /home/simark/src/binutils-gdb/gdb/stack.c:1976

Since the user-created frame has level 0 (identified by the saved level
-1), lookup_selected_frame just reselects the target's current frame,
and the user-created frame is lost.

My goal here is to fix this particular problem.

Currently, select_frame does not set selected_frame_id and
selected_frame_level for frames with level 0.  It leaves them at
null_frame_id / -1, indicating to restore_selected_frame to use the
target's current frame.  User-created frames also have level 0, so add a
special case them such that select_frame saves their selected id and
level.

save_selected_frame does not need any change.

Change the assertion in restore_selected_frame that checks `frame_level
!= 0` to account for the fact that we can restore user-created frames,
which have level 0.

Finally, change lookup_selected_frame to make it able to re-create
user-created frame_info objects from selected_frame_level and
selected_frame_id.

Add a minimal test case for the case described above, that is the
"select-frame view" command followed by the "frame" command twice.  In
order to have a known stack frame to switch to, the test spawns a second
thread, and tells the first thread to use the other thread's top frame.

Change-Id: Ifc77848dc465fbd21324b9d44670833e09fe98c7
Reviewed-By: Bruno Larsen <[email protected]>
a4lg pushed a commit that referenced this pull request Feb 9, 2023
…ames to the frame cache

The test gdb.base/frame-view.exp fails like this on AArch64:

    frame^M
    #0  baz (z1=hahaha, /home/simark/src/binutils-gdb/gdb/value.c:4056: internal-error: value_fetch_lazy_register: Assertion `next_frame != NULL' failed.^M
    A problem internal to GDB has been detected,^M
    further debugging may prove unreliable.^M
    FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame (GDB internal error)

The sequence of events leading to this is the following:

 - When we create the user frame (the "select-frame view" command), we
   create a sentinel frame just for our user-created frame, in
   create_new_frame.  This sentinel frame has the same id as the regular
   sentinel frame.

 - When printing the frame, after doing the "select-frame view" command,
   the argument's pretty printer is invoked, which does an inferior
   function call (this is the point of the test).  This clears the frame
   cache, including the "real" sentinel frame, which sets the
   sentinel_frame global to nullptr.

 - Later in the frame-printing process (when printing the second
   argument), the auto-reinflation mechanism re-creates the user frame
   by calling create_new_frame again, creating its own special sentinel
   frame again.  However, note that the "real" sentinel frame, the
   sentinel_frame global, is still nullptr.  If the selected frame had
   been a regular frame, we would have called get_current_frame at some
   point during the reinflation, which would have re-created the "real"
   sentinel frame.  But it's not the case when reinflating a user frame.

 - Deep down the stack, something wants to fill in the unwind stop
   reason for frame 0, which requires trying to unwind frame 1.  This
   leads us to trying to unwind the PC of frame 1:

     #0  gdbarch_unwind_pc (gdbarch=0xffff8d010080, next_frame=...) at /home/simark/src/binutils-gdb/gdb/gdbarch.c:2955
     #1  0x000000000134569c in dwarf2_tailcall_sniffer_first (this_frame=..., tailcall_cachep=0xffff773fcae0, entry_cfa_sp_offsetp=0xfffff7f7d450)
         at /home/simark/src/binutils-gdb/gdb/dwarf2/frame-tailcall.c:390
     #2  0x0000000001355d84 in dwarf2_frame_cache (this_frame=..., this_cache=0xffff773fc928) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1089
     #3  0x00000000013562b0 in dwarf2_frame_unwind_stop_reason (this_frame=..., this_cache=0xffff773fc928) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1101
     #4  0x0000000001990f64 in get_prev_frame_always_1 (this_frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2281
     #5  0x0000000001993034 in get_prev_frame_always (this_frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2376
     #6  0x000000000199b814 in get_frame_unwind_stop_reason (frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:3051
     #7  0x0000000001359cd8 in dwarf2_frame_cfa (this_frame=...) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1356
     #8  0x000000000132122c in dwarf_expr_context::execute_stack_op (this=0xfffff7f80170, op_ptr=0xffff8d8883ee "\217\002", op_end=0xffff8d8883ee "\217\002")
         at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:2110
     #9  0x0000000001317b30 in dwarf_expr_context::eval (this=0xfffff7f80170, addr=0xffff8d8883ed "\234\217\002", len=1) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1239
     #10 0x000000000131d68c in dwarf_expr_context::execute_stack_op (this=0xfffff7f80170, op_ptr=0xffff8d88840e "", op_end=0xffff8d88840e "") at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1811
     #11 0x0000000001317b30 in dwarf_expr_context::eval (this=0xfffff7f80170, addr=0xffff8d88840c "\221p", len=2) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1239
     #12 0x0000000001314c3c in dwarf_expr_context::evaluate (this=0xfffff7f80170, addr=0xffff8d88840c "\221p", len=2, as_lval=true, per_cu=0xffff90b03700, frame=..., addr_info=0x0,
         type=0xffff8f6c8400, subobj_type=0xffff8f6c8400, subobj_offset=0) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1078
     #13 0x000000000149f9e0 in dwarf2_evaluate_loc_desc_full (type=0xffff8f6c8400, frame=..., data=0xffff8d88840c "\221p", size=2, per_cu=0xffff90b03700, per_objfile=0xffff9070b980,
         subobj_type=0xffff8f6c8400, subobj_byte_offset=0, as_lval=true) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1513
     #14 0x00000000014a0100 in dwarf2_evaluate_loc_desc (type=0xffff8f6c8400, frame=..., data=0xffff8d88840c "\221p", size=2, per_cu=0xffff90b03700, per_objfile=0xffff9070b980, as_lval=true)
         at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1557
     #15 0x00000000014aa584 in locexpr_read_variable (symbol=0xffff8f6cd770, frame=...) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:3052

 - AArch64 defines a special "prev register" function,
   aarch64_dwarf2_prev_register, to handle unwinding the PC.  This
   function does

     frame_unwind_register_unsigned (this_frame, AARCH64_LR_REGNUM);

 - frame_unwind_register_unsigned ultimately creates a lazy register
   value, saving the frame id of this_frame->next.  this_frame is the
   user-created frame, to this_frame->next is the special sentinel frame
   we created for it.  So the saved ID is the sentinel frame ID.

 - When time comes to un-lazify the value, value_fetch_lazy_register
   calls frame_find_by_id, to find the frame with the ID we saved.

 - frame_find_by_id sees it's the sentinel frame ID, so returns the
   sentinel_frame global, which is, if you remember, nullptr.

 - We hit the `gdb_assert (next_frame != NULL)` assertion in
   value_fetch_lazy_register.

The issues I see here are:

 - The ID of the sentinel frame created for the user-created frame is
   not distinguishable from the ID of the regular sentinel frame.  So
   there's no way frame_find_by_id could find the right frame, in
   value_fetch_lazy_register.
 - Even if they had distinguishable IDs, sentinel frames created for
   user frames are not registered anywhere, so there's no easy way
   frame_find_by_id could find it.

This patch addresses these two issues:

 - Give sentinel frames created for user frames their own distinct IDs
 - Register sentinel frames in the frame cache, so they can be found
   with frame_find_by_id.

I initially had this split in two patches, but I then found that it was
easier to explain as a single patch.

Rergarding the first part of the change: with this patch, the sentinel
frames created for user frames (in create_new_frame) still have
stack_status == FID_STACK_SENTINEL, but their code_addr and stack_addr
fields are now filled with the addresses used to create the user frame.
This ensures this sentinel frame ID is different from the "target"
sentinel frame ID, as well as any other "user" sentinel frame ID.  If
the user tries to create the same frame, with the same addresses,
multiple times, create_sentinel_frame just reuses the existing frame.
So we won't end up with multiple user sentinels with the same ID.

Regular "target" sentinel frames remain with code_addr and stack_addr
unset.

The concrete changes for that part are:

 - Remove the sentinel_frame_id constant, since there isn't one
   "sentinel frame ID" now.  Add the frame_id_build_sentinel function
   for building sentinel frame IDs and a is_sentinel_frame_id function
   to check if a frame id represents a sentinel frame.
 - Replace the sentinel_frame_id check in frame_find_by_id with a
   comparison to `frame_id_build_sentinel (0, 0)`.  The sentinel_frame
   global is meant to contain a reference to the "target" sentinel, so
   the one with addresses (0, 0).
 - Add stack and code address parameters to create_sentinel_frame, to be
   able to create the various types of sentinel frames.
 - Adjust get_current_frame to create the regular "target" sentinel.
 - Adjust create_new_frame to create a sentinel with the ID specific to
   the created user frame.
 - Adjust sentinel_frame_prev_register to get the sentinel frame ID from
   the frame_info object, since there isn't a single "sentinel frame ID"
   now.
 - Change get_next_frame_sentinel_okay to check for a
   sentinel-frame-id-like frame ID, rather than for sentinel_frame
   specifically, since this function could be called with another
   sentinel frame (and we would want the assert to catch it).

The rest of the change is about registering the sentinel frame in the
frame cache:

 - Change frame_stash_add's assertion to allow sentinel frame levels
   (-1).
 - Make create_sentinel_frame add the frame to the frame cache.
 - Change the "sentinel_frame != NULL" check in reinit_frame_cache for a
   check that the frame stash is not empty.  The idea is that if we only
   have some user-created frames in the cache when reinit_frame_cache is
   called, we probably want to emit the frames invalid annotation.  The
   goal of that check is to avoid unnecessary repeated annotations, I
   suppose, so the "frame cache not empty" check should achieve that.

After this change, I think we could theoritically get rid of the
sentienl_frame global.  That sentinel frame could always be found by
looking up `frame_id_build_sentinel (0, 0)` in the frame cache.
However, I left the global there to avoid slowing the typical case down
for nothing.  I however, noted in its comment that it is an
optimization.

With this fix applied, the gdb.base/frame-view.exp now passes for me on
AArch64.  value_of_register_lazy now saves the special sentinel frame ID
in the value, and value_fetch_lazy_register is able to find that
sentinel frame after the frame cache reinit and after the user-created
frame was reinflated.

Tested-By: Alexandra Petlanova Hajkova <[email protected]>
Tested-By: Luis Machado <[email protected]>
Change-Id: I8b77b3448822c8aab3e1c3dda76ec434eb62704f
a4lg pushed a commit that referenced this pull request Mar 1, 2023
Tom de Vries reported [1] a regression in gdb.btrace/record_goto.exp
caused by 6d3717d ("gdb: call frame unwinders' dealloc_cache methods
through destroying the frame cache").  This issue is caught by ASan.  On
a non-ASan build, it may or may not cause a crash or some other issue, I
haven't tried.

I managed to narrow it down to:

    $ ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.btrace/record_goto/record_goto -ex "start" -ex "record btrace" -ex "next"

... and then doing repeatedly "record goto 19" and "record goto 27".
Eventually, I get:

    (gdb) record goto 27
    =================================================================
    ==1527735==ERROR: AddressSanitizer: heap-use-after-free on address 0x6210003392a8 at pc 0x55e4c26eef86 bp 0x7ffd229f24e0 sp 0x7ffd229f24d8
    READ of size 8 at 0x6210003392a8 thread T0
        #0 0x55e4c26eef85 in bfcache_eq /home/simark/src/binutils-gdb/gdb/record-btrace.c:1639
        #1 0x55e4c37cdeff in htab_find_slot_with_hash /home/simark/src/binutils-gdb/libiberty/hashtab.c:659
        #2 0x55e4c37ce24a in htab_find_slot /home/simark/src/binutils-gdb/libiberty/hashtab.c:703
        #3 0x55e4c26ef0c6 in bfcache_new /home/simark/src/binutils-gdb/gdb/record-btrace.c:1653
        #4 0x55e4c26f1242 in record_btrace_frame_sniffer /home/simark/src/binutils-gdb/gdb/record-btrace.c:1820
        #5 0x55e4c1b926a1 in frame_unwind_try_unwinder /home/simark/src/binutils-gdb/gdb/frame-unwind.c:136
        #6 0x55e4c1b930d7 in frame_unwind_find_by_frame(frame_info_ptr, void**) /home/simark/src/binutils-gdb/gdb/frame-unwind.c:196
        #7 0x55e4c1bb867f in get_frame_type(frame_info_ptr) /home/simark/src/binutils-gdb/gdb/frame.c:2925
        #8 0x55e4c2ae6798 in print_frame_info(frame_print_options const&, frame_info_ptr, int, print_what, int, int) /home/simark/src/binutils-gdb/gdb/stack.c:1049
        #9 0x55e4c2ade3e1 in print_stack_frame(frame_info_ptr, int, print_what, int) /home/simark/src/binutils-gdb/gdb/stack.c:367
        #10 0x55e4c26fda03 in record_btrace_set_replay /home/simark/src/binutils-gdb/gdb/record-btrace.c:2779
        #11 0x55e4c26fddc3 in record_btrace_target::goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/record-btrace.c:2843
        #12 0x55e4c2de2bb2 in target_goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/target.c:4169
        #13 0x55e4c275ed98 in record_goto(char const*) /home/simark/src/binutils-gdb/gdb/record.c:372
        #14 0x55e4c275edba in cmd_record_goto /home/simark/src/binutils-gdb/gdb/record.c:383

    0x6210003392a8 is located 424 bytes inside of 4064-byte region [0x621000339100,0x62100033a0e0)
    freed by thread T0 here:
        #0 0x7f6ca34a5b6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
        #1 0x55e4c38a4c17 in rpl_free /home/simark/src/binutils-gdb/gnulib/import/free.c:44
        #2 0x55e4c1bbd378 in xfree<void> /home/simark/src/binutils-gdb/gdb/../gdbsupport/gdb-xfree.h:37
        #3 0x55e4c37d1b63 in call_freefun /home/simark/src/binutils-gdb/libiberty/obstack.c:103
        #4 0x55e4c37d25a2 in _obstack_free /home/simark/src/binutils-gdb/libiberty/obstack.c:280
        #5 0x55e4c1bad701 in reinit_frame_cache() /home/simark/src/binutils-gdb/gdb/frame.c:2112
        #6 0x55e4c27705a3 in registers_changed_ptid(process_stratum_target*, ptid_t) /home/simark/src/binutils-gdb/gdb/regcache.c:564
        #7 0x55e4c27708c7 in registers_changed_thread(thread_info*) /home/simark/src/binutils-gdb/gdb/regcache.c:573
        #8 0x55e4c26fd922 in record_btrace_set_replay /home/simark/src/binutils-gdb/gdb/record-btrace.c:2772
        #9 0x55e4c26fddc3 in record_btrace_target::goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/record-btrace.c:2843
        #10 0x55e4c2de2bb2 in target_goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/target.c:4169
        #11 0x55e4c275ed98 in record_goto(char const*) /home/simark/src/binutils-gdb/gdb/record.c:372
        #12 0x55e4c275edba in cmd_record_goto /home/simark/src/binutils-gdb/gdb/record.c:383

    previously allocated by thread T0 here:
        #0 0x7f6ca34a5e8f in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
        #1 0x55e4c0b55c60 in xmalloc /home/simark/src/binutils-gdb/gdb/alloc.c:57
        #2 0x55e4c37d1a6d in call_chunkfun /home/simark/src/binutils-gdb/libiberty/obstack.c:94
        #3 0x55e4c37d1c20 in _obstack_begin_worker /home/simark/src/binutils-gdb/libiberty/obstack.c:141
        #4 0x55e4c37d1ed7 in _obstack_begin /home/simark/src/binutils-gdb/libiberty/obstack.c:164
        #5 0x55e4c1bad728 in reinit_frame_cache() /home/simark/src/binutils-gdb/gdb/frame.c:2113
        #6 0x55e4c27705a3 in registers_changed_ptid(process_stratum_target*, ptid_t) /home/simark/src/binutils-gdb/gdb/regcache.c:564
        #7 0x55e4c27708c7 in registers_changed_thread(thread_info*) /home/simark/src/binutils-gdb/gdb/regcache.c:573
        #8 0x55e4c26fd922 in record_btrace_set_replay /home/simark/src/binutils-gdb/gdb/record-btrace.c:2772
        #9 0x55e4c26fddc3 in record_btrace_target::goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/record-btrace.c:2843
        #10 0x55e4c2de2bb2 in target_goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/target.c:4169
        #11 0x55e4c275ed98 in record_goto(char const*) /home/simark/src/binutils-gdb/gdb/record.c:372
        #12 0x55e4c275edba in cmd_record_goto /home/simark/src/binutils-gdb/gdb/record.c:383

The problem is a stale entry in the bfcache hash table (in
record-btrace.c), left across a reinit_frame_cache.  This entry points
to something that used to be allocated on the frame obstack, that has
since been wiped by reinit_frame_cache.

Before the aforementioned, unwinder deallocation functions were called
by iterating on the frame chain, starting with the sentinel frame, like
so:

  /* Tear down all frame caches.  */
  for (frame_info *fi = sentinel_frame; fi != NULL; fi = fi->prev)
    {
      if (fi->prologue_cache && fi->unwind->dealloc_cache)
	fi->unwind->dealloc_cache (fi, fi->prologue_cache);
      if (fi->base_cache && fi->base->unwind->dealloc_cache)
	fi->base->unwind->dealloc_cache (fi, fi->base_cache);
    }

After that patch, we relied on the fact that all frames are (supposedly)
in the frame_stash.  A deletion function was added to the frame_stash
hash table, so that dealloc functions would be called when emptying the
frame stash.  There is one case, however, where a frame_info is not in
the frame stash.  That is when we create the frame_info for the current
frame (level 0, unwound from the sentinel frame), but don't compute its
frame id.  The computation of the frame id for that frame (and only that
frame, AFAIK) is done lazily.  And putting a frame_info in the frame stash
requires knowing its id.  So a frame 0 whose frame id is not computed
yet is necessarily not in the frame stash.

When replaying with btrace, record_btrace_frame_sniffer insert entries
corresponding to frames in the "bfcache" hash table.  It then relies on
record_btrace_frame_dealloc_cache being called for each frame to remove
all those entries when the frames get invalidated.  If a frame reinit
happens while frame 0's id is not computed  (and therefore that frame is
not in frame_stash), record_btrace_frame_dealloc_cache does not get
called for it, and it leaves a stale entry in bfcache.  That then leads
to a use-after-free when that entry is accessed later, which ASan
catches.

The proposed solution is to explicitly call frame_info_del on frame 0,
if it exists, and if its frame id is not computed.  If its frame id is
computed, it is expected that it will be in the frame stash, so it will
be "deleted" through that.

[1] https://inbox.sourceware.org/gdb-patches/[email protected]/T/#mcf1340ce2906a72ec7ed535ec0c97dba11c3d977

Reported-By: Tom de Vries <[email protected]>
Tested-By: Tom de Vries <[email protected]>
Change-Id: I2351882dd511f3bbc01e4152e9db13b69b3ba384
a4lg pushed a commit that referenced this pull request Mar 1, 2023
I noticed that if Ctrl-C was typed just while GDB is evaluating a
breakpoint condition in the background, and GDB ends up reaching out
to the Python interpreter, then the breakpoint condition would still
fail, like:

  c&
  Continuing.
  (gdb) Error in testing breakpoint condition:
  Quit

That happens because while evaluating the breakpoint condition, we
enter Python, and end up calling PyErr_SetInterrupt (it's called by
gdbpy_set_quit_flag, in frame #0):

 (top-gdb) bt
 #0  gdbpy_set_quit_flag (extlang=0x558c68f81900 <extension_language_python>) at ../../src/gdb/python/python.c:288
 #1  0x0000558c6845f049 in set_quit_flag () at ../../src/gdb/extension.c:785
 #2  0x0000558c6845ef98 in set_active_ext_lang (now_active=0x558c68f81900 <extension_language_python>) at ../../src/gdb/extension.c:743
 #3  0x0000558c686d3e56 in gdbpy_enter::gdbpy_enter (this=0x7fff2b70bb90, gdbarch=0x558c6ab9eac0, language=0x0) at ../../src/gdb/python/python.c:212
 #4  0x0000558c68695d49 in python_on_memory_change (inferior=0x558c6a830b00, addr=0x555555558014, len=4, data=0x558c6af8a610 "") at ../../src/gdb/python/py-inferior.c:146
 #5  0x0000558c6823a071 in std::__invoke_impl<void, void (*&)(inferior*, unsigned long, long, unsigned char const*), inferior*, unsigned long, long, unsigned char const*> (__f=@0x558c6a8ecd98: 0x558c68695d01 <python_on_memory_change(inferior*, CORE_ADDR, ssize_t, bfd_byte const*)>) at /usr/include/c++/11/bits/invoke.h:61
 #6  0x0000558c68237591 in std::__invoke_r<void, void (*&)(inferior*, unsigned long, long, unsigned char const*), inferior*, unsigned long, long, unsigned char const*> (__fn=@0x558c6a8ecd98: 0x558c68695d01 <python_on_memory_change(inferior*, CORE_ADDR, ssize_t, bfd_byte const*)>) at /usr/include/c++/11/bits/invoke.h:111
 #7  0x0000558c68233e64 in std::_Function_handler<void (inferior*, unsigned long, long, unsigned char const*), void (*)(inferior*, unsigned long, long, unsigned char const*)>::_M_invoke(std::_Any_data const&, inferior*&&, unsigned long&&, long&&, unsigned char const*&&) (__functor=..., __args#0=@0x7fff2b70bd40: 0x558c6a830b00, __args#1=@0x7fff2b70bd38: 93824992247828, __args#2=@0x7fff2b70bd30: 4, __args#3=@0x7fff2b70bd28: 0x558c6af8a610 "") at /usr/include/c++/11/bits/std_function.h:290
 #8  0x0000558c6830a96e in std::function<void (inferior*, unsigned long, long, unsigned char const*)>::operator()(inferior*, unsigned long, long, unsigned char const*) const (this=0x558c6a8ecd98, __args#0=0x558c6a830b00, __args#1=93824992247828, __args#2=4, __args#3=0x558c6af8a610 "") at /usr/include/c++/11/bits/std_function.h:590
 #9  0x0000558c6830a620 in gdb::observers::observable<inferior*, unsigned long, long, unsigned char const*>::notify (this=0x558c690828c0 <gdb::observers::memory_changed>, args#0=0x558c6a830b00, args#1=93824992247828, args#2=4, args#3=0x558c6af8a610 "") at ../../src/gdb/../gdbsupport/observable.h:166
 #10 0x0000558c68309d95 in write_memory_with_notification (memaddr=0x555555558014, myaddr=0x558c6af8a610 "", len=4) at ../../src/gdb/corefile.c:363
 #11 0x0000558c68904224 in value_assign (toval=0x558c6afce910, fromval=0x558c6afba6c0) at ../../src/gdb/valops.c:1190
 #12 0x0000558c681e3869 in expr::assign_operation::evaluate (this=0x558c6af8e150, expect_type=0x0, exp=0x558c6afcfe60, noside=EVAL_NORMAL) at ../../src/gdb/expop.h:1902
 #13 0x0000558c68450c89 in expr::logical_or_operation::evaluate (this=0x558c6afab060, expect_type=0x0, exp=0x558c6afcfe60, noside=EVAL_NORMAL) at ../../src/gdb/eval.c:2330
 #14 0x0000558c6844a896 in expression::evaluate (this=0x558c6afcfe60, expect_type=0x0, noside=EVAL_NORMAL) at ../../src/gdb/eval.c:110
 #15 0x0000558c6844a95e in evaluate_expression (exp=0x558c6afcfe60, expect_type=0x0) at ../../src/gdb/eval.c:124
 #16 0x0000558c682061ef in breakpoint_cond_eval (exp=0x558c6afcfe60) at ../../src/gdb/breakpoint.c:4971
 ...


The fix is to disable cooperative SIGINT handling while handling
inferior events, so that SIGINT is saved in the global quit flag, and
not in the extension language, while handling an event.

This commit augments the testcase added by the previous commit to test
this scenario as well.

Approved-By: Tom Tromey <[email protected]>
Change-Id: Idf8ab815774ee6f4b45ca2d0caaf30c9b9f127bb
a4lg pushed a commit that referenced this pull request Mar 1, 2023
…l/kernel mode addresses

At the moment GDB only handles pointer authentication (pauth) for userspace
addresses and if we're debugging a Linux-hosted program.

The Linux Kernel can be configured to use pauth instructions for some
additional security hardening, but GDB doesn't handle this well.

To overcome this limitation, GDB needs a couple things:

1 - The target needs to advertise pauth support.
2 - The hook to remove non-address bits from a pointer needs to be registered
    in aarch64-tdep.c as opposed to aarch64-linux-tdep.c.

There is a patch for QEMU that addresses the first point, and it makes
QEMU's gdbstub expose a couple more pauth mask registers, so overall we will
have up to 4 pauth masks (2 masks or 4 masks):

pauth_dmask
pauth_cmask
pauth_dmask_high
pauth_cmask_high

pauth_dmask and pauth_cmask are the masks used to remove pauth signatures
from userspace addresses. pauth_dmask_high and pauth_cmask_high masks are used
to remove pauth signatures from kernel addresses.

The second point is easily addressed by moving code around.

When debugging a Linux Kernel built with pauth with an unpatched GDB, we get
the following backtrace:

 #0  __fput (file=0xffff0000c17a6400) at /repos/linux/fs/file_table.c:296
 #1  0xffff8000082bd1f0 in ____fput (work=<optimized out>) at /repos/linux/fs/file_table.c:348
 #2  0x30008000080ade30 [PAC] in ?? ()
 #3  0x30d48000080ade30 in ?? ()
 Backtrace stopped: previous frame identical to this frame (corrupt stack?)

With a patched GDB, we get something a lot more meaningful:

 #0  __fput (file=0xffff0000c1bcfa00) at /repos/linux/fs/file_table.c:296
 #1  0xffff8000082bd1f0 in ____fput (work=<optimized out>) at /repos/linux/fs/file_table.c:348
 #2  0xffff8000080ade30 [PAC] in task_work_run () at /repos/linux/kernel/task_work.c:179
 #3  0xffff80000801db90 [PAC] in resume_user_mode_work (regs=0xffff80000a96beb0) at /repos/linux/include/linux/resume_user_mode.h:49
 #4  do_notify_resume (regs=regs@entry=0xffff80000a96beb0, thread_flags=4) at /repos/linux/arch/arm64/kernel/signal.c:1127
 #5  0xffff800008fb9974 [PAC] in prepare_exit_to_user_mode (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:137
 #6  exit_to_user_mode (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:142
 #7  el0_svc (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:638
 #8  0xffff800008fb9d34 [PAC] in el0t_64_sync_handler (regs=<optimized out>) at /repos/linux/arch/arm64/kernel/entry-common.c:655
 #9  0xffff800008011548 [PAC] in el0t_64_sync () at /repos/linux/arch/arm64/kernel/entry.S:586
 Backtrace stopped: Cannot access memory at address 0xffff80000a96c0c8
a4lg pushed a commit that referenced this pull request Mar 16, 2023
With test-case gdb.python/py-completion.exp and target board
native-extended-gdbserver I get this warning:
...
(gdb) PASS: gdb.python/py-completion.exp: discard #2
completefilecommandcond $outputs/gdb.python/py-completion/py-completion-t^G\
  PASS: gdb.python/py-completion.exp: completefilecommandcond completion
Remote debugging from host ::1, port 53346^M
monitor exit^M
not implemented^M
(gdb) WARNING: Timed out waiting for EOF in server after monitor exit
...

Fix this by adding the missing "discard #3", such that we have instead:
...
(gdb) PASS: gdb.python/py-completion.exp: discard #2
completefilecommandcond $outputs/gdb.python/py-completion/py-completion-t^G\
  PASS: gdb.python/py-completion.exp: completefilecommandcond completion
 ^M
not implemented^M
(gdb) PASS: gdb.python/py-completion.exp: discard #3
Remote debugging from host ::1, port 36278^M
monitor exit^M
(gdb)
...

Tested on x86_64-linux.
a4lg pushed a commit that referenced this pull request Jul 24, 2023
Commit 7a8de0c ("Remove ALL_BREAKPOINTS_SAFE") introduced a
use-after-free in the breakpoints iterations (see below for full ASan
report).  This makes gdb.base/stale-infcall.exp fail when GDB is build
with ASan.

check_longjmp_breakpoint_for_call_dummy iterates on all breakpoints,
possibly deleting the current breakpoint as well as related breakpoints.
The problem arises when a breakpoint in the B->related_breakpoint chain
is also B->next.  In that case, deleting that related breakpoint frees
the breakpoint that all_breakpoints_safe has saved.

The old code worked around that by manually changing B_TMP, which was
the next breakpoint saved by the "safe iterator":

	while (b->related_breakpoint != b)
	  {
	    if (b_tmp == b->related_breakpoint)
	      b_tmp = b->related_breakpoint->next;
	    delete_breakpoint (b->related_breakpoint);
	  }

(Note that this seemed to assume that b->related_breakpoint->next was
the same as b->next->next, not sure this is guaranteed.)

The new code kept the B_TMP variable, but it's not useful in that
context.  We can't go change the next breakpoint as saved by the safe
iterator, like we did before.  I suggest fixing that by saving the
breakpoints to delete in a map and deleting them all at the end.

Here's the full ASan report:

    (gdb) PASS: gdb.base/stale-infcall.exp: continue to breakpoint: break-run1
    print infcall ()
    =================================================================
    ==47472==ERROR: AddressSanitizer: heap-use-after-free on address 0x611000034980 at pc 0x563f7012c7bc bp 0x7ffdf3804d70 sp 0x7ffdf3804d60
    READ of size 8 at 0x611000034980 thread T0
        #0 0x563f7012c7bb in next_iterator<breakpoint>::operator++() /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/next-iterator.h:66
        #1 0x563f702ce8c0 in basic_safe_iterator<next_iterator<breakpoint> >::operator++() /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/safe-iterator.h:84
        #2 0x563f7021522a in check_longjmp_breakpoint_for_call_dummy(thread_info*) /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:7611
        #3 0x563f714567b1 in process_event_stop_test /home/smarchi/src/binutils-gdb/gdb/infrun.c:6881
        #4 0x563f71454e07 in handle_signal_stop /home/smarchi/src/binutils-gdb/gdb/infrun.c:6769
        #5 0x563f7144b680 in handle_inferior_event /home/smarchi/src/binutils-gdb/gdb/infrun.c:6023
        #6 0x563f71436165 in fetch_inferior_event() /home/smarchi/src/binutils-gdb/gdb/infrun.c:4387
        #7 0x563f7136ff51 in inferior_event_handler(inferior_event_type) /home/smarchi/src/binutils-gdb/gdb/inf-loop.c:42
        #8 0x563f7168038d in handle_target_event /home/smarchi/src/binutils-gdb/gdb/linux-nat.c:4219
        #9 0x563f72fccb6d in handle_file_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:573
        #10 0x563f72fcd503 in gdb_wait_for_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:694
        #11 0x563f72fcaf2b in gdb_do_one_event(int) /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:217
        #12 0x563f7262b9bb in wait_sync_command_done() /home/smarchi/src/binutils-gdb/gdb/top.c:426
        #13 0x563f7137a7c3 in run_inferior_call /home/smarchi/src/binutils-gdb/gdb/infcall.c:650
        #14 0x563f71381295 in call_function_by_hand_dummy(value*, type*, gdb::array_view<value*>, void (*)(void*, int), void*) /home/smarchi/src/binutils-gdb/gdb/infcall.c:1332
        #15 0x563f7137c0e2 in call_function_by_hand(value*, type*, gdb::array_view<value*>) /home/smarchi/src/binutils-gdb/gdb/infcall.c:780
        #16 0x563f70fe5960 in evaluate_subexp_do_call(expression*, noside, value*, gdb::array_view<value*>, char const*, type*) /home/smarchi/src/binutils-gdb/gdb/eval.c:649
        #17 0x563f70fe6617 in expr::operation::evaluate_funcall(type*, expression*, noside, char const*, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:677
        #18 0x563f6fd19668 in expr::operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/expression.h:136
        #19 0x563f70fe6bba in expr::var_value_operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:689
        #20 0x563f704b71dc in expr::funcall_operation::evaluate(type*, expression*, noside) /home/smarchi/src/binutils-gdb/gdb/expop.h:2219
        #21 0x563f70fe0f02 in expression::evaluate(type*, noside) /home/smarchi/src/binutils-gdb/gdb/eval.c:110
        #22 0x563f71b1373e in process_print_command_args /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1319
        #23 0x563f71b1391b in print_command_1 /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1332
        #24 0x563f71b147ec in print_command /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1465
        #25 0x563f706029b8 in do_simple_func /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:95
        #26 0x563f7061972a in cmd_func(cmd_list_element*, char const*, int) /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:2735
        #27 0x563f7262d0ef in execute_command(char const*, int) /home/smarchi/src/binutils-gdb/gdb/top.c:572
        #28 0x563f7100ed9c in command_handler(char const*) /home/smarchi/src/binutils-gdb/gdb/event-top.c:543
        #29 0x563f7101014b in command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /home/smarchi/src/binutils-gdb/gdb/event-top.c:779
        #30 0x563f72777942 in tui_command_line_handler /home/smarchi/src/binutils-gdb/gdb/tui/tui-interp.c:104
        #31 0x563f7100d059 in gdb_rl_callback_handler /home/smarchi/src/binutils-gdb/gdb/event-top.c:250
        #32 0x7f5a80418246 in rl_callback_read_char (/usr/lib/libreadline.so.8+0x3b246) (BuildId: 092e91fc4361b0ef94561e3ae03a75f69398acbb)
        #33 0x563f7100ca06 in gdb_rl_callback_read_char_wrapper_noexcept /home/smarchi/src/binutils-gdb/gdb/event-top.c:192
        #34 0x563f7100cc5e in gdb_rl_callback_read_char_wrapper /home/smarchi/src/binutils-gdb/gdb/event-top.c:225
        #35 0x563f728c70db in stdin_event_handler /home/smarchi/src/binutils-gdb/gdb/ui.c:155
        #36 0x563f72fccb6d in handle_file_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:573
        #37 0x563f72fcd503 in gdb_wait_for_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:694
        #38 0x563f72fcb15c in gdb_do_one_event(int) /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:264
        #39 0x563f7177ec1c in start_event_loop /home/smarchi/src/binutils-gdb/gdb/main.c:412
        #40 0x563f7177f12e in captured_command_loop /home/smarchi/src/binutils-gdb/gdb/main.c:476
        #41 0x563f717846e4 in captured_main /home/smarchi/src/binutils-gdb/gdb/main.c:1320
        #42 0x563f71784821 in gdb_main(captured_main_args*) /home/smarchi/src/binutils-gdb/gdb/main.c:1339
        #43 0x563f6fcedfbd in main /home/smarchi/src/binutils-gdb/gdb/gdb.c:32
        #44 0x7f5a7e43984f  (/usr/lib/libc.so.6+0x2384f) (BuildId: 2f005a79cd1a8e385972f5a102f16adba414d75e)
        #45 0x7f5a7e439909 in __libc_start_main (/usr/lib/libc.so.6+0x23909) (BuildId: 2f005a79cd1a8e385972f5a102f16adba414d75e)
        #46 0x563f6fcedd84 in _start (/home/smarchi/build/binutils-gdb/gdb/gdb+0xafb0d84) (BuildId: 50bd32e6e9d5e84543e9897b8faca34858ca3995)

    0x611000034980 is located 0 bytes inside of 208-byte region [0x611000034980,0x611000034a50)
    freed by thread T0 here:
        #0 0x7f5a7fce312a in operator delete(void*, unsigned long) /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:164
        #1 0x563f702bd1fa in momentary_breakpoint::~momentary_breakpoint() /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:304
        #2 0x563f702771c5 in delete_breakpoint(breakpoint*) /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:12404
        #3 0x563f702150a7 in check_longjmp_breakpoint_for_call_dummy(thread_info*) /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:7673
        #4 0x563f714567b1 in process_event_stop_test /home/smarchi/src/binutils-gdb/gdb/infrun.c:6881
        #5 0x563f71454e07 in handle_signal_stop /home/smarchi/src/binutils-gdb/gdb/infrun.c:6769
        #6 0x563f7144b680 in handle_inferior_event /home/smarchi/src/binutils-gdb/gdb/infrun.c:6023
        #7 0x563f71436165 in fetch_inferior_event() /home/smarchi/src/binutils-gdb/gdb/infrun.c:4387
        #8 0x563f7136ff51 in inferior_event_handler(inferior_event_type) /home/smarchi/src/binutils-gdb/gdb/inf-loop.c:42
        #9 0x563f7168038d in handle_target_event /home/smarchi/src/binutils-gdb/gdb/linux-nat.c:4219
        #10 0x563f72fccb6d in handle_file_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:573
        #11 0x563f72fcd503 in gdb_wait_for_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:694
        #12 0x563f72fcaf2b in gdb_do_one_event(int) /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:217
        #13 0x563f7262b9bb in wait_sync_command_done() /home/smarchi/src/binutils-gdb/gdb/top.c:426
        #14 0x563f7137a7c3 in run_inferior_call /home/smarchi/src/binutils-gdb/gdb/infcall.c:650
        #15 0x563f71381295 in call_function_by_hand_dummy(value*, type*, gdb::array_view<value*>, void (*)(void*, int), void*) /home/smarchi/src/binutils-gdb/gdb/infcall.c:1332
        #16 0x563f7137c0e2 in call_function_by_hand(value*, type*, gdb::array_view<value*>) /home/smarchi/src/binutils-gdb/gdb/infcall.c:780
        #17 0x563f70fe5960 in evaluate_subexp_do_call(expression*, noside, value*, gdb::array_view<value*>, char const*, type*) /home/smarchi/src/binutils-gdb/gdb/eval.c:649
        #18 0x563f70fe6617 in expr::operation::evaluate_funcall(type*, expression*, noside, char const*, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:677
        #19 0x563f6fd19668 in expr::operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/expression.h:136
        #20 0x563f70fe6bba in expr::var_value_operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:689
        #21 0x563f704b71dc in expr::funcall_operation::evaluate(type*, expression*, noside) /home/smarchi/src/binutils-gdb/gdb/expop.h:2219
        #22 0x563f70fe0f02 in expression::evaluate(type*, noside) /home/smarchi/src/binutils-gdb/gdb/eval.c:110
        #23 0x563f71b1373e in process_print_command_args /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1319
        #24 0x563f71b1391b in print_command_1 /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1332
        #25 0x563f71b147ec in print_command /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1465
        #26 0x563f706029b8 in do_simple_func /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:95
        #27 0x563f7061972a in cmd_func(cmd_list_element*, char const*, int) /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:2735
        #28 0x563f7262d0ef in execute_command(char const*, int) /home/smarchi/src/binutils-gdb/gdb/top.c:572
        #29 0x563f7100ed9c in command_handler(char const*) /home/smarchi/src/binutils-gdb/gdb/event-top.c:543

    previously allocated by thread T0 here:
        #0 0x7f5a7fce2012 in operator new(unsigned long) /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:95
        #1 0x563f7029a9a3 in new_momentary_breakpoint<program_space*&, frame_id&, int&> /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:8129
        #2 0x563f702212f6 in momentary_breakpoint_from_master /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:8169
        #3 0x563f70212db1 in set_longjmp_breakpoint_for_call_dummy() /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:7582
        #4 0x563f713804db in call_function_by_hand_dummy(value*, type*, gdb::array_view<value*>, void (*)(void*, int), void*) /home/smarchi/src/binutils-gdb/gdb/infcall.c:1260
        #5 0x563f7137c0e2 in call_function_by_hand(value*, type*, gdb::array_view<value*>) /home/smarchi/src/binutils-gdb/gdb/infcall.c:780
        #6 0x563f70fe5960 in evaluate_subexp_do_call(expression*, noside, value*, gdb::array_view<value*>, char const*, type*) /home/smarchi/src/binutils-gdb/gdb/eval.c:649
        #7 0x563f70fe6617 in expr::operation::evaluate_funcall(type*, expression*, noside, char const*, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:677
        #8 0x563f6fd19668 in expr::operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/expression.h:136
        #9 0x563f70fe6bba in expr::var_value_operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:689
        #10 0x563f704b71dc in expr::funcall_operation::evaluate(type*, expression*, noside) /home/smarchi/src/binutils-gdb/gdb/expop.h:2219
        #11 0x563f70fe0f02 in expression::evaluate(type*, noside) /home/smarchi/src/binutils-gdb/gdb/eval.c:110
        #12 0x563f71b1373e in process_print_command_args /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1319
        #13 0x563f71b1391b in print_command_1 /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1332
        #14 0x563f71b147ec in print_command /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1465
        #15 0x563f706029b8 in do_simple_func /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:95
        #16 0x563f7061972a in cmd_func(cmd_list_element*, char const*, int) /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:2735
        #17 0x563f7262d0ef in execute_command(char const*, int) /home/smarchi/src/binutils-gdb/gdb/top.c:572
        #18 0x563f7100ed9c in command_handler(char const*) /home/smarchi/src/binutils-gdb/gdb/event-top.c:543
        #19 0x563f7101014b in command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /home/smarchi/src/binutils-gdb/gdb/event-top.c:779
        #20 0x563f72777942 in tui_command_line_handler /home/smarchi/src/binutils-gdb/gdb/tui/tui-interp.c:104
        #21 0x563f7100d059 in gdb_rl_callback_handler /home/smarchi/src/binutils-gdb/gdb/event-top.c:250
        #22 0x7f5a80418246 in rl_callback_read_char (/usr/lib/libreadline.so.8+0x3b246) (BuildId: 092e91fc4361b0ef94561e3ae03a75f69398acbb)

Change-Id: Id00c17ab677f847fbf4efdf0f4038373668d3d88
Approved-By: Tom Tromey <[email protected]>
a4lg pushed a commit that referenced this pull request Jul 24, 2023
Commit b5661ff ("gdb: fix possible use-after-free when
executing commands") attempted to fix possible use-after-free
in case command redefines itself.

Commit 37e5833 ("gdb: fix command lookup in execute_command ()")
updated the previous fix to handle subcommands as well by using the
original command string to lookup the command again after its execution.

This fixed the test in gdb.base/define.exp but it turned out that it
does not work (at least) for "target remote" and "target extended-remote".

The problem is that the command buffer P passed to execute_command ()
gets overwritten in dont_repeat () while executing "target remote"
command itself:

	#0  dont_repeat () at top.c:822
	#1  0x000055555730982a in target_preopen (from_tty=1) at target.c:2483
	#2  0x000055555711e911 in remote_target::open_1 (name=0x55555881c7fe ":1234", from_tty=1, extended_p=0)
	    at remote.c:5946
	#3  0x000055555711d577 in remote_target::open (name=0x55555881c7fe ":1234", from_tty=1) at remote.c:5272
	#4  0x00005555573062f2 in open_target (args=0x55555881c7fe ":1234", from_tty=1, command=0x5555589d0490)
	    at target.c:853
	#5  0x0000555556ad22fa in cmd_func (cmd=0x5555589d0490, args=0x55555881c7fe ":1234", from_tty=1)
	    at cli/cli-decode.c:2737
	#6  0x00005555573487fd in execute_command (p=0x55555881c802 "4", from_tty=1) at top.c:688

Therefore the second call to lookup_cmd () at line 697 fails to find
command because the original command string is gone.

This commit addresses this particular problem by creating a *copy* of
original command string for the sole purpose of using it after command
execution to lookup the command again. It may not be the most efficient
way but it's safer given that command buffer is shared and overwritten
in hard-to-foresee situations.

Tested on x86_64-linux.

PR 30249
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30249

Approved-By: Tom Tromey <[email protected]>
a4lg pushed a commit that referenced this pull request Jul 24, 2023
After this commit:

  commit baab375
  Date:   Tue Jul 13 14:44:27 2021 -0400

      gdb: building inferior strings from within GDB

It was pointed out that a new ASan failure had been introduced which
was triggered by gdb.base/internal-string-values.exp:

  (gdb) PASS: gdb.base/internal-string-values.exp: test_setting: all langs: lang=ada: ptype "foo"
  print $_gdb_maint_setting("test-settings string")
  =================================================================
  ==80377==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000068034 at pc 0x564785cba682 bp 0x7ffd20644620 sp 0x7ffd20644610
  READ of size 1 at 0x603000068034 thread T0
      #0 0x564785cba681 in find_command_name_length(char const*) /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:2129
      #1 0x564785cbacb2 in lookup_cmd_1(char const**, cmd_list_element*, cmd_list_element**, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, int, bool) /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:2186
      #2 0x564785cbb539 in lookup_cmd_1(char const**, cmd_list_element*, cmd_list_element**, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, int, bool) /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:2248
      #3 0x564785cbbcf3 in lookup_cmd(char const**, cmd_list_element*, char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, int, int) /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:2339
      #4 0x564785c82df2 in setting_cmd /tmp/src/binutils-gdb/gdb/cli/cli-cmds.c:2219
      #5 0x564785c84274 in gdb_maint_setting_internal_fn /tmp/src/binutils-gdb/gdb/cli/cli-cmds.c:2348
      #6 0x564788167b3b in call_internal_function(gdbarch*, language_defn const*, value*, int, value**) /tmp/src/binutils-gdb/gdb/value.c:2321
      #7 0x5647854b6ebd in expr::ada_funcall_operation::evaluate(type*, expression*, noside) /tmp/src/binutils-gdb/gdb/ada-lang.c:11254
      #8 0x564786658266 in expression::evaluate(type*, noside) /tmp/src/binutils-gdb/gdb/eval.c:111
      #9 0x5647871242d6 in process_print_command_args /tmp/src/binutils-gdb/gdb/printcmd.c:1322
      #10 0x5647871244b3 in print_command_1 /tmp/src/binutils-gdb/gdb/printcmd.c:1335
      #11 0x564787125384 in print_command /tmp/src/binutils-gdb/gdb/printcmd.c:1468
      #12 0x564785caac44 in do_simple_func /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:95
      #13 0x564785cc18f0 in cmd_func(cmd_list_element*, char const*, int) /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:2735
      #14 0x564787c70c68 in execute_command(char const*, int) /tmp/src/binutils-gdb/gdb/top.c:574
      #15 0x564786686180 in command_handler(char const*) /tmp/src/binutils-gdb/gdb/event-top.c:543
      #16 0x56478668752f in command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /tmp/src/binutils-gdb/gdb/event-top.c:779
      #17 0x564787dcb29a in tui_command_line_handler /tmp/src/binutils-gdb/gdb/tui/tui-interp.c:104
      #18 0x56478668443d in gdb_rl_callback_handler /tmp/src/binutils-gdb/gdb/event-top.c:250
      #19 0x7f4efd506246 in rl_callback_read_char (/usr/lib/libreadline.so.8+0x3b246) (BuildId: 092e91fc4361b0ef94561e3ae03a75f69398acbb)
      #20 0x564786683dea in gdb_rl_callback_read_char_wrapper_noexcept /tmp/src/binutils-gdb/gdb/event-top.c:192
      #21 0x564786684042 in gdb_rl_callback_read_char_wrapper /tmp/src/binutils-gdb/gdb/event-top.c:225
      #22 0x564787f1b119 in stdin_event_handler /tmp/src/binutils-gdb/gdb/ui.c:155
      #23 0x56478862438d in handle_file_event /tmp/src/binutils-gdb/gdbsupport/event-loop.cc:573
      #24 0x564788624d23 in gdb_wait_for_event /tmp/src/binutils-gdb/gdbsupport/event-loop.cc:694
      #25 0x56478862297c in gdb_do_one_event(int) /tmp/src/binutils-gdb/gdbsupport/event-loop.cc:264
      #26 0x564786df99f0 in start_event_loop /tmp/src/binutils-gdb/gdb/main.c:412
      #27 0x564786dfa069 in captured_command_loop /tmp/src/binutils-gdb/gdb/main.c:476
      #28 0x564786dff61f in captured_main /tmp/src/binutils-gdb/gdb/main.c:1320
      #29 0x564786dff75c in gdb_main(captured_main_args*) /tmp/src/binutils-gdb/gdb/main.c:1339
      #30 0x564785381b6d in main /tmp/src/binutils-gdb/gdb/gdb.c:32
      #31 0x7f4efbc3984f  (/usr/lib/libc.so.6+0x2384f) (BuildId: 2f005a79cd1a8e385972f5a102f16adba414d75e)
      #32 0x7f4efbc39909 in __libc_start_main (/usr/lib/libc.so.6+0x23909) (BuildId: 2f005a79cd1a8e385972f5a102f16adba414d75e)
      #33 0x564785381934 in _start (/tmp/build/binutils-gdb/gdb/gdb+0xabc5934) (BuildId: 90de353ac158646e7dab501b76a18a76628fca33)

  0x603000068034 is located 0 bytes after 20-byte region [0x603000068020,0x603000068034) allocated by thread T0 here:
      #0 0x7f4efcee0cd1 in __interceptor_calloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:77
      #1 0x5647856265d8 in xcalloc /tmp/src/binutils-gdb/gdb/alloc.c:97
      #2 0x564788610c6b in xzalloc(unsigned long) /tmp/src/binutils-gdb/gdbsupport/common-utils.cc:29
      #3 0x56478815721a in value::allocate_contents(bool) /tmp/src/binutils-gdb/gdb/value.c:929
      #4 0x564788157285 in value::allocate(type*, bool) /tmp/src/binutils-gdb/gdb/value.c:941
      #5 0x56478815733a in value::allocate(type*) /tmp/src/binutils-gdb/gdb/value.c:951
      #6 0x5647854ae81c in expr::ada_string_operation::evaluate(type*, expression*, noside) /tmp/src/binutils-gdb/gdb/ada-lang.c:10675
      #7 0x5647854b63b8 in expr::ada_funcall_operation::evaluate(type*, expression*, noside) /tmp/src/binutils-gdb/gdb/ada-lang.c:11184
      #8 0x564786658266 in expression::evaluate(type*, noside) /tmp/src/binutils-gdb/gdb/eval.c:111
      #9 0x5647871242d6 in process_print_command_args /tmp/src/binutils-gdb/gdb/printcmd.c:1322
      #10 0x5647871244b3 in print_command_1 /tmp/src/binutils-gdb/gdb/printcmd.c:1335
      #11 0x564787125384 in print_command /tmp/src/binutils-gdb/gdb/printcmd.c:1468
      #12 0x564785caac44 in do_simple_func /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:95
      #13 0x564785cc18f0 in cmd_func(cmd_list_element*, char const*, int) /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:2735
      #14 0x564787c70c68 in execute_command(char const*, int) /tmp/src/binutils-gdb/gdb/top.c:574
      #15 0x564786686180 in command_handler(char const*) /tmp/src/binutils-gdb/gdb/event-top.c:543
      #16 0x56478668752f in command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /tmp/src/binutils-gdb/gdb/event-top.c:779
      #17 0x564787dcb29a in tui_command_line_handler /tmp/src/binutils-gdb/gdb/tui/tui-interp.c:104
      #18 0x56478668443d in gdb_rl_callback_handler /tmp/src/binutils-gdb/gdb/event-top.c:250
      #19 0x7f4efd506246 in rl_callback_read_char (/usr/lib/libreadline.so.8+0x3b246) (BuildId: 092e91fc4361b0ef94561e3ae03a75f69398acbb)

The problem is in cli/cli-cmds.c, in the function setting_cmd, where
we do this:

  const char *a0 = (const char *) argv[0]->contents ().data ();

Here argv[0] is a value* which we know is either a TYPE_CODE_ARRAY or
a TYPE_CODE_STRING.  The problem is that the above line is casting the
value contents directly to a C-string, i.e. one that is assumed to
have a null-terminator at the end.

After the above commit this can no longer be assumed to be true.  A
string value will be represented just as it would be in the current
language, so for Ada and Fortran the string will be an array of
characters with no null-terminator at the end.

My proposed solution is to copy the string contents into a std::string
object, and then use the std::string::c_str() value, this will ensure
that a null-terminator has been added.

I had a check through GDB at places TYPE_CODE_STRING was used and
couldn't see any other obvious places where this type of assumption
was being made, so hopefully this is the only offender.

Running the above test with ASan compiled in no longer gives an error.

Reviewed-By: Tom Tromey <[email protected]>
a4lg pushed a commit that referenced this pull request Jul 29, 2023
Expect a `.MIPS.options' section alternatively to `.reginfo' and ignore
contents of either as irrelevant for all the affected compact EH tests,
removing these regressions:

mips64-openbsd  -FAIL: Compact EH EB #1 with personality ID and FDE data
mips64-openbsd  -FAIL: Compact EH EB #2 with personality routine and FDE data
mips64-openbsd  -FAIL: Compact EH EB #3 with personality id and large FDE data
mips64-openbsd  -FAIL: Compact EH EB #4 with personality id, FDE data and LSDA
mips64-openbsd  -FAIL: Compact EH EB #5 with personality routine, FDE data and LSDA
mips64-openbsd  -FAIL: Compact EH EB #6 with personality id, LSDA and large FDE data
mips64-openbsd  -FAIL: Compact EH EL #1 with personality ID and FDE data
mips64-openbsd  -FAIL: Compact EH EL #2 with personality routine and FDE data
mips64-openbsd  -FAIL: Compact EH EL #3 with personality id and large FDE data
mips64-openbsd  -FAIL: Compact EH EL #4 with personality id, FDE data and LSDA
mips64-openbsd  -FAIL: Compact EH EL #5 with personality routine, FDE data and LSDA
mips64-openbsd  -FAIL: Compact EH EL #6 with personality id, LSDA and large FDE data
mips64el-openbsd  -FAIL: Compact EH EB #1 with personality ID and FDE data
mips64el-openbsd  -FAIL: Compact EH EB #2 with personality routine and FDE data
mips64el-openbsd  -FAIL: Compact EH EB #3 with personality id and large FDE data
mips64el-openbsd  -FAIL: Compact EH EB #4 with personality id, FDE data and LSDA
mips64el-openbsd  -FAIL: Compact EH EB #5 with personality routine, FDE data and LSDA
mips64el-openbsd  -FAIL: Compact EH EB #6 with personality id, LSDA and large FDE data
mips64el-openbsd  -FAIL: Compact EH EL #1 with personality ID and FDE data
mips64el-openbsd  -FAIL: Compact EH EL #2 with personality routine and FDE data
mips64el-openbsd  -FAIL: Compact EH EL #3 with personality id and large FDE data
mips64el-openbsd  -FAIL: Compact EH EL #4 with personality id, FDE data and LSDA
mips64el-openbsd  -FAIL: Compact EH EL #5 with personality routine, FDE data and LSDA
mips64el-openbsd  -FAIL: Compact EH EL #6 with personality id, LSDA and large FDE data

Co-Authored-By: Maciej W. Rozycki <[email protected]>

	gas/
	* testsuite/gas/mips/compact-eh-eb-1.d: Accept `.MIPS.options'
	section as an alternative to `.reginfo' and ignore contents of
	either.
	* testsuite/gas/mips/compact-eh-eb-2.d: Likewise.
	* testsuite/gas/mips/compact-eh-eb-3.d: Likewise.
	* testsuite/gas/mips/compact-eh-eb-4.d: Likewise.
	* testsuite/gas/mips/compact-eh-eb-5.d: Likewise.
	* testsuite/gas/mips/compact-eh-eb-6.d: Likewise.
	* testsuite/gas/mips/compact-eh-el-1.d: Likewise.
	* testsuite/gas/mips/compact-eh-el-2.d: Likewise.
	* testsuite/gas/mips/compact-eh-el-3.d: Likewise.
	* testsuite/gas/mips/compact-eh-el-4.d: Likewise.
	* testsuite/gas/mips/compact-eh-el-5.d: Likewise.
	* testsuite/gas/mips/compact-eh-el-6.d: Likewise.
a4lg pushed a commit that referenced this pull request Aug 6, 2023
With gdb build with -fsanitize=thread and test-case gdb.base/index-cache.exp I
run into:
...
(gdb) file build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache
Reading symbols from build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache...
(gdb) show index-cache enabled
The index cache is off.
(gdb) PASS: gdb.base/index-cache.exp: test_basic_stuff: index-cache is disabled by default
set index-cache enabled on
==================
WARNING: ThreadSanitizer: data race (pid=32248)
  Write of size 1 at 0x00000321f540 by main thread:
    #0 index_cache::enable() gdb/dwarf2/index-cache.c:76 (gdb+0x82cfdd)
    #1 set_index_cache_enabled_command gdb/dwarf2/index-cache.c:270 (gdb+0x82d9af)
    #2 bool setting::set<bool>(bool const&) gdb/command.h:353 (gdb+0x6fe5f2)
    #3 do_set_command(char const*, int, cmd_list_element*) gdb/cli/cli-setshow.c:414 (gdb+0x6fcd21)
    #4 execute_command(char const*, int) gdb/top.c:567 (gdb+0xff2e64)
    #5 command_handler(char const*) gdb/event-top.c:552 (gdb+0x94acc0)
    #6 command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) gdb/event-top.c:788 (gdb+0x94b37d)
    #7 tui_command_line_handler gdb/tui/tui-interp.c:104 (gdb+0x103467e)
    #8 gdb_rl_callback_handler gdb/event-top.c:259 (gdb+0x94a265)
    #9 rl_callback_read_char readline/readline/callback.c:290 (gdb+0x11bdd3f)
    #10 gdb_rl_callback_read_char_wrapper_noexcept gdb/event-top.c:195 (gdb+0x94a064)
    #11 gdb_rl_callback_read_char_wrapper gdb/event-top.c:234 (gdb+0x94a125)
    #12 stdin_event_handler gdb/ui.c:155 (gdb+0x1074922)
    #13 handle_file_event gdbsupport/event-loop.cc:573 (gdb+0x1d94de4)
    #14 gdb_wait_for_event gdbsupport/event-loop.cc:694 (gdb+0x1d9551c)
    #15 gdb_do_one_event(int) gdbsupport/event-loop.cc:264 (gdb+0x1d93908)
    #16 start_event_loop gdb/main.c:412 (gdb+0xb5a256)
    #17 captured_command_loop gdb/main.c:476 (gdb+0xb5a445)
    #18 captured_main gdb/main.c:1320 (gdb+0xb5c5c5)
    #19 gdb_main(captured_main_args*) gdb/main.c:1339 (gdb+0xb5c674)
    #20 main gdb/gdb.c:32 (gdb+0x416776)

  Previous read of size 1 at 0x00000321f540 by thread T12:
    #0 index_cache::enabled() const gdb/dwarf2/index-cache.h:48 (gdb+0x82e1a6)
    #1 index_cache::store(dwarf2_per_bfd*) gdb/dwarf2/index-cache.c:94 (gdb+0x82d0bc)
    #2 cooked_index::maybe_write_index(dwarf2_per_bfd*) gdb/dwarf2/cooked-index.c:638 (gdb+0x7f1b97)
    #3 operator() gdb/dwarf2/cooked-index.c:468 (gdb+0x7f0f24)
    #4 _M_invoke /usr/include/c++/7/bits/std_function.h:316 (gdb+0x7f285b)
    #5 std::function<void ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x700952)
    #6 void std::__invoke_impl<void, std::function<void ()>&>(std::__invoke_other, std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:60 (gdb+0x7381a0)
    #7 std::__invoke_result<std::function<void ()>&>::type std::__invoke<std::function<void ()>&>(std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x737e91)
    #8 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}::operator()() const /usr/include/c++/7/future:1421 (gdb+0x737b59)
    #9 std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void>::operator()() const /usr/include/c++/7/future:1362 (gdb+0x738660)
    #10 std::_Function_handler<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> (), std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void> >::_M_invoke(std::_Any_data const&) /usr/include/c++/7/bits/std_function.h:302 (gdb+0x73825c)
    #11 std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x733623)
    #12 std::__future_base::_State_baseV2::_M_do_set(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*) /usr/include/c++/7/future:561 (gdb+0x732bdf)
    #13 void std::__invoke_impl<void, void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::__invoke_memfun_deref, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x734c4f)
    #14 std::__invoke_result<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>::type std::__invoke<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x733bc5)
    #15 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#1}::operator()() const /usr/include/c++/7/mutex:672 (gdb+0x73300d)
    #16 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::operator()() const /usr/include/c++/7/mutex:677 (gdb+0x7330b2)
    #17 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::_FUN() /usr/include/c++/7/mutex:677 (gdb+0x7330f2)
    #18 pthread_once <null> (libtsan.so.0+0x4457c)
    #19 __gthread_once /usr/include/c++/7/x86_64-suse-linux/bits/gthr-default.h:699 (gdb+0x72f5dd)
    #20 void std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/mutex:684 (gdb+0x733224)
    #21 std::__future_base::_State_baseV2::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>, bool) /usr/include/c++/7/future:401 (gdb+0x732852)
    #22 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run() /usr/include/c++/7/future:1423 (gdb+0x737bef)
    #23 std::packaged_task<void ()>::operator()() /usr/include/c++/7/future:1556 (gdb+0x1dac492)
    #24 gdb::thread_pool::thread_function() gdbsupport/thread-pool.cc:242 (gdb+0x1dabdb4)
    #25 void std::__invoke_impl<void, void (gdb::thread_pool::*)(), gdb::thread_pool*>(std::__invoke_memfun_deref, void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x1dace63)
    #26 std::__invoke_result<void (gdb::thread_pool::*)(), gdb::thread_pool*>::type std::__invoke<void (gdb::thread_pool::*)(), gdb::thread_pool*>(void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x1dac294)
    #27 decltype (__invoke((_S_declval<0ul>)(), (_S_declval<1ul>)())) std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/include/c++/7/thread:234 (gdb+0x1daf5c6)
    #28 std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::operator()() /usr/include/c++/7/thread:243 (gdb+0x1daf551)
    #29 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> > >::_M_run() /usr/include/c++/7/thread:186 (gdb+0x1daf506)
    #30 <null> <null> (libstdc++.so.6+0xdcac2)

  Location is global 'global_index_cache' of size 48 at 0x00000321f520 (gdb+0x00000321f540)
  ...
SUMMARY: ThreadSanitizer: data race gdb/dwarf2/index-cache.c:76 in index_cache::enable()
...

The race happens when issuing a "file $exec" command followed by a
"set index-cache enabled on" command.

The race is between:
- a worker thread reading index_cache::m_enabled to determine whether an
  index-cache entry for $exec needs to be written
  (due to command "file $exec"), and
- the main thread setting index_cache::m_enabled
  (due to command "set index-cache enabled on").

Fix this by capturing the value of index_cache::m_enabled in the main thread,
and using the captured value in the worker thread.

Tested on x86_64-linux.

PR symtab/30392
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30392
a4lg pushed a commit that referenced this pull request Aug 6, 2023
With gdb build with -fsanitize=thread and test-case gdb.base/index-cache.exp I
run into:
...
(gdb) file build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache
Reading symbols from build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache...
==================
WARNING: ThreadSanitizer: data race (pid=12261)
  Write of size 4 at 0x7b4400097d08 by main thread:
    #0 bfd_open_file bfd/cache.c:584 (gdb+0x148bb92)
    #1 bfd_cache_lookup_worker bfd/cache.c:261 (gdb+0x148b12a)
    #2 cache_bseek bfd/cache.c:289 (gdb+0x148b324)
    #3 bfd_seek bfd/bfdio.c:459 (gdb+0x1489c31)
    #4 _bfd_generic_get_section_contents bfd/libbfd.c:1069 (gdb+0x14977a4)
    #5 bfd_get_section_contents bfd/section.c:1606 (gdb+0x149cc7c)
    #6 gdb_bfd_scan_elf_dyntag(int, bfd*, unsigned long*, unsigned long*) gdb/solib.c:1601 (gdb+0xed8eca)
    #7 elf_locate_base gdb/solib-svr4.c:705 (gdb+0xec28ac)
    #8 svr4_iterate_over_objfiles_in_search_order gdb/solib-svr4.c:3430 (gdb+0xeca55d)
    #9 gdbarch_iterate_over_objfiles_in_search_order(gdbarch*, gdb::function_view<bool (objfile*)>, objfile*) gdb/gdbarch.c:5041 (gdb+0x537cad)
    #10 find_main_name gdb/symtab.c:6270 (gdb+0xf743a5)
    #11 main_language() gdb/symtab.c:6313 (gdb+0xf74499)
    #12 set_initial_language() gdb/symfile.c:1700 (gdb+0xf4285c)
    #13 symbol_file_add_main_1 gdb/symfile.c:1212 (gdb+0xf40e2a)
    #14 symbol_file_command(char const*, int) gdb/symfile.c:1681 (gdb+0xf427d1)
    #15 file_command gdb/exec.c:554 (gdb+0x94f74b)
    #16 do_simple_func gdb/cli/cli-decode.c:95 (gdb+0x6d9528)
    #17 cmd_func(cmd_list_element*, char const*, int) gdb/cli/cli-decode.c:2735 (gdb+0x6e0f69)
    #18 execute_command(char const*, int) gdb/top.c:575 (gdb+0xff303c)
    #19 command_handler(char const*) gdb/event-top.c:552 (gdb+0x94adde)
    #20 command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) gdb/event-top.c:788 (gdb+0x94b49b)
    #21 tui_command_line_handler gdb/tui/tui-interp.c:104 (gdb+0x103479c)
    #22 gdb_rl_callback_handler gdb/event-top.c:259 (gdb+0x94a383)
    #23 rl_callback_read_char readline/readline/callback.c:290 (gdb+0x11bde5d)
    #24 gdb_rl_callback_read_char_wrapper_noexcept gdb/event-top.c:195 (gdb+0x94a182)
    #25 gdb_rl_callback_read_char_wrapper gdb/event-top.c:234 (gdb+0x94a243)
    #26 stdin_event_handler gdb/ui.c:155 (gdb+0x1074a40)
    #27 handle_file_event gdbsupport/event-loop.cc:573 (gdb+0x1d94f02)
    #28 gdb_wait_for_event gdbsupport/event-loop.cc:694 (gdb+0x1d9563a)
    #29 gdb_do_one_event(int) gdbsupport/event-loop.cc:264 (gdb+0x1d93a26)
    #30 start_event_loop gdb/main.c:412 (gdb+0xb5a374)
    #31 captured_command_loop gdb/main.c:476 (gdb+0xb5a563)
    #32 captured_main gdb/main.c:1320 (gdb+0xb5c6e3)
    #33 gdb_main(captured_main_args*) gdb/main.c:1339 (gdb+0xb5c792)
    #34 main gdb/gdb.c:32 (gdb+0x416776)

  Previous read of size 1 at 0x7b4400097d08 by thread T12:
    #0 bfd_check_format_matches bfd/format.c:323 (gdb+0x1492db4)
    #1 bfd_check_format bfd/format.c:94 (gdb+0x1492104)
    #2 build_id_bfd_get(bfd*) gdb/build-id.c:42 (gdb+0x6648f7)
    #3 index_cache::store(dwarf2_per_bfd*, index_cache_store_context*) gdb/dwarf2/index-cache.c:110 (gdb+0x82d205)
    #4 cooked_index::maybe_write_index(dwarf2_per_bfd*) gdb/dwarf2/cooked-index.c:640 (gdb+0x7f1bf1)
    #5 operator() gdb/dwarf2/cooked-index.c:470 (gdb+0x7f0f40)
    #6 _M_invoke /usr/include/c++/7/bits/std_function.h:316 (gdb+0x7f28f7)
    #7 std::function<void ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x700952)
    #8 void std::__invoke_impl<void, std::function<void ()>&>(std::__invoke_other, std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:60 (gdb+0x7381a0)
    #9 std::__invoke_result<std::function<void ()>&>::type std::__invoke<std::function<void ()>&>(std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x737e91)
    #10 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}::operator()() const /usr/include/c++/7/future:1421 (gdb+0x737b59)
    #11 std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void>::operator()() const /usr/include/c++/7/future:1362 (gdb+0x738660)
    #12 std::_Function_handler<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> (), std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void> >::_M_invoke(std::_Any_data const&) /usr/include/c++/7/bits/std_function.h:302 (gdb+0x73825c)
    #13 std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x733623)
    #14 std::__future_base::_State_baseV2::_M_do_set(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*) /usr/include/c++/7/future:561 (gdb+0x732bdf)
    #15 void std::__invoke_impl<void, void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::__invoke_memfun_deref, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x734c4f)
    #16 std::__invoke_result<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>::type std::__invoke<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x733bc5)
    #17 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#1}::operator()() const /usr/include/c++/7/mutex:672 (gdb+0x73300d)
    #18 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::operator()() const /usr/include/c++/7/mutex:677 (gdb+0x7330b2)
    #19 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::_FUN() /usr/include/c++/7/mutex:677 (gdb+0x7330f2)
    #20 pthread_once <null> (libtsan.so.0+0x4457c)
    #21 __gthread_once /usr/include/c++/7/x86_64-suse-linux/bits/gthr-default.h:699 (gdb+0x72f5dd)
    #22 void std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/mutex:684 (gdb+0x733224)
    #23 std::__future_base::_State_baseV2::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>, bool) /usr/include/c++/7/future:401 (gdb+0x732852)
    #24 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run() /usr/include/c++/7/future:1423 (gdb+0x737bef)
    #25 std::packaged_task<void ()>::operator()() /usr/include/c++/7/future:1556 (gdb+0x1dac5b0)
    #26 gdb::thread_pool::thread_function() gdbsupport/thread-pool.cc:242 (gdb+0x1dabed2)
    #27 void std::__invoke_impl<void, void (gdb::thread_pool::*)(), gdb::thread_pool*>(std::__invoke_memfun_deref, void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x1dacf81)
    #28 std::__invoke_result<void (gdb::thread_pool::*)(), gdb::thread_pool*>::type std::__invoke<void (gdb::thread_pool::*)(), gdb::thread_pool*>(void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x1dac3b2)
    #29 decltype (__invoke((_S_declval<0ul>)(), (_S_declval<1ul>)())) std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/include/c++/7/thread:234 (gdb+0x1daf6e4)
    #30 std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::operator()() /usr/include/c++/7/thread:243 (gdb+0x1daf66f)
    #31 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> > >::_M_run() /usr/include/c++/7/thread:186 (gdb+0x1daf624)
    #32 <null> <null> (libstdc++.so.6+0xdcac2)
  ...
SUMMARY: ThreadSanitizer: data race bfd/cache.c:584 in bfd_open_file
...

The race happens when issuing the "file $exec" command.

The race is between:
- a worker thread getting the build id while writing the index cache, and in
  the process reading bfd::format, and
- the main thread calling find_main_name, and in the process setting
  bfd::cacheable.

The two bitfields bfd::cacheable and bfd::format share the same bitfield
container.

Fix this by capturing the build id in the main thread, and using the captured
value in the worker thread.

Likewise for the dwz build id, which likely suffers from the same issue.

While we're at it, also move the creation of the cache directory to
the index_cache_store_context constructor, to:
- make sure there's no race between subsequent file commands, and
- issue any related warning or error messages during the file command.

Tested on x86_64-linux.

Approved-By: Tom Tromey <[email protected]>

PR symtab/30392
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30392
a4lg pushed a commit that referenced this pull request Aug 6, 2023
With gdb build with -fsanitize=thread and test-case gdb.base/index-cache.exp I
run into:
...
(gdb) file build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache
Reading symbols from build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache...
==================
WARNING: ThreadSanitizer: data race (pid=24296)
  Write of size 1 at 0x7b200000420d by main thread:
    #0 queue_comp_unit gdb/dwarf2/read.c:5564 (gdb+0x8939ce)
    #1 dw2_do_instantiate_symtab gdb/dwarf2/read.c:1754 (gdb+0x885b96)
    #2 dw2_instantiate_symtab gdb/dwarf2/read.c:1792 (gdb+0x885d86)
    #3 dw2_expand_symtabs_matching_one(dwarf2_per_cu_data*, dwarf2_per_objfile*, gdb::function_view<bool (char const*, bool)>, gdb::function_view<bool (compunit_symtab*)>) gdb/dwarf2/read.c:3042 (gdb+0x88ac77)
    #4 cooked_index_functions::expand_symtabs_matching(objfile*, gdb::function_view<bool (char const*, bool)>, lookup_name_info const*, gdb::function_view<bool (char const*)>, gdb::function_view<bool (compunit_symtab*)>, enum_flags<block_search_flag_values>, domain_enum, search_domain) gdb/dwarf2/read.c:16915 (gdb+0x8c1c8a)
    #5 objfile::lookup_symbol(block_enum, char const*, domain_enum) gdb/symfile-debug.c:288 (gdb+0xf389a1)
    #6 lookup_symbol_via_quick_fns gdb/symtab.c:2385 (gdb+0xf66403)
    #7 lookup_symbol_in_objfile gdb/symtab.c:2516 (gdb+0xf66a67)
    #8 operator() gdb/symtab.c:2562 (gdb+0xf66bbe)
    #9 operator() gdb/../gdbsupport/function-view.h:305 (gdb+0xf76ffd)
    #10 _FUN gdb/../gdbsupport/function-view.h:299 (gdb+0xf77054)
    #11 gdb::function_view<bool (objfile*)>::operator()(objfile*) const gdb/../gdbsupport/function-view.h:289 (gdb+0xc3f5e3)
    #12 svr4_iterate_over_objfiles_in_search_order gdb/solib-svr4.c:3455 (gdb+0xeca793)
    #13 gdbarch_iterate_over_objfiles_in_search_order(gdbarch*, gdb::function_view<bool (objfile*)>, objfile*) gdb/gdbarch.c:5041 (gdb+0x537cad)
    #14 lookup_global_or_static_symbol gdb/symtab.c:2559 (gdb+0xf66e47)
    #15 lookup_global_symbol(char const*, block const*, domain_enum) gdb/symtab.c:2615 (gdb+0xf670cc)
    #16 language_defn::lookup_symbol_nonlocal(char const*, block const*, domain_enum) const gdb/symtab.c:2447 (gdb+0xf666ba)
    #17 lookup_symbol_aux gdb/symtab.c:2123 (gdb+0xf655ff)
    #18 lookup_symbol_in_language(char const*, block const*, domain_enum, language, field_of_this_result*) gdb/symtab.c:1931 (gdb+0xf646f7)
    #19 set_initial_language() gdb/symfile.c:1708 (gdb+0xf429c0)
    #20 symbol_file_add_main_1 gdb/symfile.c:1212 (gdb+0xf40f54)
    #21 symbol_file_command(char const*, int) gdb/symfile.c:1681 (gdb+0xf428fb)
    #22 file_command gdb/exec.c:554 (gdb+0x94f875)
    #23 do_simple_func gdb/cli/cli-decode.c:95 (gdb+0x6d9528)
    #24 cmd_func(cmd_list_element*, char const*, int) gdb/cli/cli-decode.c:2735 (gdb+0x6e0f69)
    #25 execute_command(char const*, int) gdb/top.c:575 (gdb+0xff3166)
    #26 command_handler(char const*) gdb/event-top.c:552 (gdb+0x94af08)
    #27 command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) gdb/event-top.c:788 (gdb+0x94b5c5)
    #28 tui_command_line_handler gdb/tui/tui-interp.c:104 (gdb+0x10348c6)
    #29 gdb_rl_callback_handler gdb/event-top.c:259 (gdb+0x94a4ad)
    #30 rl_callback_read_char readline/readline/callback.c:290 (gdb+0x11bdf87)
    #31 gdb_rl_callback_read_char_wrapper_noexcept gdb/event-top.c:195 (gdb+0x94a2ac)
    #32 gdb_rl_callback_read_char_wrapper gdb/event-top.c:234 (gdb+0x94a36d)
    #33 stdin_event_handler gdb/ui.c:155 (gdb+0x1074b6a)
    #34 handle_file_event gdbsupport/event-loop.cc:573 (gdb+0x1d9502c)
    #35 gdb_wait_for_event gdbsupport/event-loop.cc:694 (gdb+0x1d95764)
    #36 gdb_do_one_event(int) gdbsupport/event-loop.cc:264 (gdb+0x1d93b50)
    #37 start_event_loop gdb/main.c:412 (gdb+0xb5a49e)
    #38 captured_command_loop gdb/main.c:476 (gdb+0xb5a68d)
    #39 captured_main gdb/main.c:1320 (gdb+0xb5c80d)
    #40 gdb_main(captured_main_args*) gdb/main.c:1339 (gdb+0xb5c8bc)
    #41 main gdb/gdb.c:32 (gdb+0x416776)

  Previous read of size 1 at 0x7b200000420d by thread T12:
    #0 write_gdbindex gdb/dwarf2/index-write.c:1229 (gdb+0x8310c8)
    #1 write_dwarf_index(dwarf2_per_bfd*, char const*, char const*, char const*, dw_index_kind) gdb/dwarf2/index-write.c:1484 (gdb+0x83232f)
    #2 index_cache::store(dwarf2_per_bfd*, index_cache_store_context*) gdb/dwarf2/index-cache.c:177 (gdb+0x82d62b)
    #3 cooked_index::maybe_write_index(dwarf2_per_bfd*) gdb/dwarf2/cooked-index.c:640 (gdb+0x7f1bf7)
    #4 operator() gdb/dwarf2/cooked-index.c:470 (gdb+0x7f0f40)
    #5 _M_invoke /usr/include/c++/7/bits/std_function.h:316 (gdb+0x7f2909)
    #6 std::function<void ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x700952)
    #7 void std::__invoke_impl<void, std::function<void ()>&>(std::__invoke_other, std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:60 (gdb+0x7381a0)
    #8 std::__invoke_result<std::function<void ()>&>::type std::__invoke<std::function<void ()>&>(std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x737e91)
    #9 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}::operator()() const /usr/include/c++/7/future:1421 (gdb+0x737b59)
    #10 std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void>::operator()() const /usr/include/c++/7/future:1362 (gdb+0x738660)
    #11 std::_Function_handler<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> (), std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void> >::_M_invoke(std::_Any_data const&) /usr/include/c++/7/bits/std_function.h:302 (gdb+0x73825c)
    #12 std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x733623)
    #13 std::__future_base::_State_baseV2::_M_do_set(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*) /usr/include/c++/7/future:561 (gdb+0x732bdf)
    #14 void std::__invoke_impl<void, void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::__invoke_memfun_deref, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x734c4f)
    #15 std::__invoke_result<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>::type std::__invoke<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x733bc5)
    #16 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#1}::operator()() const /usr/include/c++/7/mutex:672 (gdb+0x73300d)
    #17 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::operator()() const /usr/include/c++/7/mutex:677 (gdb+0x7330b2)
    #18 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::_FUN() /usr/include/c++/7/mutex:677 (gdb+0x7330f2)
    #19 pthread_once <null> (libtsan.so.0+0x4457c)
    #20 __gthread_once /usr/include/c++/7/x86_64-suse-linux/bits/gthr-default.h:699 (gdb+0x72f5dd)
    #21 void std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/mutex:684 (gdb+0x733224)
    #22 std::__future_base::_State_baseV2::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>, bool) /usr/include/c++/7/future:401 (gdb+0x732852)
    #23 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run() /usr/include/c++/7/future:1423 (gdb+0x737bef)
    #24 std::packaged_task<void ()>::operator()() /usr/include/c++/7/future:1556 (gdb+0x1dac6da)
    #25 gdb::thread_pool::thread_function() gdbsupport/thread-pool.cc:242 (gdb+0x1dabffc)
    #26 void std::__invoke_impl<void, void (gdb::thread_pool::*)(), gdb::thread_pool*>(std::__invoke_memfun_deref, void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x1dad0ab)
    #27 std::__invoke_result<void (gdb::thread_pool::*)(), gdb::thread_pool*>::type std::__invoke<void (gdb::thread_pool::*)(), gdb::thread_pool*>(void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x1dac4dc)
    #28 decltype (__invoke((_S_declval<0ul>)(), (_S_declval<1ul>)())) std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/include/c++/7/thread:234 (gdb+0x1daf80e)
    #29 std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::operator()() /usr/include/c++/7/thread:243 (gdb+0x1daf799)
    #30 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> > >::_M_run() /usr/include/c++/7/thread:186 (gdb+0x1daf74e)
    #31 <null> <null> (libstdc++.so.6+0xdcac2)
 ...
SUMMARY: ThreadSanitizer: data race gdb/dwarf2/read.c:5564 in queue_comp_unit
...

The race happens when issuing the "file $exec" command.

The race is between:
- a worker thread writing the index cache, and in the process reading
  dwarf2_per_cu_data::is_debug_type, and
- the main thread expanding the CU containing main, and in the process setting
  dwarf2_per_cu_data::queued.

The two bitfields dwarf2_per_cu_data::queue and
dwarf2_per_cu_data::is_debug_type share the same bitfield container.

Fix this by making dwarf2_per_cu_data::queued a packed<bool, 1>.

Tested on x86_64-linux.

Approved-By: Tom Tromey <[email protected]>

PR symtab/30392
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30392
a4lg pushed a commit that referenced this pull request Aug 6, 2023
…s_debug_type}

With gdb build with -fsanitize=thread and test-case gdb.base/index-cache.exp
and target board debug-types, I run into:
...
(gdb) file build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache
Reading symbols from build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache...
==================
WARNING: ThreadSanitizer: data race (pid=9654)
  Write of size 1 at 0x7b200000420d by main thread:
    #0 dwarf2_per_cu_data::get_header() const gdb/dwarf2/read.c:21513 (gdb+0x8d1eee)
    #1 dwarf2_per_cu_data::addr_size() const gdb/dwarf2/read.c:21524 (gdb+0x8d1f4e)
    #2 dwarf2_cu::addr_type() const gdb/dwarf2/cu.c:112 (gdb+0x806327)
    #3 set_die_type gdb/dwarf2/read.c:21932 (gdb+0x8d3870)
    #4 read_base_type gdb/dwarf2/read.c:15448 (gdb+0x8bcacb)
    #5 read_type_die_1 gdb/dwarf2/read.c:19832 (gdb+0x8cc0a5)
    #6 read_type_die gdb/dwarf2/read.c:19767 (gdb+0x8cbe6d)
    #7 lookup_die_type gdb/dwarf2/read.c:19739 (gdb+0x8cbdc7)
    #8 die_type gdb/dwarf2/read.c:19593 (gdb+0x8cb68a)
    #9 read_subroutine_type gdb/dwarf2/read.c:14648 (gdb+0x8b998e)
    #10 read_type_die_1 gdb/dwarf2/read.c:19792 (gdb+0x8cbf2f)
    #11 read_type_die gdb/dwarf2/read.c:19767 (gdb+0x8cbe6d)
    #12 read_func_scope gdb/dwarf2/read.c:10154 (gdb+0x8a4f36)
    #13 process_die gdb/dwarf2/read.c:6667 (gdb+0x898daa)
    #14 read_file_scope gdb/dwarf2/read.c:7682 (gdb+0x89bad8)
    #15 process_die gdb/dwarf2/read.c:6654 (gdb+0x898ced)
    #16 process_full_comp_unit gdb/dwarf2/read.c:6418 (gdb+0x8981de)
    #17 process_queue gdb/dwarf2/read.c:5690 (gdb+0x894433)
    #18 dw2_do_instantiate_symtab gdb/dwarf2/read.c:1770 (gdb+0x88623a)
    #19 dw2_instantiate_symtab gdb/dwarf2/read.c:1792 (gdb+0x886300)
    #20 dw2_expand_symtabs_matching_one(dwarf2_per_cu_data*, dwarf2_per_objfile*, gdb::function_view<bool (char const*, bool)>, gdb::function_view<bool (compunit_symtab*)>) gdb/dwarf2/read.c:3042 (gdb+0x88b1f1)
    #21 cooked_index_functions::expand_symtabs_matching(objfile*, gdb::function_view<bool (char const*, bool)>, lookup_name_info const*, gdb::function_view<bool (char const*)>, gdb::function_view<bool (compunit_symtab*)>, enum_flags<block_search_flag_values>, domain_enum, search_domain) gdb/dwarf2/read.c:16917 (gdb+0x8c228e)
    #22 objfile::lookup_symbol(block_enum, char const*, domain_enum) gdb/symfile-debug.c:288 (gdb+0xf39055)
    #23 lookup_symbol_via_quick_fns gdb/symtab.c:2385 (gdb+0xf66ab7)
    #24 lookup_symbol_in_objfile gdb/symtab.c:2516 (gdb+0xf6711b)
    #25 operator() gdb/symtab.c:2562 (gdb+0xf67272)
    #26 operator() gdb/../gdbsupport/function-view.h:305 (gdb+0xf776b1)
    #27 _FUN gdb/../gdbsupport/function-view.h:299 (gdb+0xf77708)
    #28 gdb::function_view<bool (objfile*)>::operator()(objfile*) const gdb/../gdbsupport/function-view.h:289 (gdb+0xc3fc97)
    #29 svr4_iterate_over_objfiles_in_search_order gdb/solib-svr4.c:3455 (gdb+0xecae47)
    #30 gdbarch_iterate_over_objfiles_in_search_order(gdbarch*, gdb::function_view<bool (objfile*)>, objfile*) gdb/gdbarch.c:5041 (gdb+0x537cad)
    #31 lookup_global_or_static_symbol gdb/symtab.c:2559 (gdb+0xf674fb)
    #32 lookup_global_symbol(char const*, block const*, domain_enum) gdb/symtab.c:2615 (gdb+0xf67780)
    #33 language_defn::lookup_symbol_nonlocal(char const*, block const*, domain_enum) const gdb/symtab.c:2447 (gdb+0xf66d6e)
    #34 lookup_symbol_aux gdb/symtab.c:2123 (gdb+0xf65cb3)
    #35 lookup_symbol_in_language(char const*, block const*, domain_enum, language, field_of_this_result*) gdb/symtab.c:1931 (gdb+0xf64dab)
    #36 set_initial_language() gdb/symfile.c:1708 (gdb+0xf43074)
    #37 symbol_file_add_main_1 gdb/symfile.c:1212 (gdb+0xf41608)
    #38 symbol_file_command(char const*, int) gdb/symfile.c:1681 (gdb+0xf42faf)
    #39 file_command gdb/exec.c:554 (gdb+0x94ff29)
    #40 do_simple_func gdb/cli/cli-decode.c:95 (gdb+0x6d9528)
    #41 cmd_func(cmd_list_element*, char const*, int) gdb/cli/cli-decode.c:2735 (gdb+0x6e0f69)
    #42 execute_command(char const*, int) gdb/top.c:575 (gdb+0xff379c)
    #43 command_handler(char const*) gdb/event-top.c:552 (gdb+0x94b5bc)
    #44 command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) gdb/event-top.c:788 (gdb+0x94bc79)
    #45 tui_command_line_handler gdb/tui/tui-interp.c:104 (gdb+0x1034efc)
    #46 gdb_rl_callback_handler gdb/event-top.c:259 (gdb+0x94ab61)
    #47 rl_callback_read_char readline/readline/callback.c:290 (gdb+0x11be4ef)
    #48 gdb_rl_callback_read_char_wrapper_noexcept gdb/event-top.c:195 (gdb+0x94a960)
    #49 gdb_rl_callback_read_char_wrapper gdb/event-top.c:234 (gdb+0x94aa21)
    #50 stdin_event_handler gdb/ui.c:155 (gdb+0x10751a0)
    #51 handle_file_event gdbsupport/event-loop.cc:573 (gdb+0x1d95bac)
    #52 gdb_wait_for_event gdbsupport/event-loop.cc:694 (gdb+0x1d962e4)
    #53 gdb_do_one_event(int) gdbsupport/event-loop.cc:264 (gdb+0x1d946d0)
    #54 start_event_loop gdb/main.c:412 (gdb+0xb5ab52)
    #55 captured_command_loop gdb/main.c:476 (gdb+0xb5ad41)
    #56 captured_main gdb/main.c:1320 (gdb+0xb5cec1)
    #57 gdb_main(captured_main_args*) gdb/main.c:1339 (gdb+0xb5cf70)
    #58 main gdb/gdb.c:32 (gdb+0x416776)

  Previous read of size 1 at 0x7b200000420d by thread T11:
    #0 write_gdbindex gdb/dwarf2/index-write.c:1229 (gdb+0x831630)
    #1 write_dwarf_index(dwarf2_per_bfd*, char const*, char const*, char const*, dw_index_kind) gdb/dwarf2/index-write.c:1484 (gdb+0x832897)
    #2 index_cache::store(dwarf2_per_bfd*, index_cache_store_context const&) gdb/dwarf2/index-cache.c:173 (gdb+0x82db8d)
    #3 cooked_index::maybe_write_index(dwarf2_per_bfd*, index_cache_store_context const&) gdb/dwarf2/cooked-index.c:645 (gdb+0x7f1d49)
    #4 operator() gdb/dwarf2/cooked-index.c:474 (gdb+0x7f0f31)
    #5 _M_invoke /usr/include/c++/7/bits/std_function.h:316 (gdb+0x7f2a13)
    #6 std::function<void ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x700952)
    #7 void std::__invoke_impl<void, std::function<void ()>&>(std::__invoke_other, std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:60 (gdb+0x7381a0)
    #8 std::__invoke_result<std::function<void ()>&>::type std::__invoke<std::function<void ()>&>(std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x737e91)
    #9 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}::operator()() const /usr/include/c++/7/future:1421 (gdb+0x737b59)
    #10 std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void>::operator()() const /usr/include/c++/7/future:1362 (gdb+0x738660)
    #11 std::_Function_handler<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> (), std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void> >::_M_invoke(std::_Any_data const&) /usr/include/c++/7/bits/std_function.h:302 (gdb+0x73825c)
    #12 std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x733623)
    #13 std::__future_base::_State_baseV2::_M_do_set(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*) /usr/include/c++/7/future:561 (gdb+0x732bdf)
    #14 void std::__invoke_impl<void, void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::__invoke_memfun_deref, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x734c4f)
    #15 std::__invoke_result<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>::type std::__invoke<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x733bc5)
    #16 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#1}::operator()() const /usr/include/c++/7/mutex:672 (gdb+0x73300d)
    #17 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::operator()() const /usr/include/c++/7/mutex:677 (gdb+0x7330b2)
    #18 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::_FUN() /usr/include/c++/7/mutex:677 (gdb+0x7330f2)
    #19 pthread_once <null> (libtsan.so.0+0x4457c)
    #20 __gthread_once /usr/include/c++/7/x86_64-suse-linux/bits/gthr-default.h:699 (gdb+0x72f5dd)
    #21 void std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/mutex:684 (gdb+0x733224)
    #22 std::__future_base::_State_baseV2::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>, bool) /usr/include/c++/7/future:401 (gdb+0x732852)
    #23 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run() /usr/include/c++/7/future:1423 (gdb+0x737bef)
    #24 std::packaged_task<void ()>::operator()() /usr/include/c++/7/future:1556 (gdb+0x1dad25a)
    #25 gdb::thread_pool::thread_function() gdbsupport/thread-pool.cc:242 (gdb+0x1dacb7c)
    #26 void std::__invoke_impl<void, void (gdb::thread_pool::*)(), gdb::thread_pool*>(std::__invoke_memfun_deref, void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x1dadc2b)
    #27 std::__invoke_result<void (gdb::thread_pool::*)(), gdb::thread_pool*>::type std::__invoke<void (gdb::thread_pool::*)(), gdb::thread_pool*>(void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x1dad05c)
    #28 decltype (__invoke((_S_declval<0ul>)(), (_S_declval<1ul>)())) std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/include/c++/7/thread:234 (gdb+0x1db038e)
    #29 std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::operator()() /usr/include/c++/7/thread:243 (gdb+0x1db0319)
    #30 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> > >::_M_run() /usr/include/c++/7/thread:186 (gdb+0x1db02ce)
    #31 <null> <null> (libstdc++.so.6+0xdcac2)
  ...
SUMMARY: ThreadSanitizer: data race gdb/dwarf2/read.c:21513 in dwarf2_per_cu_data::get_header() const
...

The race happens when issuing the "file $exec" command.

The race is between:
- a worker thread writing the index cache, and in the process reading
   dwarf2_per_cu_data::is_debug_type, and
- the main thread writing to dwarf2_per_cu_data::m_header_read_in.

The two bitfields dwarf2_per_cu_data::m_header_read_in and
dwarf2_per_cu_data::is_debug_type share the same bitfield container.

Fix this by making dwarf2_per_cu_data::m_header_read_in a packed<bool, 1>.

Tested on x86_64-linux.

Approved-By: Tom Tromey <[email protected]>

PR symtab/30392
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30392
a4lg pushed a commit that referenced this pull request Aug 6, 2023
With gdb build with -fsanitize=thread, and the exec from test-case
gdb.base/index-cache.exp, I run into:
...
$ rm -f ~/.cache/gdb/*; \
  gdb -q -batch -iex "set index-cache enabled on" index-cache \
    -ex "print foobar"
  ...
WARNING: ThreadSanitizer: data race (pid=23970)
  Write of size 1 at 0x7b200000410d by main thread:
    #0 dw_expand_symtabs_matching_file_matcher(dwarf2_per_objfile*, gdb::function_view<bool (char const*, bool)>) gdb/dwarf2/read.c:3077 (gdb+0x7ac54e)
    #1 cooked_index_functions::expand_symtabs_matching(objfile*, gdb::function_view<bool (char const*, bool)>, lookup_name_info const*, gdb::function_view<bool (char const*)>, gdb::function_view<bool (compunit_symtab*)>, enum_flags<block_search_flag_values>, domain_enum, search_domain) gdb/dwarf2/read.c:16812 (gdb+0x7d039f)
    #2 objfile::map_symtabs_matching_filename(char const*, char const*, gdb::function_view<bool (symtab*)>) gdb/symfile-debug.c:219 (gdb+0xda5aee)
    #3 iterate_over_symtabs(char const*, gdb::function_view<bool (symtab*)>) gdb/symtab.c:648 (gdb+0xdc439d)
    #4 lookup_symtab(char const*) gdb/symtab.c:662 (gdb+0xdc44a2)
    #5 classify_name gdb/c-exp.y:3083 (gdb+0x61afec)
    #6 c_yylex gdb/c-exp.y:3251 (gdb+0x61dd13)
    #7 c_yyparse() build/gdb/c-exp.c.tmp:1988 (gdb+0x61f07e)
    #8 c_parse(parser_state*) gdb/c-exp.y:3417 (gdb+0x62d864)
    #9 language_defn::parser(parser_state*) const gdb/language.c:598 (gdb+0x9771c5)
    #10 parse_exp_in_context gdb/parse.c:414 (gdb+0xb10a9b)
    #11 parse_expression(char const*, innermost_block_tracker*, enum_flags<parser_flag>) gdb/parse.c:462 (gdb+0xb110ae)
    #12 process_print_command_args gdb/printcmd.c:1321 (gdb+0xb4bf0c)
    #13 print_command_1 gdb/printcmd.c:1335 (gdb+0xb4ca2a)
    #14 print_command gdb/printcmd.c:1468 (gdb+0xb4cd5a)
    #15 do_simple_func gdb/cli/cli-decode.c:95 (gdb+0x65b078)
    #16 cmd_func(cmd_list_element*, char const*, int) gdb/cli/cli-decode.c:2735 (gdb+0x65ed53)
    #17 execute_command(char const*, int) gdb/top.c:575 (gdb+0xe3a76a)
    #18 catch_command_errors gdb/main.c:518 (gdb+0xa1837d)
    #19 execute_cmdargs gdb/main.c:617 (gdb+0xa1853f)
    #20 captured_main_1 gdb/main.c:1289 (gdb+0xa1aa58)
    #21 captured_main gdb/main.c:1310 (gdb+0xa1b95a)
    #22 gdb_main(captured_main_args*) gdb/main.c:1339 (gdb+0xa1b95a)
    #23 main gdb/gdb.c:39 (gdb+0x42506a)

  Previous read of size 1 at 0x7b200000410d by thread T1:
    #0 write_gdbindex gdb/dwarf2/index-write.c:1214 (gdb+0x75bb30)
    #1 write_dwarf_index(dwarf2_per_bfd*, char const*, char const*, char const*, dw_index_kind) gdb/dwarf2/index-write.c:1469 (gdb+0x75f803)
    #2 index_cache::store(dwarf2_per_bfd*, index_cache_store_context const&) gdb/dwarf2/index-cache.c:173 (gdb+0x755a36)
    #3 cooked_index::maybe_write_index(dwarf2_per_bfd*, index_cache_store_context const&) gdb/dwarf2/cooked-index.c:642 (gdb+0x71c96d)
    #4 operator() gdb/dwarf2/cooked-index.c:471 (gdb+0x71c96d)
    #5 _M_invoke /usr/include/c++/7/bits/std_function.h:316 (gdb+0x71c96d)
    #6 std::function<void ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x72a57c)
    #7 void std::__invoke_impl<void, std::function<void ()>&>(std::__invoke_other, std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:60 (gdb+0x72a5db)
    #8 std::__invoke_result<std::function<void ()>&>::type std::__invoke<std::function<void ()>&>(std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x72a5db)
    #9 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}::operator()() const /usr/include/c++/7/future:1421 (gdb+0x72a5db)
    #10 std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void>::operator()() const /usr/include/c++/7/future:1362 (gdb+0x72a5db)
    #11 std::_Function_handler<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> (), std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void> >::_M_invoke(std::_Any_data const&) /usr/include/c++/7/bits/std_function.h:302 (gdb+0x72a5db)
    #12 std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x724954)
    #13 std::__future_base::_State_baseV2::_M_do_set(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*) /usr/include/c++/7/future:561 (gdb+0x724954)
    #14 void std::__invoke_impl<void, void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::__invoke_memfun_deref, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x72434a)
    #15 std::__invoke_result<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>::type std::__invoke<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x72434a)
    #16 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#1}::operator()() const /usr/include/c++/7/mutex:672 (gdb+0x72434a)
    #17 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::operator()() const /usr/include/c++/7/mutex:677 (gdb+0x72434a)
    #18 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::_FUN() /usr/include/c++/7/mutex:677 (gdb+0x72434a)
    #19 pthread_once <null> (libtsan.so.0+0x4457c)
    #20 __gthread_once /usr/include/c++/7/x86_64-suse-linux/bits/gthr-default.h:699 (gdb+0x72532b)
    #21 void std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/mutex:684 (gdb+0x72532b)
    #22 std::__future_base::_State_baseV2::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>, bool) /usr/include/c++/7/future:401 (gdb+0x174568d)
    #23 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run() /usr/include/c++/7/future:1423 (gdb+0x174568d)
    #24 std::packaged_task<void ()>::operator()() /usr/include/c++/7/future:1556 (gdb+0x174568d)
    #25 gdb::thread_pool::thread_function() gdbsupport/thread-pool.cc:242 (gdb+0x174568d)
    #26 void std::__invoke_impl<void, void (gdb::thread_pool::*)(), gdb::thread_pool*>(std::__invoke_memfun_deref, void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x1748040)
    #27 std::__invoke_result<void (gdb::thread_pool::*)(), gdb::thread_pool*>::type std::__invoke<void (gdb::thread_pool::*)(), gdb::thread_pool*>(void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x1748040)
    #28 decltype (__invoke((_S_declval<0ul>)(), (_S_declval<1ul>)())) std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/include/c++/7/thread:234 (gdb+0x1748040)
    #29 std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::operator()() /usr/include/c++/7/thread:243 (gdb+0x1748040)
    #30 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> > >::_M_run() /usr/include/c++/7/thread:186 (gdb+0x1748040)
    #31 <null> <null> (libstdc++.so.6+0xdcac2)
  ...
SUMMARY: ThreadSanitizer: data race gdb/dwarf2/read.c:3077 in dw_expand_symtabs_matching_file_matcher(dwarf2_per_objfile*, gdb::function_view<bool (char const*, bool)>)
...

The race happens when issuing the "file $exec" command.

The race is between:
- a worker thread writing the index cache, and in the process reading
  dwarf2_per_cu_data::is_debug_type, and
- the main thread writing to dwarf2_per_cu_data::mark.

The two bitfields dwarf2_per_cu_data::mark and
dwarf2_per_cu_data::is_debug_type share the same bitfield container.

Fix this by making dwarf2_per_cu_data::mark a packed<unsigned int, 1>.

Tested on x86_64-linux.

PR symtab/30718
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30718
a4lg pushed a commit that referenced this pull request Aug 6, 2023
…g_types}

With gdb build with -fsanitize=thread, and the exec from test-case
gdb.base/index-cache.exp, I run into:
...
$ rm -f ~/.cache/gdb/*; \
  gdb -q -batch -iex "set index-cache enabled on" index-cache \
    -ex "print foobar"
  ...
WARNING: ThreadSanitizer: data race (pid=25018)
  Write of size 1 at 0x7b200000410d by main thread:
    #0 dw2_get_file_names_reader gdb/dwarf2/read.c:2033 (gdb+0x7ab023)
    #1 dw2_get_file_names gdb/dwarf2/read.c:2130 (gdb+0x7ab023)
    #2 dw_expand_symtabs_matching_file_matcher(dwarf2_per_objfile*, gdb::function_view<bool (char const*, bool)>) gdb/dwarf2/read.c:3105 (gdb+0x7ac6e9)
    #3 cooked_index_functions::expand_symtabs_matching(objfile*, gdb::function_view<bool (char const*, bool)>, lookup_name_info const*, gdb::function_view<bool (char const*)>, gdb::function_view<bool (compunit_symtab*)>, enum_flags<block_search_flag_values>, domain_enum, search_domain) gdb/dwarf2/read.c:16812 (gdb+0x7d040f)
    #4 objfile::map_symtabs_matching_filename(char const*, char const*, gdb::function_view<bool (symtab*)>) gdb/symfile-debug.c:219 (gdb+0xda5b6e)
    #5 iterate_over_symtabs(char const*, gdb::function_view<bool (symtab*)>) gdb/symtab.c:648 (gdb+0xdc441d)
    #6 lookup_symtab(char const*) gdb/symtab.c:662 (gdb+0xdc4522)
    #7 classify_name gdb/c-exp.y:3083 (gdb+0x61afec)
    #8 c_yylex gdb/c-exp.y:3251 (gdb+0x61dd13)
    #9 c_yyparse() build/gdb/c-exp.c.tmp:1988 (gdb+0x61f07e)
    #10 c_parse(parser_state*) gdb/c-exp.y:3417 (gdb+0x62d864)
    #11 language_defn::parser(parser_state*) const gdb/language.c:598 (gdb+0x977245)
    #12 parse_exp_in_context gdb/parse.c:414 (gdb+0xb10b1b)
    #13 parse_expression(char const*, innermost_block_tracker*, enum_flags<parser_flag>) gdb/parse.c:462 (gdb+0xb1112e)
    #14 process_print_command_args gdb/printcmd.c:1321 (gdb+0xb4bf8c)
    #15 print_command_1 gdb/printcmd.c:1335 (gdb+0xb4caaa)
    #16 print_command gdb/printcmd.c:1468 (gdb+0xb4cdda)
    #17 do_simple_func gdb/cli/cli-decode.c:95 (gdb+0x65b078)
    #18 cmd_func(cmd_list_element*, char const*, int) gdb/cli/cli-decode.c:2735 (gdb+0x65ed53)
    #19 execute_command(char const*, int) gdb/top.c:575 (gdb+0xe3a7ea)
    #20 catch_command_errors gdb/main.c:518 (gdb+0xa183fd)
    #21 execute_cmdargs gdb/main.c:617 (gdb+0xa185bf)
    #22 captured_main_1 gdb/main.c:1289 (gdb+0xa1aad8)
    #23 captured_main gdb/main.c:1310 (gdb+0xa1b9da)
    #24 gdb_main(captured_main_args*) gdb/main.c:1339 (gdb+0xa1b9da)
    #25 main gdb/gdb.c:39 (gdb+0x42506a)

  Previous read of size 1 at 0x7b200000410d by thread T2:
    #0 write_gdbindex gdb/dwarf2/index-write.c:1214 (gdb+0x75bb30)
    #1 write_dwarf_index(dwarf2_per_bfd*, char const*, char const*, char const*, dw_index_kind) gdb/dwarf2/index-write.c:1469 (gdb+0x75f803)
    #2 index_cache::store(dwarf2_per_bfd*, index_cache_store_context const&) gdb/dwarf2/index-cache.c:173 (gdb+0x755a36)
    #3 cooked_index::maybe_write_index(dwarf2_per_bfd*, index_cache_store_context const&) gdb/dwarf2/cooked-index.c:642 (gdb+0x71c96d)
    #4 operator() gdb/dwarf2/cooked-index.c:471 (gdb+0x71c96d)
    #5 _M_invoke /usr/include/c++/7/bits/std_function.h:316 (gdb+0x71c96d)
    #6 std::function<void ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x72a57c)
    #7 void std::__invoke_impl<void, std::function<void ()>&>(std::__invoke_other, std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:60 (gdb+0x72a5db)
    #8 std::__invoke_result<std::function<void ()>&>::type std::__invoke<std::function<void ()>&>(std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x72a5db)
    #9 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}::operator()() const /usr/include/c++/7/future:1421 (gdb+0x72a5db)
    #10 std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void>::operator()() const /usr/include/c++/7/future:1362 (gdb+0x72a5db)
    #11 std::_Function_handler<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> (), std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void> >::_M_invoke(std::_Any_data const&) /usr/include/c++/7/bits/std_function.h:302 (gdb+0x72a5db)
    #12 std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x724954)
    #13 std::__future_base::_State_baseV2::_M_do_set(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*) /usr/include/c++/7/future:561 (gdb+0x724954)
    #14 void std::__invoke_impl<void, void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::__invoke_memfun_deref, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x72434a)
    #15 std::__invoke_result<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>::type std::__invoke<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x72434a)
    #16 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#1}::operator()() const /usr/include/c++/7/mutex:672 (gdb+0x72434a)
    #17 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::operator()() const /usr/include/c++/7/mutex:677 (gdb+0x72434a)
    #18 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::_FUN() /usr/include/c++/7/mutex:677 (gdb+0x72434a)
    #19 pthread_once <null> (libtsan.so.0+0x4457c)
    #20 __gthread_once /usr/include/c++/7/x86_64-suse-linux/bits/gthr-default.h:699 (gdb+0x72532b)
    #21 void std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/mutex:684 (gdb+0x72532b)
    #22 std::__future_base::_State_baseV2::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>, bool) /usr/include/c++/7/future:401 (gdb+0x174570d)
    #23 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run() /usr/include/c++/7/future:1423 (gdb+0x174570d)
    #24 std::packaged_task<void ()>::operator()() /usr/include/c++/7/future:1556 (gdb+0x174570d)
    #25 gdb::thread_pool::thread_function() gdbsupport/thread-pool.cc:242 (gdb+0x174570d)
    #26 void std::__invoke_impl<void, void (gdb::thread_pool::*)(), gdb::thread_pool*>(std::__invoke_memfun_deref, void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x17480c0)
    #27 std::__invoke_result<void (gdb::thread_pool::*)(), gdb::thread_pool*>::type std::__invoke<void (gdb::thread_pool::*)(), gdb::thread_pool*>(void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x17480c0)
    #28 decltype (__invoke((_S_declval<0ul>)(), (_S_declval<1ul>)())) std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/include/c++/7/thread:234 (gdb+0x17480c0)
    #29 std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::operator()() /usr/include/c++/7/thread:243 (gdb+0x17480c0)
    #30 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> > >::_M_run() /usr/include/c++/7/thread:186 (gdb+0x17480c0)
    #31 <null> <null> (libstdc++.so.6+0xdcac2)
  ...
SUMMARY: ThreadSanitizer: data race gdb/dwarf2/read.c:2033 in dw2_get_file_names_reader
...

The race happens when issuing the "file $exec" command.

The race is between:
- a worker thread writing the index cache, and in the process reading
  dwarf2_per_cu_data::is_debug_type, and
- the main thread writing to dwarf2_per_cu_data::files_read.

The two bitfields dwarf2_per_cu_data::files_read and
dwarf2_per_cu_data::is_debug_type share the same bitfield container.

Fix this by making dwarf2_per_cu_data::files_read a packed<bool, 1>.

Tested on x86_64-linux.

PR symtab/30718
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30718
a4lg pushed a commit that referenced this pull request Aug 21, 2023
It was pointed out on the mailing list[1] that after this commit:

  commit b1e0126
  Date:   Wed Jun 21 14:18:54 2023 +0100

      gdb: don't resume vfork parent while child is still running

the test gdb.base/vfork-follow-parent.exp now has some failures when
run with the native-gdbserver or native-extended-gdbserver boards:

  FAIL: gdb.base/vfork-follow-parent.exp: resolution_method=schedule-multiple: continue to end of inferior 2 (timeout)
  FAIL: gdb.base/vfork-follow-parent.exp: resolution_method=schedule-multiple: inferior 1 (timeout)
  FAIL: gdb.base/vfork-follow-parent.exp: resolution_method=schedule-multiple: print unblock_parent = 1 (timeout)
  FAIL: gdb.base/vfork-follow-parent.exp: resolution_method=schedule-multiple: continue to break_parent (timeout)

The reason that these failures don't show up when run on the standard
unix board is that the test is only run in the default operating mode,
so for Linux this will be all-stop on top of non-stop.

If we adjust the test script so that it runs in the default mode and
with target-non-stop turned off, then we see the same failures on the
unix board.  This commit includes this change.

The way that the test is written means that it is not (currently)
possible to turn on non-stop mode and have the test still work, so
this commit does not do that.

I have also updated the test script so that the vfork child performs
an exec as well as the current exit.  Exec and exit are the two ways
in which a vfork child can release the vfork parent, so testing both
of these cases is useful I think.

In this test the inferior performs a vfork and the vfork-child
immediately exits.  The vfork-parent will wait for the vfork-child and
then blocks waiting for gdb.  Once gdb has released the vfork-parent,
the vfork-parent also exits.

In the test that fails, GDB sets 'detach-on-fork off' and then runs to
the vfork.  At this point the test tries to just "continue", but this
fails as the vfork-parent is still selected, and the parent can't
continue until the vfork-child completes.  As the vfork-child is
stopped by GDB the parent will never stop once resumed, so GDB refuses
to resume it.

The test script then sets 'schedule-multiple on' and once again
continues.  This time GDB, in theory, resumes both the parent and the
child, the parent will be held blocked by the kernel, but the child
will run until it exits, and which point GDB stops again, this time
with inferior 2, the newly exited vfork-child, selected.

What happens after this in the test script is irrelevant as far as
this failure is concerned.

To understand why the test started failing we should consider the
behaviour of four different cases:

  1. All-stop-on-non-stop before commit b1e0126,

  2. All-stop-on-non-stop after commit b1e0126,

  3. All-stop-on-all-stop before commit b1e0126, and

  4. All-stop-on-all-stop after commit b1e0126.

Only case #4 is failing after commit b1e0126, but I think the
other cases are interesting because, (a) they inform how we might fix
the regression, and (b) it turns out the behaviour of #2 changed too
with the commit, but the change was harmless.

For #1 All-stop-on-non-stop before commit b1e0126, what happens
is:

  1. GDB calls proceed with the vfork-parent selected, as schedule
     multiple is on user_visible_resume_ptid returns -1 (everything)
     as the resume_ptid (see proceed function),

  2. As this is all-stop-on-non-stop, every thread is resumed
    individually, so GDB tries to resume both the vfork-parent and the
    vfork-child, both of which succeed,

  3. The vfork-parent is held stopped by the kernel,

  4. The vfork-child completes (exits) at which point the GDB sees the
     EXITED event for the vfork-child and the VFORK_DONE event for the
     vfork-parent,

  5. At this point we might take two paths depending on which event
     GDB handles first, if GDB handles the VFORK_DONE first then:

     (a) As GDB is controlling both parent and child the VFORK_DONE is
         ignored (see handle_vfork_done), the vfork-parent will be
	 resumed,

     (b) GDB processes the EXITED event, selects the (now defunct)
         vfork-child, and stops, returning control to the user.

     Alternatively, if GDB selects the EXITED event first then:

     (c) GDB processes the EXITED event, selects the (now defunct)
         vfork-child, and stops, returning control to the user.

     (d) At some future time the user resumes the vfork-parent, at
         which point the VFORK_DONE is reported to GDB, however, GDB
	 is ignoring the VFORK_DONE (see handle_vfork_done), so the
	 parent is resumed.

For case #2, all-stop-on-non-stop after commit b1e0126, the
important difference is in step (2) above, now, instead of resuming
both the vfork-parent and the vfork-child, only the vfork-child is
resumed.  As such, when we get to step (5), only a single event, the
EXITED event is reported.

GDB handles the EXITED just as in (5)(c), then, later, when the user
resumes the vfork-parent, the VFORKED_DONE is immediately delivered
from the kernel, but this is ignored just as in (5)(d), and so,
though the pattern of when the vfork-parent is resumed changes, the
overall pattern of which events are reported and when, doesn't
actually change.  In fact, by not resuming the vfork-parent, the order
of events (in this test) is now deterministic, which (maybe?) is a
good thing.

If we now consider case #3, all-stop-on-all-stop before commit
b1e0126, then what happens is:

  1. GDB calls proceed with the vfork-parent selected, as schedule
     multiple is on user_visible_resume_ptid returns -1 (everything)
     as the resume_ptid (see proceed function),

  2. As this is all-stop-on-all-stop, the resume is passed down to the
     linux-nat target, the vfork-parent is the event thread, while the
     vfork-child is a sibling of the event thread,

  3. In linux_nat_target::resume, GDB calls linux_nat_resume_callback
     for all threads, this causes the vfork-child to be resumed.  Then
     in linux_nat_target::resume, the event thread, the vfork-parent,
     is also resumed.

  4. The vfork-parent is held stopped by the kernel,

  5. The vfork-child completes (exits) at which point the GDB sees the
     EXITED event for the vfork-child and the VFORK_DONE event for the
     vfork-parent,

  6. We are now in a situation identical to step (5) as for
     all-stop-on-non-stop above, GDB selects one of the events to
     handle, and whichever we select the user sees the correct
     behaviour.

And so, finally, we can consider #4, all-stop-on-all-stop after commit
b1e0126, this is the case that started failing.

We start out just like above, in proceed, the resume_ptid is
-1 (resume everything), due to schedule multiple being on.  And just
like above, due to the target being all-stop, we call
proceed_resume_thread_checked just once, for the current thread,
which, remember, is the vfork-parent thread.

The change in commit b1e0126 was to avoid resuming a vfork-parent
thread, read the commit message for the justification for this change.

However, this means that GDB now rejects resuming the vfork-parent in
this case, which means that nothing gets resumed!  Obviously, if
nothing resumes, then nothing will ever stop, and so GDB appears to
hang.

I considered a couple of solutions which, in the end, I didn't go
with, these were:

  1. Move the vfork-parent check out of proceed_resume_thread_checked,
     and place it in proceed, but only on the all-stop-on-non-stop
     path, this should still address the issue seen in b1e0126,
     but would avoid the issue seen here.  I rejected this just
     because it didn't feel great to split the checks that exist in
     proceed_resume_thread_checked like this,

  2. Extend the condition in proceed_resume_thread_checked by adding a
     target_is_non_stop_p check.  This would have the same effect as
     idea 1, but leaves all the checks in the same place, which I
     think would be better, but this still just didn't feel right to
     me, and so,

What I noticed was that for the all-stop-on-non-stop, after commit
b1e0126, we only resumed the vfork-child, and this seems fine.
The vfork-parent isn't going to run anyway (the kernel will hold it
back), so if feels like we there's no harm in just waiting for the
child to complete, and then resuming the parent.

So then I started looking at follow_fork, which is called from the top
of proceed.  This function already has the task of switching between
the parent and child based on which the user wishes to follow.  So, I
wondered, could we use this to switch to the vfork-child in the case
that we are attached to both?

Turns out this is pretty simple to do.

Having done that, now the process is for all-stop-on-all-stop after
commit b1e0126, and with this new fix is:

  1. GDB calls proceed with the vfork-parent selected, but,

  2. In follow_fork, and follow_fork_inferior, GDB switches the
     selected thread to be that of the vfork-child,

  3. Back in proceed user_visible_resume_ptid returns -1 (everything)
     as the resume_ptid still, but now,

  4. When GDB calls proceed_resume_thread_checked, the vfork-child is
     the current selected thread, this is not a vfork-parent, and so
     GDB allows the proceed to continue to the linux-nat target,

  5. In linux_nat_target::resume, GDB calls linux_nat_resume_callback
     for all threads, this does not resume the vfork-parent (because
     it is a vfork-parent), and then the vfork-child is resumed as
     this is the event thread,

At this point we are back in the same situation as for
all-stop-on-non-stop after commit b1e0126, that is, the
vfork-child is resumed, while the vfork-parent is held stopped by
GDB.

Eventually the vfork-child will exit or exec, at which point the
vfork-parent will be resumed.

[1] https://inbox.sourceware.org/gdb-patches/[email protected]/
a4lg pushed a commit that referenced this pull request Aug 30, 2023
After running a number of programs under Windows gdb and detaching
them, I typed run in gdb, and got a hang, here:

 (top-gdb) bt
 #0  sharing_input_terminal (pid=4672) at /home/pedro/gdb/src/gdb/mingw-hdep.c:388
 #1  0x00007ff71a2d8678 in sharing_input_terminal (inf=0x23bf23dafb0) at /home/pedro/gdb/src/gdb/inflow.c:269
 #2  0x00007ff71a2d887b in child_terminal_save_inferior (self=0x23bf23de060) at /home/pedro/gdb/src/gdb/inflow.c:423
 #3  0x00007ff71a2c80c0 in inf_child_target::terminal_save_inferior (this=0x23bf23de060) at /home/pedro/gdb/src/gdb/inf-child.c:111
 #4  0x00007ff71a429c0f in target_terminal_is_ours_kind (desired_state=target_terminal_state::is_ours_for_output) at /home/pedro/gdb/src/gdb/target.c:1037
 #5  0x00007ff71a429e02 in target_terminal::ours_for_output () at /home/pedro/gdb/src/gdb/target.c:1094
 #6  0x00007ff71a2ccc8e in post_create_inferior (from_tty=0) at /home/pedro/gdb/src/gdb/infcmd.c:245
 #7  0x00007ff71a2cd431 in run_command_1 (args=0x0, from_tty=0, run_how=RUN_NORMAL) at /home/pedro/gdb/src/gdb/infcmd.c:502
 #8  0x00007ff71a2cd58b in run_command (args=0x0, from_tty=0) at /home/pedro/gdb/src/gdb/infcmd.c:527

The problem is that the loop around GetConsoleProcessList looped
forever, because there were exactly 10 processes to return.
GetConsoleProcessList's documentation says:

  If the buffer is too small to hold all the valid process identifiers,
  the return value is the required number of array elements. The
  function will have stored no identifiers in the buffer. In this
  situation, use the return value to allocate a buffer that is large
  enough to store the entire list and call the function again.

In this case, the buffer wasn't too small, it was exactly the right
size, so we should have broken out of the loop.  We didn't due to a
"<" check that should have been "<=".  That is fixed by this patch.

Approved-By: Tom Tromey <[email protected]>
Reviewed-By: Eli Zaretskii <[email protected]>
Change-Id: I14e4909f2ac2fa83d0d9b6e64418b5831ac4e4e3
a4lg pushed a commit that referenced this pull request Sep 5, 2023
When running test-case gdb.base/add-symbol-file-attach.exp with target board
unix/-m32, we run into:
...
(gdb) attach 3955^M
Attaching to process 3955^M
Load new symbol table from "add-symbol-file-attach"? (y or n) y^M
Reading symbols from add-symbol-file-attach/add-symbol-file-attach...^M
Reading symbols from /lib/libm.so.6...^M
Reading symbols from /usr/lib/debug/lib/libm-2.31.so-i386.debug...^M
Reading symbols from /lib/libc.so.6...^M
Reading symbols from /usr/lib/debug/lib/libc-2.31.so-i386.debug...^M
Reading symbols from /lib/ld-linux.so.2...^M
Reading symbols from /usr/lib/debug/lib/ld-2.31.so-i386.debug...^M
0xf7f53549 in __kernel_vsyscall ()^M
(gdb) FAIL: gdb.base/add-symbol-file-attach.exp: attach
...

The test fails because this regexp is used:
...
    -re ".*in \[_A-Za-z0-9\]*pause.*$gdb_prompt $" {
...

The regexp attempts to detect that the exec is somewhere in pause ():
...
int
main (int argc, char **argv)
{
  pause ();
  return 0;
}
...
but when the exec is blocked in pause, the backtrace is:
...
 (gdb) bt
 #0  0xf7fd2549 in __kernel_vsyscall ()
 #1  0xf7d84966 in __libc_pause () at ../sysdeps/unix/sysv/linux/pause.c:29
 #2  0x0804844c in main (argc=1, argv=0xffffce84)
     at /data/vries/gdb/src/gdb/testsuite/gdb.base/add-symbol-file-attach.c:26
...

We could simply extend the regexp to also match __kernel_vsyscall, but the
more fundamental problem is that the test is racy.

The attach can happen before the exec is blocked in pause (), somewhere in the
dynamic linker resolving the call to pause, in main or even earlier.

Note that for the test-case to be effective, the exec is not required to be in
pause ().  I added a "while (1);" loop at the start of main, reverted the
patch fixing the corresponding PR and reproduced the problem it's supposed to
detect.

Fix this by simply matching the "Reading symbols from" line, similar to what
an earlier test is doing.

While we're at it, rewrite the earlier test to also use the -wrap idiom.

Tested on x86_64-linux.
a4lg pushed a commit that referenced this pull request Oct 12, 2023
This commit fixes an issue that was discovered while writing the tests
for the previous commit.

I noticed that, when GDB restarts an inferior, the executable_changed
event would trigger twice.  The first notification would originate
from:

  #0  exec_file_attach (filename=0x4046680 "/tmp/hello.x", from_tty=0) at ../../src/gdb/exec.c:513
  #1  0x00000000006f3adb in reopen_exec_file () at ../../src/gdb/corefile.c:122
  #2  0x0000000000e6a3f2 in generic_mourn_inferior () at ../../src/gdb/target.c:3682
  #3  0x0000000000995121 in inf_child_target::mourn_inferior (this=0x2fe95c0 <the_amd64_linux_nat_target>) at ../../src/gdb/inf-child.c:192
  #4  0x0000000000995cff in inf_ptrace_target::mourn_inferior (this=0x2fe95c0 <the_amd64_linux_nat_target>) at ../../src/gdb/inf-ptrace.c:125
  #5  0x0000000000a32472 in linux_nat_target::mourn_inferior (this=0x2fe95c0 <the_amd64_linux_nat_target>) at ../../src/gdb/linux-nat.c:3609
  #6  0x0000000000e68a40 in target_mourn_inferior (ptid=...) at ../../src/gdb/target.c:2761
  #7  0x0000000000a323ec in linux_nat_target::kill (this=0x2fe95c0 <the_amd64_linux_nat_target>) at ../../src/gdb/linux-nat.c:3593
  #8  0x0000000000e64d1c in target_kill () at ../../src/gdb/target.c:924
  #9  0x00000000009a19bc in kill_if_already_running (from_tty=1) at ../../src/gdb/infcmd.c:328
  #10 0x00000000009a1a6f in run_command_1 (args=0x0, from_tty=1, run_how=RUN_STOP_AT_MAIN) at ../../src/gdb/infcmd.c:381
  #11 0x00000000009a20a5 in start_command (args=0x0, from_tty=1) at ../../src/gdb/infcmd.c:527
  #12 0x000000000068dc5d in do_simple_func (args=0x0, from_tty=1, c=0x35c7200) at ../../src/gdb/cli/cli-decode.c:95

While the second originates from:

  #0  exec_file_attach (filename=0x3d7a1d0 "/tmp/hello.x", from_tty=0) at ../../src/gdb/exec.c:513
  #1  0x0000000000dfe525 in reread_symbols (from_tty=1) at ../../src/gdb/symfile.c:2517
  #2  0x00000000009a1a98 in run_command_1 (args=0x0, from_tty=1, run_how=RUN_STOP_AT_MAIN) at ../../src/gdb/infcmd.c:398
  #3  0x00000000009a20a5 in start_command (args=0x0, from_tty=1) at ../../src/gdb/infcmd.c:527
  #4  0x000000000068dc5d in do_simple_func (args=0x0, from_tty=1, c=0x35c7200) at ../../src/gdb/cli/cli-decode.c:95

In the first case the call to exec_file_attach first passes through
reopen_exec_file.  The reopen_exec_file performs a modification time
check on the executable file, and only calls exec_file_attach if the
executable has changed on disk since it was last loaded.

However, in the second case things work a little differently.  In this
case GDB is really trying to reread the debug symbol.  As such, we
iterate over the objfiles list, and for each of those we check the
modification time, if the file on disk has changed then we reload the
debug symbols from that file.

However, there is an additional check, if the objfile has the same
name as the executable then we will call exec_file_attach, but we do
so without checking the cached modification time that indicates when
the executable was last reloaded, as a result, we reload the
executable twice.

In this commit I propose that reread_symbols be changed to
unconditionally call reopen_exec_file before performing the objfile
iteration.  This will ensure that, if the executable has changed, then
the executable will be reloaded, however, if the executable has
already been recently reloaded, we will not reload it for a second
time.

After handling the executable, GDB can then iterate over the objfiles
list and reload them in the normal way.

With this done I now see the executable reloaded only once when GDB
restarts an inferior, which means I can remove the kfail that I added
to the gdb.python/py-exec-file.exp test in the previous commit.

Approved-By: Tom Tromey <[email protected]>
a4lg pushed a commit that referenced this pull request Oct 12, 2023
It was pointed out on the mailing list that a recently added
test (gdb.python/py-progspace-events.exp) was failing when run with
the native-extended-gdbserver board.  This test was added with this
commit:

  commit 59912fb
  Date:   Tue Sep 19 11:45:36 2023 +0100

      gdb: add Python events for program space addition and removal

It turns out though that the test is failing due to a existing bug
in GDB, the new test just exposes the problem.  Additionally, the
failure really doesn't even rely on the new functionality added in the
above commit.  I reduced the test to a simple set of steps that
reproduced the failure and tested against GDB 13, and the test passes;
so the bug was introduced since then.  In fact, the bug was introduced
with this commit:

  commit a282736
  Date:   Fri Sep 8 15:48:16 2023 +0100

      gdb: remove final user of the executable_changed observer

This commit changed how the per-inferior auxv data cache is managed,
specifically, when the cache is cleared, and it is this that leads to
the failure.

This bug is interesting because it exposes a number of issues with
GDB, I'll explain all of the problems I see, though ultimately, I only
propose fixing one problem in this commit, which is enough to resolve
the crash we are currently seeing.

The crash that we are seeing manifests like this:

  ...
  [Inferior 2 (process 3970384) exited normally]
  +inferior 1
  [Switching to inferior 1 [process 3970383] (/tmp/build/gdb/testsuite/outputs/gdb.python/py-progspace-events/py-progspace-events)]
  [Switching to thread 1.1 (Thread 3970383.3970383)]
  #0  breakpt () at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.python/py-progspace-events.c:28
  28	{ /* Nothing.  */ }
  (gdb) step
  +step
  terminate called after throwing an instance of 'gdb_exception_error'

  Fatal signal: Aborted
  ... etc ...

What's happening is that GDB attempts to refill the auxv cache as a
result of the gdbarch_has_shared_address_space call in
program_space::~program_space, the backtrace looks like this:

  #0  0x00007fb4f419a9a5 in raise () from /lib64/libpthread.so.0
  #1  0x00000000008b635d in handle_fatal_signal (sig=6) at ../../src/gdb/event-top.c:912
  #2  <signal handler called>
  #3  0x00007fb4f38e3625 in raise () from /lib64/libc.so.6
  #4  0x00007fb4f38cc8d9 in abort () from /lib64/libc.so.6
  #5  0x00007fb4f3c70756 in __gnu_cxx::__verbose_terminate_handler() [clone .cold] () from /lib64/libstdc++.so.6
  #6  0x00007fb4f3c7c6dc in __cxxabiv1::__terminate(void (*)()) () from /lib64/libstdc++.so.6
  #7  0x00007fb4f3c7b6e9 in __cxa_call_terminate () from /lib64/libstdc++.so.6
  #8  0x00007fb4f3c7c094 in __gxx_personality_v0 () from /lib64/libstdc++.so.6
  #9  0x00007fb4f3a80c63 in _Unwind_RaiseException_Phase2 () from /lib64/libgcc_s.so.1
  #10 0x00007fb4f3a8154e in _Unwind_Resume () from /lib64/libgcc_s.so.1
  #11 0x0000000000e8832d in target_read_alloc_1<unsigned char> (ops=0x408a3a0, object=TARGET_OBJECT_AUXV, annex=0x0) at ../../src/gdb/target.c:2266
  #12 0x0000000000e73dea in target_read_alloc (ops=0x408a3a0, object=TARGET_OBJECT_AUXV, annex=0x0) at ../../src/gdb/target.c:2315
  #13 0x000000000058248c in target_read_auxv_raw (ops=0x408a3a0) at ../../src/gdb/auxv.c:379
  #14 0x000000000058243d in target_read_auxv () at ../../src/gdb/auxv.c:368
  #15 0x000000000058255c in target_auxv_search (match=0x0, valp=0x7ffdee17c598) at ../../src/gdb/auxv.c:415
  #16 0x0000000000a464bb in linux_is_uclinux () at ../../src/gdb/linux-tdep.c:433
  #17 0x0000000000a464f6 in linux_has_shared_address_space (gdbarch=0x409a2d0) at ../../src/gdb/linux-tdep.c:440
  #18 0x0000000000510eae in gdbarch_has_shared_address_space (gdbarch=0x409a2d0) at ../../src/gdb/gdbarch.c:4889
  #19 0x0000000000bc7558 in program_space::~program_space (this=0x4544aa0, __in_chrg=<optimized out>) at ../../src/gdb/progspace.c:124
  #20 0x00000000009b245d in delete_inferior (inf=0x47b3de0) at ../../src/gdb/inferior.c:290
  #21 0x00000000009b2c10 in prune_inferiors () at ../../src/gdb/inferior.c:480
  #22 0x00000000009c5e3e in fetch_inferior_event () at ../../src/gdb/infrun.c:4558
  #23 0x000000000099b4dc in inferior_event_handler (event_type=INF_REG_EVENT) at ../../src/gdb/inf-loop.c:42
  #24 0x0000000000cbc64f in remote_async_serial_handler (scb=0x4090a30, context=0x408a6b0) at ../../src/gdb/remote.c:14859
  #25 0x0000000000d83d3a in run_async_handler_and_reschedule (scb=0x4090a30) at ../../src/gdb/ser-base.c:138
  #26 0x0000000000d83e1f in fd_event (error=0, context=0x4090a30) at ../../src/gdb/ser-base.c:189

So this is problem #1, if we throw an exception while deleting a
program_space then this is not caught, and is going to crash GDB.

Problem #2 becomes evident when we ask why GDB is throwing an error in
this case; the error is thrown because the remote target, operating in
non-async mode, can't read the auxv data while an inferior is running
and GDB is waiting for a stop reply.  The problem here then, is why
does GDB get into a position where it tries to interact with the
remote target in this way, at this time?  The problem is caused by the
prune_inferiors call which can be seen in the above backtrace.

In prune_inferiors we check if the inferior is deletable, and if it
is, we delete it.  The problem is, I think, we should also check if
the target is currently in a state that would allow us to delete the
inferior.  We don't currently have such a check available, we'd need
to add one, but for the remote target, this would return false if the
remote is in async mode and the remote is currently waiting for a stop
reply.  With this change in place GDB would defer deleting the
inferior until the remote target has stopped, at which point GDB would
be able to refill the auxv cache successfully.

And then, problem #3 becomes evident when we ask why GDB is needing to
refill the auxv cache now when it didn't need to for GDB 13.  This is
where the second commit mentioned above (a282736) comes in.
Prior to this commit, the auxv cache was cleared by the
executable_changed observer, while after that commit the auxv cache
was cleared by the new_objfile observer -- but only when the
new_objfile observer is used in the special mode that actually means
that all objfiles have been unloaded (I know, the overloading of the
new_objfile observer is horrible, and unnecessary, but it's not really
important for this bug).

The difference arises because the new_objfile observer is triggered
from clear_symtab_users, which in turn is called from
program_space::~program_space.  The new_objfile observer for auxv does
this:

  static void
  auxv_new_objfile_observer (struct objfile *objfile)
  {
    if (objfile == nullptr)
      invalidate_auxv_cache_inf (current_inferior ());
  }

That is, when all the objfiles are unloaded, we clear the auxv cache
for the current inferior.

The problem is, then when we look at the prune_inferiors ->
delete_inferior -> ~program_space path, we see that the current
inferior is not going to be an inferior that exists within the
program_space being deleted; delete_inferior removes the deleted
inferior from the global inferior list, and then only deletes the
program_space if program_space::empty() returns true, which is only
the case if the current inferior isn't within the program_space to
delete, and no other inferior exists within that program_space
either.

What this means is that when the new_objfile observer is called we
can't rely on the current inferior having any relationship with the
program space in which the objfiles were removed.  This was an error
in the commit a282736, the only thing we can rely on is the
current program space.  As a result of this mistake, after commit
a282736, GDB was sometimes clearing the auxv cache for a random
inferior.  In the native target case this was harmless as we can
always refill the cache when needed, but in the remote target case, if
we need to refill the cache when the remote target is executing, then
we get the crash we observed.

And additionally, if we think about this a little more, we see that
commit a282736 made another mistake.  When all the objfiles are
removed, they are removed from a program_space, a program_space might
contain multiple inferiors, so surely, we should clear the auxv cache
for all of the matching inferiors?

Given these two insights, that the current_inferior is not relevant,
only the current_program_space, and that we should be clearing the
cache for all inferiors in the current_program_space, we can update
auxv_new_objfile_observer to:

  if (objfile == nullptr)
    {
      for (inferior *inf : all_inferiors ())
	{
	  if (inf->pspace == current_program_space)
	    invalidate_auxv_cache_inf (inf);
	}
    }

With this change we now correctly clear the auxv cache for the correct
inferiors, and GDB no longer needs to refill the cache at an
inconvenient time, this avoids the crash we were seeing.

And finally, we reach problem #4.  Inspired by the observation that
using the current_inferior from within the ~program_space function was
not correct, I added some debug to see if current_inferior() was
called anywhere else (below ~program_space), and the answer is yes,
it's called a often.  Mostly the culprit is GDB doing:

  current_inferior ()->top_target ()-> ....

But I think all of these calls are most likely doing the wrong thing,
and only work because the top target in all these cases is shared
between all inferiors, e.g. it's the native target, or the remote
target for all inferiors.  But if we had a truly multi-connection
setup, then we might start to see odd behaviour.

Problem #1 I'm just ignoring for now, I guess at some point we might
run into this again, and then we'd need to solve this.  But in this
case I wasn't sure what a "good" solution would look like.  We need
the auxv data in order to implement the linux_is_uclinux() function.
If we can't get the auxv data then what should we do, assume yes, or
assume no?  The right answer would probably be to propagate the error
back up the stack, but then we reach ~program_space, and throwing
exceptions from a destructor is problematic, so we'd need to catch and
deal at this point.  The linux_is_uclinux() call is made from within
gdbarch_has_shared_address_space(), which is used like:

  if (!gdbarch_has_shared_address_space (target_gdbarch ()))
    delete this->aspace;

So, we would have to choose; delete the address space or not.  If we
delete it on error, then we might delete an address space that is
shared within another program space.  If we don't delete the address
space, then we might leak it.  Neither choice is great.

A better solution might be to have the address spaces be reference
counted, then we could remove the gdbarch_has_shared_address_space
call completely, and just rely on the reference count to auto-delete
the address space when appropriate.

The solution for problem #2 I already hinted at above, we should have
a new target_can_delete_inferiors() call, which should be called from
prune_inferiors, this would prevent GDB from trying to delete
inferiors when a (remote) target is in a state where we know it can't
delete the inferior.  Deleting an inferior often (always?) requires
sending packets to the remote, and if the remote is waiting for a stop
reply then this will never work, so the pruning should be deferred in
this case.

The solution for problem #3 is included in this commit.

And, for problem #4, I'm not sure what the right solution is.  Maybe
delete_inferior should ensure the inferior to be deleted is in place
when ~program_space is called?  But that seems a little weird, as the
current inferior would, in theory, still be using the current
program_space...

Anyway, after this commit, the gdb.python/py-progspace-events.exp test
now passes when run with the native-extended-remote board.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30935
Approved-By: Simon Marchi <[email protected]>
Change-Id: I41f0e6e2d7ecc1e5e55ec170f37acd4052f46eaf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant