Currently, gdb cannot step outside of a signal handler on RISC-V
platforms. This causes multiple failures in gdb.base/sigstep.exp:
FAIL: gdb.base/sigstep.exp: continue to handler, nothing in handler, step from handler: leave handler (timeout)
FAIL: gdb.base/sigstep.exp: continue to handler, si+advance in handler, step from handler: leave handler (timeout)
FAIL: gdb.base/sigstep.exp: continue to handler, nothing in handler, next from handler: leave handler (timeout)
FAIL: gdb.base/sigstep.exp: continue to handler, si+advance in handler, next from handler: leave handler (timeout)
FAIL: gdb.base/sigstep.exp: stepi from handleri: leave signal trampoline
FAIL: gdb.base/sigstep.exp: nexti from handleri: leave signal trampoline
=== gdb Summary ===
# of expected passes 587
# of unexpected failures 6
This patch adds support for stepping outside of a signal handler on
riscv*-*-linux*.
Implementation is heavily inspired from mips_linux_syscall_next_pc and
surroundings as advised by Pedro Alves.
After this patch, all tests in gdb.base/sigstep.exp pass.
Build and tested on riscv64-linux-gnu.
Many tests fail in gdb/testsuite/gdb.base/sigstep.exp on
riscv64-linux-gnu. Those tests check that when stepping, if the
debuggee received a signal it should step inside the signal handler.
This feature requires hardware support for single stepping (or at least
kernel support), but none are available on riscv*-linux-gnu hosts, at
the moment at least.
This patch adds RISC-V to the list of configurations that does not
have hardware single step capability, disabling tests relying on such
feature.
Tested on riscv64-linux-gnu.
While working on my series to replace the DWARF psymbol reader, I
noticed that the expand_symtabs_matching has an undocumented
invariant. I think that, if this invariant is not followed, then GDB
will crash. So, this patch documents this in the relevant spots and
introduces some asserts to make it clear.
Regression tested on x86-64 Fedora 32.
Investigation of using the Python API with an Ada program showed that
an array of dynamic types was not being handled properly. I tracked
this down to an oddity of how array strides are handled.
In gdb, an array stride can be attached to the range type, via the
range_bounds object. However, the stride can also be put into the
array's first field. From create_range_type_with_stride:
else if (bit_stride > 0)
TYPE_FIELD_BITSIZE (result_type, 0) = bit_stride;
It's hard to be sure why this is done, but I would guess a combination
of historical reasons plus a desire (mentioned in a comment somewhere)
to avoid modifying the range type.
This patch fixes the problem by changing type::bit_stride to
understand this convention. It also fixes one spot that reproduces
this logic.
Regression tested on x86-64 Fedora 32.
bfd * elf32-or1k.c (or1k_elf_relocate_section): Use a separate entry
in switch case R_OR1K_INSN_REL_26 where we need to check for
!SYMBOL_CALLS_LOCAL() instead of !SYMBOL_REFERENCES_LOCAL().
PR gdb/28093 points out that gdb crashes when language is set to
"unknown" and expression parsing is attempted. At first I thought
this was a regression due to the expression rewrite, but it turns out
that older versions crash as well.
This patch avoids the crash by changing the default expression parser
to throw an exception. I think this is preferable -- the current
behavior of silently doing nothing does not really make sense.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28093
GDB doesn't handle well the case of an inferior using the JIT interface
to register JIT-ed objfiles and forking. If an inferior registers a
code object using the JIT interface and then forks, the child process
conceptually has the same code object loaded, so GDB should look it up
and learn about it (it currently doesn't).
To achieve this, I think it would make sense to have the
inferior_created observable called when an inferior is created due to a
fork in follow_fork_inferior. The inferior_created observable is
currently called both after starting a new inferior and after attaching
to an inferior, allowing various sub-components to learn about that new
executing inferior. We can see handling a fork child just like
attaching to it, so any work done when attaching should also be done in
the case of a fork child.
Instead of just calling the inferior_created observable, this patch
makes follow_fork_inferior call the whole post_create_inferior function.
This way, the attach and follow-fork code code paths are more alike.
Given that post_create_inferior calls solib_create_inferior_hook,
follow_fork_inferior doesn't need to do it itself, so those calls to
solib_create_inferior_hook are removed.
One question you may have: why not just call post_create_inferior at the
places where solib_create_inferior_hook is currently called, instead of
after target_follow_fork?
- there's something fishy for the second solib_create_inferior_hook
call site: at this point we have switched the current program space
to the child's, but not the current inferior nor the current thread.
So solib_create_inferior_hook (and everything under, including
check_for_thread_db, for example) is called with inferior 1 as the
current inferior and inferior 2's program space as the current
program space. I think that's wrong, because at this point we are
setting up inferior 2, and all that code relies on the current
inferior. We could just add a switch_to_thread call before it to
make inferior 2 the current one, but there are other problems (see
below).
- solib_create_inferior_hook is currently not called on the
`follow_child && detach_fork` path. I think we need to call it,
because we still get a new inferior in that case (even though we
detach the parent). If we only call post_create_inferior where
solib_create_inferior_hook used to be called, then the JIT
subcomponent doesn't get informed about the new inferior, and that
introduces a failure in the new gdb.base/jit-elf-fork.exp test.
- if we try to put the post_create_inferior just after the
switch_to_thread that was originally at line 662, or just before the
call to target_follow_fork, we introduce a subtle failure in
gdb.threads/fork-thread-pending.exp. What happens then is that
libthread_db gets loaded (somewhere under post_create_inferior)
before the linux-nat target learns about the LWPs (which happens in
linux_nat_target::follow_fork). As a result, the ALL_LWPS loop in
try_thread_db_load_1 doesn't see the child LWP, and the thread-db
target doesn't have the chance to fill in thread_info::priv. A bit
later, when the test does "info threads", and
thread_db_target::pid_to_str is called, the thread-db target doesn't
recognize the thread as one of its own, and delegates the request to
the target below. Because the pid_to_str output is not the expected
one, the test fails.
This tells me that we need to call the process target's follow_fork
first, to make the process target create the necessary LWP and thread
structures. Then, we can call post_create_inferior to let the other
components of GDB do their thing.
But then you may ask: check_for_thread_db is already called today,
somewhere under solib_create_inferior_hook, and that is before
target_follow_fork, why don't we see this ordering problem!? Well,
because of the first bullet point: when check_for_thread_db /
thread_db_load are called, the current inferior is (erroneously)
inferior 1, the parent. Because libthread_db is already loaded for
the parent, thread_db_load early returns. check_for_thread_db later
gets called by linux_nat_target::follow_fork. At this point, the
current inferior is the correct one and the child's LWP exists, so
all is well.
Since we now call post_create_inferior after target_follow_fork, which
calls the inferior_created observable, which calls check_for_thread_db,
I don't think linux_nat_target needs to explicitly call
check_for_thread_db itself, so that is removed.
In terms of testing, this patch adds a new gdb.base/jit-elf-fork.exp
test. It makes an inferior register a JIT code object and then fork.
It then verifies that whatever the detach-on-fork and follow-fork-child
parameters are, GDB knows about the JIT code object in all the inferiors
that survive the fork. It verifies that the inferiors can unload that
code object.
There isn't currently a way to get visibility into GDB's idea of the JIT
code objects for each inferior. For the purpose of this test, add the
"maintenance info jit" command. There isn't much we can print about the
JIT code objects except their load address. So the output looks a bit
bare, but it's good enough for the test.
gdb/ChangeLog:
* NEWS: Mention "maint info jit" command.
* infrun.c (follow_fork_inferior): Don't call
solib_create_inferior_hook, call post_create_inferior if a new
inferior was created.
* jit.c (maint_info_jit_cmd): New.
(_initialize_jit): Register new command.
* linux-nat.c (linux_nat_target::follow_fork): Don't call
check_for_thread_db.
* linux-nat.h (check_for_thread_db): Remove declaration.
* linux-thread-db.c (check_thread_signals): Make static.
gdb/doc/ChangeLog:
* gdb.texinfo (Maintenance Commands): Mention "maint info jit".
gdb/testsuite/ChangeLog:
* gdb.base/jit-elf-fork-main.c: New test.
* gdb.base/jit-elf-fork-solib.c: New test.
* gdb.base/jit-elf-fork.exp: New test.
Change-Id: I9a192e55b8a451c00e88100669283fc9ca60de5c
It fixes a regression caused by commit
1edb66d856 where thread_info::suspend was
made private.
The public thread_info API has to be used to get stop signal and avoid
build failures.
gdb/ChangeLog:
2021-07-14 Libor Bukata <libor.bukata@oracle.com>
* gdb/procfs.c (find_stop_signal): Use thread_info API.
Change-Id: I53bc57a05cd0eca5f28ef0726d6faeeb306e7904
gas/write.c provides a fallback TC_VALIDATE_FIX_SUB define that can be
a problem for some targets, the problem being that a non-zero
definition of TC_VALIDATE_FIX_SUB says that some uses of fx_subsy are
OK, in effect that the target will handle fx_subsy in md_apply_fix
and/or tc_gen_reloc. A lot of targets don't have the necessary
md_apply_fix and tc_gen_reloc support. So a safer default is to
disallow fx_subsy by default.
I've had a good look over target usage of fx_subsy, and think I've
caught all the cases where targets need TC_VALIDATE_FIX_SUB. Possible
failures would be limited to alpha, microblaze, ppc and s390 (the
targets that define UNDEFINED_DIFFERENCE_OK), or targets that generate
fixups with BFD_RELOC_GPREL32/16 and use a syntax explicitly showing
a difference expression.
* write.c (TC_VALIDATE_FIX_SUB): Default to 0.
* config/tc-hppa.h (TC_VALIDATE_FIX_SUB): Define.
* config/tc-microblaze.h (TC_VALIDATE_FIX_SUB): Define.
* config/tc-alpha.h (TC_VALIDATE_FIX_SUB): Define for ECOFF.
* config/tc-ppc.h (TC_VALIDATE_FIX_SUB): Don't define for ELF.
Do define for XCOFF.
DWARF sections have special names on AIX which need be handled
by objdump in order to correctly print them.
This patch also adds the correlation in bfd for future uses.
bfd/
* libxcoff.h (struct xcoff_dwsect_name): Add DWARF name.
* coff-rs6000.c (xcoff_dwsect_names): Update.
* coffcode.h (sec_to_styp_flags): Likewise.
(coff_new_section_hook): Likewise.
binutils/
* dwarf.h (struct dwarf_section): Add XCOFF name.
* dwarf.c (struct dwarf_section_display): Update.
* objdump.c (load_debug_section): Add XCOFF name handler.
(dump_dwarf_section): Likewise.
gas/
* config/tc-ppc.c (ppc_change_debug_section): Update to
match new name's field.
When running test-case gdb.base/gold-gdb-index.exp on openSUSE Tumbleweed,
I run into:
...
FAIL: gdb.base/gold-gdb-index.exp: maint info symtabs
...
This is due to a dummy .gdb_index:
...
Contents of the .gdb_index section:
Version 7
CU table:
TU table:
Address table:
Symbol table:
...
The dummy .gdb_index is ignored when loading the symbols, and instead partial
symbols are used. Consequently, we get the same result as if we'd removed
-Wl,--gdb-index from the compilation.
Presumably, gold fails to generate a proper .gdb_index because it lacks
DWARF5 support.
Anyway, without a proper .gdb_index we can't test the gdb behaviour we're
trying to excercise. Fix this by detecting whether we actually used a
.gdb_index for symbol loading.
Tested on x86_64-linux.
gdb/testsuite/ChangeLog:
2021-07-14 Tom de Vries <tdevries@suse.de>
* lib/gdb.exp (have_index): New proc.
* gdb.base/gold-gdb-index.exp: Use have_index.
When building gdb with --disable-tui, we run into:
...
(gdb) frame apply all -- -^M
Undefined command: "-". Try "help".^M
(gdb) ERROR: Undefined command "frame apply all -- -".
UNRESOLVED: gdb.base/options.exp: test-frame-apply: frame apply all -- -
...
Fix this by detecting whether tui is supported, and skipping the tui-related
tests otherwise. Same in some gdb.tui test-cases.
Tested on x86_64-linux.
gdb/testsuite/ChangeLog:
2021-07-13 Tom de Vries <tdevries@suse.de>
* gdb.base/options.exp: Skip tui-related tests when tui is not
supported.
* gdb.python/tui-window-disabled.exp: Same.
* gdb.python/tui-window.exp: Same.
While testing the NixOS[1] packaging for gdb-11.0.90.tar.xz, I got the
following error:
[...]
CXX aarch32-tdep.o
CXX gdb.o
GEN init.c
/nix/store/26a78ync552m8j4sbjavhvkmnqir8c9y-bash-4.4-p23/bin/bash: ./make-init-c: /usr/bin/env: bad interpreter: No such file or directory
make[2]: *** [Makefile:1866: stamp-init] Error 126
make[2]: *** Waiting for unfinished jobs....
make[2]: Leaving directory '/build/gdb-11.0.90/gdb'
make[1]: *** [Makefile:9814: all-gdb] Error 2
make[1]: Leaving directory '/build/gdb-11.0.90'
make: *** [Makefile:903: all] Error 2
builder for '/nix/store/xs8my3rrc3l4kdlbpx0azh6q0v0jxphr-gdb-gdb-11.0.90.drv' failed with exit code 2
error: build of '/nix/store/xs8my3rrc3l4kdlbpx0azh6q0v0jxphr-gdb-gdb-11.0.90.drv' failed
In the nix build environment, /usr/bin/env is not present, only /bin/sh
is. This patch makes sure that gdb/make-init-c uses '/bin/sh' as
interpreter as this is the only one available on this platform.
I do not think this change will cause regressions on any other
configuration.
[1] https://nixos.org/
gdb/Changelog
* make-init-c: Use /bin/sh as shebang.
In particular, this supports register sets described by a regcache_map
which are fetched and stored with dedicated ptrace operations. These
functions are intended to be used in architecture-specific
fetch_registers and store_registers target methods.
Before PR gdb/28080 was fixed by the previous patch, GDB was crashing
like this:
(gdb) detach
Detaching from program: target:/any/program, process 3671843
Detaching from process 3671843
Ending remote debugging.
[Inferior 1 (process 3671843) detached]
In main
terminate called after throwing an instance of 'gdb_exception_error'
Aborted (core dumped)
Here's the exception above being thrown:
(top-gdb) bt
#0 throw_error (error=TARGET_CLOSE_ERROR, fmt=0x555556035588 "Remote connection closed") at src/gdbsupport/common-exceptions.cc:222
#1 0x0000555555bbaa46 in remote_target::readchar (this=0x555556a11040, timeout=10000) at src/gdb/remote.c:9440
#2 0x0000555555bbb9e5 in remote_target::getpkt_or_notif_sane_1 (this=0x555556a11040, buf=0x555556a11058, forever=0, expecting_notif=0, is_notif=0x0) at src/gdb/remote.c:9928
#3 0x0000555555bbbda9 in remote_target::getpkt_sane (this=0x555556a11040, buf=0x555556a11058, forever=0) at src/gdb/remote.c:10030
#4 0x0000555555bc0e75 in remote_target::remote_hostio_send_command (this=0x555556a11040, command_bytes=13, which_packet=14, remote_errno=0x7fffffffcfd0, attachment=0x0, attachment_len=0x0) at src/gdb/remote.c:12137
#5 0x0000555555bc1b6c in remote_target::remote_hostio_close (this=0x555556a11040, fd=8, remote_errno=0x7fffffffcfd0) at src/gdb/remote.c:12455
#6 0x0000555555bc1bb4 in remote_target::fileio_close (During symbol reading: .debug_line address at offset 0x64f417 is 0 [in module build/gdb/gdb]
this=0x555556a11040, fd=8, remote_errno=0x7fffffffcfd0) at src/gdb/remote.c:12462
#7 0x0000555555c9274c in target_fileio_close (fd=3, target_errno=0x7fffffffcfd0) at src/gdb/target.c:3365
#8 0x000055555595a19d in gdb_bfd_iovec_fileio_close (abfd=0x555556b9f8a0, stream=0x555556b11530) at src/gdb/gdb_bfd.c:439
#9 0x0000555555e09e3f in opncls_bclose (abfd=0x555556b9f8a0) at src/bfd/opncls.c:599
#10 0x0000555555e0a2c7 in bfd_close_all_done (abfd=0x555556b9f8a0) at src/bfd/opncls.c:847
#11 0x0000555555e0a27a in bfd_close (abfd=0x555556b9f8a0) at src/bfd/opncls.c:814
#12 0x000055555595a9d3 in gdb_bfd_close_or_warn (abfd=0x555556b9f8a0) at src/gdb/gdb_bfd.c:626
#13 0x000055555595ad29 in gdb_bfd_unref (abfd=0x555556b9f8a0) at src/gdb/gdb_bfd.c:715
#14 0x0000555555ae4730 in objfile::~objfile (this=0x555556515540, __in_chrg=<optimized out>) at src/gdb/objfiles.c:573
#15 0x0000555555ae955a in std::_Sp_counted_ptr<objfile*, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=0x555556c20db0) at /usr/include/c++/9/bits/shared_ptr_base.h:377
#16 0x000055555572b7c8 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x555556c20db0) at /usr/include/c++/9/bits/shared_ptr_base.h:155
#17 0x00005555557263c3 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=0x555556bf0588, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr_base.h:730
#18 0x0000555555ae745e in std::__shared_ptr<objfile, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=0x555556bf0580, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr_base.h:1169
#19 0x0000555555ae747e in std::shared_ptr<objfile>::~shared_ptr (this=0x555556bf0580, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr.h:103
#20 0x0000555555b1c1dc in __gnu_cxx::new_allocator<std::_List_node<std::shared_ptr<objfile> > >::destroy<std::shared_ptr<objfile> > (this=0x5555564cdd60, __p=0x555556bf0580) at /usr/include/c++/9/ext/new_allocator.h:153
#21 0x0000555555b1bb1d in std::allocator_traits<std::allocator<std::_List_node<std::shared_ptr<objfile> > > >::destroy<std::shared_ptr<objfile> > (__a=..., __p=0x555556bf0580) at /usr/include/c++/9/bits/alloc_traits.h:497
#22 0x0000555555b1b73e in std::__cxx11::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::_M_erase (this=0x5555564cdd60, __position=std::shared_ptr<objfile> (expired, weak count 1) = {get() = 0x555556515540}) at /usr/include/c++/9/bits/stl_list.h:1921
#23 0x0000555555b1afeb in std::__cxx11::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::erase (this=0x5555564cdd60, __position=std::shared_ptr<objfile> (expired, weak count 1) = {get() = 0x555556515540}) at /usr/include/c++/9/bits/list.tcc:158
#24 0x0000555555b19576 in program_space::remove_objfile (this=0x5555564cdd20, objfile=0x555556515540) at src/gdb/progspace.c:210
#25 0x0000555555ae4502 in objfile::unlink (this=0x555556515540) at src/gdb/objfiles.c:487
#26 0x0000555555ae5a12 in objfile_purge_solibs () at src/gdb/objfiles.c:875
#27 0x0000555555c09686 in no_shared_libraries (ignored=0x0, from_tty=1) at src/gdb/solib.c:1236
#28 0x00005555559e3f5f in detach_command (args=0x0, from_tty=1) at src/gdb/infcmd.c:2769
Note frame #14:
#14 0x0000555555ae4730 in objfile::~objfile (this=0x555556515540, __in_chrg=<optimized out>) at src/gdb/objfiles.c:573
That's a dtor, thus noexcept. That's the reason for the
std::terminate.
The previous patch fixed things such that the exception above isn't
thrown anymore. However, it's possible that e.g., the remote
connection drops just while a user types "nosharedlibrary", or some
other reason that leads to objfile::~objfile, and then we end up the
same std::terminate problem.
Also notice that frames #9-#11 are BFD frames:
#9 0x0000555555e09e3f in opncls_bclose (abfd=0x555556bc27e0) at src/bfd/opncls.c:599
#10 0x0000555555e0a2c7 in bfd_close_all_done (abfd=0x555556bc27e0) at src/bfd/opncls.c:847
#11 0x0000555555e0a27a in bfd_close (abfd=0x555556bc27e0) at src/bfd/opncls.c:814
BFD is written in C and thus throwing exceptions over such frames may
either not clean up properly, or, may abort if bfd is not compiled
with -fasynchronous-unwind-tables (x86-64 defaults that on, but not
all GCC ports do).
Thus frame #8 seems like a good place to swallow exceptions. More so
since in this spot we already ignore target_fileio_close return
errors. That's what this commit does. Without the previous fix, we'd
see:
(gdb) detach
Detaching from program: target:/any/program, process 2197701
Ending remote debugging.
[Inferior 1 (process 2197701) detached]
warning: cannot close "target:/lib64/ld-linux-x86-64.so.2": Remote connection closed
Note it prints a warning, which would still be a regression compared
to GDB 10, if it weren't for the previous fix.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <pedro@palves.net>
PR gdb/28080
* gdb_bfd.c (gdb_bfd_close_warning): New.
(gdb_bfd_iovec_fileio_close): Wrap target_fileio_close in
try/catch and print warning on exception.
(gdb_bfd_close_or_warn): Use gdb_bfd_close_warning.
Change-Id: Ic7a26ddba0a4444e3377b0e7c1c89934a84545d7
Commit 408f66864a ("detach in all-stop
with threads running") regressed "detach" with "target remote":
(gdb) detach
Detaching from program: target:/any/program, process 3671843
Detaching from process 3671843
Ending remote debugging.
[Inferior 1 (process 3671843) detached]
In main
terminate called after throwing an instance of 'gdb_exception_error'
Aborted (core dumped)
Here's the exception above being thrown:
(top-gdb) bt
#0 throw_error (error=TARGET_CLOSE_ERROR, fmt=0x555556035588 "Remote connection closed") at src/gdbsupport/common-exceptions.cc:222
#1 0x0000555555bbaa46 in remote_target::readchar (this=0x555556a11040, timeout=10000) at src/gdb/remote.c:9440
#2 0x0000555555bbb9e5 in remote_target::getpkt_or_notif_sane_1 (this=0x555556a11040, buf=0x555556a11058, forever=0, expecting_notif=0, is_notif=0x0) at src/gdb/remote.c:9928
#3 0x0000555555bbbda9 in remote_target::getpkt_sane (this=0x555556a11040, buf=0x555556a11058, forever=0) at src/gdb/remote.c:10030
#4 0x0000555555bc0e75 in remote_target::remote_hostio_send_command (this=0x555556a11040, command_bytes=13, which_packet=14, remote_errno=0x7fffffffcfd0, attachment=0x0, attachment_len=0x0) at src/gdb/remote.c:12137
#5 0x0000555555bc1b6c in remote_target::remote_hostio_close (this=0x555556a11040, fd=8, remote_errno=0x7fffffffcfd0) at src/gdb/remote.c:12455
#6 0x0000555555bc1bb4 in remote_target::fileio_close (During symbol reading: .debug_line address at offset 0x64f417 is 0 [in module build/gdb/gdb]
this=0x555556a11040, fd=8, remote_errno=0x7fffffffcfd0) at src/gdb/remote.c:12462
#7 0x0000555555c9274c in target_fileio_close (fd=3, target_errno=0x7fffffffcfd0) at src/gdb/target.c:3365
#8 0x000055555595a19d in gdb_bfd_iovec_fileio_close (abfd=0x555556b9f8a0, stream=0x555556b11530) at src/gdb/gdb_bfd.c:439
#9 0x0000555555e09e3f in opncls_bclose (abfd=0x555556b9f8a0) at src/bfd/opncls.c:599
#10 0x0000555555e0a2c7 in bfd_close_all_done (abfd=0x555556b9f8a0) at src/bfd/opncls.c:847
#11 0x0000555555e0a27a in bfd_close (abfd=0x555556b9f8a0) at src/bfd/opncls.c:814
#12 0x000055555595a9d3 in gdb_bfd_close_or_warn (abfd=0x555556b9f8a0) at src/gdb/gdb_bfd.c:626
#13 0x000055555595ad29 in gdb_bfd_unref (abfd=0x555556b9f8a0) at src/gdb/gdb_bfd.c:715
#14 0x0000555555ae4730 in objfile::~objfile (this=0x555556515540, __in_chrg=<optimized out>) at src/gdb/objfiles.c:573
#15 0x0000555555ae955a in std::_Sp_counted_ptr<objfile*, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=0x555556c20db0) at /usr/include/c++/9/bits/shared_ptr_base.h:377
#16 0x000055555572b7c8 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x555556c20db0) at /usr/include/c++/9/bits/shared_ptr_base.h:155
#17 0x00005555557263c3 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=0x555556bf0588, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr_base.h:730
#18 0x0000555555ae745e in std::__shared_ptr<objfile, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=0x555556bf0580, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr_base.h:1169
#19 0x0000555555ae747e in std::shared_ptr<objfile>::~shared_ptr (this=0x555556bf0580, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr.h:103
#20 0x0000555555b1c1dc in __gnu_cxx::new_allocator<std::_List_node<std::shared_ptr<objfile> > >::destroy<std::shared_ptr<objfile> > (this=0x5555564cdd60, __p=0x555556bf0580) at /usr/include/c++/9/ext/new_allocator.h:153
#21 0x0000555555b1bb1d in std::allocator_traits<std::allocator<std::_List_node<std::shared_ptr<objfile> > > >::destroy<std::shared_ptr<objfile> > (__a=..., __p=0x555556bf0580) at /usr/include/c++/9/bits/alloc_traits.h:497
#22 0x0000555555b1b73e in std::__cxx11::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::_M_erase (this=0x5555564cdd60, __position=std::shared_ptr<objfile> (expired, weak count 1) = {get() = 0x555556515540}) at /usr/include/c++/9/bits/stl_list.h:1921
#23 0x0000555555b1afeb in std::__cxx11::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::erase (this=0x5555564cdd60, __position=std::shared_ptr<objfile> (expired, weak count 1) = {get() = 0x555556515540}) at /usr/include/c++/9/bits/list.tcc:158
#24 0x0000555555b19576 in program_space::remove_objfile (this=0x5555564cdd20, objfile=0x555556515540) at src/gdb/progspace.c:210
#25 0x0000555555ae4502 in objfile::unlink (this=0x555556515540) at src/gdb/objfiles.c:487
#26 0x0000555555ae5a12 in objfile_purge_solibs () at src/gdb/objfiles.c:875
#27 0x0000555555c09686 in no_shared_libraries (ignored=0x0, from_tty=1) at src/gdb/solib.c:1236
#28 0x00005555559e3f5f in detach_command (args=0x0, from_tty=1) at src/gdb/infcmd.c:2769
So frame #28 already detached the remote process, and then we're
purging the shared libraries. GDB had opened remote shared libraries
via the target: sysroot, so it tries closing them. GDBserver is
tearing down already, so remote communication breaks down and we close
the remote target and throw TARGET_CLOSE_ERROR.
Note frame #14:
#14 0x0000555555ae4730 in objfile::~objfile (this=0x555556515540, __in_chrg=<optimized out>) at src/gdb/objfiles.c:573
That's a dtor, thus noexcept. That's the reason for the
std::terminate.
Stepping back a bit, why do we still have open remote files if we've
managed to detach already, and, we're debugging with "target remote"?
The reason is that commit 408f66864a
makes detach_command hold a reference to the target, so the remote
target won't be finally closed until frame #28 returns. It's closing
the target that invalidates target file I/O handles.
This commit fixes the issue by not relying on target_close to
invalidate the target file I/O handles, instead invalidate them
immediately in remote_unpush_target. So when GDB purges the solibs,
and we end up in target_fileio_close (frame #7 above), there's nothing
to do, and we don't try to talk with the remote target anymore.
The regression isn't seen when testing with
--target_board=native-gdbserver, because that does "set sysroot" to
disable the "target:" sysroot, for test run speed reasons. So this
commit adds a testcase that explicitly tests detach with "set sysroot
target:".
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <pedro@palves.net>
PR gdb/28080
* remote.c (remote_unpush_target): Invalidate file I/O target
handles.
* target.c (fileio_handles_invalidate_target): Make extern.
* target.h (fileio_handles_invalidate_target): Declare.
gdb/testsuite/ChangeLog:
yyyy-mm-dd Pedro Alves <pedro@palves.net>
PR gdb/28080
* gdb.base/detach-sysroot-target.exp: New.
* gdb.base/detach-sysroot-target.c: New.
Reported-By: Jonah Graham <jonah@kichwacoders.com>
Change-Id: I851234910172f42a1b30e731161376c344d2727d
When running test-case gdb.threads/check-libthread-db.exp on openSUSE
Tumbleweed with glibc 2.33, I get:
...
(gdb) maint check libthread-db^M
Running libthread_db integrity checks:^M
Got thread 0x7ffff7c79b80 => 9354 => 0x7ffff7c79b80; errno = 0 ... OK^M
libthread_db integrity checks passed.^M
(gdb) FAIL: gdb.threads/check-libthread-db.exp: user-initiated check: \
libpthread.so not initialized (pattern 2)
...
The test-case expects instead:
...
Got thread 0x0 => 9354 => 0x0 ... OK^M
...
which is what I get on openSUSE Leap 15.2 with glibc 2.26, and what is
described in the test-case like this:
...
# libthread_db should fake a single thread with th_unique == NULL.
...
Using a breakpoint on check_thread_db_callback we can compare the two
scenarios, and find that in the latter case we hit this code in glibc function
iterate_thread_list in nptl_db/td_ta_thr_iter.c:
...
if (next == 0 && fake_empty)
{
/* __pthread_initialize_minimal has not run. There is just the main
thread to return. We cannot rely on its thread register. They
sometimes contain garbage that would confuse us, left by the
kernel at exec. So if it looks like initialization is incomplete,
we only fake a special descriptor for the initial thread. */
td_thrhandle_t th = { ta, 0 };
return callback (&th, cbdata_p) != 0 ? TD_DBERR : TD_OK;
}
...
while in the former case we don't because this preceding statement doesn't
result in next == 0:
...
err = DB_GET_FIELD (next, ta, head, list_t, next, 0);
...
Note that the comment mentions __pthread_initialize_minimal, but in both cases
it has already run before we hit the callback, so it's possible the comment is
no longer accurate.
The change in behaviour bisect to glibc commit 1daccf403b "nptl: Move stack
list variables into _rtld_global", which moves the initialization of stack
list variables such as __stack_user to an earlier moment, which explains well
enough the observed difference.
Fix this by updating the regexp patterns to agree with what libthread-db is
telling us.
Tested on x86_64-linux, both with glibc 2.33 and 2.26.
gdb/testsuite/ChangeLog:
2021-07-07 Tom de Vries <tdevries@suse.de>
PR testsuite/27690
* gdb.threads/check-libthread-db.exp: Update patterns for glibc 2.33.
During prefix resolution, if the parent is a subprogram, there is no need
to go to the parent of the subprogram. The DIE will be local.
For a program like:
~~~
class F1
{
public:
int a;
int
vvv ()
{
class F2
{
int f;
};
F2 abcd;
return 1;
}
};
~~~
The class F2 should not be seen as a member of F1.
Before:
~~~
(gdb) ptype abcd
type = class F1::F2 {
private:
int f;
}
~~~
After:
~~~
(gdb) ptype abcd
type = class F2 {
private:
int f;
}
~~~
gdb/ChangeLog:
2021-06-23 Felix Willgerodt <felix.willgerodt@intel.com>
* dwarf2/read.c (determine_prefix): Return an empty prefix if the
parent is a subprogram.
gdb/testsuite/ChangeLog:
2021-06-23 Felix Willgerodt <felix.willgerodt@intel.com>
* gdb.cp/nested-class-func-class.cc: New file.
* gdb.cp/nested-class-func-class.exp: New file.
As reported in PR gdb/28077, we hit an internal error when using
-exec-interrupt with --thread-group:
info threads
&"info threads\n"
~" Id Target Id Frame \n"
~"* 1 process 403312 \"loop\" (running)\n"
^done
(gdb)
-exec-interrupt --thread-group i1
~"/home/simark/src/binutils-gdb/gdb/target.c:3768: internal-error: void target_stop(ptid_t): Assertion `!proc_target->commit_resumed_state' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit this debugging session? (y or n) "
This is because this code path never disables commit-resumed (a
requirement for calling target_stop, as documented in
process_stratum_target::»commit_resumed_state) before calling
target_stop.
The other 3 code paths in mi_cmd_exec_interrupt use interrupt_target_1,
which does it. But the --thread-group code path uses its own thing
which doesn't do it. Fix this by adding a scoped_disable_commit_resumed
in this code path.
Calling -exec-interrupt with --thread-group is apparently not tested at
the moment (which is why this bug could creep in). Add a new test for
that. The test runs two inferiors and tries to interrupt them with
"-exec-interrupt --thread-group X".
This will need to be merged in the gdb-11-branch, so here are ChangeLog
entries:
gdb/ChangeLog:
* mi/mi-main.c (mi_cmd_exec_interrupt): Use
scoped_disable_commit_resumed in the --thread-group case.
gdb/testsuite/ChangeLog:
* gdb.mi/interrupt-thread-group.c: New.
* gdb.mi/interrupt-thread-group.exp: New.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28077
Change-Id: I615efefcbcaf2c15d47caf5e4b9d82854b2a2fcb
Since gcc commit, 3c70b3ca1ef58f302bf8c16d9e7c7bb8626408bf, we now enable
elf attributes for all riscv targets by default in gcc. Therefore, I
think binutils should have the same behavior, in case users are writing
assembly files. If --enable-default-riscv-attribute isn't set, then we
enable the elf attributes for all riscv targets by default.
ChangLog:
binutils/
* testsuite/binutils-all/readelf.s: Add comments for riscv.
* testsuite/binutils-all/readelf.s-64: Likewise.
* testsuite/binutils-all/readelf.s-64-unused: Likewise.
* testsuite/binutils-all/readelf.ss: Likewise.
* testsuite/binutils-all/readelf.ss-64: Likewise.
* testsuite/binutils-all/readelf.ss-64-unused: Likewise.
gas/
* configure.ac: If --enable-default-riscv-attribute isn't set,
then we enable the elf attributes for all riscv targets by
default.
* configure: Regenerated.
These files were renamed in 1b71cfcfdc,
but evidentially a few dangling references were left behind. This causes
builds to fail:
$ ./configure --target i686-netbsdelf
$ make
make: *** No rule to make target 'nbsd-tdep.c', needed by 'nbsd-tdep.o'. Stop.
all_matching_threads_iterator is used extensively in some pretty fast
paths, often under the all_non_exited_threads function.
If a filter target and thread-specific ptid are given, it iterates on
all threads of all inferiors of that target, to ultimately yield exactly
on thread. And this happens quite often, which means we unnecessarily
spend time iterating on threads to find the one we are looking for. The
same thing happens if an inferior-specific ptid is given, although there
the iterator yields all the threads of that inferior.
In those cases, the callers of all_non_exited_threads could have
different behaviors depending on the kind of ptid, to avoid this
inefficiency, but that would be very tedious. Using
all_non_exited_threads has the advantage that one simple implementation
can work seamlessly on multiple threads or on one specific thread, just
by playing with the ptid.
Instead, optimize all_matching_threads_iterator directly to detect these
different cases and limiting what we iterate on to just what we need.
- if filter_ptid is minus_one_ptid, do as we do now: filter inferiors
based on filter_target, iterate on all of the matching inferiors'
threads
- if filter_ptid is a pid-only ptid (then a filter_target must
necessarily be given), look up that inferior and iterate on all its
threads
- otherwise, filter_ptid is a thread-specific ptid, so look up that
specific thread and "iterate" only on it
For the last case, what was an iteration on all threads of the filter
target now becomes a call to find_thread_ptid, which is quite efficient
now thanks to inferior::ptid_thread_map.
gdb/ChangeLog:
* thread-iter.h (class all_matching_threads_iterator)
<all_matching_threads_iterator>: Use default.
<enum class mode>: New.
<m_inf, m_thr>: Initialize.
<m_filter_ptid>: Remove.
* thread-iter.c (all_matching_threads_iterator::m_inf_matches):
Don't filter on m_filter_ptid.
(all_matching_threads_iterator::all_matching_threads_iterator):
Choose path based on filter_ptid (all threads, all threads of
inferior, single thread).
(all_matching_threads_iterator::advance): Likewise.
Change-Id: Ic6a19845f5f760fa1b8eac8145793c0ff431bbc9
When debugging a large number of threads (thousands), looking up a
thread by ptid_t using the inferior::thread_list linked list can add up.
Add inferior::thread_map, an std::unordered_map indexed by ptid_t, and
change the find_thread_ptid function to look up a thread using
std::unordered_map::find, instead of iterating on all of the
inferior's threads. This should make it faster to look up a thread
from its ptid.
Change-Id: I3a8da0a839e18dee5bb98b8b7dbeb7f3dfa8ae1c
Co-Authored-By: Pedro Alves <pedro@palves.net>
Consider a case where many threads (thousands) keep hitting a breakpoint
whose condition evaluates to false. random_pending_event_thread is
responsible for selecting a thread from an inferior among all that are
resumed with a pending wait status. It is currently implemented by
walking the inferior's thread list twice: once to count the number of
candidates and once to select a random one.
Since we now maintain a per target list of resumed threads with pending
event, we can implement this more efficiently by walking that list and
selecting the first thread that matches the criteria
(random_pending_event_thread looks for an thread from a specific
inferior, and possibly a filter ptid). It will be faster especially in
the common case where there isn't any resumed thread with pending
event. Currently, we have to iterate the thread list to figure this
out. With this patch, the list of resumed threads with pending event
will be empty, so it's quick to figure out.
The random selection is kept, but is moved to
process_stratum_target::random_resumed_with_pending_wait_status. The
same technique is used: do a first pass to count the number of
candidates, and do a second pass to select a random one. But given that
the list of resumed threads with pending wait statuses will generally be
short, or at least shorter than the full thread list, it should be
quicker.
Note that this isn't completely true, in case there are multiple
inferiors on the same target. Imagine that inferior A has 10k resumed
threads with pending wait statuses, and random_pending_event_thread is
called with inferior B. We'll need to go through the list that contains
inferior A's threads to realize that inferior B has no resumed threads
with pending wait status. But I think that this is a corner /
pathological case. And a possible fix for this situation would be to
make random_pending_event_thread work per-process-target, rather than
per-inferior.
Change-Id: I1b71d01beaa500a148b5b9797745103e13917325
Consider a test case where many threads (thousands) keep hitting a
breakpoint whose condition evaluates to false.
maybe_set_commit_resumed_all_targets is called at each handled event,
when the scoped_disable_commit_resumed object in fetch_inferior_event is
reset_and_commit-ed. One particularly expensive check in there is
whether the target has at least one resumed thread with a pending wait
status (in which case, we don't want to commit the resumed threads, as
we want to consume this status first). It is currently implemented as
walking all threads of the target.
Since we now maintain a per-target list of resumed threads with pending
status, we can do this check efficiently, by checking whether that list
is empty or not.
Add the process_stratum_target::has_resumed_with_pending_wait_status
method for this, and use it in maybe_set_commit_resumed_all_targets.
Change-Id: Ia1595baa1b358338f94fc3cb3af7f27092dad5b6
Looking up threads that are both resumed and have a pending wait
status to report is something that we do quite often in the fast path
and is expensive if there are many threads, since it currently requires
walking whole thread lists.
The first instance is in maybe_set_commit_resumed_all_targets. This is
called after handling each event in fetch_inferior_event, to see if we
should ask targets to commit their resumed threads or not. If at least
one thread is resumed but has a pending wait status, we don't ask the
targets to commit their resumed threads, because we want to consume and
handle the pending wait status first.
The second instance is in random_pending_event_thread, where we want to
select a random thread among all those that are resumed and have a
pending wait status. This is called every time we try to consume
events, to see if there are any pending events that we we want to
consume, before asking the targets for more events.
To allow optimizing these cases, maintain a per-process-target list of
threads that are resumed and have a pending wait status.
In maybe_set_commit_resumed_all_targets, we'll be able to check in O(1)
if there are any such threads simply by checking whether the list is
empty.
In random_pending_event_thread, we'll be able to use that list, which
will be quicker than iterating the list of threads, especially when
there are no resumed with pending wait status threads.
About implementation details: using the new setters on class
thread_info, it's relatively easy to maintain that list. Any time the
"resumed" or "pending wait status" property is changed, we check whether
that should cause the thread to be added or removed from the list.
In set_thread_exited, we try to remove the thread from the list, because
keeping an exited thread in that list would make no sense (especially if
the thread is freed). My first implementation assumed that a process
stratum target was always present when set_thread_exited is called.
That's however, not the case: in some cases, targets unpush themselves
from an inferior and then call "exit_inferior", which exits all the
threads. If the target is unpushed before set_thread_exited is called
on the threads, it means we could mistakenly leave some threads in the
list. I tried to see how hard it would be to make it such that targets
have to exit all threads before unpushing themselves from the inferior
(that would seem logical to me, we don't want threads belonging to an
inferior that has no process target). That seemed quite difficult and
not worth the time at the moment. Instead, I changed
inferior::unpush_target to remove all threads of that inferior from the
list.
As of this patch, the list is not used, this is done in the subsequent
patches.
The debug messages in process-stratum-target.c need to print some ptids.
However, they can't use target_pid_to_str to print them without
introducing a dependency on the current inferior (the current inferior
is used to get the current target stack). For debug messages, I find it
clearer to print the spelled out ptid anyway (the pid, lwp and tid
values). Add a ptid_t::to_string method that returns a string
representation of the ptid that is meant for debug messages, a bit like
we already have frame_id::to_string.
Change-Id: Iad8f93db2d13984dd5aa5867db940ed1169dbb67
A following patch will want to take some action when a pending wait
status is set on or removed from a thread. Add a getter and a setter on
thread_info for the pending waitstatus, so that we can add some code in
the setter later.
The thing is, the pending wait status field is in the
thread_suspend_state, along with other fields that we need to backup
before and restore after the thread does an inferior function call.
Therefore, make the thread_suspend_state member private
(thread_info::suspend becomes thread_info::m_suspend), and add getters /
setters for all of its fields:
- pending wait status
- stop signal
- stop reason
- stop pc
For the pending wait status, add the additional has_pending_waitstatus
and clear_pending_waitstatus methods.
I think this makes the thread_info interface a bit nicer, because we
now access the fields as:
thread->stop_pc ()
rather than
thread->suspend.stop_pc
The stop_pc field being in the `suspend` structure is an implementation
detail of thread_info that callers don't need to be aware of.
For the backup / restore of the thread_suspend_state structure, add
save_suspend_to and restore_suspend_from methods. You might wonder why
`save_suspend_to`, as opposed to a simple getter like
thread_suspend_state &suspend ();
I want to make it clear that this is to be used only for backing up and
restoring the suspend state, _not_ to access fields like:
thread->suspend ()->stop_pc
Adding some getters / setters allows adding some assertions. I find
that this helps understand how things are supposed to work. Add:
- When getting the pending status (pending_waitstatus method), ensure
that there is a pending status.
- When setting a pending status (set_pending_waitstatus method), ensure
there is no pending status.
There is one case I found where this wasn't true - in
remote_target::process_initial_stop_replies - which needed adjustments
to respect that contract. I think it's because
process_initial_stop_replies is kind of (ab)using the
thread_info::suspend::waitstatus to store some statuses temporarily, for
its internal use (statuses it doesn't intent on leaving pending).
process_initial_stop_replies pulls out stop replies received during the
initial connection using target_wait. It always stores the received
event in `evthread->suspend.waitstatus`. But it only sets
waitstatus_pending_p, if it deems the event interesting enough to leave
pending, to be reported to the core:
if (ws.kind != TARGET_WAITKIND_STOPPED
|| ws.value.sig != GDB_SIGNAL_0)
evthread->suspend.waitstatus_pending_p = 1;
It later uses this flag a bit below, to choose which thread to make the
"selected" one:
if (selected == NULL
&& thread->suspend.waitstatus_pending_p)
selected = thread;
And ultimately that's used if the user-visible mode is all-stop, so that
we print the stop for that interesting thread:
/* In all-stop, we only print the status of one thread, and leave
others with their status pending. */
if (!non_stop)
{
thread_info *thread = selected;
if (thread == NULL)
thread = lowest_stopped;
if (thread == NULL)
thread = first;
print_one_stopped_thread (thread);
}
But in any case (all-stop or non-stop), print_one_stopped_thread needs
to access the waitstatus value of these threads that don't have a
pending waitstatus (those that had TARGET_WAITKIND_STOPPED +
GDB_SIGNAL_0). This doesn't work with the assertions I've
put.
So, change the code to only set the thread's wait status if it is an
interesting one that we are going to leave pending. If the thread
stopped due to a non-interesting event (TARGET_WAITKIND_STOPPED +
GDB_SIGNAL_0), don't store it. Adjust print_one_stopped_thread to
understand that if a thread has no pending waitstatus, it's because it
stopped with TARGET_WAITKIND_STOPPED + GDB_SIGNAL_0.
The call to set_last_target_status also uses the pending waitstatus.
However, given that the pending waitstatus for the thread may have been
cleared in print_one_stopped_thread (and that there might not even be a
pending waitstatus in the first place, as explained above), it is no
longer possible to do it at this point. To fix that, move the call to
set_last_target_status in print_one_stopped_thread. I think this will
preserve the existing behavior, because set_last_target_status is
currently using the current thread's wait status. And the current
thread is the last one for which print_one_stopped_thread is called. So
by calling set_last_target_status in print_one_stopped_thread, we'll get
the same result. set_last_target_status will possibly be called
multiple times, but only the last call will matter. It just means
possibly more calls to set_last_target_status, but those are cheap.
Change-Id: Iedab9653238eaf8231abcf0baa20145acc8b77a7
A following patch will want to do things when a thread's resumed state
changes. Make the `resumed` field private (renamed to `m_resumed`) and
add a getter and a setter for it. The following patch in question will
therefore be able to add some code to the setter.
Change-Id: I360c48cc55a036503174313261ce4e757d795319
The threads that need a step-over are currently linked using an
hand-written intrusive doubly-linked list, so that seems a very good
candidate for intrusive_list, convert it.
For this, we have a use case of appending a list to another one (in
start_step_over). Based on the std::list and Boost APIs, add a splice
method. However, only support splicing the other list at the end of the
`this` list, since that's all we need.
Add explicit default assignment operators to
reference_to_pointer_iterator, which are otherwise implicitly deleted.
This is needed because to define thread_step_over_list_safe_iterator, we
wrap reference_to_pointer_iterator inside a basic_safe_iterator, and
basic_safe_iterator needs to be able to copy-assign the wrapped
iterator. The move-assignment operator is therefore not needed, only
the copy-assignment operator is. But for completeness, add both.
Change-Id: I31b2ff67c7b78251314646b31887ef1dfebe510c
Change inferior_list, the global list of inferiors, to use
intrusive_list. I think most other changes are somewhat obvious
fallouts from this change.
There is a small change in behavior in scoped_mock_context. Before this
patch, constructing a scoped_mock_context would replace the whole
inferior list with only the new mock inferior. Tests using two
scoped_mock_contexts therefore needed to manually link the two inferiors
together, as the second scoped_mock_context would bump the first mock
inferior from the thread list. With this patch, a scoped_mock_context
adds its mock inferior to the inferior list on construction, and removes
it on destruction. This means that tests run with mock inferiors in the
inferior list in addition to any pre-existing inferiors (there is always
at least one). There is no possible pid clash problem, since each
scoped mock inferior uses its own process target, and pids are per
process target.
Co-Authored-By: Simon Marchi <simon.marchi@efficios.com>
Change-Id: I7eb6a8f867d4dcf8b8cd2dcffd118f7270756018
GDB currently has several objects that are put in a singly linked list,
by having the object's type have a "next" pointer directly. For
example, struct thread_info and struct inferior. Because these are
simply-linked lists, and we don't keep track of a "tail" pointer, when
we want to append a new element on the list, we need to walk the whole
list to find the current tail. It would be nice to get rid of that
walk. Removing elements from such lists also requires a walk, to find
the "previous" position relative to the element being removed. To
eliminate the need for that walk, we could make those lists
doubly-linked, by adding a "prev" pointer alongside "next". It would be
nice to avoid the boilerplate associated with maintaining such a list
manually, though. That is what the new intrusive_list type addresses.
With an intrusive list, it's also possible to move items out of the
list without destroying them, which is interesting in our case for
example for threads, when we exit them, but can't destroy them
immediately. We currently keep exited threads on the thread list, but
we could change that which would simplify some things.
Note that with std::list, element removal is O(N). I.e., with
std::list, we need to walk the list to find the iterator pointing to
the position to remove. However, we could store a list iterator
inside the object as soon as we put the object in the list, to address
it, because std::list iterators are not invalidated when other
elements are added/removed. However, if you need to put the same
object in more than one list, then std::list<object> doesn't work.
You need to instead use std::list<object *>, which is less efficient
for requiring extra memory allocations. For an example of an object
in multiple lists, see the step_over_next/step_over_prev fields in
thread_info:
/* Step-over chain. A thread is in the step-over queue if these are
non-NULL. If only a single thread is in the chain, then these
fields point to self. */
struct thread_info *step_over_prev = NULL;
struct thread_info *step_over_next = NULL;
The new intrusive_list type gives us the advantages of an intrusive
linked list, while avoiding the boilerplate associated with manually
maintaining it.
intrusive_list's API follows the standard container interface, and thus
std::list's interface. It is based the API of Boost's intrusive list,
here:
https://www.boost.org/doc/libs/1_73_0/doc/html/boost/intrusive/list.html
Our implementation is relatively simple, while Boost's is complicated
and intertwined due to a lot of customization options, which our version
doesn't have.
The easiest way to use an intrusive_list is to make the list's element
type inherit from intrusive_node. This adds a prev/next pointers to
the element type. However, to support putting the same object in more
than one list, intrusive_list supports putting the "node" info as a
field member, so you can have more than one such nodes, one per list.
As a first guinea pig, this patch makes the per-inferior thread list use
intrusive_list using the base class method.
Unlike Boost's implementation, ours is not a circular list. An earlier
version of the patch was circular: the intrusive_list type included an
intrusive_list_node "head". In this design, a node contained pointers
to the previous and next nodes, not the previous and next elements.
This wasn't great for when debugging GDB with GDB, as it was difficult
to get from a pointer to the node to a pointer to the element. With the
design proposed in this patch, nodes contain pointers to the previous
and next elements, making it easy to traverse the list by hand and
inspect each element.
The intrusive_list object contains pointers to the first and last
elements of the list. They are nullptr if the list is empty.
Each element's node contains a pointer to the previous and next
elements. The first element's previous pointer is nullptr and the last
element's next pointer is nullptr. Therefore, if there's a single
element in the list, both its previous and next pointers are nullptr.
To differentiate such an element from an element that is not linked into
a list, the previous and next pointers contain a special value (-1) when
the node is not linked. This is necessary to be able to reliably tell
if a given node is currently linked or not.
A begin() iterator points to the first item in the list. An end()
iterator contains nullptr. This makes iteration until end naturally
work, as advancing past the last element will make the iterator contain
nullptr, making it equal to the end iterator. If the list is empty,
a begin() iterator will contain nullptr from the start, and therefore be
immediately equal to the end.
Iterating on an intrusive_list yields references to objects (e.g.
`thread_info&`). The rest of GDB currently expects iterators and ranges
to yield pointers (e.g. `thread_info*`). To bridge the gap, add the
reference_to_pointer_iterator type. It is used to define
inf_threads_iterator.
Add a Python pretty-printer, to help inspecting intrusive lists when
debugging GDB with GDB. Here's an example of the output:
(top-gdb) p current_inferior_.m_obj.thread_list
$1 = intrusive list of thread_info = {0x61700002c000, 0x617000069080, 0x617000069400, 0x61700006d680, 0x61700006eb80}
It's not possible with current master, but with this patch [1] that I
hope will be merged eventually, it's possible to index the list and
access the pretty-printed value's children:
(top-gdb) p current_inferior_.m_obj.thread_list[1]
$2 = (thread_info *) 0x617000069080
(top-gdb) p current_inferior_.m_obj.thread_list[1].ptid
$3 = {
m_pid = 406499,
m_lwp = 406503,
m_tid = 0
}
Even though iterating the list in C++ yields references, the Python
pretty-printer yields pointers. The reason for this is that the output
of printing the thread list above would be unreadable, IMO, if each
thread_info object was printed in-line, since they contain so much
information. I think it's more useful to print pointers, and let the
user drill down as needed.
[1] https://sourceware.org/pipermail/gdb-patches/2021-April/178050.html
Co-Authored-By: Simon Marchi <simon.marchi@efficios.com>
Change-Id: I3412a14dc77f25876d742dab8f44e0ba7c7586c0
Recently I started to see this fail with trunk:
...
(gdb) record instruction-history^M
1 0x00000000004004ab <main+4>: call 0x4004b7 <test>^M
2 0x00000000004004c6 <test+15>: mov $0x1,%eax^M
3 0x00000000004004cb <test+20>: ret ^M
(gdb) FAIL: gdb.btrace/tsx.exp: speculation indication
...
This is due to an intel microcode update (1) that disables Intel TSX by default.
Fix this by updating the pattern.
Tested on x86_64-linux, with both gcc 7.5.0 and clang 12.0.1.
[1] https://www.intel.com/content/www/us/en/support/articles/000059422/processors.html
gdb/testsuite/ChangeLog:
2021-07-12 Tom de Vries <tdevries@suse.de>
PR testsuite/28057
* gdb.btrace/tsx.exp: Add pattern for system with tsx disabled in
microcode.
When running test-case gdb.mi/mi-info-sources.exp, I run into:
...
Running src/gdb/testsuite/gdb.mi/mi-info-sources.exp ...
ERROR: internal buffer is full.
...
due to extra debug info from the shared libraries.
Fix this by using "nosharedlibrary".
Then I run into these FAILs:
...
FAIL: gdb.mi/mi-info-sources.exp: debug_read=false: \
-file-list-exec-source-files (unexpected output)
FAIL: gdb.mi/mi-info-sources.exp: debug_read=true: \
-file-list-exec-source-files (unexpected output)
FAIL: gdb.mi/mi-info-sources.exp: debug_read=true: \
-file-list-exec-source-files --group-by-objfile, look for \
mi-info-sources.c (unexpected output)
FAIL: gdb.mi/mi-info-sources.exp: debug_read=true: \
-file-list-exec-source-files --group-by-objfile, look for \
mi-info-sources-base.c (unexpected output)
...
due to openSUSE executables which have debug info for objects from sources
like sysdeps/x86_64/crtn.S.
Fix these by updating the patterns, and adding "maint expand-symtabs" to
reliably get fully-read objfiles.
Then I run into FAILs when using the readnow target board. Fix these by
skipping the relevant tests.
Then I run into FAILs when using the cc-with-gnu-debuglink board. Fix these
by updating the patterns.
Tested on x86_64-linux, with native, check-read1, readnow, cc-with-gdb-index,
cc-with-debug-names, cc-with-gnu-debuglink, cc-with-dwz, cc-with-dwz-m.
gdb/testsuite/ChangeLog:
2021-07-05 Tom de Vries <tdevries@suse.de>
* lib/mi-support.exp (mi_readnow): New proc.
* gdb.mi/mi-info-sources.exp: Use nosharedlibrary. Update patterns.
Skip tests for readnow. Use "maint expand-symtabs".
