416 Commits

Author SHA1 Message Date
Pedro Alves
7f8acedeeb gdbserver: track current process as well as current thread
The recent commit 421490af33bf ("gdbserver/linux: Access memory even
if threads are running") caused a regression in
gdb.threads/access-mem-running-thread-exit.exp with gdbserver, which I
somehow missed.  Like so:

 (gdb) print global_var
 Cannot access memory at address 0x555555558010
 (gdb) FAIL: gdb.threads/access-mem-running-thread-exit.exp: non-stop: access mem (print global_var after writing, inf=2, iter=1)

The problem starts with GDB telling GDBserver to select a thread, via
the Hg packet, which GDBserver complies with, then that thread exits,
and GDB, without knowing the thread is gone, tries to write to memory,
through the context of the previously selected Hg thread.

GDBserver's GDB-facing memory access routines, gdb_read_memory and
gdb_write_memory, call set_desired_thread to make GDBserver re-select
the thread that GDB has selected with the Hg packet.  Since the thread
is gone, set_desired_thread returns false, and the memory access
fails.

Now, to access memory, it doesn't really matter which thread is
selected.  All we should need is the target process.  Even if the
thread that GDB previously selected is gone, and GDB does not yet know
about that exit, it shouldn't matter, GDBserver should still know
which process that thread belonged to.

Fix this by making GDBserver track the current process separately,
like GDB also does.  Add a new set_desired_process routine that is
similar to set_desired_thread, but just sets the current process,
leaving the current thread as NULL.  Use it in the GDB-facing memory
read and write routines, to avoid failing if the selected thread is
gone, but the process is still around.

Change-Id: I4ff97cb6f42558efbed224b30d5c71f6112d44cd
2022-05-03 15:10:22 +01:00
Tom Tromey
fc0b8a976d Fix libinproctrace.so build on PPC
The recent gnulib import caused a build failure of libinproctrace.so
on PPC:

    alloc.c:(.text+0x20): undefined reference to `rpl_malloc'
    alloc.c:(.text+0x70): undefined reference to `rpl_realloc'

This patch fixes the problem using the same workaround that was
previously used for free.
2022-04-28 12:47:11 -06:00
Tom Tromey
801eb70f9a Fix gdbserver build for x86-64 Windows
I broke the gdbserver build on x86-64 Windows a little while back.
Previously, I could not build this configuration, but today I found
out that if I configure with:

    --host=x86_64-w64-mingw32 --target=x86_64-w64-mingw32

using the Fedora 34 tools, it will in fact build.  I'm not certain,
but maybe the gnulib update helped with this.

This patch fixes the build.  I'm checking it in.
2022-04-27 10:45:10 -06:00
Simon Marchi
f551c8ef32 gdbserver/linux: free process_info_private and arch_process_info when failing to attach
Running

  $ ../gdbserver/gdbserver --once --attach :1234 539436

with ASan while /proc/sys/kernel/yama/ptrace_scope is set to 1 (prevents
attaching) shows that we fail to free some platform-specific objects
tied to the process_info (process_info_private and arch_process_info):

    Direct leak of 32 byte(s) in 1 object(s) allocated from:
        #0 0x7f6b558b3fb9 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
        #1 0x562eaf15d04a in xcalloc /home/simark/src/binutils-gdb/gdbserver/../gdb/alloc.c:100
        #2 0x562eaf251548 in xcnew<process_info_private> /home/simark/src/binutils-gdb/gdbserver/../gdbsupport/poison.h:122
        #3 0x562eaf22810c in linux_process_target::add_linux_process_no_mem_file(int, int) /home/simark/src/binutils-gdb/gdbserver/linux-low.cc:426
        #4 0x562eaf22d33f in linux_process_target::attach(unsigned long) /home/simark/src/binutils-gdb/gdbserver/linux-low.cc:1132
        #5 0x562eaf1a7222 in attach_inferior /home/simark/src/binutils-gdb/gdbserver/server.cc:308
        #6 0x562eaf1c1016 in captured_main /home/simark/src/binutils-gdb/gdbserver/server.cc:3949
        #7 0x562eaf1c1d60 in main /home/simark/src/binutils-gdb/gdbserver/server.cc:4084
        #8 0x7f6b552f630f in __libc_start_call_main (/usr/lib/libc.so.6+0x2d30f)

    Indirect leak of 56 byte(s) in 1 object(s) allocated from:
        #0 0x7f6b558b3fb9 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
        #1 0x562eaf15d04a in xcalloc /home/simark/src/binutils-gdb/gdbserver/../gdb/alloc.c:100
        #2 0x562eaf2a0d79 in xcnew<arch_process_info> /home/simark/src/binutils-gdb/gdbserver/../gdbsupport/poison.h:122
        #3 0x562eaf295e2c in x86_target::low_new_process() /home/simark/src/binutils-gdb/gdbserver/linux-x86-low.cc:723
        #4 0x562eaf22819b in linux_process_target::add_linux_process_no_mem_file(int, int) /home/simark/src/binutils-gdb/gdbserver/linux-low.cc:428
        #5 0x562eaf22d33f in linux_process_target::attach(unsigned long) /home/simark/src/binutils-gdb/gdbserver/linux-low.cc:1132
        #6 0x562eaf1a7222 in attach_inferior /home/simark/src/binutils-gdb/gdbserver/server.cc:308
        #7 0x562eaf1c1016 in captured_main /home/simark/src/binutils-gdb/gdbserver/server.cc:3949
        #8 0x562eaf1c1d60 in main /home/simark/src/binutils-gdb/gdbserver/server.cc:4084
        #9 0x7f6b552f630f in __libc_start_call_main (/usr/lib/libc.so.6+0x2d30f)

Those objects are deleted by linux_process_target::mourn, but that is
not called if we fail to attach, we only call remove_process.  I
initially fixed this by making linux_process_target::attach call
linux_process_target::mourn on failure (before calling error).  But this
isn't done anywhere else (including in GDB) so it would just be
confusing to do things differently here.

Instead, add a linux_process_target::remove_linux_process helper method
(which calls remove_process), and call that instead of remove_process in
the Linux target.  Move the free-ing of the extra data from the mourn
method to that new method.

Change-Id: I277059a69d5f08087a7f3ef0b8f1792a1fcf7a85
2022-04-22 14:04:36 -04:00
Simon Marchi
7ab2607f97 gdbsupport: make gdb_abspath return an std::string
I'm trying to switch these functions to use std::string instead of char
arrays, as much as possible.  Some callers benefit from it (can avoid
doing a copy of the result), while others suffer (have to make one more
copy).

Change-Id: Iced49b8ee2f189744c5072a3b217aab5af17a993
2022-04-18 15:48:03 -04:00
Pedro Alves
8e347faf8f gdbserver: Eliminate prepare_to_access_memory
Given:

 - The prepare_to_access_memory machinery was added for non-stop mode.

 - Only Linux supports non-stop.

 - Linux no longer needs the prepare_to_access_memory machinery.  In
   fact, after the previous patch,
   linux_process_target::prepare_to_access_memory became a nop.

Thus, prepare_to_access_memory can go away, simplifying core GDBserver
code.

Change-Id: I93ac8bfe66bd61c3d1c4a0e7d419335163120ecf
2022-04-14 20:22:56 +01:00
Pedro Alves
421490af33 gdbserver/linux: Access memory even if threads are running
Similarly to how the native Linux target was changed
and subsequently reworked in these commits:

 05c06f318fd9 Linux: Access memory even if threads are running
 8a89ddbda2ec Avoid /proc/pid/mem races (PR 28065)

... teach GDBserver to access memory even when the current thread is
running, by always accessing memory via /proc/PID/mem.

The existing comment:

  /* Neither ptrace nor /proc/PID/mem allow accessing memory through a
     running LWP.  */

... is incorrect for /proc/PID/mem does allow that.

Actually, from GDB's perspective, GDBserver could already access
memory while threads were running, but at the expense of pausing all
threads for the duration of the memory access, via
prepare_to_access_memory.  This new implementation does not require
pausing any thread, thus
linux_process_target::prepare_to_access_memory /
linux_process_target::done_accessing_memory become nops.  A subsequent
patch will remove the whole prepare_to_access_memory infrastructure
completely.

The GDBserver linux-low.cc implementation is simpler than GDB's
linux-nat.c's, because GDBserver always adds the unfollowed vfork/fork
children to the process list immediately when the fork/vfork event is
seen out of ptrace.  I.e., there's no need to keep the file descriptor
stored on a side map, we can store it directly in the process
structure.

Change-Id: I0abfd782ceaa4ddce8d3e5f3e2dfc5928862ef61
2022-04-14 20:22:56 +01:00
Pedro Alves
366e3746c5 gdbserver: special case target_write_memory len==0
The next patch in this series adds a common helper routine for both
memory reads and writes, like this:

 static int
 proc_xfer_memory (CORE_ADDR memaddr, unsigned char *readbuf,
		  const gdb_byte *writebuf, int len)
 {
   gdb_assert ((readbuf == nullptr) != (writebuf == nullptr));
   ...
 }

 int
 linux_process_target::read_memory (CORE_ADDR memaddr,
                                    unsigned char *myaddr, int len)
 {
   return proc_xfer_memory (memaddr, myaddr, nullptr, len);
 }

 linux_process_target::write_memory (CORE_ADDR memaddr,
                                    const unsigned char *myaddr, int len)
 {
   return proc_xfer_memory (memaddr, nullptr, myaddr, len);
 }

Surprisingly, the assertion fails.  That happens because it can happen
that target_write_memory is called with LEN==0, due to this in
gdb/remote.c:

 /* Determine whether the remote target supports binary downloading.
    This is accomplished by sending a no-op memory write of zero length
    to the target at the specified address. (...) */

 void
 remote_target::check_binary_download (CORE_ADDR addr)
 {
 ...
       p = rs->buf.data ();
       *p++ = 'X';
       p += hexnumstr (p, (ULONGEST) addr);
       *p++ = ',';
       p += hexnumstr (p, (ULONGEST) 0);
       *p++ = ':';
       *p = '\0';

In this scenario, in gdbserver's target_write_memory, the "myaddr"
argument of the_target->write_memory is passed the data() of a local
gdb::byte_vector (which is a specialized std::vector).  It's valid for
std::vector::data() to return NULL when the vector is empty.

This commit adds an early return to target_write_memory to avoid
target backends having to care about this.  For good measure, do the
same on the read side, in read_inferior_memory.

Change-Id: Iac8f04fcf99014c624ef4036bd318ca1771ad491
2022-04-14 20:22:42 +01:00
Pedro Alves
330d63093c gdbserver/qXfer::threads, prepare_to_access_memory=>target_pause_all
handle_qxfer_threads_proper needs to pause all threads even if the
target can read memory when threads are running, so use
target_pause_all instead, which is what the Linux implementation of
prepare_to_access_memory uses.  (Only Linux implements this hook.)

A following patch will make the Linux backend be able to access memory
when threads are running, and thus will also make
prepare_to_access_memory do nothing, which would cause testsuite
regressions without this change.

Change-Id: I127fec7246b7c45b60dfa7341e781606bf54b5da
2022-04-14 20:21:11 +01:00
Tom Tromey
8bbdbd6985 Use GetThreadDescription on Windows
Windows 10 introduced SetThreadDescription and GetThreadDescription, a
simpler way to set a thread's name.  This changes gdb and gdbserver to
use this convention when it is available.

This is part of PR win32/29050.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29050
2022-04-14 12:12:35 -06:00
Tom Tromey
42a5971407 Implement thread_name for gdbserver
This changes gdbserver to implement thread_name method.
2022-04-14 12:12:34 -06:00
Tom Tromey
44ac251ad2 Share handle_ms_vc_exception with gdbserver
Currently, gdb's native Windows target implements the exception-based
approach for setting thread names, but gdbserver does not.  This patch
moves handle_ms_vc_exception to the shared nat/windows-nat.c code, as
preparation for adding this support to gdbserver.
2022-04-14 12:12:34 -06:00
Tom Tromey
b17c7ab380 Move target_read_string to target/target.c
This moves the two overloads of target_read_string to a new file,
target/target.c, and updates both gdb and gdbserver to build this.
2022-04-14 12:12:34 -06:00
Tom Tromey
c560a5fbae Let std::thread check pass even without pthreads
Currently, the configure check for std::thread relies on pthreads
existing.  However, this means that if std::thread is implemented for
a non-pthreads host, then the check will yield the wrong answer.  This
happened in AdaCore internal builds.  Here, we have this GCC patch:

    https://gcc.gnu.org/legacy-ml/gcc-patches/2019-06/msg01840.html

... which adds mingw support to GCC's gthreads implementation, and
also to std::thread.

This configure change fixes this problem and enables threading for
gdb.
2022-04-14 09:28:56 -06:00
Simon Marchi
e88cf517e9 gdbserver: report correct status in thread stop race condition
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-04 22:11:53 -04:00
Tom Tromey
0578e87f93 Remove some globals from nat/windows-nat.c
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-04-04 13:58:37 -06:00
Simon Marchi
20471e00e2 gdbserver/linux: set lwp !stopped when failing to resume
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-31 13:04:22 -04:00
Tom Tromey
c50e54825b Consolidate definition of current_directory
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-30 09:08:48 -06:00
John Baldwin
4bd817e71e nat: Split out platform-independent aarch64 debug register support.
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-22 12:05:43 -07:00
Pedro Alves
4414150d33 gdbserver: Fixup previous patch
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-21 17:01:49 +00:00
Pedro Alves
04f0c03a22 gdbserver: Fix incorrect assertion
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-21 16:45:49 +00:00
Pedro Alves
8a841a3579 Re-add zombie leader on exit, gdbserver/linux
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-10 11:35:54 +00:00
Pedro Alves
aa40a989ef gdbserver: Reindent check_zombie_leaders
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-10 11:35:53 +00:00
Pedro Alves
5406bc3f1b gdbserver: Reorganize linux_process_target::filter_event
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-10 11:35:53 +00:00
Pedro Alves
e48359eaa8 Fix gdbserver/linux target_waitstatus logging assert
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-10 11:35:53 +00:00
Roland McGrath
8674f082e3 Avoid conflict with gnulib open/close macros.
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-03-03 11:21:36 -08:00
John Baldwin
cdc8e9b285 gdbserver linux-low: Convert linux_event_pipe to the event_pipe class.
Use event_pipe from gdbsupport in place of the existing file
descriptor array.
2022-02-22 11:22:14 -08:00
Simon Marchi
a288518611 gdb/linux: remove ptrace support check for exec, fork, vfork, vforkdone, 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-02-10 20:10:12 -05:00
Markus Metzger
696c0d5ef2 gdb, gdbserver: update thread identifier in enable_btrace target method
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-27 13:31:20 +01:00
Simon Marchi
91f94053dd gdbserver: introduce remote_debug_printf
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-18 13:44:38 -05:00
Simon Marchi
c058728c31 gdbserver: introduce threads_debug_printf, THREADS_SCOPED_DEBUG_ENTER_EXIT
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-18 13:44:32 -05:00
Simon Marchi
c68665c726 gdbserver: turn debug_threads into a boolean
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-18 13:44:32 -05:00
Andrew Burgess
4b74833d1a gdb: don't use -Wmissing-prototypes with g++
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-13 10:25:45 +00:00
Tom Tromey
8d5e08e10e Do not use CC_HAS_LONG_LONG
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-07 14:00:33 -07:00
Joel Brobecker
4a94e36819 Automatic Copyright Year update after running gdb/copyright.py
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-01 19:13:23 +04:00
Joel Brobecker
dd10f20452 Update Copyright Year in gdb, gdbserver and gdbreplay version output
This commit changes the copyright year printed by gdb, gdbserver
and gdbreplay when printing the tool's version.
2022-01-01 19:13:22 +04:00
Luis Machado
261b07488b New --enable-threading configure option to control use of threads in 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-15 17:00:00 -03:00
Simon Marchi
237f6eac15 gdbserver/tracepoint.cc: use snprintf in gdb_agent_socket_init
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-14 14:35:05 -05:00
Simon Marchi
b2c7007bdd gdbserver/tracepoint.cc: work around -Wstringop-truncation error
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-14 14:35:03 -05:00
Tankut Baris Aktemur
20ac1cdb8c gdbserver/linux-low: replace direct assignment to current_thread
Use scoped_restore_current_thread and switch_to_thread in
linux_process_target::wait_for_sigstop.
2021-12-13 12:22:48 +01:00
Tankut Baris Aktemur
24583e45ef gdbserver: replace direct assignments to current_thread
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-13 12:22:48 +01:00
Tankut Baris Aktemur
f24791b72e gdbserver: introduce scoped_restore_current_thread and switch_to_thread
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-13 12:22:48 +01:00
Simon Marchi
df5ad10200 gdb, gdbserver: detach fork child when detaching from fork parent
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-08 21:00:39 -05:00
Simon Marchi
7b961964f8 gdbserver: hide fork child threads from GDB
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-08 21:00:39 -05:00
Tom de Vries
2e18755037 [gdb/tdep] Fix avx512 -m32 support in gdbserver
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-12-02 18:20:13 +01:00
Simon Marchi
c272a98cbf gdb: pass more const target_waitstatus by reference
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-22 13:57:54 -05:00
Simon Marchi
7dca2ea7ff gdb: rename target_waitstatus_to_string to target_waitstatus::to_string
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-22 13:57:43 -05:00
Tom Tromey
b7c9d393d6 Remove config.cache in gdbserver's "distclean"
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-16 09:13:24 -07:00
Tom Tromey
0b03c6f03d Fix build on rhES5
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-09 08:21:18 -07:00
Simon Marchi
6c130ba387 gdbserver: re-generate configure
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-04 14:14:36 -04:00