'g' command returns hex-string as response so simply checking for 'E'
to determine if it failed is not enough and can trigger spurious error
messages. For example, invalid behaviour can be easily triggered on
Cortex-M as follows:
(gdb) set $r0 = 0xe0
Sending packet: $P0=e0000000#72...Packet received: OK
Packet P (set-register) is supported
Sending packet: $g#67...Packet received: E0000000849A0020...
Remote failure reply: E0000000849A0020...
This patch fixes the problem by calling putpkt()/getpkt() directly and
checking result with packet_check_result(). This works fine since Enn
response has odd number of bytes while proper response has even number
of bytes.
Also, remote_send() is now not used anywhere so it can be removed.
gdb/Changelog:
2018-04-26 Andrzej Kaczmarek <andrzej.kaczmarek@codecoup.pl>
PR remote/9665
* remote.c (send_g_packet): Use putpkt/getpkt/packet_check_result
instead of remote_send.
(remote_send): Remove.
Since f67c0c9171 ("Enable 'set print inferior-events' and improve
detach/fork/kill/exit messages"), when detaching a remote process, we
get, for detach against a remote target:
(gdb) detach
Detaching from program: ...., process 5388
Ending remote debugging.
[Inferior 1 (Thread 5388.5388) detached]
^^^^^^^^^^^^^^^^
That is incorrect, for it is printing a thread id as string while we
should be printing the process id instead. I.e., either one of:
[Inferior 1 (process 5388) detached]
[Inferior 1 (Remote target) detached]
depending on remote stub support for the multi-process extensions.
Similarly, after killing a process, we're printing thread ids while we
should be printing process ids. E.g., on native GNU/Linux:
(gdb) k
Kill the program being debugged? (y or n) y
[Inferior 1 (Thread 0x7ffff7faa8c0 (LWP 30721)) has been killed]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
while it should have been:
Kill the program being debugged? (y or n) y
[Inferior 1 (process 30721) has been killed]
^^^^^^^^^^^^^
There's a wording inconsistency between detach and kill:
[Inferior 1 (process 30721) has been killed]
[Inferior 1 (process 30721) detached]
Given we were already saying "detached" instead of "has been
detached", and we used to say just "exited", and given that the "has
been" doesn't really add any information, this commit changes the
message to just "killed":
[Inferior 1 (process 30721) killed]
gdb/ChangeLog:
2018-04-25 Pedro Alves <palves@redhat.com>
* infcmd.c (kill_command): Print the pid as string, not the whole
thread's ptid. Add comment. s/has been killed/killed/ in output
message.
* remote.c (remote_detach_1): Print the pid as string, not the
whole thread's ptid.
gdb/testsuite/ChangeLog:
2018-04-25 Pedro Alves <palves@redhat.com>
* gdb.base/hook-stop.exp: Expect "killed" instead of "has been
killed".
* gdb.base/kill-after-signal.exp: Likewise.
* gdb.threads/kill.exp: Likewise.
This patch aims to turn 'set print inferior-events' always on, and do
some cleanup on the messages printed by GDB when various inferior
events happen (attach, detach, fork, kill, exit).
To make sure that the patch is correct, I've tested it with a handful
of combinations of 'set follow-fork-mode', 'set detach-on-fork' and
'set print inferior-events'. In the end, I decided to make my
hand-made test into an official testcase. More on that below.
Using the following program as an example:
#include <unistd.h>
int main ()
{
fork ();
return 0;
}
We see the following outputs from the patched GDB:
- With 'set print inferior-events on':
(gdb) r
Starting program: a.out
[Detaching after fork from child process 27749]
[Inferior 1 (process 27745) exited normally]
(gdb)
- With 'set print inferior-events off':
(gdb) r
Starting program: a.out
[Inferior 1 (process 27823) exited normally]
(gdb)
Comparing this against an unpatched GDB:
- With 'set print inferior-events off' and 'set follow-fork-mode
child':
(gdb) r
Starting program: a.out
[Inferior 2 (process 5993) exited normally]
(gdb)
Compare this against an unpatched GDB:
(unpatched-gdb) r
Starting program: a.out
[New process 5702]
[Inferior 2 (process 5702) exited normally]
(unpatched-gdb)
It is possible to notice that, in this scenario, the patched GDB
will lose the '[New process %d]' message.
- With 'set print inferior-events on', 'set follow-fork-mode child'
and 'set detach-on-fork on':
(gdb) r
Starting program: a.out
[Attaching after process 27905 fork to child process 27909]
[New inferior 2 (process 27909)]
[Detaching after fork from parent process 27905]
[Inferior 1 (process 27905) detached]
[Inferior 2 (process 27909) exited normally]
(gdb)
Compare this output with an unpatched GDB, using the same settings:
(unpatched-gdb) r
Starting program: a.out
[New inferior 28033]
[Inferior 28029 detached]
[New process 28033]
[Inferior 2 (process 28033) exited normally]
[Inferior 28033 exited]
(unpatched-gdb)
As can be seen above, I've also made a few modifications to messages
that are printed when 'set print inferior-events' is on. For example,
a few of the messages did not contain the '[' and ']' as
prefix/suffix, which led to a few inconsistencies like:
Attaching after process 22995 fork to child process 22999.
[New inferior 22999]
Detaching after fork from child process 22999.
[Inferior 22995 detached]
[Inferior 2 (process 22999) exited normally]
So I took the opportunity and included the square brackets where
applicable. I have also made the existing messages more uniform, by
always printing "Inferior %d (process %d)..." where applicable. This
makes it easier to identify the inferior number and the PID number
from the messages.
As suggested by Pedro, the "[Inferior %d exited]" message from
'exit_inferior' has been removed, because it got duplicated when
'inferior-events' is on. I'm also using the
'add_{thread,inferior}_silent' versions (instead of their verbose
counterparts) on some locations, also to avoid duplicated messages.
For example, a patched GDB with 'set print inferior-events on', 'set
detach-on-fork on' and 'set follow-fork-mode child', but using
'add_thread', would print:
(gdb) run
Starting program: a.out
[Attaching after process 25088 fork to child process 25092.]
[New inferior 25092] <--- duplicated
[Detaching after fork from child process 25092.]
[Inferior 25088 detached]
[New process 25092] <--- duplicated
[Inferior 2 (process 25092) exited normally]
But if we use 'add_thread_silent' (with the same configuration as
before):
(gdb) run
Starting program: a.out
[Attaching after process 31606 fork to child process 31610]
[New inferior 2 (process 31610)]
[Detaching after fork from parent process 31606]
[Inferior 1 (process 31606) detached]
[Inferior 2 (process 31610) exited normally]
As for the tests, the configuration options being exercised are:
- follow-fork-mode: child/parent
- detach-on-fork: on/off
- print inferior-events: on/off
It was also necessary to perform adjustments on several testcases,
because the expected messages changed considerably.
Built and regtested on BuildBot, without regressions.
gdb/ChangeLog:
2018-04-24 Jan Kratochvil <jan.kratochvil@redhat.com>
Sergio Durigan Junior <sergiodj@redhat.com>
Pedro Alves <palves@redhat.com>
* infcmd.c (kill_command): Print message when inferior has
been killed.
* inferior.c (print_inferior_events): Remove 'static'. Set as
'1'.
(add_inferior): Improve message printed when
'print_inferior_events' is on.
(exit_inferior): Remove message printed when
'print_inferior_events' is on.
(detach_inferior): Improve message printed when
'print_inferior_events' is on.
(initialize_inferiors): Use 'add_inferior_silent' to set
'current_inferior_'.
* inferior.h (print_inferior_events): Declare here as
'extern'.
* infrun.c (follow_fork_inferior): Print '[Attaching...]' or
'[Detaching...]' messages when 'print_inferior_events' is on.
Use 'add_thread_silent' instead of 'add_thread'. Add '[' and ']'
as prefix/suffix for messages. Remove periods. Fix erroneous
'Detaching after fork from child...', replace it by '... from
parent...'.
(handle_vfork_child_exec_or_exit): Add '[' and ']' as
prefix/suffix when printing 'Detaching...' messages. Print
them when 'print_inferior_events' is on.
* remote.c (remote_detach_1): Print message when detaching
from inferior and '!is_fork_parent'.
gdb/testsuite/ChangeLog:
2018-04-24 Jan Kratochvil <jan.kratochvil@redhat.com>
Sergio Durigan Junior <sergiodj@redhat.com>
Pedro Alves <palves@redhat.com>
* gdb.base/attach-non-pgrp-leader.exp: Adjust 'Detaching...'
regexps to expect for '[Inferior ... detached]' as well.
* gdb.base/attach.exp: Likewise.
* gdb.base/catch-syscall.exp (check_for_program_end): Adjust
"gdb_continue_to_end".
(test_catch_syscall_with_wrong_args): Likewise.
* gdb.base/foll-fork.exp: Adjust regexps to match '[' and
']'. Don't set 'verbose' on.
* gdb.base/foll-vfork.exp: Likewise.
* gdb.base/fork-print-inferior-events.c: New file.
* gdb.base/fork-print-inferior-events.exp: New file.
* gdb.base/hook-stop.exp: Adjust regexps to expect for new
'[Inferior ... has been killed]' message.
* gdb.base/kill-after-signal.exp: Likewise.
* gdb.base/solib-overlap.exp: Adjust regexps to expect for new
detach message.
* gdb.threads/kill.exp: Adjust regexps to expect for new kill
message.
* gdb.threads/clone-attach-detach.exp: Adjust 'Detaching...'
regexps to expect for '[Inferior ... detached]' as well.
* gdb.threads/process-dies-while-detaching.exp: Likewise.
As Rajendra SY reported at
<https://sourceware.org/ml/gdb-patches/2018-04/msg00399.html>, several
attach-related tests are failing on FreeBSD. The "attach" command
errors with "Couldn't get registers: Device busy".
When the "attach" command is executed, it calls target_attach ->
inf_ptrace_attach, which just does the ptrace(PT_ATTACH), it does not
wait for the child to stop with SIGSTOP. Afterwards, the command is
complete and we go back to the event loop. The event loop wakes up
and we end up in target_wait -> fbsd_wait, and handle the SIGSTOP
stop.
At the end of execute_command, though, before going back to the event
loop, we check if the frame language changed via
check_frame_language_change(). That reads the current PC, which is
what leads to the registers read that fails.
The problem is that we fail to mark the attached-to thread as
executing between the initial attach, and the subsequent target_wait.
Until we see the thread stop with SIGSTOP, we shouldn't try to read
registers off of it. I guess there may a timing issue here - if
you're "lucky", the thread may stop before gdb reads its registers,
masking the problem.
With that fixed, check_frame_language_change() becomes a nop until the
thread is marked not-executing again, after target_wait is called and
we go through handle_inferior_event -> normal_stop.
We haven't seen the problem on Linux because there, the target_attach
implementation waits for the thread to stop before returning. Still,
that's supposedly hidden from the core, since the Linux target, like
most targets, is a '!to_attach_no_wait' target.
This fixes:
FAIL: gdb.base/attach.exp: attach1, after setting file
FAIL: gdb.base/attach.exp: attach2, with no file
FAIL: gdb.base/attach.exp: load file manually, after attach2 (re-read) (got interactive prompt)
FAIL: gdb.base/attach.exp: attach when process' a.out not in cwd
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=gdb dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=gdb dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=call dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=call dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=agent dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=agent dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=gdb dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=gdb dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=call dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=call dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=agent dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=agent dd=off: re-attach to inferior
gdb/ChangeLog:
2018-04-21 Pedro Alves <palves@redhat.com>
Rajendra SY <rajendra.sy@gmail.com>
* inf-ptrace.c (inf_ptrace_attach): Mark the thread as executing.
* remote.c (extended_remote_attach): In all-stop mode, mark the
thread as executing.
A future patch will propose making the remote target's target_ops be
heap-allocated (to make it possible to have multiple instances of
remote targets, for multiple simultaneous connections), and will
delete/destroy the remote target at target_close time.
That change trips on a latent problem, though. File I/O handles
remain open even after the target is gone, with a dangling pointer to
a target that no longer exists. This results in GDB crashing when it
calls the target_ops backend associated with the file handle:
(gdb) Disconnect
Ending remote debugging.
* GDB crashes deferencing a dangling pointer
Backtrace:
#0 0x00007f79338570a0 in main_arena () at /lib64/libc.so.6
#1 0x0000000000858bfe in target_fileio_close(int, int*) (fd=1, target_errno=0x7ffe0499a4c8)
at src/gdb/target.c:2980
#2 0x00000000007088bd in gdb_bfd_iovec_fileio_close(bfd*, void*) (abfd=0x1a631b0, stream=0x223c9d0)
at src/gdb/gdb_bfd.c:353
#3 0x0000000000930906 in opncls_bclose (abfd=0x1a631b0) at src/bfd/opncls.c:528
#4 0x0000000000930cf9 in bfd_close_all_done (abfd=0x1a631b0) at src/bfd/opncls.c:768
#5 0x0000000000930cb3 in bfd_close (abfd=0x1a631b0) at src/bfd/opncls.c:735
#6 0x0000000000708dc5 in gdb_bfd_close_or_warn(bfd*) (abfd=0x1a631b0) at src/gdb/gdb_bfd.c:511
#7 0x00000000007091a2 in gdb_bfd_unref(bfd*) (abfd=0x1a631b0) at src/gdb/gdb_bfd.c:615
#8 0x000000000079ed8e in objfile::~objfile() (this=0x2154730, __in_chrg=<optimized out>)
at src/gdb/objfiles.c:682
#9 0x000000000079fd1a in objfile_purge_solibs() () at src/gdb/objfiles.c:1065
#10 0x00000000008162ca in no_shared_libraries(char const*, int) (ignored=0x0, from_tty=1)
at src/gdb/solib.c:1251
#11 0x000000000073b89b in disconnect_command(char const*, int) (args=0x0, from_tty=1)
at src/gdb/infcmd.c:3035
This goes unnoticed in current master, because the current remote
target's target_ops is never destroyed nowadays, so we end up calling:
remote_hostio_close -> remote_hostio_send_command
which gracefully fails with FILEIO_ENOSYS if remote_desc is NULL
(because the target is closed).
Fix this by invalidating a target's file I/O handles when the target
is closed.
With this change, remote_hostio_send_command no longer needs to handle the
case of being called with a closed remote target, originally added here:
<https://sourceware.org/ml/gdb-patches/2008-08/msg00359.html>.
gdb/ChangeLog:
2018-04-11 Pedro Alves <palves@redhat.com>
* target.c (fileio_fh_t::t): Add comment.
(target_fileio_pwrite, target_fileio_pread, target_fileio_fstat)
(target_fileio_close): Handle a NULL target.
(invalidate_fileio_fh): New.
(target_close): Call it.
* remote.c (remote_hostio_send_command): No longer check whether
remote_desc is open.
This patch removes VEC(tsv_s), using an std::vector instead. I C++ified
trace_state_variable a bit in the process, using std::string for the
name. I also thought it would be nicer to pass a const reference to
target_download_trace_state_variable, since we know it will never be
NULL. This highlighted that the make-target-delegates script didn't
handle references well, so I adjusted this as well. It will surely be
useful in the future.
gdb/ChangeLog:
* tracepoint.h (struct trace_state_variable): Add constructor.
<name>: Change type to std::string.
* tracepoint.c (tsv_s): Remove.
(DEF_VEC_O(tsv_s)): Remove.
(tvariables): Change to std::vector.
(create_trace_state_variable): Adjust to std::vector.
(find_trace_state_variable): Likewise.
(find_trace_state_variable_by_number): Likewise.
(delete_trace_state_variable): Likewise.
(trace_variable_command): Adjust to std::string.
(delete_trace_variable_command): Likewise.
(tvariables_info_1): Adjust to std::vector.
(save_trace_state_variables): Likewise.
(start_tracing): Likewise.
(merge_uploaded_trace_state_variables): Adjust to std::vector
and std::string.
* target.h (struct target_ops)
<to_download_trace_state_variable>: Pass reference to
trace_state_variable.
* target-debug.h (target_debug_print_const_trace_state_variable_r): New.
* target-delegates.c: Re-generate.
* mi/mi-interp.c (mi_tsv_created): Adjust to std::string.
(mi_tsv_deleted): Likewise.
* mi/mi-main.c (mi_cmd_trace_frame_collected): Likewise.
* remote.c (remote_download_trace_state_variable): Change
pointer to reference and adjust.
* make-target-delegates (parse_argtypes): Handle references.
(write_function_header): Likewise.
(munge_type): Likewise.
This patch started by changing target_read_alloc_1 to return a
byte_vector, to avoid manual memory management (in target_read_alloc_1
and in the callers). To communicate failures to the callers, it
actually returns a gdb::optional<gdb::byte_vector>.
Adjusting target_read_stralloc was a bit more tricky, since it wants to
return a buffer of char, and not gdb_byte. Since you can't just cast a
gdb::byte_vector into a gdb::def_vector<char>, I made
target_read_alloc_1 templated, so both versions (that return vectors of
gdb_byte and char) are generated. Since target_read_stralloc now
returns a gdb::char_vector instead of a gdb::unique_xmalloc_ptr<char>, a
few callers need to be adjusted.
gdb/ChangeLog:
* common/byte-vector.h (char_vector): New type.
* target.h (target_read_alloc): Return
gdb::optional<byte_vector>.
(target_read_stralloc): Return gdb::optional<char_vector>.
(target_get_osdata): Return gdb::optional<char_vector>.
* target.c (target_read_alloc_1): Templatize. Replacement
manual memory management with vector.
(target_read_alloc): Change return type, adjust.
(target_read_stralloc): Change return type, adjust.
(target_get_osdata): Change return type, adjust.
* auxv.c (struct auxv_info) <length>: Remove.
<data>: Change type to gdb::optional<byte_vector>.
(auxv_inferior_data_cleanup): Free auxv_info with delete.
(get_auxv_inferior_data): Allocate auxv_info with new, adjust.
(target_auxv_search): Adjust.
(fprint_target_auxv): Adjust.
* avr-tdep.c (avr_io_reg_read_command): Adjust.
* linux-tdep.c (linux_spu_make_corefile_notes): Adjust.
(linux_make_corefile_notes): Adjust.
* osdata.c (get_osdata): Adjust.
* remote.c (remote_get_threads_with_qxfer): Adjust.
(remote_memory_map): Adjust.
(remote_traceframe_info): Adjust.
(btrace_read_config): Adjust.
(remote_read_btrace): Adjust.
(remote_pid_to_exec_file): Adjust.
* solib-aix.c (solib_aix_get_library_list): Adjust.
* solib-dsbt.c (decode_loadmap): Don't free buf.
(dsbt_get_initial_loadmaps): Adjust.
* solib-svr4.c (svr4_current_sos_via_xfer_libraries): Adjust.
* solib-target.c (solib_target_current_sos): Adjust.
* tracepoint.c (sdata_make_value): Adjust.
* xml-support.c (xinclude_start_include): Adjust.
(xml_fetch_content_from_file): Adjust.
* xml-support.h (xml_fetch_another): Change return type.
(xml_fetch_content_from_file): Change return type.
* xml-syscall.c (xml_init_syscalls_info): Adjust.
* xml-tdesc.c (file_read_description_xml): Adjust.
(fetch_available_features_from_target): Change return type.
(target_fetch_description_xml): Adjust.
(target_read_description_xml): Adjust.
This patch replaces VEC(static_tracepoint_marker_p) with std::vector,
and does some c++ification around that. I thought a new overload of
hex2str was useful, so I added it as well as corresponding unit tests.
I also added an overload of ui_out::field_string that takes an
std::string directly.
gdb/ChangeLog:
* tracepoint.h (struct static_tracepoint_marker): Initialize
fields, define default constructor, move constructor and move
assignment, disable the rest.
<str_id, extra>: Make std::string.
(release_static_tracepoint_marker): Remove.
(free_current_marker): Remove.
* tracepoint.c (free_current_marker): Remove.
(parse_static_tracepoint_marker_definition): Adjust to
std::string, use new hex2str overload.
(release_static_tracepoint_marker): Remove.
(print_one_static_tracepoint_marker): Get marker by reference
and adjust to std::string.
(info_static_tracepoint_markers_command): Adjust to std::vector
changes
* target.h (static_tracepoint_marker_p): Remove typedef.
(DEF_VEC_P(static_tracepoint_marker_p)): Remove.
(struct target_ops) <to_static_tracepoint_marker_at>: Return
bool.
<to_static_tracepoint_markers_by_strid>: Return std::vector.
* target-debug.h
(target_debug_print_VEC_static_tracepoint_marker_p_p): Remove.
(target_debug_print_std_vector_static_tracepoint_marker): New.
(target_debug_print_struct_static_tracepoint_marker_p): Rename
to...
(target_debug_print_static_tracepoint_marker_p): ... this.
* target-delegates.c: Re-generate.
* breakpoint.h (struct tracepoint) <static_trace_marker_id>:
Make std::string.
* breakpoint.c (init_breakpoint_sal): Adjust to std::string.
(decode_static_tracepoint_spec): Adjust to std::vector.
(tracepoint_print_one_detail): Adjust to std::string.
(strace_marker_decode_location): Adjust to std::string.
(update_static_tracepoint): Adjust to std::string, remove call
to release_static_tracepoint_marker.
* linux-nat.c (linux_child_static_tracepoint_markers_by_strid):
Adjust to std::vector.
* remote.c (remote_static_tracepoint_marker_at): Return bool.
(remote_static_tracepoint_markers_by_strid): Adjust to
std::vector.
* common/rsp-low.h (hex2str): New overload with explicit count
of bytes.
* common/rsp-low.c (hex2str): New overload with explicit count
of bytes.
* unittests/rsp-low-selftests.c (test_hex2str): New function.
(_initialize_rsp_low_selftests): Add test_hex2str test.
* unittests/tracepoint-selftests.c
(test_parse_static_tracepoint_marker_definition): Adjust to
std::string.
This converts observers from using a special source-generating script
to be plain C++. This version of the patch takes advantage of C++11
by using std::function and variadic templates; incorporates Pedro's
patches; and renames the header file to "observable.h" (this change
eliminates the need for a clean rebuild).
Note that Pedro's patches used a template lambda in tui-hooks.c, but
this failed to compile on some buildbot instances (presumably due to
differing C++ versions); I replaced this with an ordinary template
function.
Regression tested on the buildbot.
gdb/ChangeLog
2018-03-19 Pedro Alves <palves@redhat.com>
Tom Tromey <tom@tromey.com>
* unittests/observable-selftests.c: New file.
* common/observable.h: New file.
* observable.h: New file.
* ada-lang.c, ada-tasks.c, agent.c, aix-thread.c, annotate.c,
arm-tdep.c, auto-load.c, auxv.c, break-catch-syscall.c,
breakpoint.c, bsd-uthread.c, cli/cli-interp.c, cli/cli-setshow.c,
corefile.c, dummy-frame.c, event-loop.c, event-top.c, exec.c,
extension.c, frame.c, gdbarch.c, guile/scm-breakpoint.c,
infcall.c, infcmd.c, inferior.c, inflow.c, infrun.c, jit.c,
linux-tdep.c, linux-thread-db.c, m68klinux-tdep.c,
mi/mi-cmd-break.c, mi/mi-interp.c, mi/mi-main.c, objfiles.c,
ppc-linux-nat.c, ppc-linux-tdep.c, printcmd.c, procfs.c,
python/py-breakpoint.c, python/py-finishbreakpoint.c,
python/py-inferior.c, python/py-unwind.c, ravenscar-thread.c,
record-btrace.c, record-full.c, record.c, regcache.c, remote.c,
riscv-tdep.c, sol-thread.c, solib-aix.c, solib-spu.c, solib.c,
spu-multiarch.c, spu-tdep.c, stack.c, symfile-mem.c, symfile.c,
symtab.c, thread.c, top.c, tracepoint.c, tui/tui-hooks.c,
tui/tui-interp.c, valops.c: Update all users.
* tui/tui-hooks.c (tui_bp_created_observer)
(tui_bp_deleted_observer, tui_bp_modified_observer)
(tui_inferior_exit_observer, tui_before_prompt_observer)
(tui_normal_stop_observer, tui_register_changed_observer):
Remove.
(tui_observers_token): New global.
(attach_or_detach, tui_attach_detach_observers): New functions.
(tui_install_hooks, tui_remove_hooks): Use
tui_attach_detach_observers.
* record-btrace.c (record_btrace_thread_observer): Remove.
(record_btrace_thread_observer_token): New global.
* observer.sh: Remove.
* observer.c: Rename to observable.c.
* observable.c (namespace gdb_observers): Define new objects.
(observer_debug): Move into gdb_observers namespace.
(struct observer, struct observer_list, xalloc_observer_list_node)
(xfree_observer_list_node, generic_observer_attach)
(generic_observer_detach, generic_observer_notify): Remove.
(_initialize_observer): Update.
Don't include observer.inc.
* Makefile.in (generated_files): Remove observer.h, observer.inc.
(clean mostlyclean): Likewise.
(observer.h, observer.inc): Remove targets.
(SUBDIR_UNITTESTS_SRCS): Add observable-selftests.c.
(COMMON_SFILES): Use observable.c, not observer.c.
* .gitignore: Remove observer.h.
gdb/doc/ChangeLog
2018-03-19 Tom Tromey <tom@tromey.com>
* observer.texi: Remove.
gdb/testsuite/ChangeLog
2018-03-19 Tom Tromey <tom@tromey.com>
* gdb.gdb/observer.exp: Remove.
In remote.c, when the output of "set debug remote" is truncated, the
number of characters reported is incorrect. What is reported is the
number of characters added by the quoting, not the number of characters
that were truncated.
gdb/ChangeLog:
* remote.c (putpkt_binary): Fix omitted bytes reporting.
(getpkt_or_notif_sane_1): Likewise.
This changes to_fileio_readlink and target_fileio_readlink to return a
gdb::optional<std::sring>, and then fixes up the callers and
implementations. This allows the removal of some cleanups.
Regression tested by the buildbot.
gdb/ChangeLog
2018-03-07 Tom Tromey <tom@tromey.com>
* linux-tdep.c (linux_info_proc): Update.
* target.h (struct target_ops) <to_fileio_readlink>: Return
optional<string>.
(target_fileio_readlink): Return optional<string>.
* remote.c (remote_hostio_readlink): Return optional<string>.
* inf-child.c (inf_child_fileio_readlink): Return
optional<string>.
* target.c (target_fileio_readlink): Return optional<string>.
Remove the to_supports_btrace target method and instead rely on detecting errors
when trying to enable recording. This will also provide a suitable error
message explaining why recording is not possible.
For remote debugging, gdbserver will now always advertise branch tracing related
packets. When talking to an older GDB, this will cause GDB to try to enable
branch tracing and gdbserver to report a suitable error message every time.
An older gdbserver will not advertise branch tracing related packets if the
one-time check failed, so a newer GDB with this patch will fail to enable branch
tracing at remote_enable_btrace() rather than at btrace_enable(). The error
message is the same in both cases so there should be no user-visible change.
gdb/
* btrace.c (btrace_enable): Remove target_supports_btrace call.
* nat/linux-btrace.c (perf_event_pt_event_type): Move.
(kernel_supports_bts, kernel_supports_pt, linux_supports_bts)
(linux_supports_pt, linux_supports_btrace): Remove.
(linux_enable_bts): Call cpu_supports_bts.
* nat/linux-btrace.h (linux_supports_btrace): Remove.
* remote.c (remote_supports_btrace): Remove.
(init_remote_ops): Remove remote_supports_btrace.
* target-delegates.c: Regenerated.
* target.c (target_supports_btrace): Remove.
* target.h (target_ops) <to_supports_btrace>: Remove
(target_supports_btrace): Remove.
* x86-linux-nat.c (x86_linux_create_target): Remove
linux_supports_btrace.
gdbserver/
* linux-low.c (linux_target_ops): Remove linux_supports_btrace.
* nto-low.c (nto_target_ops): Remove NULL for supports_btrace.
* spu-low.c (spu_target_ops): Likewise.
* win32-low.c (win32_target_ops): Likewise.
* server.c (supported_btrace_packets): Report packets unconditionally.
* target.h (target_ops) <supports_btrace>: Remove.
(target_supports_btrace): Remove.
In my multi-target branch I ran into problems with GDB's terminal
handling that exist in master as well, with multi-inferior debugging.
This patch adds a testcase for said problems
(gdb.multi/multi-term-settings.exp), fixes the problems, fixes PR
gdb/13211 as well (and adds a testcase for that too,
gdb.base/interrupt-daemon.exp).
The basis of the problem I ran into is the following. Consider a
scenario where you have:
- inferior 1 - started with "attach", process is running on some
other terminal.
- inferior 2 - started with "run", process is sharing gdb's terminal.
In this scenario, when you stop/resume both inferiors, you want GDB to
save/restore the terminal settings of inferior 2, the one that is
sharing GDB's terminal. I.e., you want inferior 2 to "own" the
terminal (in target_terminal::is_ours/target_terminal::is_inferior
sense).
Unfortunately, that's not what you get currently. Because GDB doesn't
know whether an attached inferior is actually sharing GDB's terminal,
it tries to save/restore its settings anyway, ignoring errors. In
this case, this is pointless, because inferior 1 is running on a
different terminal, but GDB doesn't know better.
And then, because it is only possible to have the terminal settings of
a single inferior be in effect at a time, or make one inferior/pgrp be
the terminal's foreground pgrp (aka, only one inferior can "own" the
terminal, ignoring fork children here), if GDB happens to try to
restore the terminal settings of inferior 1 first, then GDB never
restores the terminal settings of inferior 2.
This patch fixes that and a few things more along the way:
- Moves enum target_terminal::terminal_state out of the
target_terminal class (it's currently private) and makes it a
scoped enum so that it can be easily used elsewhere.
- Replaces the inflow.c:terminal_is_ours boolean with a
target_terminal_state variable. This allows distinguishing is_ours
and is_ours_for_output states. This allows finally making
child_terminal_ours_1 do something with its "output_only"
parameter.
- Makes each inferior have its own copy of the
is_ours/is_ours_for_output/is_inferior state.
- Adds a way for GDB to tell whether the inferior is sharing GDB's
terminal. Works best on Linux and Solaris; the fallback works just
as well as currently.
- With that, we can remove the inf->attach_flag tests from
child_terminal_inferior/child_terminal_ours.
- Currently target_ops.to_ours is responsible for both saving the
current inferior's terminal state, and restoring gdb's state.
Because each inferior has its own terminal state (possibly handled
by different targets in a multi-target world, even), we need to
split the inferior-saving part from the gdb-restoring part. The
patch adds a new target_ops.to_save_inferior target method for
that.
- Adds a new target_terminal::save_inferior() function, so that
sequences like:
scoped_restore_terminal_state save_state;
target_terminal::ours_for_output ();
... restore back inferiors that were
target_terminal_state::is_inferior before back to is_inferior, and
leaves inferiors that were is_ours alone.
- Along the way, this adds a default implementation of
target_pass_ctrlc to inflow.c (for inf-child.c), that handles
passing the Ctrl-C to a process running on GDB's terminal or to
some other process otherwise.
- Similarly, adds a new target default implementation of
target_interrupt, for the "interrupt" command. The current
implementation of this hook in inf-ptrace.c kills the whole process
group, but that's incorrect/undesirable because we may not be
attached to all processes in the process group. And also, it's
incorrect because inferior_process_group() doesn't really return
the inferior's real process group id if the inferior is not a
process group leader... This is the cause of PR gdb/13211 [1],
which this patch fixes. While at it, that target method's "ptid"
parameter is eliminated, because it's not really used.
- A new test is included that exercises and fixes PR gdb/13211, and
also fixes a GDB issue reported on stackoverflow that I ran into
while working on this [2]. The problem is similar to PR gdb/13211,
except that it also triggers with Ctrl-C. When debugging a daemon
(i.e., a process that disconnects from the controlling terminal and
is not a process group leader, then Ctrl-C doesn't work, you just
can't interrupt the inferior at all, resulting in a hung debug
session. The problem is that since the inferior is no longer
associated with gdb's session / controlling terminal, then trying
to put the inferior in the foreground fails. And so Ctrl-C never
reaches the inferior directly. pass_signal is only used when the
inferior is attached, but that is not the case here. This is fixed
by the new child_pass_ctrlc. Without the fix, the new
interrupt-daemon.exp testcase fails with timeout waiting for a
SIGINT that never arrives.
[1] PR gdb/13211 - Async / Process group and interrupt not working
https://sourceware.org/bugzilla/show_bug.cgi?id=13211
[2] GDB not reacting Ctrl-C when after fork() and setsid()
https://stackoverflow.com/questions/46101292/gdb-not-reacting-ctrl-c-when-after-fork-and-setsid
Note this patch does _not_ fix:
- PR gdb/14559 - The 'interrupt' command does not work if sigwait is in use
https://sourceware.org/bugzilla/show_bug.cgi?id=14559
- PR gdb/9425 - When using "sigwait" GDB doesn't trap SIGINT. Ctrl+C terminates program when should break gdb.
https://sourceware.org/bugzilla/show_bug.cgi?id=9425
The only way to fix that that I know of (without changing the kernel)
is to make GDB put inferiors in a separate session (create a
pseudo-tty master/slave pair, make the inferior run with the slave as
its terminal, and have gdb pump output/input on the master end).
gdb/ChangeLog:
2018-01-30 Pedro Alves <palves@redhat.com>
PR gdb/13211
* config.in, configure: Regenerate.
* configure.ac: Check for getpgid.
* go32-nat.c (go32_pass_ctrlc): New.
(go32_target): Install it.
* inf-child.c (inf_child_target): Install
child_terminal_save_inferior, child_pass_ctrlc and
child_interrupt.
* inf-ptrace.c (inf_ptrace_interrupt): Delete.
(inf_ptrace_target): No longer install it.
* infcmd.c (interrupt_target_1): Adjust.
* inferior.h (child_terminal_save_inferior, child_pass_ctrlc)
(child_interrupt): Declare.
(inferior::terminal_state): New.
* inflow.c (struct terminal_info): Update comments.
(inferior_process_group): Delete.
(terminal_is_ours): Delete.
(gdb_tty_state): New.
(child_terminal_init): Adjust.
(is_gdb_terminal, sharing_input_terminal_1)
(sharing_input_terminal): New functions.
(child_terminal_inferior): Adjust. Use sharing_input_terminal.
Set the process's actual process group in the foreground if
possible. Handle is_ours_for_output/is_ours distinction. Don't
mark terminal as the inferior's if not sharing GDB's terminal.
Don't check attach_flag.
(child_terminal_ours_for_output, child_terminal_ours): Adjust to
pass down a target_terminal_state.
(child_terminal_save_inferior): New, factored out from ...
(child_terminal_ours_1): ... this. Handle
target_terminal_state::is_ours_for_output.
(child_interrupt, child_pass_ctrlc): New.
(inflow_inferior_exit): Clear the inferior's terminal_state.
(copy_terminal_info): Copy the inferior's terminal state.
(_initialize_inflow): Remove reference to terminal_is_ours.
* inflow.h (inferior_process_group): Delete.
* nto-procfs.c (nto_handle_sigint, procfs_interrupt): Adjust.
* procfs.c (procfs_target): Don't install procfs_interrupt.
(procfs_interrupt): Delete.
* remote.c (remote_serial_quit_handler): Adjust.
(remote_interrupt): Remove ptid parameter. Adjust.
* target-delegates.c: Regenerate.
* target.c: Include "terminal.h".
(target_terminal::terminal_state): Rename to ...
(target_terminal::m_terminal_state): ... this.
(target_terminal::init): Adjust.
(target_terminal::inferior): Adjust to per-inferior
terminal_state.
(target_terminal::restore_inferior, target_terminal_is_ours_kind): New.
(target_terminal::ours, target_terminal::ours_for_output): Use
target_terminal_is_ours_kind.
(target_interrupt): Remove ptid parameter. Adjust.
(default_target_pass_ctrlc): Adjust.
* target.h (target_ops::to_terminal_save_inferior): New field.
(target_ops::to_interrupt): Remove ptid_t parameter.
(target_interrupt): Remove ptid_t parameter. Update comment.
(target_pass_ctrlc): Update comment.
* target/target.h (target_terminal_state): New scoped enum,
factored out of ...
(target_terminal::terminal_state): ... here.
(target_terminal::inferior): Update comments.
(target_terminal::restore_inferior): New.
(target_terminal::is_inferior, target_terminal::is_ours)
(target_terminal::is_ours_for_output): Adjust.
(target_terminal::scoped_restore_terminal_state): Adjust to
rename, and call restore_inferior() instead of inferior().
(target_terminal::scoped_restore_terminal_state::m_state): Change
type.
(target_terminal::terminal_state): Rename to ...
(target_terminal::m_terminal_state): ... this and change type.
gdb/gdbserver/ChangeLog:
2018-01-30 Pedro Alves <palves@redhat.com>
PR gdb/13211
* target.c (target_terminal::terminal_state): Rename to ...
(target_terminal::m_terminal_state): ... this.
gdb/testsuite/ChangeLog:
2018-01-30 Pedro Alves <palves@redhat.com>
PR gdb/13211
* gdb.base/interrupt-daemon.c: New.
* gdb.base/interrupt-daemon.exp: New.
* gdb.multi/multi-term-settings.c: New.
* gdb.multi/multi-term-settings.exp: New.
This patch fixes a regression that has been introduced by:
commit bc09b0c14f
Date: Fri Jan 19 11:48:11 2018 -0500
Make linux_nat_detach/thread_db_detach use the inferior parameter
It is possible to trigger this failure with gdb.base/foll-fork.exp (in
which case a bunch of ERROR's will be printed), but one can also use
the test below.
Consider the following example program:
#include <unistd.h>
int
main (int argc, char *argv[])
{
fork ();
return 0;
}
When running it under gdbserver:
# ./gdb/gdbserver/gdbserver --multi --once :2345
And debugging it under GDB, we see a segmentation fault:
# ./gdb/gdb -q -batch -ex 'set remote exec-file ./a.out' -ex 'tar extended-remote :2345' -ex r ./a.out
Starting program:
...
[Detaching after fork from child process 16102.]
Segmentation fault (core dumped)
The problem happens on inferior.c:detach_inferior:
void
detach_inferior (inferior *inf)
{
/* Save the pid, since exit_inferior_1 will reset it. */
int pid = inf->pid;
^^^^^^^^^
exit_inferior_1 (inf, 0);
if (print_inferior_events)
printf_unfiltered (_("[Inferior %d detached]\n"), pid);
}
When this code is called from remote.c:remote_follow_fork, the PID is
valid but there is no 'inferior' associated with it, which means that
'inf == NULL'.
The proper fix here is to not call "detach_inferior" when doing remote
follow-fork, because we don't have an inferior to detach on the host
side.
Before bc09b0c1, that call was already a nop (exit_inferior_1 bails
out early if you pass it a NULL inferior), except that it printed
"Inferior PID detached" when "set print inferior-events" is on. Since
native debugging doesn't call detach_inferior in this case, removing
the call from remote aligns remote debugging output with native
debugging output further.
This has been regtested using BuildBot and no regressions were found.
gdb/ChangeLog:
2018-01-29 Sergio Durigan Junior <sergiodj@redhat.com>
* remote.c (remote_follow_fork): Don't call "detach_inferior".
The to_detach target_ops method implementations are currently expected
to work on current_inferior/inferior_ptid. In order to make things more
explicit, and remove some "shadow" parameter passing through globals,
this patch adds an "inferior" parameter to to_detach. Implementations
will be expected to use this instead of relying on the global. However,
to keep things simple, this patch only does the minimum that is
necessary to add the parameter. The following patch gives an example of
how one such implementation would be adapted. If the approach is deemed
good, we can then look into adapting more implementations. Until then,
they'll continue to work as they do currently.
gdb/ChangeLog:
* target.h (struct target_ops) <to_detach>: Add inferior
parameter.
(target_detach): Likewise.
* target.c (dispose_inferior): Pass inferior down.
(target_detach): Pass inferior down. Assert that it is equal to
the current inferior.
* aix-thread.c (aix_thread_detach): Pass inferior down.
* corefile.c (core_file_command): Pass current_inferior() down.
* corelow.c (core_detach): Add inferior parameter.
* darwin-nat.c (darwin_detach): Likewise.
* gnu-nat.c (gnu_detach): Likewise.
* inf-ptrace.c (inf_ptrace_detach): Likewise.
* infcmd.c (detach_command): Pass current_inferior() down to
target_detach.
* infrun.c (follow_fork_inferior): Pass parent_inf to
target_detach.
(handle_vfork_child_exec_or_exit): Pass inf->vfork_parent to
target_detach.
* linux-nat.c (linux_nat_detach): Add inferior parameter.
* linux-thread-db.c (thread_db_detach): Likewise.
* nto-procfs.c (procfs_detach): Likewise.
* procfs.c (procfs_detach): Likewise.
* record.c (record_detach): Likewise.
* record.h (struct inferior): Forward-declare.
(record_detach): Add inferior parameter.
* remote-sim.c (gdbsim_detach): Likewise.
* remote.c (remote_detach_1): Likewise.
(remote_detach): Likewise.
(extended_remote_detach): Likewise.
* sol-thread.c (sol_thread_detach): Likewise.
* target-debug.h (target_debug_print_inferior_p): New macro.
* target-delegates.c: Re-generate.
* top.c (kill_or_detach): Pass inferior down to target_detach.
* windows-nat.c (windows_detach): Add inferior parameter.
I was looking into adding a parameter to target_detach, and was
wondering what the args parameter was. It seems like in the distant
past, it was possible to specify a signal number when detaching. That
signal was injected in the process before it was detached. There is an
example of code handling this in linux_nat_detach. With today's GDB, I
can't get this to work. Doing "detach 15" (15 == SIGTERM) doesn't work,
because detach is a prefix command and doesn't recognize the sub-command
15. Doing "detach inferiors 15" doesn't work because it expects a list
of inferior id to detach. Therefore, I don't think there's a way of
invoking detach_command with a non-NULL args. I also didn't find any
documentation related to this feature.
I assume that this feature stopped working when detach was made a prefix
command, which is in f73adfeb8b (sorry,
there's no commit title) from 2006. Given that this feature was broken
for such a long time and we haven't heard anything (AFAIK, I did not
find any related bug), I think it's safe to remove it, as well as the
args parameter to target_detach. If someone wants to re-introduce it, I
would suggest rethinking the user interface, and in particular would
suggest using signal name instead of numbers.
I tried to fix all the impacted code, but I might have forgotten some
spots. It shouldn't be hard to fix if that's the case. I also couldn't
build-test everything I changed, especially the nto and solaris stuff.
gdb/ChangeLog:
* target.h (struct target_ops) <to_detach>: Remove args
parameter.
(target_detach): Likewise.
* target.c (dispose_inferior): Adjust.
(target_detach): Remove args parameter, adjust.
* aix-thread.c (aix_thread_detach): Adjust.
* corefile.c (core_file_command): Adjust.
* corelow.c (core_detach): Adjust.
* darwin-nat.c (darwin_detach): Adjust.
* gnu-nat.c (gnu_detach): Adjust.
* inf-ptrace.c (inf_ptrace_detach): Adjust.
* infcmd.c (detach_command): Adjust
* infrun.c (follow_fork_inferior): Adjust.
(handle_vfork_child_exec_or_exit): Adjust.
* linux-fork.c (linux_fork_detach): Remove args parameter.
* linux-fork.h (linux_fork_detach): Likewise.
* linux-nat.c (linux_nat_detach): Likewise, and adjust.
* linux-thread-db.c (thread_db_detach): Likewise.
* nto-procfs.c (procfs_detach): Likewise.
* procfs.c (procfs_detach): Likewise.
(do_detach): Remove signo parameter.
* record.c (record_detach): Remove args parameter.
* record.h (record_detach): Likewise.
* remote-sim.c (gdbsim_detach): Likewise.
* remote.c (remote_detach_1): Likewise.
(remote_detach): Likewise.
(extended_remote_detach): Likewise.
* sol-thread.c (sol_thread_detach): Likewise.
* target-delegates.c: Re-generate.
* top.c (struct qt_args) <args>: Remove field.
(kill_or_detach): Don't pass args.
(quit_force): Don't set args.
* windows-nat.c (windows_detach): Remove args parameter.
At <https://sourceware.org/ml/gdb-patches/2017-12/msg00285.html>,
Maciej reported that commit:
commit 5cd63fda03
Date: Wed Oct 4 18:21:10 2017 +0100
Subject: Fix "Remote 'g' packet reply is too long" problems with multiple inferiors
made GDB stop working with older stubs. Any attempt to continue
execution after the initial connection fails with:
[...]
Process .../gdb/testsuite/outputs/gdb.base/advance/advance created; pid = 2670
Listening on port 2346
target remote [...]:2346
Remote debugging using [...]:2346
Reading symbols from .../lib64/ld.so.1...done.
[Switching to Thread <main>]
(gdb) continue
Cannot execute this command without a live selected thread.
(gdb)
The problem is:
(gdb) c
Cannot execute this command without a live selected thread.
(gdb) info threads
Id Target Id Frame
1 Thread 14917 0x00007f341cd98ed0 in _start () from /lib64/ld-linux-x86-64.so.2
The current thread <Thread ID 2> has terminated. See `help thread'.
^^^^^^^^^^^
(gdb)
Note, thread _2_. There's really only one thread in the inferior
(it's still at the entry point), but still GDB added a bogus second
thread.
The reason GDB started adding a second thread after 5cd63fda03 is
this hunk:
+ if (event->ptid == null_ptid)
+ {
+ const char *thr = strstr (p1 + 1, ";thread:");
+ if (thr != NULL)
+ event->ptid = read_ptid (thr + strlen (";thread:"),
+ NULL);
+ else
+ event->ptid = magic_null_ptid;
+ }
Note the else branch that falls back to magic_null_ptid. We reach
that when we process the initial stop reply sent back in response to
the the "?" (status) packet early in the connection setup:
Sending packet: $?#3f...Ack
Packet received: T0506:0000000000000000;07:40a510f4fd7f0000;10:d0fe1201577f0000;
And note that that response does not include a ";thread:XXX" part.
This stop reply is processed after listing threads with qfThreadInfo /
qsThreadInfo :
Sending packet: $qfThreadInfo#bb...Ack
Packet received: m3915
Sending packet: $qsThreadInfo#c8...Ack
Packet received: l
meaning, when we process that stop reply, we treat the event as coming
from a thread with ptid == magic_null_ptid, which is not yet in the
thread list, so we add it then:
(top-gdb) p ptid
$1 = {m_pid = 42000, m_lwp = -1, m_tid = 1}
(top-gdb) bt
#0 0x0000000000840a8c in add_thread_silent(ptid_t) (ptid=...) at src/gdb/thread.c:269
#1 0x00000000007ad61d in remote_add_thread(ptid_t, int, int) (ptid=..., running=0, executing=0)
at src/gdb/remote.c:1838
#2 0x00000000007ad8de in remote_notice_new_inferior(ptid_t, int) (currthread=..., executing=0)
at src/gdb/remote.c:1921
#3 0x00000000007b758b in process_stop_reply(stop_reply*, target_waitstatus*) (stop_reply=0x1158860, status=0x7fffffffcc00)
at src/gdb/remote.c:7217
#4 0x00000000007b7a38 in remote_wait_as(ptid_t, target_waitstatus*, int) (ptid=..., status=0x7fffffffcc00, options=0)
at src/gdb/remote.c:7380
#5 0x00000000007b7cd1 in remote_wait(target_ops*, ptid_t, target_waitstatus*, int) (ops=0x102fac0 <remote_ops>, ptid=..., status=0x7fffffffcc00, options=0) at src/gdb/remote.c:7446
#6 0x000000000081587b in delegate_wait(target_ops*, ptid_t, target_waitstatus*, int) (self=0x102fac0 <remote_ops>, arg1=..., arg2=0x7fffffffcc00, arg3=0) at src/gdb/target-delegates.c:138
#7 0x0000000000827d77 in target_wait(ptid_t, target_waitstatus*, int) (ptid=..., status=0x7fffffffcc00, options=0)
at src/gdb/target.c:2179
#8 0x0000000000715fda in do_target_wait(ptid_t, target_waitstatus*, int) (ptid=..., status=0x7fffffffcc00, options=0)
at src/gdb/infrun.c:3589
#9 0x0000000000716351 in wait_for_inferior() () at src/gdb/infrun.c:3707
#10 0x0000000000715435 in start_remote(int) (from_tty=1) at src/gdb/infrun.c:3212
things go downhill from this.
We don't see the problem with current master gdbserver, because that
version always sends the ";thread:" part in the initial stop reply:
Sending packet: $?#3f...Packet received: T0506:0000000000000000;07:a0d4ffffff7f0000;10:d05eddf7ff7f0000;thread:p3cea.3cea;core:3;
Years ago I had added a "--disable-packet=" command line option to
gdbserver which comes in handy for testing this, since the existing
"--disable-packet=Tthread" precisely makes gdbserver not send that
";thread:" part in stop replies. The testcase added by this commit
emulates old gdbserver making use of that.
I've compared a testrun at 5cd63fda035d^ (before regression) with
'current master+patch', against old gdbserver at f8b73d13b7ca^. I
hacked out --once, and "monitor exit" to be able to test. The results
are a bit too unstable to tell accurately, but it looked like there
were no regressions. Maciej confirmed this worked for him as well.
No regressions on master (against master gdbserver).
gdb/ChangeLog:
2018-01-11 Pedro Alves <palves@redhat.com>
PR remote/22597
* remote.c (remote_parse_stop_reply): Default to the last-set
general thread instead of to 'magic_null_ptid'.
gdb/testsuite/ChangeLog:
2018-01-11 Pedro Alves <palves@redhat.com>
PR remote/22597
* gdb.server/stop-reply-no-thread.c: New file.
* gdb.server/stop-reply-no-thread.exp: New file.
This patch replaces VEC(gdb_xml_value_s), which is passed to XML
visitors, with an std::vector. In order to be able to remove the
cleanup in gdb_xml_parser::start_element, the gdb_xml_parser structure
is made to own the value with a gdb::unique_xmalloc_ptr.
This patch has been tested on the buildbot.
gdb/ChangeLog:
* xml-support.h (struct gdb_xml_value): Add constructor.
<value>: Change type to unique_xmalloc_ptr.
(gdb_xml_value_s): Remove typedef.
(DEF_VEC_O (gdb_xml_value_s)): Remove.
(gdb_xml_element_start_handler): Change parameter type to
std::vector.
(xml_find_attribute): Likewise.
* xml-support.c (xml_find_attribute): Change parameter type to
std::vector and adjust.
(gdb_xml_values_cleanup): Remove.
(gdb_xml_parser::start_element): Adjust to std::vector.
(xinclude_start_include): Change paraeter type to std::vector
and adjust.
* btrace.c (check_xml_btrace_version): Likewise.
(parse_xml_btrace_block): Likewise.
(parse_xml_btrace_pt_config_cpu): Likewise.
(parse_xml_btrace_pt): Likewise.
(parse_xml_btrace_conf_bts): Likewise.
(parse_xml_btrace_conf_pt): Likewise.
* memory-map.c (memory_map_start_memory): Likewise.
(memory_map_start_property): Likewise.
* osdata.c (osdata_start_osdata): Likewise.
(osdata_start_item): Likewise.
(osdata_start_column): Likewise.
* remote.c (start_thread): Likewise.
* solib-aix.c (library_list_start_library): Likewise.
(library_list_start_list): Likewise.
* solib-svr4.c (library_list_start_library): Likewise.
(svr4_library_list_start_list): Likewise.
* solib-target.c (library_list_start_segment): Likewise.
(library_list_start_section): Likewise.
(library_list_start_library): Likewise.
(library_list_start_list): Likewise.
* tracepoint.c (traceframe_info_start_memory): Likewise.
(traceframe_info_start_tvar): Likewise.
* xml-syscall.c (syscall_start_syscall): Likewise.
* xml-tdesc.c (tdesc_start_target): Likewise.
(tdesc_start_feature): Likewise.
(tdesc_start_reg): Likewise.
(tdesc_start_union): Likewise.
(tdesc_start_struct): Likewise.
(tdesc_start_flags): Likewise.
(tdesc_start_enum): Likewise.
(tdesc_start_field): Likewise.
(tdesc_start_enum_value): Likewise.
(tdesc_start_vector): Likewise.
Commit
remote: C++ify thread_item and threads_listing_context
21fe1c752e
broke the test gdb.threads/names.exp. The problem is that since we now
use an std::string to hold the extra_info, an empty string is returned
by target_extra_thread_info to print_thread_info_1 when the remote stub
didn't send any extra info, instead of NULL before. Because of that,
print_thread_info_1 prints the extra info between parentheses, which
results in some spurious empty parentheses.
Expected: * 1 Thread 22752.22752 "main" all_threads_ready () at ...
Actual : * 1 Thread 22752.22752 "main" () all_threads_ready () a ...
Since the bug was introduced by a behavior change in the remote target,
I chose to fix it on the remote target side by making it return NULL
when the extra string is empty. This will avoid possibly changing the
behavior of the common code and affecting other targets.
The name field has the same problem. If a remote stub returns no thread
names, remote_thread_name will return an empty string instead of NULL,
so print_thread_info_1 will show empty quotes ("") instead of nothing.
gdb/ChangeLog:
PR gdb/22556
* remote.c (remote_thread_name): Return NULL if name is empty.
(remote_threads_extra_info): Return NULL if extra info is empty.
I noticed that we're passing down a data/size pair to
target_ops::to_set_syscall_catchpoint. This commit makes use of
gdb::array_view instead. While at it, use bool where appropriate as
well.
gdb/ChangeLog:
* break-catch-syscall.c (insert_catch_syscall)
(remove_catch_syscall): Adjust to pass reference to
inf_data->syscalls_counts directly via gdb::array_view.
* fbsd-nat.c (fbsd_set_syscall_catchpoint): Adjust to use bool
and gdb::array_view.
* linux-nat.c (linux_child_set_syscall_catchpoint): Likewise.
* remote.c (remote_set_syscall_catchpoint): Likewise.
* target-debug.h (target_debug_print_bool): New.
(define target_debug_print_gdb_array_view_const_int): New.
* target-delegates.c: Regenerate.
* target.h (target_ops) <to_set_syscall_catchpoint>: Use
gdb::array_view and bool.
(target_set_syscall_catchpoint): Likewise.
I've noticed that "set remote target-features-packet off" before
connecting has no effect -- GDB still fetches a target description
anyway.
The problem is that while most "set remote foo-packet" commands were
fixed by:
From 4082afcc3d Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 25 Apr 2014 18:07:02 +0100
Subject: [PATCH] Fix several "set remote foo-packet on/off" commands.
<https://sourceware.org/ml/gdb-patches/2014-04/msg00006.html>
the "qXfer" packets where missed. This commit fixes that.
I've changed remote_search_memory too for consistency (seems like
those are the last direct references to packet->support), though the
difference is not observable because the qSearch:memory packet is auto
probed. Note gdb.base/find-unmapped.exp already exercises explicit
"set remote search-memory-packet off".
gdb/ChangeLog:
2017-12-06 Pedro Alves <palves@redhat.com>
* remote.c (remote_query_supported): Don't send "xmlRegisters=" if
"qXfer:features:read"" is disabled.
(remote_write_qxfer, remote_read_qxfer, remote_search_memory):
Check packet_config_support instead of packet->support directly.
gdb/testsuite/ChangeLog:
2017-12-06 Pedro Alves <palves@redhat.com>
* gdb.arch/i386-avx.exp: If testing with a RSP target, check
force-disabling XML descriptions.
--
gdb/remote.c | 16 +++++++++-------
gdb/testsuite/gdb.arch/i386-avx.exp | 25 +++++++++++++++++++++++++
2 files changed, 34 insertions(+), 7 deletions(-)
There are multiple definitions of the private_thread_info structure
compiled in the same GDB build. Because of the one definition rule, we
need to change this if we want to be able to make them non-POD (e.g. use
std::vector fields). This patch creates a class hierarchy, with
private_thread_info being an abstract base class, and all the specific
implementations inheriting from it.
In order to poison XNEW/xfree for non-POD types, it is also needed to
get rid of the xfree in thread_info::~thread_info, which operates on an
opaque type. This is replaced by thread_info::priv now being a
unique_ptr, which calls the destructor of the private_thread_info
subclass when the thread is being destroyed.
Including gdbthread.h from darwin-nat.h gave these errors:
/Users/simark/src/binutils-gdb/gdb/gdbthread.h:609:3: error: must use 'class' tag to refer to type 'thread_info' in this scope
thread_info *m_thread;
^
class
/usr/include/mach/thread_act.h:240:15: note: class 'thread_info' is hidden by a non-type declaration of 'thread_info' here
kern_return_t thread_info
^
It turns out that there is a thread_info function in the Darwin/XNU/mach API:
http://web.mit.edu/darwin/src/modules/xnu/osfmk/man/thread_info.html
Therefore, I had to add the class keyword at a couple of places in gdbthread.h,
I don't really see a way around it.
gdb/ChangeLog:
* gdbthread.h (private_thread_info): Define structure type, add
virtual pure destructor.
(thread_info) <priv>: Change type to unique_ptr.
<private_dtor>: Remove.
* thread.c (add_thread_with_info): Adjust to use of unique_ptr.
(private_thread_info::~private_thread_info): Provide default
implementation.
(thread_info::~thread_info): Don't call private_dtor nor
manually free priv.
* aix-thread.c (private_thread_info): Rename to ...
(aix_thread_info): ... this.
(get_aix_thread_info): New.
(sync_threadlists): Adjust.
(iter_tid): Adjust.
(aix_thread_resume): Adjust.
(aix_thread_fetch_registers): Adjust.
(aix_thread_store_registers): Adjust.
(aix_thread_extra_thread_info): Adjust.
* darwin-nat.h (private_thread_info): Rename to ...
(darwin_thread_info): ... this.
(get_darwin_thread_info): New.
* darwin-nat.c (darwin_init_thread_list): Adjust.
(darwin_check_new_threads): Adjust.
(thread_info_from_private_thread_info): Adjust.
* linux-thread-db.c (private_thread_info): Rename to ...
(thread_db_thread_info): ... this, initialize fields.
(get_thread_db_thread_info): New.
<dying>: Change type to bool.
(update_thread_state): Adjust to type rename.
(record_thread): Adjust to type rename an use of unique_ptr.
(thread_db_pid_to_str): Likewise.
(thread_db_extra_thread_info): Likewise.
(thread_db_thread_handle_to_thread_info): Likewise.
(thread_db_get_thread_local_address): Likewise.
* nto-tdep.h (private_thread_info): Rename to ...
(nto_thread_info): ... this, initialize fields.
(get_nto_thread_info): New.
<name>: Change type to std::string.
* nto-tdep.c (nto_extra_thread_info): Adjust to type rename and
use of unique_ptr.
* nto-procfs.c (update_thread_private_data_name): Adjust to
std::string change, allocate nto_private_thread_info with new.
(update_thread_private_data): Adjust to unique_ptr.
* remote.c (private_thread_info): Rename to ...
(remote_thread_info): ... this, initialize data members with
default values.
<extra, name>: Change type to std::string.
<thread_handle>: Change type to non-pointer.
(free_private_thread_info): Remove.
(get_private_info_thread): Rename to...
(get_remote_thread_info): ... this, change return type, adjust to
use of unique_ptr, use remote_thread_info constructor.
(remote_add_thread): Adjust.
(get_private_info_ptid): Rename to...
(get_remote_thread_info): ...this, change return type.
(remote_thread_name): Use get_remote_thread_info, adjust to
change to std::string.
(struct thread_item) <~thread_item>: Remove.
<thread_handle>: Make non pointer.
(start_thread): Adjust to thread_item::thread_handle type
change.
(remote_update_thread_list): Adjust to type name change, move
strings from temporary to long-lived object instead of
duplicating.
(remote_threads_extra_info): Use get_remote_thread_info.
(process_initial_stop_replies): Likewise.
(resume_clear_thread_private_info): Likewise.
(remote_resume): Adjust to type name change.
(remote_commit_resume): Use get_remote_thread_info.
(process_stop_reply): Adjust to type name change.
(remote_stopped_by_sw_breakpoint): Use get_remote_thread_info.
(remote_stopped_by_hw_breakpoint): Likewise.
(remote_stopped_by_watchpoint): Likewise.
(remote_stopped_data_address): Likewise.
(remote_core_of_thread): Likewise.
(remote_thread_handle_to_thread_info): Use
get_private_info_thread, adjust to thread_handle field type
change.
This patch C++ifies the thread_item and threads_listing_context
structures in remote.c. thread_item::{extra,name} are changed to
std::string. As a result, there's a bit of awkwardness in
remote_update_thread_list, where we have to xstrdup those strings when
filling the private_thread_info structure. This is removed in the
following patch, where private_thread_info is also C++ified and its
corresponding fields made std::string too. The xstrdup then becomes an
std::move.
Other than that there's nothing really special, it's a usual day-to-day
VEC -> vector and char* -> std::string change. It allows removing a
cleanup in remote_update_thread_list.
Note that an overload of hex2bin that returns a gdb::byte_vector is
added, with corresponding selftests.
gdb/ChangeLog:
* remote.c (struct thread_item): Add constructor, disable copy
construction and copy assignment, define default move
construction and move assignment.
<extra, name>: Change type to std::string.
<core>: Initialize.
<thread_handle>: Make non-pointer.
(thread_item_t): Remove typedef.
(DEF_VEC_O(thread_item_t)): Remove.
(threads_listing_context) <contains_thread>: New method.
<remove_thread>: New method.
<items>: Change type to std::vector.
(clear_threads_listing_context): Remove.
(threads_listing_context_remove): Remove.
(remote_newthread_step): Use thread_item constructor, adjust to
change to std::vector.
(start_thread): Use thread_item constructor, adjust to change to
std::vector.
(end_thread): Adjust to change to std::vector and std::string.
(remote_get_threads_with_qthreadinfo): Use thread_item
constructor, adjust to std::vector.
(remote_update_thread_list): Adjust to change to std::vector and
std::string, use threads_listing_context methods.
(remove_child_of_pending_fork): Adjust.
(remove_new_fork_children): Adjust.
* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add rsp-low-selftests.c.
(SUBDIR_UNITTESTS_OBS): Add rsp-low-selftests.o.
* unittests/rsp-low-selftests.c: New file.
* common/rsp-low.h: Include common/byte-vector.h.
(hex2bin): New overload.
* common/rsp-low.c (hex2bin): New overload.
There are currently multiple definitions of private_inferior, defined in
remote.c and darwin-nat.h. The patch that poisons XNEW and friends for
non-POD types trips on that, because private_inferior is freed in
~inferior(), where it is an opaque type. Since the compiler can't tell
whether the type is POD, it gives an error. Also, we can't start using
C++ features in these structures (make them non-POD) as long as there
are multiple definitions with the same name. For these reasons, this
patch makes a class hierarchy, with private_inferior being the abstract
base class, and darwin_inferior & remote_inferior inheriting from it.
Destruction is done through the virtual destructor.
I stumbled on some suspicious code in the darwin implementation though.
darwin_check_new_threads does an XCNEW(darwin_thread_t) when it finds a
new thread, allocating a new structure for it (darwin_thread_t is a
typedef for private_thread_info). It then VEC_safe_pushes it in a
vector defined as DEF_VEC_O (a vector of objects). This means that the
structure content gets copied in the vector. The thread_info object is
created with the XCNEW'ed structure as the private thread info, while
the rest of the code works with the instance in the vector. We have
therefore two distinct instances of darwin_thread_t/private_thread_info
for each thread. This is not really a problem in practice, because
thread_info::priv is not used in the darwin code. I still find it weird
and far from ideal, so I tried to fix it by changing the vector to be a
vector of pointers. There should now be a single instance of the
structure for each thread. The deallocation of the
darwin_thread_t/private_thread_info structure is done by the thread_info
destructor.
I am able to build on macOS, but not really test, since the port seems a
bit broken. I am not able to debug reliably on the machine I have
access to, which runs macOS 10.12.6.
gdb/ChangeLog:
* inferior.h (private_inferior): Define structure type, add
virtual pure destructor.
(inferior) <priv>: Change type to unique_ptr.
* inferior.c (private_inferior::~private_inferior): Provide
default implementation.
(inferior::~inferior): Don't free priv field.
(exit_inferior_1): Likewise.
* darwin-nat.h (struct darwin_exception_info): Initialize fields.
(darwin_exception_info): Remove typedef.
(DEF_VEC_O (darwin_thread_t)); Remove.
(private_inferior): Rename to ...
(darwin_private_inferior): ... this, extend private_inferior.
(get_darwin_inferior): New.
<threads>: Change type to std::vector of darwin_thread_t pointers.
* darwin-nat.c (darwin_check_new_threads): Adjust.
(find_inferior_task_it): Adjust.
(darwin_find_thread); Adjust.
(darwin_suspend_inferior): Adjust.
(darwin_resume_inferior): Adjust.
(darwin_find_new_inferior): Adjust.
(darwin_decode_notify_message): Adjust.
(darwin_send_reply): Adjust.
(darwin_resume_inferior_threads): Adjust.
(darwin_suspend_inferior_threads): Adjust.
(darwin_decode_message): Adjust.
(darwin_wait): Adjust.
(darwin_interrupt): Adjust.
(darwin_deallocate_threads): Adjust.
(darwin_mourn_inferior): Adjust, don't free private data.
(darwin_reply_to_all_pending_messages): Adjust.
(darwin_stop_inferior): Adjust.
(darwin_setup_exceptions): Adjust.
(darwin_kill_inferior): Adjust.
(darwin_setup_request_notification): Adjust.
(darwin_attach_pid): Adjust.
(darwin_init_thread_list): Adjust.
(darwin_setup_fake_stop_event): Adjust.
(darwin_attach): Adjust.
(darwin_detach): Adjust.
(darwin_xfer_partial): Adjust.
(set_enable_mach_exceptions): Adjust.
(darwin_pid_to_exec_file): Adjust.
(darwin_get_ada_task_ptid): Adjust.
* darwin-nat-info.c (get_task_from_args): Adjust.
(info_mach_ports_command): Adjust.
(info_mach_region_command): Adjust.
(info_mach_exceptions_command): Adjust.
* remote.c (private_inferior): Rename to ...
(remote_private_inferior): ... this, initialize fields.
(get_remote_inferior); New.
(remote_commit_resume): Use get_remote_inferior.
(check_pending_event_prevents_wildcard_vcont_callback): Likewise.
We have a customer who is using a Corelis gdb server to connect to gdb.
Occasionally, the gdb server will send a 0-byte block of memory for a
read. When this happens, gdb gives an assertion from target.c:
internal-error: target_xfer_partial: Assertion `*xfered_len > 0' failed.
This problem is almost identical to that fixed in
https://sourceware.org/ml/gdb-patches/2014-02/msg00636.html
In this case, remote.c needs to be modified to return TARGET_XFER_EOF
instead of TARGET_XFER_OK or TARGET_XFER_UNAVAILABLE when 0 bytes are
transferred.
gdb/ChangeLog:
PR gdb/22388
* remote.c (remote_write_bytes_aux, remote_read_bytes_1,
remote_read_bytes, remote_write_qxfer, remote_xfer_partial):
Return TARGET_XFER_EOF if size of returned data is 0.
Currently, encode_actions_rsp returns two malloc'ed arrays of malloc'ed
strings (char *) by pointer. Change this to use
std::vector<std::string>. This eliminates some cleanups in remote.c.
Regtested on the buildbot.
gdb/ChangeLog:
* tracepoint.h (class collection_list) <stringify>: Return
std::vector<std::string>.
(encode_actions_rsp): Change parameters to
std::vector<std::string> *.
* tracepoint.c (collection_list::stringify): Return
std::vector<std::string> and adjust accordingly.
(encode_actions_rsp): Changee parameters to
std::vector<std::string> and adjust accordingly.
* remote.c (free_actions_list),
free_actions_list_cleanup_wrapper): Remove.
(remote_download_tracepoint): Adjust to std::vector.
Simplify the code a little bit using std::string + string_appendf.
gdb/ChangeLog:
2017-10-30 Pedro Alves <palves@redhat.com>
Simon Marchi <simon.marchi@ericsson.com>
* remote.c (remote_set_syscall_catchpoint): Build a std::string
instead of a gdb::unique_xmalloc_ptr, using string_appendf.
remote.c:remote_async_terminal_ours_p stopped being useful after
048094accc ("target remote: Don't rely on immediate_quit (introduce
quit handlers)") and commit 41fd2b0f5d ("Make input_fd be per UI"),
which turned remote's terminal_inferior/ours methods into nops.
gdb/ChangeLog:
2017-10-26 Pedro Alves <palves@redhat.com>
* remote.c (remote_async_terminal_ours_p): Delete.
(remote_open_1, remote_terminal_inferior, remote_terminal_ours):
Remove references to 'remote_async_terminal_ours_p'.
This patch removes VEC (mem_region). Doing so requires touching a lot
of little things here and there.
The fields in mem_attrib are now initialized during construction. The
values match those that were in default_mem_attrib (now removed).
unknown_mem_attrib is also removed, and replaced with a static method
(mem_attrib::unknown) that returns the equivalent.
mem_region is initialized in a way similar to mem_region_init (now
removed) did.
I found the organization of mem_region_list and target_mem_region_list a
bit confusing. Sometimes mem_region_list points to the same vector as
target_mem_region_list (and therefore does not own it), and sometimes
(when the user manually edits the mem regions) points to another vector,
and in this case owns it. To avoid this ambiguity, I think it is
simpler to have two vectors, one for target-defined regions and one for
user-defined regions, and have mem_region_list point to one or the
other. There are now no vector objects dynamically allocated, both are
static.
The make-target-delegates script does not generate valid code when a
target method returns a type with a parameter list. For this reason, I
created a typedef (mem_region_vector) that's only used in the target_ops
structure. If you speak perl, you are welcome to improve the script!
Regtested on the buildbot.
gdb/ChangeLog:
* memattr.h: Don't include vec.h.
(struct mem_attrib): Initialize fields.
<unknown>: New static method.
(struct mem_region): Add constructors, operator<, initialize
fields.
* memattr.c: Include algorithm.
(default_mem_attrib, unknown_mem_attrib): Remove.
(user_mem_region_list): New global.
(target_mem_region_list, mem_region_list): Change type to
std::vector<mem_region>.
(mem_use_target): Now a function.
(target_mem_regions_valid): Change type to bool.
(mem_region_lessthan, mem_region_cmp, mem_region_init): Remove.
(require_user_regions): Adjust.
(require_target_regions): Adjust.
(create_mem_region): Adjust.
(lookup_mem_region): Adjust.
(invalidate_target_mem_regions): Adjust.
(mem_clear): Rename to...
(user_mem_clear): ... this, and adjust.
(mem_command): Adjust.
(info_mem_command): Adjust.
(mem_enable, enable_mem_command, mem_disable,
disable_mem_command): Adjust.
(mem_delete): Adjust.
(delete_mem_command): Adjust.
* memory-map.h (parse_memory_map): Return an std::vector.
* memory-map.c (parse_memory_map): Likewise.
(struct memory_map_parsing_data): Add constructor.
<memory_map>: Point to std::vector.
(memory_map_start_memory): Adjust.
(memory_map_end_memory): Adjust.
(memory_map_end_property): Adjust.
(clear_result): Remove.
* remote.c (remote_memory_map): Return an std::vector.
* target-debug.h (target_debug_print_VEC_mem_region_s__p):
Remove.
(target_debug_print_mem_region_vector): New.
* target-delegates.c: Regenerate.
* target.h (mem_region_vector): New typedef.
(to_memory_map): Return mem_region_vector.
(target_memory_map): Return an std::vector.
* target.c (target_memory_map): Return an std::vector.
(flash_erase_command): Adjust.
This removes a few cleanups in remote.c using the usual techniques:
std::vector, unique_xmalloc_ptr, and gdb::def_vector.
ChangeLog
2017-10-16 Tom Tromey <tom@tromey.com>
* remote.c (remote_register_number_and_offset): Use std::vector.
(remote_set_syscall_catchpoint): Use gdb::unique_xmalloc_ptr.
(putpkt_binary): Use gdb::def_vector.
(compare_sections_command): Use gdb::byte_vector.
This patch replaces the last usages of VEC(mem_range_s) with
std::vector<mem_range>. This allows getting rid of a few cleanups and
of the DEF_VEC_O(mem_range_s).
I added a test for normalize_mem_ranges to make sure I didn't break
anything there.
Regtested on the buildbot.
gdb/ChangeLog:
* memrange.h (struct mem_range): Define operator< and operator==.
(mem_range_s): Remove.
(DEF_VEC_O (mem_range_s)): Remove.
(normalize_mem_ranges): Change parameter type to std::vector.
* memrange.c (compare_mem_ranges): Remove.
(normalize_mem_ranges): Change parameter type to std::vector,
adjust to vector change.
* exec.c (section_table_available_memory): Return vector, remove
parameter.
(section_table_read_available_memory): Adjust to std::vector
change.
* remote.c (remote_read_bytes): Adjust to std::vector
change.
* tracepoint.h (traceframe_available_memory): Change parameter
type to std::vector.
* tracepoint.c (traceframe_available_memory): Change parameter
type to std::vector, adjust.
* gdb/mi/mi-main.c (mi_cmd_trace_frame_collected): Adjust to
std::vector change.
* gdb/Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
unittests/memrange-selftests.c.
(SUBDIR_UNITTESTS_OBS): Add memrange-selftests.o.
* gdb/unittests/memrange-selftests.c: New file.
Since this target method returns an allocated object, return a
unique_ptr. It allows getting rid a some cleanups here and there.
I had to shuffle the includes around. First, target.h now needs to
include tracepoint.h, to get the definition of traceframe_info_up.
However, the definition of enum trace_find_type was later in target, so
I had to move it to tracepoint.h, so that the declaration of tfind_1
could know about it. I then had to remove the include of target.h from
tracepoint.h, which caused a circular dependency (it was probably
included to get enum trace_find_type in the first place anyway).
Regression tested on the buildbot.
gdb/ChangeLog:
* target.h: Include tracepoint.h.
(enum trace_find_type): Move to tracepoint.h.
(struct target_ops) <to_traceframe_info>: Return a unique ptr.
* tracepoint.h: Don't include target.h
(enum trace_find_type): Move from target.h.
(parse_traceframe_info): Return a unique ptr.
* tracepoint.c (current_traceframe_info): Change type to unique
ptr.
(free_traceframe_info): Remove.
(clear_traceframe_info): Don't manually free
current_traceframe_info.
(free_result): Remove.
(parse_traceframe_info): Return a unique ptr.
(get_traceframe_info): Adjust to unique ptr.
* ctf.c (ctf_traceframe_info): Return a unique ptr.
* remote.c (remote_traceframe_info): Return a unique ptr.
* tracefile-tfile.c (tfile_traceframe_info): Return a unique
ptr.
* target-debug.h (target_debug_print_traceframe_info_up): New
macro.
* target-delegates.c: Regenerate.
When debugging two inferiors (or more) against gdbserver, and the
inferiors have different architectures, such as e.g., on x86_64
GNU/Linux and one inferior is 64-bit while the other is 32-bit, then
GDB can get confused with the different architectures in a couple
spots.
In both cases I ran into, GDB incorrectly ended up using the
architecture of whatever happens to be the selected inferior instead
of the architecture of some other given inferior:
#1 - When parsing the expedited registers in stop replies.
#2 - In the default implementation of the target_thread_architecture
target method.
These resulted in instances of the infamous "Remote 'g' packet reply
is too long" error. For example, with the test added in this commit,
we get:
~~~
Continuing.
Remote 'g' packet reply is too long (expected 440 bytes, got 816 bytes): ad064000000000000[snip]
(gdb) FAIL: gdb.multi/multi-arch.exp: inf1 event with inf2 selected: continue to hello_loop
c
Continuing.
Truncated register 50 in remote 'g' packet
(gdb) PASS: gdb.multi/multi-arch.exp: inf2 event with inf1 selected: c
~~~
This commit fixes that.
gdb/ChangeLog:
2017-10-04 Pedro Alves <palves@redhat.com>
* remote.c (get_remote_arch_state): New 'gdbarch' parameter. Use
it instead of target_gdbarch.
(get_remote_state, get_remote_packet_size): Adjust
get_remote_arch_state calls, passing down target_gdbarch
explicitly.
(packet_reg_from_regnum, packet_reg_from_pnum): New parameter
'gdbarch' and use it instead of target_gdbarch.
(get_memory_packet_size): Adjust get_remote_arch_state calls,
passing down target_gdbarch explicitly.
(struct stop_reply) <arch>: New field.
(remote_parse_stop_reply): Use the stopped thread's architecture,
not the current inferior's. Save the architecture in the
stop_reply.
(process_stop_reply): Use the stop reply's architecture.
(process_g_packet, remote_fetch_registers)
(remote_prepare_to_store, store_registers_using_G)
(remote_store_registers): Adjust get_remote_arch_state calls,
using the regcache's architecture.
(remote_get_trace_status): Adjust get_remote_arch_state calls,
passing down target_gdbarch explicitly.
* spu-multiarch.c (spu_thread_architecture): Defer to the target
beneath instead of calling target_gdbarch.
* target.c (default_thread_architecture): Use the specified
inferior's architecture, instead of the current inferior's
architecture (via target_gdbarch).
gdb/testsuite/ChangeLog:
2017-10-04 Pedro Alves <palves@redhat.com>
* gdb.multi/hangout.c: Include <unistd.h>.
(hangout_loop): New function.
(main): Call alarm. Call hangout_loop in a loop.
* gdb.multi/hello.c: Include <unistd.h>.
(hello_loop): New function.
(main): Call alarm. Call hangout_loop in a loop.
* gdb.multi/multi-arch.exp: Test running to a breakpoint one
inferior with the other selected.
This is the "natural" extension necessary for the "set cwd" command
(and the whole "set the inferior's cwd" logic) to work on gdbserver.
The idea here is to have a new remote packet, QSetWorkingDir (name
adopted from LLDB's extension to the RSP, as can be seen at
<https://raw.githubusercontent.com/llvm-mirror/lldb/master/docs/lldb-gdb-remote.txt>),
which sends an hex-encoded string representing the working directory
that the remote inferior will use. There is a slight difference from
the packet proposed by LLDB: GDB's version will accept empty
arguments, meaning that the user wants to clear the previously set
working directory for the inferior (i.e., "set cwd" without arguments
on GDB).
For UNIX-like targets this feature is already implemented on
nat/fork-inferior.c, and all gdbserver has to do is to basically
implement "set_inferior_cwd" and call it whenever such packet arrives.
For other targets, like Windows, it is possible to use the existing
"get_inferior_cwd" function and do the necessary steps to make sure
that the inferior will use the specified working directory.
Aside from that, the patch consists basically of updates to the
testcase (making it available on remote targets) and the
documentation.
No regressions found.
gdb/ChangeLog:
2017-10-04 Sergio Durigan Junior <sergiodj@redhat.com>
* NEWS (Changes since GDB 8.0): Add entry about new
'set-cwd-on-gdbserver' feature.
(New remote packets): Add entry for QSetWorkingDir.
* common/common-inferior.h (set_inferior_cwd): New prototype.
* infcmd.c (set_inferior_cwd): Remove "static".
(show_cwd_command): Expand text to include remote debugging.
* remote.c: Add PACKET_QSetWorkingDir.
(remote_protocol_features) <QSetWorkingDir>: New entry for
PACKET_QSetWorkingDir.
(extended_remote_set_inferior_cwd): New function.
(extended_remote_create_inferior): Call
"extended_remote_set_inferior_cwd".
(_initialize_remote): Call "add_packet_config_cmd" for
QSetWorkingDir.
gdb/gdbserver/ChangeLog:
2017-10-04 Sergio Durigan Junior <sergiodj@redhat.com>
* inferiors.c (set_inferior_cwd): New function.
* server.c (handle_general_set): Handle QSetWorkingDir packet.
(handle_query): Inform that QSetWorkingDir is supported.
* win32-low.c (create_process): Pass the inferior's cwd to
CreateProcess.
gdb/testsuite/ChangeLog:
2017-10-04 Sergio Durigan Junior <sergiodj@redhat.com>
* gdb.base/set-cwd.exp: Make it available on
native-extended-gdbserver.
gdb/doc/ChangeLog:
2017-10-04 Sergio Durigan Junior <sergiodj@redhat.com>
* gdb.texinfo (Starting your Program) <The working directory.>:
Mention remote debugging.
(Working Directory) <Your Program's Working Directory>:
Likewise.
(Connecting) <Remote Packet>: Add "set-working-dir"
and "QSetWorkingDir" to the table.
(Remote Protocol) <QSetWorkingDir>: New item, explaining the
packet.
This finally eliminates an old hack left in place when tracepoint RSP
support was migrated from tracepoint.c to remote.c, back in
35b1e5cca0 ("Make tracepoint operations go through target vector.")
over 7 years ago.
Tested on x86_64 GNU/Linux.
gdb/ChangeLog:
2017-09-29 Pedro Alves <palves@redhat.com>
* remote.c (target_buf, target_buf_size): Delete.
(remote_get_noisy_reply): Remove buf_p and sizeof_buf parameters.
Use the connection's packet buffer instead.
All callers adjusted.
(_initialize_remote): Remove references to target_buf and
target_buf_size.
This patch adds support to remote targets for converting a thread
handle to a thread_info struct pointer.
A thread handle is fetched via a "handle" attribute which has been
added to the qXfer:threads:read query packet. An implementation is
provided in gdbserver for targets using the Linux kernel.
gdb/gdbserver/ChangeLog:
* linux-low.h (struct lwp_info): Add new field, thread_handle.
(thread_db_thread_handle): Declare.
* linux-low.c (linux_target_ops): Initialize thread_handle.
* server.c (handle_qxfer_threads_worker): Add support for
"handle" attribute.
* target.h (struct target_ops): Add new function pointer,
thread_handle.
(target_thread_handle): Define.
* thread-db.c (find_one_thread, attach_thread): Set thread_handle
field in lwp.
(thread_db_thread_handle): New function.
gdb/ChangeLog:
* remote.c (vector): Include.
(struct private_thread_info): Add field, thread_handle.
(free_private_thread_info): Deallocate storage associated with
thread handle.
(get_private_info_thread): Initialize `thread_handle' field.
(struct thread_item): Add field, thread_handle.
(clear_threads_listing_context): Deallocate storage associated
with thread handle.
(start_thread): Add support for "handle" attribute.
(thread_attributes): Add "handle".
(remote_get_threads_with_qthreadinfo): Initialize thread_handle
field.
(remote_update_thread_list): Update thread_handle.
(remote_thread_handle_to_thread_info): New function.
(init_remote_ops): Initialize to_thread_handle_to_thread_info.
... by adding the expected size, and the received size. I found this
useful when debugging gdbarch/remote issues, since it gives a hint of
what gdb expects and what the remote sent.
gdb/ChangeLog:
* remote.c (process_g_packet): Update error message.
This patch implements the ability to set/unset environment variables
on the remote target, mimicking what GDB already offers to the user.
There are two features present here: user-set and user-unset
environment variables.
User-set environment variables are only the variables that are
explicitly set by the user, using the 'set environment' command. This
means that variables that were already present in the environment when
starting GDB/GDBserver are not transmitted/considered by this feature.
User-unset environment variables are variables that are explicitly
unset by the user, using the 'unset environment' command.
The idea behind this patch is to store user-set and user-unset
environment variables in two separate sets, both part of gdb_environ.
Then, when extended_remote_create_inferior is preparing to start the
inferior, it will iterate over the two sets and set/unset variables
accordingly. Three new packets are introduced:
- QEnvironmentHexEncoded, which is used to set environment variables,
and contains an hex-encoded string in the format "VAR=VALUE" (VALUE
can be empty if the user set a variable with a null value, by doing
'set environment VAR=').
- QEnvironmentUnset, which is used to unset environment variables, and
contains an hex-encoded string in the format "VAR".
- QEnvironmentReset, which is always the first packet to be
transmitted, and is used to reset the environment, i.e., discard any
changes made by the user on previous runs.
The QEnvironmentHexEncoded packet is inspired on LLDB's extensions to
the RSP. Details about it can be seen here:
<https://raw.githubusercontent.com/llvm-mirror/lldb/master/docs/lldb-gdb-remote.txt>
I decided not to implement the QEnvironment packet because it is
considered deprecated by LLDB. This packet, on LLDB, serves the same
purpose of QEnvironmentHexEncoded, but sends the information using a
plain text, non-hex-encoded string.
The other two packets are new.
This patch also includes updates to the documentation, testsuite, and
unit tests, without introducing regressions.
gdb/ChangeLog:
2017-08-31 Sergio Durigan Junior <sergiodj@redhat.com>
* NEWS (Changes since GDB 8.0): Add entry mentioning new support
for setting/unsetting environment variables on the remote target.
(New remote packets): Add entries for QEnvironmentHexEncoded,
QEnvironmentUnset and QEnvironmentReset.
* common/environ.c (gdb_environ::operator=): Extend method to
handle m_user_set_env_list and m_user_unset_env_list.
(gdb_environ::clear): Likewise.
(match_var_in_string): Change type of first parameter from 'char
*' to 'const char *'.
(gdb_environ::set): Extend method to handle
m_user_set_env_list and m_user_unset_env_list.
(gdb_environ::unset): Likewise.
(gdb_environ::clear_user_set_env): New method.
(gdb_environ::user_set_envp): Likewise.
(gdb_environ::user_unset_envp): Likewise.
* common/environ.h (gdb_environ): Handle m_user_set_env_list and
m_user_unset_env_list on move constructor/assignment.
(unset): Add new default parameter 'update_unset_list = true'.
(clear_user_set_env): New method.
(user_set_envp): Likewise.
(user_unset_envp): Likewise.
(m_user_set_env_list): New std::set.
(m_user_unset_env_list): Likewise.
* common/rsp-low.c (hex2str): New function.
(bin2hex): New overload for bin2hex function.
* common/rsp-low.c (hex2str): New prototype.
(str2hex): New overload prototype.
* remote.c: Include "environ.h". Add QEnvironmentHexEncoded,
QEnvironmentUnset and QEnvironmentReset.
(remote_protocol_features): Add QEnvironmentHexEncoded,
QEnvironmentUnset and QEnvironmentReset packets.
(send_environment_packet): New function.
(extended_remote_environment_support): Likewise.
(extended_remote_create_inferior): Call
extended_remote_environment_support.
(_initialize_remote): Add QEnvironmentHexEncoded,
QEnvironmentUnset and QEnvironmentReset packet configs.
* unittests/environ-selftests.c (gdb_selftest_env_var):
New variable.
(test_vector_initialization): New function.
(test_init_from_host_environ): Likewise.
(test_reinit_from_host_environ): Likewise.
(test_set_A_unset_B_unset_A_cannot_find_A_can_find_B):
Likewise.
(test_unset_set_empty_vector): Likewise.
(test_vector_clear): Likewise.
(test_std_move): Likewise.
(test_move_constructor):
(test_self_move): Likewise.
(test_set_unset_reset): Likewise.
(run_tests): Rewrite in terms of the functions above.
gdb/gdbserver/ChangeLog:
2017-08-31 Sergio Durigan Junior <sergiodj@redhat.com>
* server.c (handle_general_set): Handle QEnvironmentHexEncoded,
QEnvironmentUnset and QEnvironmentReset packets.
(handle_query): Inform remote that QEnvironmentHexEncoded,
QEnvironmentUnset and QEnvironmentReset are supported.
gdb/doc/ChangeLog:
2017-08-31 Sergio Durigan Junior <sergiodj@redhat.com>
* gdb.texinfo (set environment): Add @anchor. Explain that
environment variables set by the user are sent to GDBserver.
(unset environment): Likewise, but for unsetting variables.
(Connecting) <Remote Packet>: Add "environment-hex-encoded",
"QEnvironmentHexEncoded", "environment-unset", "QEnvironmentUnset",
"environment-reset" and "QEnvironmentReset" to the table.
(Remote Protocol) <QEnvironmentHexEncoded, QEnvironmentUnset,
QEnvironmentReset>: New item, explaining the packet.
gdb/testsuite/ChangeLog:
2017-08-31 Sergio Durigan Junior <sergiodj@redhat.com>
* gdb.base/share-env-with-gdbserver.c: New file.
* gdb.base/share-env-with-gdbserver.exp: Likewise.
Andrzej Kaczmarek