aboutsummaryrefslogtreecommitdiff
path: root/gdbserver
AgeCommit message (Collapse)AuthorFilesLines
2022-04-04gdbserver: report correct status in thread stop race conditionSimon Marchi1-32/+28
The test introduced by the following patch would sometimes fail in this configuration: FAIL: gdb.threads/next-fork-other-thread.exp: fork_func=vfork: target-non-stop=on: non-stop=off: displaced-stepping=auto: i=14: next to for loop The test has multiple threads constantly forking or vforking while the main thread keep doing "next"s. (After writing the commit message, I realized this also fixes a similar failure in gdb.threads/forking-threads-plus-breakpoint.exp with the native-gdbserver and native-extended-gdbserver boards.) As stop_all_threads is called, because the main thread finished its "next", it inevitably happens at some point that we ask the remote target to stop a thread and wait() reports that this thread stopped with a fork or vfork event, instead of the SIGSTOP we sent to try to stop it. While running this test, I attached to GDBserver and stopped at linux-low.cc:3626. We can see that the status pulled from the kernel for 2742805 is indeed a vfork event: (gdb) p/x w $3 = 0x2057f (gdb) p WIFSTOPPED(w) $4 = true (gdb) p WSTOPSIG(w) $5 = 5 (gdb) p/x (w >> 8) & (PTRACE_EVENT_VFORK << 8) $6 = 0x200 However, the statement at line 3626 overrides that: ourstatus->set_stopped (gdb_signal_from_host (WSTOPSIG (w))); OURSTATUS becomes "stopped by a SIGTRAP". The information about the fork or vfork is lost. It's then all downhill from there, stop_all_threads eventually asks for a thread list update. That thread list includes the child of that forgotten fork or vfork, the remote target goes "oh cool, a new process, let's attach to it!", when in fact that vfork child's destiny was to be detached. My reverse-engineered understanding of the code around there is that the if/else between lines 3562 and 3583 (in the original code) makes sure OURSTATUS is always initialized (not "ignore"). Either the details are already in event_child->waitstatus (in the case of fork/vfork, for example), in which case we just copy event_child->waitstatus to ourstatus. Or, if the event is a plain "stopped by a signal" or a syscall event, OURSTATUS is set to "stopped", but without a signal number. Lines 3601 to 3629 (in the original code) serve to fill in that last bit of information. The problem is that when `w` holds the vfork status, the code wrongfully takes this branch, because WSTOPSIG(w) returns SIGTRAP: else if (current_thread->last_resume_kind == resume_stop && WSTOPSIG (w) != SIGSTOP) The intent of this branch is, for example, when we sent SIGSTOP to try to stop a thread, but wait() reports that it stopped with another signal (that it must have received from somewhere else simultaneously), say SIGWINCH. In that case, we want to report the SIGWINCH. But in our fork/vfork case, we don't want to take this branch, as the thread didn't really stop because it received a signal. For the non "stopped by a signal" and non "syscall signal" cases, we would ideally skip over all that snippet that fills in the signal or syscall number. The fix I propose is to move this snipppet of the else branch of the if/else above. In addition to moving the code, the last two "else if" branches: else if (current_thread->last_resume_kind == resume_stop && WSTOPSIG (w) != SIGSTOP) { /* A thread that has been requested to stop by GDB with vCont;t, but, it stopped for other reasons. */ ourstatus->set_stopped (gdb_signal_from_host (WSTOPSIG (w))); } else if (ourstatus->kind () == TARGET_WAITKIND_STOPPED) ourstatus->set_stopped (gdb_signal_from_host (WSTOPSIG (w))); are changed into a single else: else ourstatus->set_stopped (gdb_signal_from_host (WSTOPSIG (w))); This is the default path we take if: - W is not a syscall status - W does not represent a SIGSTOP that have sent to stop the thread and therefore want to suppress it Change-Id: If2dc1f0537a549c293f7fa3c53efd00e3e194e79
2022-04-04Remove some globals from nat/windows-nat.cTom Tromey1-112/+133
nat/windows-nat.c has a number of globals that it uses to communicate with its clients (gdb and gdbserver). However, if we ever want the Windows ports to be multi-inferior, globals won't work. This patch takes a step toward that by moving most nat/windows-nat.c globals into a new struct windows_process_info. Many functions are converted to be methods on this object. A couple of globals remain, as they are needed to truly be global due to the way that the Windows debugging APIs work. The clients still have a global for the current process. That is, this patch is a step toward the end goal, but doesn't implement the goal itself.
2022-03-31gdbserver/linux: set lwp !stopped when failing to resumeSimon Marchi1-2/+18
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
2022-03-30Consolidate definition of current_directoryTom Tromey1-4/+0
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.
2022-03-22nat: Split out platform-independent aarch64 debug register support.John Baldwin2-5/+9
Move non-Linux-specific support for hardware break/watchpoints from nat/aarch64-linux-hw-point.c to nat/aarch64-hw-point.c. Changes beyond a simple split of the code are: - aarch64_linux_region_ok_for_watchpoint and aarch64_linux_any_set_debug_regs_state renamed to drop linux_ as they are not platform specific. - Platforms must implement the aarch64_notify_debug_reg_change function which is invoked from the platform-independent code when a debug register changes for a given debug register state. This does not use the indirection of a 'low' structure as is done for x86. - The handling for kernel_supports_any_contiguous_range is not pristine. For non-Linux it is simply defined to true. Some uses of this could perhaps be implemented as new 'low' routines for the various places that check it instead? - Pass down ptid into aarch64_handle_breakpoint and aarch64_handle_watchpoint rather than using current_lwp_ptid which is only defined on Linux. In addition, pass the ptid on to aarch64_notify_debug_reg_change instead of the unused state argument.
2022-03-21gdbserver: Fixup previous patchPedro Alves1-1/+1
The previous prepare_resume_reply change missed updating the 'buf' reference that overwrites the 'T', so if 'buf' was advanced, we'd still overwrite the wrong character. This fixes it. Change-Id: Ia8ce433366b85af4e268c1c49e7b447da3130a4d
2022-03-21gdbserver: Fix incorrect assertionPedro Alves1-4/+5
While playing with adding a new event kind, I noticed that prepare_resume_reply TARGET_WAITKIND_FORKED, etc. advance 'buf', so if we force-disable the T packet, we'd fail the *buf == 'T' assertion. Fix it by tweaking the assertion to always look at the beginning of the buffer. Change-Id: I8c38e32353db115edcde418b3b1e8ba12343c22b
2022-03-10Re-add zombie leader on exit, gdbserver/linuxPedro Alves1-38/+82
Same as the previous patch, but for GDBserver. In summary, the current zombie leader detection code in linux-low.cc has a race -- if a multi-threaded inferior exits just before check_zombie_leaders finds that the leader is now zombie via checking /proc/PID/status, check_zombie_leaders deletes the leader, assuming we won't get an event for that exit (which we won't in some scenarios, but not in this one), which is a false-positive scenario, where the whole process is simply exiting. Later when we see the last LWP in our list exit, we report that LWP's exit status as exit code, even though for the (real) parent process, the exit code that counts is the child's leader thread's exit code. Like for GDB, the solution here is to: - only report whole-process exit events for the leader. - re-add the leader back to the LWP list when we finally see it exit. Change-Id: Id2d7bbb51a415534e1294fff1d555b7ecaa87f1d
2022-03-10gdbserver: Reindent check_zombie_leadersPedro Alves1-51/+50
This fixes the indentation of linux_process_target::check_zombie_leaders, which will help with keeping its comments in sync with the gdb/linux-nat.c counterpart. Change-Id: I37332343bd80423d934249e3de2d04feefad1891
2022-03-10gdbserver: Reorganize linux_process_target::filter_eventPedro Alves1-36/+40
Reorganize linux-low.cc:linux_process_target::filter_event such that all the handling for events for LWPs not in the LWP list is together. This helps make a following patch clearer. The comments and debug messages have also been tweaked to have them synchronized with the GDB counterpart. Change-Id: If9019635f63a846607cfda44b454b4254a404019
2022-03-10Fix gdbserver/linux target_waitstatus logging assertPedro Alves1-2/+2
Turning on debug output in gdbserver leads to an assertion failure if gdbserver reports a non-signal event: [threads] wait_1: LWP 3273770: extended event with waitstatus status->kind = EXECD, execd_pathname = gdb.threads/non-ldr-exc-1/non-ldr-exc-1 [threads] wait_1: Hit a non-gdbserver trap event. ../../src/gdbserver/../gdb/target/waitstatus.h:365: A problem internal to GDBserver has been detected. sig: Assertion `m_kind == TARGET_WAITKIND_STOPPED || m_kind == TARGET_WAITKIND_SIGNALLED' failed. Fix it in the obvious way, using target_waitstatus::to_string(), resulting in, for example: [threads] wait_1: ret = LWP 1542412.1542412, status->kind = STOPPED, sig = GDB_SIGNAL_TRAP Change-Id: Ia4832f9b4fa39f4af67fcaf21fd4d909a285a645
2022-03-03Avoid conflict with gnulib open/close macros.Roland McGrath1-2/+2
On some systems, the gnulib configuration will decide to define open and/or close as macros to replace the POSIX C functions. This interferes with using those names in C++ class or namespace scopes. gdbsupport/ * event-pipe.cc (event_pipe::open): Renamed to ... (event_pipe::open_pipe): ... this. (event_pipe::close): Renamed to ... (event_pipe::close_pipe): ... this. * event-pipe.h (class event_pipe): Updated. gdb/ * inf-ptrace.h (async_file_open, async_file_close): Updated. gdbserver/ * gdbserver/linux-low.cc (linux_process_target::async): Likewise.
2022-02-22gdbserver linux-low: Convert linux_event_pipe to the event_pipe class.John Baldwin1-32/+11
Use event_pipe from gdbsupport in place of the existing file descriptor array.
2022-02-10gdb/linux: remove ptrace support check for exec, fork, vfork, vforkdone, ↵Simon Marchi1-5/+4
clone, sysgood I think it's safe to remove checking support for these ptrace features, they have all been added in what is now ancient times (around the beginning of Linux 2.6). This allows removing a bit of complexity in linux-nat.c and nat/linux-ptrace.c. It also allows saving one extra fork every time we start debugging on Linux: linux_check_ptrace_features forks a child process to test if some ptrace features are supported. That child process forks a grand-child, to test whether ptrace reports an event for the fork by the child. This is no longer needed, if we assume the kernel supports reporting forks. PTRACE_O_TRACEVFORKDONE was introduced in Linux in this change, in 2003: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=45c1a159b85b3b30afd26a77b4be312226bba416 PTRACE_O_TRACESYSGOOD was supported at least as of this change, in 2002: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=acc7088569c8eef04eeed0eff51d23bb5bcff964 PTRACE_O_TRACEFORK, PTRACE_O_TRACEVFORK, PTRACE_O_TRACEEXEC and PTRACE_O_TRACECLONE were introduced in this change, in 2002: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=a0691b116f6a4473f0fa264210ab9b95771a2b46 Change-Id: Iffb906549a89cc6b619427f976ec044706ab1e8d
2022-01-27gdb, gdbserver: update thread identifier in enable_btrace target methodMarkus Metzger5-10/+11
The enable_btrace target method takes a ptid_t to identify the thread on which tracing shall be enabled. Change this to thread_info * to avoid translating back and forth between the two. This will be used in a subsequent patch.
2022-01-18gdbserver: introduce remote_debug_printfSimon Marchi5-97/+48
Add remote_debug_printf, and use it for all debug messages controlled by remote_debug. Change remote_debug to be a bool, which is trivial in this case. Change-Id: I90de13cb892faec3830047b571661822b126d6e8
2022-01-18gdbserver: introduce threads_debug_printf, THREADS_SCOPED_DEBUG_ENTER_EXITSimon Marchi13-798/+483
Add the threads_debug_printf and THREADS_SCOPED_DEBUG_ENTER_EXIT, which use the logging infrastructure from gdbsupport/common-debug.h. Replace all debug_print uses that are predicated by debug_threads with threads_dethreads_debug_printf. Replace uses of the debug_enter and debug_exit macros with THREADS_SCOPED_DEBUG_ENTER_EXIT, which serves essentially the same purpose, but allows showing what comes between the enter and the exit in an indented form. Note that "threads" debug is currently used for a bit of everything in GDBserver, not only threads related stuff. It should ideally be cleaned up and separated logically as is done in GDB, but that's out of the scope of this patch. Change-Id: I2d4546464462cb4c16f7f1168c5cec5a89f2289a
2022-01-18gdbserver: turn debug_threads into a booleanSimon Marchi5-17/+11
debug_threads is always used as a boolean. Except in ax.cc and tracepoint.cc. These files have their own macros that use debug_threads, and have a concept of verbosity level. But they both have a single level, so it's just a boolean in the end. Remove this concept of level. If we ever want to re-introduce it, I think it will be better implemented in a more common location. Change debug_threads to bool and adjust some users that were treating it as an int. Change-Id: I137f596eaf763a08c977dd74417969cedfee9ecf
2022-01-13gdb: don't use -Wmissing-prototypes with g++Andrew Burgess3-1/+69
This commit aims to not make use of -Wmissing-prototypes when compiling with g++. Use of -Wmissing-prototypes was added with this commit: commit a0761e34f054767de6d6389929d27e9015fb299b Date: Wed Mar 11 15:15:12 2020 -0400 gdb: enable -Wmissing-prototypes warning Because clang can provide helpful warnings with this flag. Unfortunately, g++ doesn't accept this flag, and will give this warning: cc1plus: warning: command line option ‘-Wmissing-prototypes’ is valid for C/ObjC but not for C++ In theory the fact that this flag is not supported should be detected by the configure check in gdbsupport/warning.m4, but for users of ccache, this check doesn't work due to a long standing ccache issue: https://github.com/ccache/ccache/issues/738 The ccache problem is that -W... options are reordered on the command line, and so -Wmissing-prototypes is seen before -Werror. Usually this doesn't matter, but the above warning (about the flag not being valid) is issued before the -Werror flag is processed, and so is not fatal. There have been two previous attempts to fix this that I'm aware of. The first is: https://sourceware.org/pipermail/gdb-patches/2021-September/182148.html In this attempt, instead of just relying on a compile to check if a flag is valid, the proposal was to both compile and link. As linking doesn't go through ccache, we don't suffer from the argument reordering problem, and the link phase will correctly fail when using -Wmissing-prototypes with g++. The configure script will then disable the use of this flag. This approach was rejected, and the suggestion was to only add the -Wmissing-prototypes flag if we are compiling with gcc. The second attempt, attempts this approach, and can be found here: https://sourceware.org/pipermail/gdb-patches/2021-November/183076.html This attempt only adds the -Wmissing-prototypes flag is the value of GCC is not 'yes'. This feels like it is doing the right thing, unfortunately, the GCC flag is really a 'is gcc like' flag, not a strict, is gcc check. As such, GCC is set to 'yes' for clang, which would mean the flag was not included for clang or gcc. The entire point of the original commit was to add this flag for clang, so clearly the second attempt is not sufficient either. In this new attempt I have added gdbsupport/compiler-type.m4, this file defines AM_GDB_COMPILER_TYPE. This macro sets the variable GDB_COMPILER_TYPE to either 'gcc', 'clang', or 'unknown'. In future the list of values might be extended to cover other compilers, if this is ever useful. I've then modified gdbsupport/warning.m4 to only add the problematic -Wmissing-prototypes flag if GDB_COMPILER_TYPE is not 'gcc'. I've tested this with both gcc and clang and see the expected results, gcc no longer attempts to use the -Wmissing-prototypes flag, while clang continues to use it. When compiling using ccache, I am no longer seeing the warning.
2022-01-07Do not use CC_HAS_LONG_LONGTom Tromey1-1/+1
ax.cc checks CC_HAS_LONG_LONG, but nothing defines this. However, PRINTF_HAS_LONG_LONG is checked in configure. This patch removes the former and keeps the latter. This is PR remote/14976 (filed by me in 2012, lol). I'm checking this in. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=14976
2022-01-01Automatic Copyright Year update after running gdb/copyright.pyJoel Brobecker88-88/+88
This commit brings all the changes made by running gdb/copyright.py as per GDB's Start of New Year Procedure. For the avoidance of doubt, all changes in this commits were performed by the script.
2022-01-01Update Copyright Year in gdb, gdbserver and gdbreplay version outputJoel Brobecker2-2/+2
This commit changes the copyright year printed by gdb, gdbserver and gdbreplay when printing the tool's version.
2021-12-15New --enable-threading configure option to control use of threads in ↵Luis Machado1-1/+23
GDB/GDBserver Add the --enable-threading configure option so multithreading can be disabled at configure time. This is useful for statically-linked builds of GDB/GDBserver, since the thread library doesn't play well with that setup. If you try to run a statically-linked GDB built with threading, it will crash when setting up the number of worker threads. This new option is also convenient when debugging GDB in a system with lots of threads, where the thread discovery code in GDB will emit too many messages, like so: [New Thread 0xfffff74d3a50 (LWP 2625599)] If you have X threads, that message will be repeated X times. The default for --enable-threading is "yes".
2021-12-14gdbserver/tracepoint.cc: use snprintf in gdb_agent_socket_initSimon Marchi1-2/+2
If we modify tracepoint.cc to try to use a too long unix socket name, for example by modifying SOCK_DIR to be: #define SOCK_DIR "/tmp/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut/salut" ... trying to start an application with libinproctrace.so loaded crashes: $ LD_PRELOAD=/usr/lib/x86_64-linux-gnu/libasan.so.6:./libinproctrace.so /bin/ls /home/smarchi/src/binutils-gdb/gdbserver/../gdbsupport/common-utils.cc:69: A problem internal to GDBserver in-process agent has been detected. xsnprintf: Assertion `ret < size' failed. Looking at the rest of the socket initialization code, the intent seems to be that if something goes wrong, we warn but let the program execute. So crashing on this failed assertions seems against the intent. Commit 6cebaf6e1ae4 ("use xsnprintf instead of snprintf.") changed this code to use xsnprintf instead of snprintf, introducing this assertion. Before that, snprintf would return a value bigger that UNIX_PATH_MAX and the "if" after would catch it and emit a warning, which is exactly what we want. That change was done because LynxOS didn't have snprintf. Since LynxOS isn't supported anymore, we can simply revert to use snprintf there. With this patch, we get a warning (printed by the caller of gdb_agent_socket_init), but the program keeps executing: $ LD_PRELOAD=/usr/lib/x86_64-linux-gnu/libasan.so.6:./libinproctrace.so /bin/ls ipa: could not create sync socket ... Change-Id: I78bca52d5dc3145335abeae45a42052701e3f5dd
2021-12-14gdbserver/tracepoint.cc: work around -Wstringop-truncation errorSimon Marchi1-2/+7
When building gdb with on AArch64 with g++ 11.1.0 (and some preceding versions too), -O2 and -fsanitize=address, I get this error. CXX tracepoint-ipa.o cc1plus: warning: command-line option ‘-Wmissing-prototypes’ is valid for C/ObjC but not for C++ In file included from /usr/include/string.h:519, from ../gnulib/import/string.h:41, from /home/simark/src/binutils-gdb/gdbserver/../gdbsupport/common-defs.h:95, from /home/simark/src/binutils-gdb/gdbserver/server.h:22, from /home/simark/src/binutils-gdb/gdbserver/tracepoint.cc:19: In function ‘char* strncpy(char*, const char*, size_t)’, inlined from ‘int init_named_socket(const char*)’ at /home/simark/src/binutils-gdb/gdbserver/tracepoint.cc:6902:11, inlined from ‘int gdb_agent_socket_init()’ at /home/simark/src/binutils-gdb/gdbserver/tracepoint.cc:6953:26, inlined from ‘void* gdb_agent_helper_thread(void*)’ at /home/simark/src/binutils-gdb/gdbserver/tracepoint.cc:7204:41: /usr/include/bits/string_fortified.h:95:34: error: ‘char* __builtin_strncpy(char*, const char*, long unsigned int)’ output may be truncated copying 107 bytes from a string of length 107 [-Werror=stringop-truncation] 95 | return __builtin___strncpy_chk (__dest, __src, __len, | ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~ 96 | __glibc_objsize (__dest)); | ~~~~~~~~~~~~~~~~~~~~~~~~~ Note that _FORTIFY_SOURCE changes the message a bit, but I get a similar error if I use -D_FORTIFY_SOURCE=0. I am pretty sure it's spurious and might be related to this GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88780 From what I can see, we are copying from a static 108 bytes long buffer (the global array agent_socket_name) to a 108 bytes long array, sockaddr_un::sun_path. I don't see anything wrong. Still, it would make things easier if we didn't see this error. Change the code to check that the source string length is smaller than the destination buffer (including space for the NULL byte) and use strcpy. For anybody who would like to try to reproduce, the full command line is: g++ -I. -I/home/simark/src/binutils-gdb/gdbserver -I/home/simark/src/binutils-gdb/gdbserver/../gdb/regformats -I/home/simark/src/binutils-gdb/gdbserver/.. -I/home/simark/src/binutils-gdb/gdbserver/../include -I/home/simark/src/binutils-gdb/gdbserver/../gdb -I/home/simark/src/binutils-gdb/gdbserver/../gnulib/import -I../gnulib/import -I/home/simark/src/binutils-gdb/gdbserver/.. -I.. -pthread -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-variable -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-error=maybe-uninitialized -Wno-mismatched-tags -Wsuggest-override -Wimplicit-fallthrough=3 -Wduplicated-cond -Wshadow=local -Wdeprecated-copy -Wdeprecated-copy-dtor -Wredundant-move -Wmissing-declarations -Wmissing-prototypes -Wstrict-null-sentinel -Wformat -Wformat-nonliteral -Werror -DGDBSERVER -DCONFIG_UST_GDB_INTEGRATION -Drpl_strerror_r=strerror_r -Drpl_free=free -fPIC -DIN_PROCESS_AGENT -fvisibility=hidden -g3 -O2 -fsanitize=address -c -o tracepoint-ipa.o -MT tracepoint-ipa.o -MMD -MP -MF ./.deps/tracepoint-ipa.Tpo /home/simark/src/binutils-gdb/gdbserver/tracepoint.cc Change-Id: I18e86c0487feead7e7677e69398405e7057cf464
2021-12-13gdbserver/linux-low: replace direct assignment to current_threadTankut Baris Aktemur1-2/+5
Use scoped_restore_current_thread and switch_to_thread in linux_process_target::wait_for_sigstop.
2021-12-13gdbserver: replace direct assignments to current_threadTankut Baris Aktemur13-136/+77
Replace the direct assignments to current_thread with switch_to_thread. Use scoped_restore_current_thread when appropriate. There is one instance remaining in linux-low.cc's wait_for_sigstop. This will be handled in a separate patch. Regression-tested on X86-64 Linux using the native-gdbserver and native-extended-gdbserver board files.
2021-12-13gdbserver: introduce scoped_restore_current_thread and switch_to_threadTankut Baris Aktemur2-0/+43
Introduce a class for restoring the current thread and a function to switch to the given thread. This is a preparation for a refactoring that aims to remove direct assignments to 'current_thread'.
2021-12-08gdb, gdbserver: detach fork child when detaching from fork parentSimon Marchi5-0/+83
While working with pending fork events, I wondered what would happen if the user detached an inferior while a thread of that inferior had a pending fork event. What happens with the fork child, which is ptrace-attached by the GDB process (or by GDBserver), but not known to the core? Sure enough, neither the core of GDB or the target detach the child process, so GDB (or GDBserver) just stays ptrace-attached to the process. The result is that the fork child process is stuck, while you would expect it to be detached and run. Make GDBserver detach of fork children it knows about. That is done in the generic handle_detach function. Since a process_info already exists for the child, we can simply call detach_inferior on it. GDB-side, make the linux-nat and remote targets detach of fork children known because of pending fork events. These pending fork events can be stored in: - thread_info::pending_waitstatus, if the core has consumed the event but then saved it for later (for example, because it got the event while stopping all threads, to present an all-stop stop on top of a non-stop target) - thread_info::pending_follow: if we ran to a "catch fork" and we detach at that moment Additionally, pending fork events can be in target-specific fields: - For linux-nat, they can be in lwp_info::status and lwp_info::waitstatus. - For the remote target, they could be stored as pending stop replies, saved in `remote_state::notif_state::pending_event`, if not acknowledged yet, or in `remote_state::stop_reply_queue`, if acknowledged. I followed the model of remove_new_fork_children for this: call remote_notif_get_pending_events to process / acknowledge any unacknowledged notification, then look through stop_reply_queue. Update the gdb.threads/pending-fork-event.exp test (and rename it to gdb.threads/pending-fork-event-detach.exp) to try to detach the process while it is stopped with a pending fork event. In order to verify that the fork child process is correctly detached and resumes execution outside of GDB's control, make that process create a file in the test output directory, and make the test wait $timeout seconds for that file to appear (it happens instantly if everything goes well). This test catches a bug in linux-nat.c, also reported as PR 28512 ("waitstatus.h:300: internal-error: gdb_signal target_waitstatus::sig() const: Assertion `m_kind == TARGET_WAITKIND_STOPPED || m_kind == TARGET_WAITKIND_SIGNALLED' failed.). When detaching a thread with a pending event, get_detach_signal unconditionally fetches the signal stored in the waitstatus (`tp->pending_waitstatus ().sig ()`). However, that is only valid if the pending event is of type TARGET_WAITKIND_STOPPED, and this is now enforced using assertions (iit would also be valid for TARGET_WAITKIND_SIGNALLED, but that would mean the thread does not exist anymore, so we wouldn't be detaching it). Add a condition in get_detach_signal to access the signal number only if the wait status is of kind TARGET_WAITKIND_STOPPED, and use GDB_SIGNAL_0 instead (since the thread was not stopped with a signal to begin with). Add another test, gdb.threads/pending-fork-event-ns.exp, specifically to verify that we consider events in pending stop replies in the remote target. This test has many threads constantly forking, and we detach from the program while the program is executing. That gives us some chance that we detach while a fork stop reply is stored in the remote target. To verify that we correctly detach all fork children, we ask the parent to exit by sending it a SIGUSR1 signal and have it write a file to the filesystem before exiting. Because the parent's main thread joins the forking threads, and the forking threads wait for their fork children to exit, if some fork child is not detach by GDB, the parent will not write the file, and the test will time out. If I remove the new remote_detach_pid calls in remote.c, the test fails eventually if I run it in a loop. There is a known limitation: we don't remove breakpoints from the children before detaching it. So the children, could hit a trap instruction after being detached and crash. I know this is wrong, and it should be fixed, but I would like to handle that later. The current patch doesn't fix everything, but it's a step in the right direction. Change-Id: I6d811a56f520e3cb92d5ea563ad38976f92e93dd Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28512
2021-12-08gdbserver: hide fork child threads from GDBSimon Marchi5-0/+62
This patch aims at fixing a bug where an inferior is unexpectedly created when a fork happens at the same time as another event, and that other event is reported to GDB first (and the fork event stays pending in GDBserver). This happens for example when we step a thread and another thread forks at the same time. The bug looks like (if I reproduce the included test by hand): (gdb) show detach-on-fork Whether gdb will detach the child of a fork is on. (gdb) show follow-fork-mode Debugger response to a program call of fork or vfork is "parent". (gdb) si [New inferior 2] Reading /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-while-fork-in-other-thread/step-while-fork-in-other-thread from remote target... Reading /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-while-fork-in-other-thread/step-while-fork-in-other-thread from remote target... Reading symbols from target:/home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-while-fork-in-other-thread/step-while-fork-in-other-thread... [New Thread 965190.965190] [Switching to Thread 965190.965190] Remote 'g' packet reply is too long (expected 560 bytes, got 816 bytes): ... <long series of bytes> The sequence of events leading to the problem is: - We are using the all-stop user-visible mode as well as the synchronous / all-stop variant of the remote protocol - We have two threads, thread A that we single-step and thread B that calls fork at the same time - GDBserver's linux_process_target::wait pulls the "single step complete SIGTRAP" and the "fork" events from the kernel. It arbitrarily choses one event to report, it happens to be the single-step SIGTRAP. The fork stays pending in the thread_info. - GDBserver send that SIGTRAP as a stop reply to GDB - While in stop_all_threads, GDB calls update_thread_list, which ends up querying the remote thread list using qXfer:threads:read. - In the reply, GDBserver includes the fork child created as a result of thread B's fork. - GDB-side, the remote target sees the new PID, calls remote_notice_new_inferior, which ends up unexpectedly creating a new inferior, and things go downhill from there. The problem here is that as long as GDB did not process the fork event, it should pretend the fork child does not exist. Ultimately, this event will be reported, we'll go through follow_fork, and that process will be detached. The remote target (GDB-side), has some code to remove from the reported thread list the threads that are the result of forks not processed by GDB yet. But that only works for fork events that have made their way to the remote target (GDB-side), but haven't been consumed by the core yet, so are still lingering as pending stop replies in the remote target (see remove_new_fork_children in remote.c). But in our case, the fork event hasn't made its way to the GDB-side remote target. We need to implement the same kind of logic GDBserver-side: if there exists a thread / inferior that is the result of a fork event GDBserver hasn't reported yet, it should exclude that thread / inferior from the reported thread list. This was actually discussed a while ago, but not implemented AFAIK: https://pi.simark.ca/gdb-patches/1ad9f5a8-d00e-9a26-b0c9-3f4066af5142@redhat.com/#t https://sourceware.org/pipermail/gdb-patches/2016-June/133906.html Implementation details-wise, the fix for this is all in GDBserver. The Linux layer of GDBserver already tracks unreported fork parent / child relationships using the lwp_info::fork_relative, in order to avoid wildcard actions resuming fork childs unknown to GDB. This information needs to be made available to the handle_qxfer_threads_worker function, so it can filter the reported threads. Add a new thread_pending_parent target function that allows the Linux target to return the parent of an eventual fork child. Testing-wise, the test replicates pretty-much the sequence of events shown above. The setup of the test makes it such that the main thread is about to fork. We stepi the other thread, so that the step completes very quickly, in a single event. Meanwhile, the main thread is resumed, so very likely has time to call fork. This means that the bug may not reproduce every time (if the main thread does not have time to call fork), but it will reproduce more often than not. The test fails without the fix applied on the native-gdbserver and native-extended-gdbserver boards. At some point I suspected that which thread called fork and which thread did the step influenced the order in which the events were reported, and therefore the reproducibility of the bug. So I made the test try both combinations: main thread forks while other thread steps, and vice versa. I'm not sure this is still necessary, but I left it there anyway. It doesn't hurt to test a few more combinations. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28288 Change-Id: I2158d5732fc7d7ca06b0eb01f88cf27bf527b990
2021-12-02[gdb/tdep] Fix avx512 -m32 support in gdbserverTom de Vries1-13/+37
PR27257 reports a problem that can be reproduced as follows: - use x86_64 machine with avx512 support - compile a hello world with -m32 to a.out - start a gdbserver session with a.out - use gdb to connect to the gdbserver session This makes us run into: ... Listening on port 2346 Remote debugging from host ::1, port 34940 src/gdbserver/regcache.cc:257: \ A problem internal to GDBserver has been detected. Unknown register zmm16h requested ... The problem is that i387_xsave_to_cache in gdbserver/i387-fp.cc can't find a register zmm16h in the register cache. To understand how this happens, first some background. SSE has 16 128-bit wide xmm registers. AVX extends the SSE registers set as follows: - it extends the 16 existing 128-bit wide xmm registers to 256-bit wide ymm registers. AVX512 extends the AVX register set as follows: - it extends the 16 existing 256-bit wide ymm registers to 512-bit wide zmm registers. - it adds 16 additional 512-bit wide zmm registers (with corresponding ymm and xmm subregisters added as well) However, in 32-bit mode, there are only 8 xmm/ymm/zmm registers. The problem we're running into is that gdbserver/i387-fp.cc uses these constants to describe the size of the register file: ... static const int num_avx512_zmmh_low_registers = 16; static const int num_avx512_zmmh_high_registers = 16; static const int num_avx512_ymmh_registers = 16; static const int num_avx512_xmm_registers = 16; ... which are all incorrect for the 32-bit case. Fix this by replacing the constants with variables that have the appropriate values in 64-bit and 32-bit mode. Tested on x86_64-linux with native and unix/-m32.
2021-11-22gdb: pass more const target_waitstatus by referenceSimon Marchi3-37/+36
While working on target_waitstatus changes, I noticed a few places where const target_waitstatus objects could be passed by reference instead of by pointers. And in some cases, places where a target_waitstatus could be passed as const, but was not. Convert them as much as possible. Change-Id: Ied552d464be5d5b87489913b95f9720a5ad50c5a
2021-11-22gdb: rename target_waitstatus_to_string to target_waitstatus::to_stringSimon Marchi2-15/+6
Make target_waitstatus_to_string a "to_string" method of target_waitstatus, a bit like we have ptid_t::to_string already. This will save a bit of typing. Change-Id: Id261b7a09fa9fa3c738abac131c191a6f9c13905
2021-11-16Remove config.cache in gdbserver's "distclean"Tom Tromey1-2/+1
PR gdb/28586 points out that "make distclean" fails to delete config.cache from gdbserver/. This patch fixes the bug, and removes a duplicate "Makefile" deletion that was also pointed out in the PR.
2021-11-09Fix build on rhES5Tom Tromey2-0/+20
The rhES5 build failed due to an upstream import a while back. The bug here is that, while the 'personality' function exists, ADDR_NO_RANDOMIZE is only defined in <linux/personality.h>, not <sys/personality.h>. However, <linux/personality.h> does not declare the 'personality' function, and <sys/personality.h> and <linux/personality.h> cannot both be included. This patch restores one of the removed configure checks and updates the code to check it. We had this as a local patch at AdaCore, because it seemed like there was no interest upstream. However, now it turns out that this fixes PR build/28555, so I'm sending it now.
2021-11-04gdbserver: re-generate configureSimon Marchi2-0/+49
I get some diffs when running autoconf in gdbserver, probably leftovers from commit 5dfe4bfcb969 ("Fix format_pieces selftest on Windows"). Re-generate configure in that directory. Change-Id: Icdc9906af95fbaf1047a579914b2983f8ec5db08
2021-11-03[AArch64] Make gdbserver register set selection dynamicLuis Machado1-85/+101
The current register set selection mechanism for AArch64 is static, based on a pre-populated array of register sets. This means that we might potentially probe register sets that are not available. This is OK if the kernel errors out during ptrace, but probing the tag_ctl register, for example, does not result in a ptrace error if the kernel supports the tagged address ABI but not MTE (PR 28355). Making the register set selection dynamic, based on feature checks, solves this and simplifies the code a bit. It allows us to list all of the register sets only once, and pick and choose based on HWCAP/HWCAP2 or other properties. I plan to backport this fix to GDB 11 as well. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28355
2021-10-29gdb: or1k: implement gdb serverStafford Horne3-0/+275
This patch adds gdbserver support for OpenRISC. This has been used for debugging the glibc port that in being worked on here: https://github.com/openrisc/or1k-glibc/tree/or1k-port-2 Hence the comment about registers definitions being inline with glibc.
2021-10-25gdbserver: make target_pid_to_str return std::stringSimon Marchi6-51/+50
I wanted to write a warning that included two target_pid_to_str calls, like this: warning (_("Blabla %s, blabla %s"), target_pid_to_str (ptid1), target_pid_to_str (ptid2)); This doesn't work, because target_pid_to_str stores its result in a static buffer, so my message would show twice the same ptid. Change target_pid_to_str to return an std::string to avoid this. I don't think we save much by using a static buffer, but it is more error-prone. Change-Id: Ie3f649627686b84930529cc5c7c691ccf5d36dc2
2021-10-22Fix 'uninstall' targetTom Tromey1-1/+4
This adds some missing code to the 'uninstall' targets in gdb and gdbserver. It also changes gdb's uninstall target so that it no longer tries to remove any man page -- this is already done (and more correctly) by doc/Makefile.in. I tested this with 'make install' followed by 'make uninstall', then examining the install tree for regular files. Only the 'dir' file remains, but this appears to just be how 'install-info' is intended to work.
2021-10-22Remove unused variables from gdbserver's MakefileTom Tromey1-26/+1
This removes a number of unused variables from gdbserver's Makefile. I found these while working on the subsequent patches, and figured it would be cleaner to have a separate patch for the deletions.
2021-10-21gdb, gdbserver: make target_waitstatus safeSimon Marchi8-208/+171
I stumbled on a bug caused by the fact that a code path read target_waitstatus::value::sig (expecting it to contain a gdb_signal value) while target_waitstatus::kind was TARGET_WAITKIND_FORKED. This meant that the active union field was in fact target_waitstatus::value::related_pid, and contained a ptid. The read signal value was therefore garbage, and that caused GDB to crash soon after. Or, since that GDB was built with ubsan, this nice error message: /home/simark/src/binutils-gdb/gdb/linux-nat.c:1271:12: runtime error: load of value 2686365, which is not a valid value for type 'gdb_signal' Despite being a large-ish change, I think it would be nice to make target_waitstatus safe against that kind of bug. As already done elsewhere (e.g. dynamic_prop), validate that the type of value read from the union matches what is supposed to be the active field. - Make the kind and value of target_waitstatus private. - Make the kind initialized to TARGET_WAITKIND_IGNORE on target_waitstatus construction. This is what most users appear to do explicitly. - Add setters, one for each kind. Each setter takes as a parameter the data associated to that kind, if any. This makes it impossible to forget to attach the associated data. - Add getters, one for each associated data type. Each getter validates that the data type fetched by the user matches the wait status kind. - Change "integer" to "exit_status", "related_pid" to "child_ptid", just because that's more precise terminology. - Fix all users. That last point is semi-mechanical. There are a lot of obvious changes, but some less obvious ones. For example, it's not possible to set the kind at some point and the associated data later, as some users did. But in any case, the intent of the code should not change in this patch. This was tested on x86-64 Linux (unix, native-gdbserver and native-extended-gdbserver boards). It was built-tested on x86-64 FreeBSD, NetBSD, MinGW and macOS. The rest of the changes to native files was done as a best effort. If I forgot any place to update in these files, it should be easy to fix (unless the change happens to reveal an actual bug). Change-Id: I0ae967df1ff6e28de78abbe3ac9b4b2ff4ad03b7
2021-10-21gdbserver: initialize the members of lwp_info in-classSimon Marchi2-29/+31
Add a constructor to initialize the waitstatus members. Initialize the others in the class directly. Change-Id: I10f885eb33adfae86e3c97b1e135335b540d7442
2021-10-21gdbserver: make thread_info non-PODSimon Marchi2-14/+18
Add a constructor and a destructor. The constructor takes care of the initialization that happened in add_thread, while the destructor takes care of the freeing that happened in free_one_thread. This is needed to make target_waitstatus non-POD, as thread_info contains a member of that type. Change-Id: I1db321b4de9dd233ede0d5c101950f1d6f1d13b7
2021-10-04[gdb/build] Add CXX_DIALECT to CXXTom de Vries1-0/+8
Say we use a gcc version that (while supporting c++11) does not support c++11 by default, and needs an -std setting to enable it. If gdb would use the default AX_CXX_COMPILE_STDCXX from autoconf-archive, then we'd have: ... CXX="g++ -std=gnu++11" ... That mechanism however has the following problem (quoting from commit 0bcda685399): ... the top level Makefile passes CXX down to subdirs, and that overrides whatever gdb/Makefile may set CXX to. The result would be that a make invocation from the build/gdb/ directory would use "g++ -std=gnu++11" as expected, while a make invocation at the top level would not. ... Commit 0bcda685399 fixes this by using a custom AX_CXX_COMPILE_STDCXX which does: ... CXX=g++ CXX_DIALECT=-std=gnu++11 ... The problem reported in PR28318 is that using the custom instead of the default AX_CXX_COMPILE_STDCXX makes the configure test for std::thread support fail. We could simply add $CXX_DIALECT to the test for std::thread support, but that would have to be repeated for each added c++ support test. Instead, fix this by doing: ... CXX="g++ -std=gnu++11" CXX_DIALECT=-std=gnu++11 ... This is somewhat awkward, since it results in -std=gnu++11 occuring twice in some situations: ... $ touch src/gdb/dwarf2/read.c $ ( cd build/gdb; make V=1 dwarf2/read.o ) g++-4.8 -std=gnu++11 -x c++ -std=gnu++11 ... ... However, both settings are needed: - the switch in CXX for the std::thread tests (and other tests) - the switch in CXX_DIALECT so it can be appended in Makefiles, to counteract the fact that the top-level Makefile overrides CXX The code added in gdb/ax_cxx_compile_stdcxx.m4 is copied from the default AX_CXX_COMPILE_STDCXX from autoconf-archive. Tested on x86_64-linux. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28318
2021-09-23Change ptid_t::tid to ULONGESTTom Tromey1-2/+2
The ptid_t 'tid' member is normally used as an address in gdb -- both bsd-uthread and ravenscar-thread use it this way. However, because the type is 'long', this can cause problems with sign extension. This patch changes the type to ULONGEST to ensure that sign extension does not occur.
2021-09-23Remove defaulted 'tid' parameter to ptid_t constructorTom Tromey3-9/+9
I wanted to find, and potentially modify, all the spots where the 'tid' parameter to the ptid_t constructor was used. So, I temporarily removed this parameter and then rebuilt. In order to make it simpler to search through the "real" (nonzero) uses of this parameter, something I knew I'd have to do multiple times, I removed any ", 0" from constructor calls. Co-Authored-By: John Baldwin <jhb@FreeBSD.org>
2021-09-07Remove unused declaration from gdbserver/win32-low.hTom Tromey1-3/+0
I noticed that gdbserver/win32-low.h has an unused declaration. This code was changed a while ago, but this declaration slipped through. This patch removes it. Tested by rebuilding.
2021-08-17gdbserver: Check r_version < 1 for Linux debugger interfaceH.J. Lu1-1/+1
Update gdbserver to check r_version < 1 instead of r_version != 1 so that r_version can be bumped for a new field in the glibc debugger interface to support multiple namespaces. Since so far, the gdbserver only reads fields defined for r_version == 1, it is compatible with r_version >= 1. All future glibc debugger interface changes will be backward compatible. If there is ever the need for backward incompatible change to the glibc debugger interface, a new DT_XXX element will be provided to access the new incompatible interface. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=11839
2021-07-26Fix the Windows buildTom Tromey1-3/+3
The gdb build was broken on Windows after the patch to change get_inferior_cwd. This patch fixes the build.