I built a random project with -gctf, in order to test the CTF support in
GDB. With my ASan/UBSan/etc-enabled build of GDB, I get:
$ ./gdb --data-directory=data-directory /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0
...
Reading symbols from /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0...
/home/simark/src/binutils-gdb/gdb/ctfread.c:1545:31: runtime error: member call on misaligned address 0xbebebebebebebebe for type 'struct buildsym_compunit', which requires 8 byte alignment
0xbebebebebebebebe: note: pointer points here
The 0xbebebebebebebebe value is a sign that the ctf_context::builder
field is uninitialized. The problem probably goes under the radar if
the field happens to be zero-initialized, because ctf_start_archive
contains this code:
if (ccx->builder == nullptr)
{
ccx->builder = new buildsym_compunit (of,
of->original_name, nullptr, language_c, 0);
If the field was zero-initialized (by chance), this will create a new
buildsym_compunit. But if the field was purposely filled with random
bytes by one of the sanitizers, we won't create a buildsym_compunit here
and we'll continue with ccx->builder equal to 0xbebebebebebebebe.
Fix this the easy way by initializing ccx->builder where the other
ctf_context fields are initialized (yeah, this code could be made nicer
C++, but I am going for the obvious fix here).
With this patch, this passes cleanly on my system:
$ make check TESTS="gdb.ctf/*.exp" RUNTESTFLAGS="CC_FOR_TARGET=/opt/gcc/git/bin/gcc"
# of expected passes 40
... where /opt/gcc/git/bin/gcc is a gcc with CTF support, given my
system gcc does not have it.
Change-Id: Idea1b0cf3e3708b72ecb16b1b60222439160f9b9
1. Since 32bit-pkeys.xml and 64bit-pkeys.xml are identical, consolidate
them into a single keys.xml.
2. Enable PKU for x32 to fix:
$ gdbserver :123456 x32-program
...
.../gdbserver/regcache.cc:255: A problem internal to GDBserver has been detected
.
Unknown register pkru requested
on Tiger Lake.
The check removed by this patch, using current_inferior, looks wrong.
When debugging multiple inferiors with the Linux native target and
linux_handle_extended_wait is called, there's no guarantee about which
is the current inferior. The vfork-done event we receive could be for
any inferior. If the vfork-done event is for a non-current inferior, we
end up wrongfully ignoring it. As a result, the core never processes a
TARGET_WAITKIND_VFORK_DONE event, program_space::breakpoints_not_allowed
is never cleared, and breakpoints are never reinserted. However,
because the Linux native target decided to ignore the event, it resumed
the thread - while breakpoints out. And that's bad.
The proposed fix is to remove this check. Always report vfork-done
events and let infrun's logic decide if it should be ignored. We don't
save much cycles by filtering the event here.
Add a test that replicates the situation described above. See comments
in the test for more details.
Change-Id: Ibe33c1716c3602e847be6c2093120696f2286fbf
I see some failures, at least in gdb.multi/multi-re-run.exp and
gdb.threads/interrupted-hand-call.exp. Running `stress -C $(nproc)` at
the same time as the test makes those tests relatively frequent.
Let's take gdb.multi/multi-re-run.exp as an example. The failure looks
like this, an unexpected "no resumed":
continue
Continuing.
No unwaited-for children left.
(gdb) FAIL: gdb.multi/multi-re-run.exp: re_run_inf=2: iter=1: continue until exit
The situation is:
- Inferior 1 is stopped somewhere, it won't really play a role here.
- Inferior 2 has 2 threads, both stopped.
- We resume inferior 2, the leader thread is expected to exit, making
the process exit.
From GDB's perspective, a failing run looks like this:
[infrun] fetch_inferior_event: enter
[infrun] scoped_disable_commit_resumed: reason=handling event
[infrun] do_target_wait: Found 2 inferiors, starting at #1
[infrun] random_pending_event_thread: None found.
[remote] wait: enter
[remote] Packet received: T0506:20dcffffff7f0000;07:20dcffffff7f0000;10:9551555555550000;thread:pae4cd.ae4cd;core:e;
[remote] wait: exit
[infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
[infrun] print_target_wait_results: 713933.713933.0 [Thread 713933.713933],
[infrun] print_target_wait_results: status->kind = STOPPED, sig = GDB_SIGNAL_TRAP
[infrun] handle_inferior_event: status->kind = STOPPED, sig = GDB_SIGNAL_TRAP
[infrun] clear_step_over_info: clearing step over info
[infrun] context_switch: Switching context from 0.0.0 to 713933.713933.0
[infrun] handle_signal_stop: stop_pc=0x555555555195
[infrun] start_step_over: enter
[infrun] start_step_over: stealing global queue of threads to step, length = 0
[infrun] operator(): step-over queue now empty
[infrun] start_step_over: exit
[infrun] process_event_stop_test: no stepping, continue
[remote] Sending packet: $Z0,555555555194,1#8e
[remote] Packet received: OK
[infrun] resume_1: step=0, signal=GDB_SIGNAL_0, trap_expected=0, current thread [713933.713933.0] at 0x555555555195
[remote] Sending packet: $QPassSignals:e;10;14;17;1a;1b;1c;21;24;25;2c;4c;97;#0a
[remote] Packet received: OK
[remote] Sending packet: $vCont;c:pae4cd.-1#9f
[infrun] prepare_to_wait: prepare_to_wait
[infrun] reset: reason=handling event
[infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target extended-remote
[infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target extended-remote
[infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target extended-remote
[infrun] fetch_inferior_event: exit
[infrun] fetch_inferior_event: enter
[infrun] scoped_disable_commit_resumed: reason=handling event
[infrun] do_target_wait: Found 2 inferiors, starting at #0
[infrun] random_pending_event_thread: None found.
[remote] wait: enter
[remote] Packet received: N
[remote] wait: exit
[infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
[infrun] print_target_wait_results: -1.0.0 [process -1],
[infrun] print_target_wait_results: status->kind = NO_RESUMED
[infrun] handle_inferior_event: status->kind = NO_RESUMED
[remote] Sending packet: $Hgp0.0#ad
[remote] Packet received: OK
[remote] Sending packet: $qXfer:threads:read::0,1000#92
[remote] Packet received: l<threads>\n<thread id="pae4cb.ae4cb" core="3" name="multi-re-run-1" handle="40c7c6f7ff7f0000"/>\n<thread id="pae4cb.ae4cc" core="2" name="multi-re-run-1" handle="40b6c6f7ff7f0000"/>\n<thread id="pae4cd.ae4ce" core="1" name="multi-re-run-2" handle="40b6c6f7ff7f0000"/>\n</threads>\n
[infrun] stop_waiting: stop_waiting
[remote] Sending packet: $qXfer:threads:read::0,1000#92
[remote] Packet received: l<threads>\n<thread id="pae4cb.ae4cb" core="3" name="multi-re-run-1" handle="40c7c6f7ff7f0000"/>\n<thread id="pae4cb.ae4cc" core="2" name="multi-re-run-1" handle="40b6c6f7ff7f0000"/>\n<thread id="pae4cd.ae4ce" core="1" name="multi-re-run-2" handle="40b6c6f7ff7f0000"/>\n</threads>\n
[infrun] infrun_async: enable=0
[infrun] reset: reason=handling event
[infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target extended-remote
[infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target extended-remote
[infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target extended-remote
[infrun] fetch_inferior_event: exit
We can see that we resume the inferior with vCont;c, but got NO_RESUMED.
When the test passes, we get an EXITED status to indicate the process
has exited.
From GDBserver's point of view, it looks like this. The logs contain
some logging I added and that are part of this patch.
[remote] getpkt: getpkt ("vCont;c:pae4cf.-1"); [no ack sent]
[threads] resume: enter
[threads] thread_needs_step_over: Need step over [LWP 713931]? Ignoring, should remain stopped
[threads] thread_needs_step_over: Need step over [LWP 713932]? Ignoring, should remain stopped
[threads] get_pc: pc is 0x555555555195
[threads] thread_needs_step_over: Need step over [LWP 713935]? No, no breakpoint found at 0x555555555195
[threads] get_pc: pc is 0x7ffff7d35a95
[threads] thread_needs_step_over: Need step over [LWP 713936]? No, no breakpoint found at 0x7ffff7d35a95
[threads] resume: Resuming, no pending status or step over needed
[threads] resume_one_thread: resuming LWP 713935
[threads] proceed_one_lwp: lwp 713935
[threads] resume_one_lwp_throw: continue from pc 0x555555555195
[threads] resume_one_lwp_throw: Resuming lwp 713935 (continue, signal 0, stop not expected)
[threads] resume_one_lwp_throw: NOW ptid=713935.713935.0 stopped=0 resumed=0
[threads] resume_one_thread: resuming LWP 713936
[threads] proceed_one_lwp: lwp 713936
[threads] resume_one_lwp_throw: continue from pc 0x7ffff7d35a95
[threads] resume_one_lwp_throw: Resuming lwp 713936 (continue, signal 0, stop not expected)
[threads] resume_one_lwp_throw: ptrace errno = 3 (No such process)
[threads] resume: exit
[threads] wait_1: enter
[threads] wait_1: [<all threads>]
[threads] wait_for_event_filtered: waitpid(-1, ...) returned 0, ERRNO-OK
[threads] resume_stopped_resumed_lwps: resuming stopped-resumed LWP LWP 713935.713936 at 7ffff7d35a95: step=0
[threads] resume_one_lwp_throw: continue from pc 0x7ffff7d35a95
[threads] resume_one_lwp_throw: Resuming lwp 713936 (continue, signal 0, stop not expected)
[threads] resume_one_lwp_throw: ptrace errno = 3 (No such process)
[threads] operator(): check_zombie_leaders: leader_pid=713931, leader_lp!=NULL=1, num_lwps=2, zombie=0
[threads] operator(): check_zombie_leaders: leader_pid=713935, leader_lp!=NULL=1, num_lwps=2, zombie=1
[threads] operator(): Thread group leader 713935 zombie (it exited, or another thread execd).
[threads] delete_lwp: deleting 713935
[threads] wait_for_event_filtered: exit (no unwaited-for LWP)
sigchld_handler
[threads] wait_1: ret = null_ptid, TARGET_WAITKIND_NO_RESUMED
[threads] wait_1: exit
What happens is:
- We resume the leader (713935) successfully.
- The leader exits.
- We resume the secondary thread (713936), we get ESRCH. This is
expected this the leader has exited.
- resume_one_lwp_throw throws, it's caught by resume_one_lwp.
- resume_one_lwp checks with check_ptrace_stopped_lwp_gone that the
failure can be explained by the LWP becoming zombie, and swallows the
error.
- Note that this means that the secondary lwp still has stopped==1.
- wait_1 is called, probably because linux_process_target::resume marks
the async pipe at the end.
- The exit event isn't ready yet, probably because the machine is under
load, so waitpid returns nothing.
- check_zombie_leaders detects that the leader is zombie and deletes
- We try to find a resumed (non-stopped) LWP to get an event from,
there's none since the leader (that was resumed) is now deleted, and
the secondary thread is still marked stopped.
wait_for_event_filtered returns -1, causing wait_1 to return
NO_RESUMED.
What I notice here is that there is some kind of race between the
availability of the process' exit notification and the call to wait_1
that results from marking the async pipe at the end of resume.
I think what we want from this wait_1 invocation is to keep waiting, as
we will eventually get thread exit notifications for both of our
threads.
The fix I came up with is to mark the secondary thread as !stopped (or
resumed) when we fail to resume it. This makes wait_1 see that there is
at least one resume lwp, so it won't return NO_RESUMED. I think this
makes sense to consider it resumed, because we are going to receive an
exit event for it. Here's the GDBserver logs with the fix applied:
[threads] resume: enter
[threads] thread_needs_step_over: Need step over [LWP 724595]? Ignoring, should remain stopped
[threads] thread_needs_step_over: Need step over [LWP 724596]? Ignoring, should remain stopped
[threads] get_pc: pc is 0x555555555195
[threads] thread_needs_step_over: Need step over [LWP 724597]? No, no breakpoint found at 0x555555555195
[threads] get_pc: pc is 0x7ffff7d35a95
[threads] thread_needs_step_over: Need step over [LWP 724598]? No, no breakpoint found at 0x7ffff7d35a95
[threads] resume: Resuming, no pending status or step over needed
[threads] resume_one_thread: resuming LWP 724597
[threads] proceed_one_lwp: lwp 724597
[threads] resume_one_lwp_throw: continue from pc 0x555555555195
[threads] resume_one_lwp_throw: Resuming lwp 724597 (continue, signal 0, stop not expected)
[threads] resume_one_lwp_throw: NOW ptid=724597.724597.0 stopped=0 resumed=0
[threads] resume_one_thread: resuming LWP 724598
[threads] proceed_one_lwp: lwp 724598
[threads] resume_one_lwp_throw: continue from pc 0x7ffff7d35a95
[threads] resume_one_lwp_throw: Resuming lwp 724598 (continue, signal 0, stop not expected)
[threads] resume_one_lwp_throw: ptrace errno = 3 (No such process)
[threads] resume: exit
[threads] wait_1: enter
[threads] wait_1: [<all threads>]
sigchld_handler
[threads] wait_for_event_filtered: waitpid(-1, ...) returned 0, ERRNO-OK
[threads] operator(): check_zombie_leaders: leader_pid=724595, leader_lp!=NULL=1, num_lwps=2, zombie=0
[threads] operator(): check_zombie_leaders: leader_pid=724597, leader_lp!=NULL=1, num_lwps=2, zombie=1
[threads] operator(): Thread group leader 724597 zombie (it exited, or another thread execd).
[threads] delete_lwp: deleting 724597
[threads] wait_for_event_filtered: sigsuspend'ing
sigchld_handler
[threads] wait_for_event_filtered: waitpid(-1, ...) returned 724598, ERRNO-OK
[threads] wait_for_event_filtered: waitpid 724598 received 0 (exited)
[threads] filter_event: 724598 exited
[threads] wait_for_event_filtered: waitpid(-1, ...) returned 724597, ERRNO-OK
[threads] wait_for_event_filtered: waitpid 724597 received 0 (exited)
[threads] wait_for_event_filtered: waitpid(-1, ...) returned 0, ERRNO-OK
sigchld_handler
[threads] wait_1: ret = LWP 724597.724598, exited with retcode 0
[threads] wait_1: exit
Change-Id: Idf0bdb4cb0313f1b49e4864071650cc83fb3c100
Since commit 3cd5229387 ("Change the pager to a ui_file"), I see these
errors when running gdb.tui/scroll.exp:
ERROR: invalid command name "_csi_P"
while executing
"::gdb_tcl_unknown _csi_P 2"
("uplevel" body line 1)
invoked from within
"uplevel 1 ::gdb_tcl_unknown $args"
(procedure "::unknown" line 5)
invoked from within
"_csi_P 2"
("eval" body line 1)
invoked from within
"eval _csi_$cmd $params"
It looks like GDB is emitting a CSI that it did not emit before, the
"Delete character" one:
https://vt100.net/docs/vt510-rm/DCH.html
Implement it.
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Change-Id: I5bf86b6104d51b0623a26a69df83d1ca9a4851b7
A race condition in how patches were pushed causes this build failure:
CXX top.o
/home/simark/src/binutils-gdb/gdb/top.c: In function ‘void print_gdb_configuration(ui_file*)’:
/home/simark/src/binutils-gdb/gdb/top.c:1622:3: error: ‘fprintf_filtered’ was not declared in this scope; did you mean ‘printf_unfiltered’?
1622 | fprintf_filtered (stream, _("\
| ^~~~~~~~~~~~~~~~
fprintf_filtered has been removed, gdb_printf must be used now. Fix
this.
Change-Id: I6a172ba0d53dab2e7cc43ed0ed2696c82925245b
FEAT_RNG is an optional Armv8.5-A extension, but it can be backported
to earlier architectures as well. GAS previously made the RNG registers
conditional on having both armv8.5-a and +rng, but only +rng should be
required.
This seems to be the only feature that was handled like this.
opcodes/
* aarch64-opc.c (SR_RNG): Don't require V8_5.
gas/
* testsuite/gas/aarch64/rng-1.s, testsuite/gas/aarch64/rng-1.d: New
test.
Add a "reason" parameter, only used to show in debug messages what is
the reason for stopping all threads. This helped me understand the
debug logs while adding some new uses of stop_all_threads, so I am
proposing to merge it.
Change-Id: I66c8c335ebf41836a7bc3d5fe1db92c195f65e55
The particular behavior we have when using that combination of settings
doesn't seem tested at all (at least, I don't find it if I grep for "Can
not resume the parent process"). Add a simple test for that.
Change-Id: Ib9454a615abba661b42f1b15056df73ed1bcd4c5
procfs.c doesn't compile on Solaris:
/vol/src/gnu/gdb/hg/master/local/gdb/procfs.c: In member function ‘virtual bool procfs_target::info_proc(const char*, info_proc_what)’:
/vol/src/gnu/gdb/hg/master/local/gdb/procfs.c:3302:3: error: ‘gdb_argv’ was not declared in this scope
3302 | gdb_argv built_argv (args);
| ^~~~~~~~
/vol/src/gnu/gdb/hg/master/local/gdb/procfs.c:3303:20: error: ‘built_argv’ was not declared in this scope; did you mean ‘buildargv’?
3303 | for (char *arg : built_argv)
| ^~~~~~~~~~
| buildargv
Fixed by including "gdbsupport/buildargv.h".
Tested on amd64-pc-solaris2.11, sparcv9-sun-solaris2.11.
While trying to review Andrew's patch here [1], I thought I spotted a
bug in the handling of a CSI, but I had no way to know for sure. So I
thought it would be useful to have unit tests for the handling of
control characters and control sequences of our toy terminal
implementation. It might help avoid chasing bugs in the GDB TUI when in
reality it's a problem with the testsuite's terminal implementation.
Add the gdb.tui/tuiterm.exp file to do that. All currently supported
control sequences and characters are tested, except _csi_m (the one that
handles colors and stuff). _csi_m should probably be tested too, but it
will require more work.
Fix a few issues that the tests spotted:
- backspace: according to [3] (table 4-1), a backspace when the cursor
is at the beginning of a line should have no effect. Our
implementation did wrap to the end of the previous line. Change our
implementation to match the doc (and the test).
- insert character: this control sequence is supposed to insert blank
characters, shifting all the rest of the line right. The current
implementation moves N characters right, but it overwrites the
characters on the right instead of shifting them. It also doesn't
insert blank characters at the cursor.
- Cursor down, forward, next line: off-by-one error when reaching the
end of the display.
- erase in display, line: off-by-one errors.
- vertical line position absolute: allowed setting the cursor outside
the display, when it should clamp it to the display size.
I found that this web page [2] gave some good clues on the expected
behavior of some control characters or sequences that some other pages
didn't.
[1] https://sourceware.org/pipermail/gdb-patches/2022-March/186433.html
[2] https://docs.microsoft.com/en-us/windows/console/console-virtual-terminal-sequences
[3] https://vt100.net/docs/vt510-rm/chapter4.html#S4.3.3
Change-Id: Iab4141fdcfb7459d1b7c45cc63bd1fcb50a78d5d
Pedro pointed out that gdb worker threads should not react to quits.
While I don't think that the new DWARF reader can call QUIT from a
worker thread (and I don't think the existing minsym threading code
can either), it seems safest to address this before checking in the
new code. This patch arranges for the QUIT macro to only work on the
main thread.
Luis pointed out that I missed a spot in the gdb_printf conversion --
namely aarch64-nat.c. While looking at this, I found another spot in
darwin-nat.c that I also missed. I can't build either of these, but I
think this patch should fix the problems.
I noticed that both gdbserver and gdb define current_directory.
However, as it is referenced by gdbsupport, it seemed better to define
it there as well. This patch also moves the declaration to
pathstuff.h. Tested by rebuilding.
In Ada, if a class implements an interface and has a dynamic
superclass, then the "offset to top" -- the offset that says how to
turn a pointer to the interface into a pointer to the whole object --
is stored in the object itself. This patch changes GDB to understand
this.
Because this only touches Ada code, and because Joel already reviewed
it internally, I am checking it in.
I noticed that a couple of deprecated hooks aren't ever called, so
they can't really be used by Insight. This patch removes them
entirely. I checked the Insight sources, and these aren't mentioned
there, either.
Various spots in gdb currently know about the wrap buffer, and so are
careful to call wrap_here to be certain that all output has been
flushed.
Now that the pager is just an ordinary stream, this isn't needed, and
a simple call to gdb_flush is enough.
Similarly, there are places where gdb prints to gdb_stderr, but first
flushes gdb_stdout. stderr_file already flushes gdb_stdout, so these
aren't needed.
There is no longer any need for ui_out_flag::unfiltered_output --
nothing ever sets this flag. This used to be needed to make the
_unfiltered output work, but now only printf_unfiltered can be used,
and it uses the puts_unfiltered method. This patch removes the flag
and the dead code.
fprintf_symbol_filtered is misnamed, because whether filtering happens
is now up to the stream. This renames it to fprintf_symbol, which
isn't a great name (the first "f" doesn't mean much and the second one
is truly meaningless here), but "print_symbol" was already taken.
puts_filtered_tabular is now misnamed, because whether filtering
happens is now up to the stream. So, rename it. (This function is
pretty weird, and should probably be rewritten to avoid using the
chars_printed global, and moved into objc-lang.c. However, I haven't
done so.)
Now that filtered and unfiltered output can be treated identically, we
can unify the printf family of functions. This is done under the name
"gdb_printf". Most of this patch was written by script.
Now that filtered and unfiltered output can be treated identically, we
can unify the putc family of functions. This is done under the name
"gdb_putc". Most of this patch was written by script.
Now that filtered and unfiltered output can be treated identically, we
can unify the puts family of functions. This is done under the name
"gdb_puts". Most of this patch was written by script.
Now that filtered and unfiltered output can be treated identically, we
can unify the vprintf family of functions: vprintf_filtered,
vprintf_unfiltered, vfprintf_filtered and vfprintf_unfiltered. (For
the gdb_stdout variants, recall that only printf_unfiltered gets truly
unfiltered output at this point.) This removes one such function and
renames the remaining two to "gdb_vprintf". All callers are updated.
Much of this patch was written by script.
This rewrites the output pager as a ui_file implementation.
A new header is introduced to declare the pager class. The
implementation remains in utils.c for the time being, because there
are some static globals there that must be used by this code. (This
could be cleaned up at some future date.)
I went through all the text output in gdb to ensure that this change
should be ok. There are a few cases:
* Any existing call to printf_unfiltered is required to be avoid the
pager. This is ensured directly in the implementation.
* All remaining calls to the f*_unfiltered functions -- the ones that
take an explicit ui_file -- either send to an unfiltered stream
(e.g., gdb_stderr), which is obviously ok; or conditionally send to
gdb_stdout
I investigated all such calls by searching for:
grep -e '\bf[a-z0-9_]*_unfiltered' *.[chyl] */*.[ch] | grep -v gdb_stdlog | grep -v gdb_stderr
This yields a number of candidates to check.
* The breakpoint _print_recreate family, and
save_trace_state_variables. These are used for "save" commands
and so are fine.
* Things printing to a temporary stream. Obviously ok.
* Disassembly selftests.
* print_gdb_help - this is non-obvious, but ok because paging isn't
yet enabled at this point during startup.
* serial.c - doens't use gdb_stdout
* The code in compile/. This is all printing to a file.
* DWARF DIE dumping - doesn't reference gdb_stdout.
* Calls to the _filtered form -- these are all clearly ok, because if
they are using gdb_stdout, then filtering will still apply; and if
not, then filtering never applied and still will not.
Therefore, at this point, there is no longer any distinction between
all the other _filtered and _unfiltered calls, and they can be
unified.
In this patch, take special note of the vfprintf_maybe_filtered and
ui_file::vprintf change. This is one instance of the above idea,
erasing the distinction between filtered and unfiltered -- in this
part of the change, the "unfiltered_output" flag is never passe to
cli_ui_out. Subsequent patches will go much further in this
direction.
Also note the can_emit_style_escape changes in ui-file.c. Checking
against gdb_stdout or gdb_stderr was always a bit of a hack; and now
it is no longer needed, because this is decision can be more fully
delegated to the particular ui_file implementation.
ui_file::can_page is removed, because this patch removed the only call
to it.
I think this is the main part of fixing PR cli/7234.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7234
This adds emit_style_escape and reset_style methods to ui_file. These
aren't used yet, but they will be once the pager is converted to be a
ui_file subclass.
When the pager is rewritten as a ui_file, gdb will still need a way to
bypass the filtering. After examining a few approaches, I chose this
patch, which adds a puts_unfiltered method to ui_file. For most
implementations of ui_file, this will just delegate to puts. This
patch also switches printf_unfiltered to use the new method.
At the end of this series, the use of unfiltered output will be very
restricted -- only places that definitely need it will use it. To
this end, I thought it would be good to reduce the number of
_unfiltered APIs that are exposed. This patch changes gdb so that
only printf_unfiltered exists. (After this patch, the f* variants
still exist as well, but those will be removed later.)
A number of spots call printf_unfiltered only because they are in code
that should not be interrupted by the pager. However, I believe these
cases are all handled by infrun's blanket ban on paging, and so can be
converted to the default (_filtered) API.
After this patch, I think all the remaining _unfiltered calls are ones
that really ought to be. A few -- namely in complete_command -- could
be replaced by a scoped assignment to pagination_enabled, but for the
remainder, the code seems simple enough like this.
It seems to me that annotations should not be filtered. While it
might be weird for an annotation-based UI to use the pager, it's not,
I think, out of the question. This patch makes this change.
When parsing the ptid out of a reply package, if the multi-process
extensions are not supported, use current_inferior's pid as the pid of
the reported thread, instead of inferior_ptid. This is needed because
the inferior_ptid may be null_ptid although a legit context exists,
due to a prior context switch via switch_to_inferior_no_thread.
Below is a scenario that illustrates what could go wrong. First,
setup a multi-target scenario. This is needed, because in a
multi-target setting, the inferior_ptid is cleared out before waiting
on targets. The second inferior below sits on top of a remote target.
Multi-process packets are disabled.
$ # First, spawn a process with PID 26253 to attach to later.
$ gdb-up a.out
Reading symbols from a.out...
(gdb) maint set target-non-stop on
(gdb) set remote multiprocess-feature-packet off
(gdb) start
...
(gdb) add-inferior -no-connection
[New inferior 2]
Added inferior 2
(gdb) inferior 2
[Switching to inferior 2 [<null>] (<noexec>)]
(gdb) target extended-remote | gdbserver --multi -
Remote debugging using | gdbserver --multi -
Remote debugging using stdio
(gdb) attach 26253
Attaching to Remote target
Attached; pid = 26253
[New Thread 26253]
[New inferior 3]
Reading /tmp/a.out from remote target...
...
[New Thread 26253]
...
Reading /usr/local/lib/debug/....debug from remote target...
>>> GDB seems to hang here.
After attaching to a process and reading some library files, GDB
seems to hang. One interesting thing to note is that
[New Thread 26253]
appears twice. We also see
[New inferior 3]
Running the same scenario with "debug infrun on" reveals more details.
...
(gdb) attach 26253
[infrun] scoped_disable_commit_resumed: reason=attaching
Attaching to Remote target
Attached; pid = 26253
[New Thread 26253]
[infrun] infrun_async: enable=1
[infrun] attach_command: immediately after attach:
[infrun] attach_command: thread 26253.26253.0, executing = 1, resumed = 0, state = RUNNING
[infrun] clear_proceed_status_thread: 26253.26253.0
[infrun] reset: reason=attaching
[infrun] maybe_set_commit_resumed_all_targets: not requesting commit-resumed for target native, no resumed threads
[infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target extended-remote
[infrun] fetch_inferior_event: enter
[infrun] scoped_disable_commit_resumed: reason=handling event
[infrun] do_target_wait: Found 2 inferiors, starting at #1
[infrun] random_pending_event_thread: None found.
[infrun] print_target_wait_results: target_wait (-1.0.0 [Thread 0], status) =
[infrun] print_target_wait_results: 26253.26253.0 [Thread 26253],
[infrun] print_target_wait_results: status->kind = STOPPED, sig = GDB_SIGNAL_0
[infrun] handle_inferior_event: status->kind = STOPPED, sig = GDB_SIGNAL_0
[infrun] start_step_over: enter
[infrun] start_step_over: stealing global queue of threads to step, length = 0
[infrun] operator(): step-over queue now empty
[infrun] start_step_over: exit
[infrun] context_switch: Switching context from 0.0.0 to 26253.26253.0
[infrun] handle_signal_stop: stop_pc=0x7f849d8cf151
[infrun] stop_waiting: stop_waiting
[infrun] stop_all_threads: starting
[infrun] stop_all_threads: pass=0, iterations=0
[New inferior 3]
Reading /tmp/a.out from remote target...
warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
Reading /tmp/a.out from remote target...
Reading symbols from target:/tmp/a.out...
[New Thread 26253]
[infrun] stop_all_threads: 4723.4723.0 not executing
[infrun] stop_all_threads: 26253.26253.0 not executing
[infrun] stop_all_threads: 42000.26253.0 executing, need stop
[infrun] print_target_wait_results: target_wait (-1.0.0 [Thread 0], status) =
[infrun] print_target_wait_results: -1.0.0 [Thread 0],
[infrun] print_target_wait_results: status->kind = IGNORE
[infrun] print_target_wait_results: target_wait (-1.0.0 [Thread 0], status) =
[infrun] print_target_wait_results: -1.0.0 [Thread 0],
[infrun] print_target_wait_results: status->kind = IGNORE
GDB tried to stop Thread 42000.26253.0, which does not exist, and we
are waiting for a stop event that will never happen. The PID in
'42000.26253.0', namely 42000, is the PID of magic_null_ptid.
It comes from gdb/remote.c:read_ptid:
/* Since the stub is not sending a process id, then default to
what's in inferior_ptid, unless it's null at this point. If so,
then since there's no way to know the pid of the reported
threads, use the magic number. */
if (inferior_ptid == null_ptid)
pid = magic_null_ptid.pid ();
else
pid = inferior_ptid.pid ();
if (obuf)
*obuf = pp;
return ptid_t (pid, tid);
Because multi-process was turned off, GDB did not parse an explicitly
specified PID. Furthermore, inferior_ptid == null_ptid, and
eventually GDB picked the PID from magic_null_ptid.
If target-non-stop is not turned on at the beginning, the same bug
reveals itself as a duplicated thread as shown below.
# Same setup as above, without 'maint set target-non-stop on'.
...
(gdb) attach 26253
Attaching to Remote target
Attached; pid = 26253
[New inferior 3]
...
[New Thread 26253]
...
(gdb) info threads
Id Target Id Frame
1.1 process 13517 "a.out" main () at test.c:3
* 2.1 Thread 26253 "a.out" 0x00007f12750c5151 in read () from target:/lib/x86_64-linux-gnu/libc.so.6
3.1 Thread 26253 "a.out" Remote 'g' packet reply is too long (expected 560 bytes, got 2496 bytes): 00feffffffffffff000a3a75127f000051510c75127f0000000400000000000060d24ef6af5500000000000000000000680d000000000000b85b31e3fc7f0000c0283a75127f000000e55b75127f000010d04ef6af550000460200000000000060c73975127f0000a0d23975127f0000000a3a75127f0000000000000000000051510c75127f000046020000330000002b0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f03000000000000ffff0000000000000000000000000000000000000000000080143a75127f000080143a75127f000040143a75127f000040143a75127f00007d0000007e0000007f00000080000000300c3a75127f0000300c3a75127f00000e000000000000000e0000000000000000000000000000000000000000000000ffffffffffffffffffffffffffffffff0400000004000000040000000400000020143a75127f000020143a75127f000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000801f0000000000000000000000e55b75127f0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
(gdb)
Fix the problem by preferring current_inferior()'s pid instead of
magic_null_ptid.
Regression-tested on X86-64 Linux.
Co-authored-by: Aleksandar Paunovic <aleksandar.paunovic@intel.com>
The test added in the commit:
commit a6b413d24c
Date: Fri Mar 11 14:44:03 2022 +0000
gdb: work around prompt corruption caused by bracketed-paste-mode
Was not written with readline 7 in mind, only readline 8+. Between
readline 7 and 8 the escape sequence used to disable bracketed paste
mode changed, an additional '\r' character was added to the end. In
fact, it was the addition of this '\r' character that triggered the
issue for which the above commit is part of the solution.
Anyway, the test tries to spot the case where the output from GDB is
not perfect, but does have the above work around applied. However,
the pattern in the test assumes that the problematic '\r' will be
present, and this is only true for readline 8+. With readline 7 the
test was failing.
In this commit I generalise the pattern a little so that the test will
still KFAIL with readline 7.
It's a little unfortunate that the test is KFAILing with readline 7,
as without the problematic '\r' there's actually no reason that GDB
couldn't "do the right thing" in this case, in which case, the test
would PASS, but that would require changes within GDB itself.
My preference then is that initially we patch the test to get it
KFAILing, then in a separate commit I can modify GDB so that it can
PASS with readline 7.
The test gdb.python/py-format-address.exp, added in commit:
commit 25209e2c69
Date: Sat Oct 23 09:59:25 2021 +0100
gdb/python: add gdb.format_address function
included 3 copy & paste errors where the wrong address was used in the
expected output patterns.
The test compiles two almost identical test binaries (one function
changes its name, that's the only difference), two inferiors are
created, each inferior using one of the test binaries.
We then take the address of the name changing function in both
inferiors ('foo' in inferior 1 and 'bar' in inferior 2) and the tests
are carried out using these addresses.
What we're checking for is that symbols 'foo' and 'bar' show up in the
correct inferior, and that (as this test is for a Python API feature),
the user can have one inferior selected, but ask about the other
inferior, and see the correct symbol in the result.
The hope is that the two binaries will be laid out identically by the
compiler, and that 'foo' and 'bar' will be at the same address. This
is fine, unless the executable is compiled as PIE (position
independent executable), in which case there is a problem.
The problem is that though inferior 1 is set running, the inferior 2
never is. If the executables are compiled as PIE, then the address in
the inferior 2 will not have been resolved, while the address in the
inferior 1 will have been, and so the two addresses we use in the
tests will be different.
This issue was reported here:
https://sourceware.org/pipermail/gdb-patches/2022-March/186911.html
The first part of the fix is to use the correct address variable in
the expected output patterns, with this change the tests pass even
when the executables are compiled as PIE.
A second part of this fix is to pass the 'nopie' option when we
compile the tests, this should ensure that the address obtained in
inferior 2 is the same as the address from inferior 1, which makes the
test more useful.
In commit:
commit a2757c4ed6
Date: Wed Mar 16 15:08:22 2022 +0000
gdb/mi: consistently notify user when GDB/MI client uses -thread-select
Changes were made to GDB to address some inconsistencies in when
notifications are sent from a MI terminal to a CLI terminal (when
multiple terminals are in use, see new-ui command).
Unfortunately, in order to track when the currently selected frame has
changed, that commit grabs a frame_info pointer before and after an MI
command has executed, and compares the pointers to see if the frame
has changed.
This is not safe.
If the frame cache is deleted for any reason then the frame_info
pointer captured before the command started, is no longer valid, and
any comparisons based on that pointer are undefined.
This was leading to random test failures for some folk, see:
https://sourceware.org/pipermail/gdb-patches/2022-March/186867.html
This commit changes GDB so we no longer hold frame_info pointers, but
instead store the frame_id and frame_level, this is safe even when the
frame cache is flushed.
Include the language identifier emitted by gas in the set of ones where
no mangled names are expected. Even if there could be "hand-mangled"
names, gas doesn't emit DW_AT_linkage_name in the first place.
Prior to entering the enclosing "else if()" the earlier associated if()
checks function->is_linkage and, if set, uses function->name. The
comment in patch context precedes (and explains) the setting
function->is_linkage. Yet with the flag set, we should then also return
the function name, just like said earlier if() would do when we came
here a 2nd time for the same "addr". And indeed passing the same address
twice on addr2line's command line would resolve the function for the 2nd
instance, but not for the 1st (if this code path is taken). (This,
obviously, is particularly relevant when there's no ELF symbol table in
the first place, like would be the case - naturally - in PE/COFF
binaries, for example.)
Restrict the PR gas/16908 workaround to just macros, matching the
original intention as well as the comment there. For constructs like
.irp or .rept the reasoning doesn't apply, as there's no separate
"invocation" point which may be of interest to record (for, as said
there, short macros).
