Linux: Access memory even if threads are running

Currently, on GNU/Linux, if you try to access memory and you have a
running thread selected, GDB fails the memory accesses, like:

 (gdb) c&
 Continuing.
 (gdb) p global_var
 Cannot access memory at address 0x555555558010

Or:

 (gdb) b main
 Breakpoint 2 at 0x55555555524d: file access-mem-running.c, line 59.
 Warning:
 Cannot insert breakpoint 2.
 Cannot access memory at address 0x55555555524d

This patch removes this limitation.  It teaches the native Linux
target to read/write memory even if the target is running.  And it
does this without temporarily stopping threads.  We now get:

 (gdb) c&
 Continuing.
 (gdb) p global_var
 $1 = 123
 (gdb) b main
 Breakpoint 2 at 0x555555555259: file access-mem-running.c, line 62.

(The scenarios above work correctly with current GDBserver, because
GDBserver temporarily stops all threads in the process whenever GDB
wants to access memory (see prepare_to_access_memory /
done_accessing_memory).  Freezing the whole process makes sense when
we need to be sure that we have a consistent view of memory and don't
race with the inferior changing it at the same time as GDB is
accessing it.  But I think that's a too-heavy hammer for the default
behavior.  I think that ideally, whether to stop all threads or not
should be policy decided by gdb core, probably best implemented by
exposing something like gdbserver's prepare_to_access_memory /
done_accessing_memory to gdb core.)

Currently, if we're accessing (reading/writing) just a few bytes, then
the Linux native backend does not try accessing memory via
/proc/<pid>/mem and goes straight to ptrace
PTRACE_PEEKTEXT/PTRACE_POKETEXT.  However, ptrace always fails when
the ptracee is running.  So the first step is to prefer
/proc/<pid>/mem even for small accesses.  Without further changes
however, that may cause a performance regression, due to constantly
opening and closing /proc/<pid>/mem for each memory access.  So the
next step is to keep the /proc/<pid>/mem file open across memory
accesses.  If we have this, then it doesn't make sense anymore to even
have the ptrace fallback, so the patch disables it.

I've made it such that GDB only ever has one /proc/<pid>/mem file open
at any time.  As long as a memory access hits the same inferior
process as the previous access, then we reuse the previously open
file.  If however, we access memory of a different process, then we
close the previous file and open a new one for the new process.

If we wanted, we could keep one /proc/<pid>/mem file open per
inferior, and never close them (unless the inferior exits or execs).
However, having seen bfd patches recently about hitting too many open
file descriptors, I kept the logic to have only one file open tops.
Also, we need to handle memory accesses for processes for which we
don't have an inferior object, for when we need to detach a
fork-child, and we'd probaly want to handle caching the open file for
that scenario (no inferior for process) too, which would probably end
up meaning caching for last non-inferior process, which is very much
what I'm proposing anyhow.  So always having one file open likely ends
up a smaller patch.

The next step is handling the case of GDB reading/writing memory
through a thread that is running and exits.  The access should not
result in a user-visible failure if the inferior/process is still
alive.

Once we manage to open a /proc/<lwpid>/mem file, then that file is
usable for memory accesses even if the corresponding lwp exits and is
reaped.  I double checked that trying to open the same
/proc/<lwpid>/mem path again fails because the lwp is really gone so
there's no /proc/<lwpid>/ entry on the filesystem anymore, but the
previously open file remains usable.  It's only when the whole process
execs that we need to reopen a new file.

When the kernel destroys the whole address space, i.e., when the
process exits or execs, the reads/writes fail with 0 aka EOF, in which
case there's nothing else to do than returning a memory access
failure.  Note this means that when we get an exec event, we need to
reopen the file, to access the process's new address space.

If we need to open (or reopen) the /proc/<pid>/mem file, and the LWP
we're opening it for exits before we open it and before we reap the
LWP (i.e., the LWP is zombie), the open fails with EACCES.  The patch
handles this by just looking for another thread until it finds one
that we can open a /proc/<pid>/mem successfully for.

If we need to open (or reopen) the /proc/<pid>/mem file, and the LWP
we're opening has exited and we already reaped it, which is the case
if the selected thread is in THREAD_EXIT state, the open fails with
ENOENT.  The patch handles this the same way as a zombie race
(EACCES), instead of checking upfront whether we're accessing a
known-exited thread, because that would result in more complicated
code, because we also need to handle accessing lwps that are not
listed in the core thread list, and it's the core thread list that
records the THREAD_EXIT state.

The patch includes two testcases:

#1 - gdb.base/access-mem-running.exp

  This is the conceptually simplest - it is single-threaded, and has
  GDB read and write memory while the program is running.  It also
  tests setting a breakpoint while the program is running, and checks
  that the breakpoint is hit immediately.

#2 - gdb.threads/access-mem-running-thread-exit.exp

  This one is more elaborate, as it continuously spawns short-lived
  threads in order to exercise accessing memory just while threads are
  exiting.  It also spawns two different processes and alternates
  accessing memory between the two processes to exercise the reopening
  the /proc file frequently.  This also ends up exercising GDB reading
  from an exited thread frequently.  I confirmed by putting abort()
  calls in the EACCES/ENOENT paths added by the patch that we do hit
  all of them frequently with the testcase.  It also exits the
  process's main thread (i.e., the main thread becomes zombie), to
  make sure accessing memory in such a corner-case scenario works now
  and in the future.

The tests fail on GNU/Linux native before the code changes, and pass
after.  They pass against current GDBserver, again because GDBserver
supports memory access even if all threads are running, by
transparently pausing the whole process.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	PR mi/15729
	PR gdb/13463
	* linux-nat.c (linux_nat_target::detach): Close the
	/proc/<pid>/mem file if it was open for this process.
	(linux_handle_extended_wait) <PTRACE_EVENT_EXEC>: Close the
	/proc/<pid>/mem file if it was open for this process.
	(linux_nat_target::mourn_inferior): Close the /proc/<pid>/mem file
	if it was open for this process.
	(linux_nat_target::xfer_partial): Adjust.  Do not fall back to
	inf_ptrace_target::xfer_partial for memory accesses.
	(last_proc_mem_file): New.
	(maybe_close_proc_mem_file): New.
	(linux_proc_xfer_memory_partial_pid): New, with bits factored out
	from linux_proc_xfer_partial.
	(linux_proc_xfer_partial): Delete.
	(linux_proc_xfer_memory_partial): New.

gdb/testsuite/ChangeLog
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	PR mi/15729
	PR gdb/13463
	* gdb.base/access-mem-running.c: New.
	* gdb.base/access-mem-running.exp: New.
	* gdb.threads/access-mem-running-thread-exit.c: New.
	* gdb.threads/access-mem-running-thread-exit.exp: New.

Change-Id: Ib3c082528872662a3fc0ca9b31c34d4876c874c9

5 files changed, 469 insertions, 0 deletions

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 38d8bbccd6c..67fa42cbe3b 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,12 @@
+2021-07-01  Pedro Alves  <pedro@palves.net>
+
+	PR mi/15729
+	PR gdb/13463
+	* gdb.base/access-mem-running.c: New.
+	* gdb.base/access-mem-running.exp: New.
+	* gdb.threads/access-mem-running-thread-exit.c: New.
+	* gdb.threads/access-mem-running-thread-exit.exp: New.
+
 2021-06-29  Simon Marchi  <simon.marchi@polymtl.ca>
 
 	* gdb.dwarf2/dw2-reg-undefined.exp: Update regexp.
diff --git a/gdb/testsuite/gdb.base/access-mem-running.c b/gdb/testsuite/gdb.base/access-mem-running.c
new file mode 100644
index 00000000000..2bb4f9766b5
--- /dev/null
+++ b/gdb/testsuite/gdb.base/access-mem-running.c
@@ -0,0 +1,47 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2021 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <unistd.h>
+
+static unsigned int global_counter = 1;
+
+static volatile unsigned int global_var = 123;
+
+static void
+maybe_stop_here ()
+{
+}
+
+int
+main (void)
+{
+  global_counter = 1;
+
+  while (global_counter > 0)
+    {
+      global_counter++;
+
+      /* Less than 1s, so the counter increments at least once while
+	 the .exp sleep 1s, but slow enough that the counter doesn't
+	 wrap in 1s.  */
+      usleep (5000);
+
+      maybe_stop_here ();
+    }
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/access-mem-running.exp b/gdb/testsuite/gdb.base/access-mem-running.exp
new file mode 100644
index 00000000000..6990d906da2
--- /dev/null
+++ b/gdb/testsuite/gdb.base/access-mem-running.exp
@@ -0,0 +1,124 @@
+# Copyright (C) 2021 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test that we can access memory while the inferior is running.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug}] == -1} {
+    return -1
+}
+
+# The test proper.  NON_STOP indicates whether we're testing in
+# non-stop, or all-stop mode.
+
+proc test { non_stop } {
+    global srcfile binfile
+    global gdb_prompt
+    global GDBFLAGS
+    global decimal
+
+    save_vars { GDBFLAGS } {
+      append GDBFLAGS " -ex \"set non-stop $non_stop\""
+      clean_restart ${binfile}
+    }
+
+    if ![runto_main] {
+	return -1
+    }
+
+    # If debugging with target remote, check whether the all-stop variant
+    # of the RSP is being used.  If so, we can't run the background tests.
+    if {!$non_stop
+	&& [target_info exists gdb_protocol]
+	&& ([target_info gdb_protocol] == "remote"
+	    || [target_info gdb_protocol] == "extended-remote")} {
+
+	gdb_test_multiple "maint show target-non-stop" "" {
+	    -wrap -re "(is|currently) on.*" {
+	    }
+	    -wrap -re "(is|currently) off.*" {
+		unsupported "can't issue commands while target is running"
+		return 0
+	    }
+	}
+    }
+
+    delete_breakpoints
+
+    if {$non_stop == "off"} {
+	set cmd "continue &"
+    } else {
+	set cmd "continue -a &"
+    }
+    gdb_test_multiple $cmd "continuing" {
+	-re "Continuing\.\r\n$gdb_prompt " {
+	    pass $gdb_test_name
+	}
+    }
+
+    # Check we can read/write variables.
+
+    # Check that we can read the counter variable, and that the
+    # counter is increasing, meaning the process really is running.
+
+    sleep 1
+    set global_counter1 \
+	[get_integer_valueof "global_counter" 0 \
+	     "get global_counter once"]
+
+    sleep 1
+    set global_counter2 \
+	[get_integer_valueof "global_counter" 0 \
+	     "get global_counter twice"]
+
+    gdb_assert {$global_counter1 != 0 \
+		    && $global_counter2 != 0 \
+		    && $global_counter1 != $global_counter2} \
+	"value changed"
+
+    # Check that we can write variables.
+
+    gdb_test "print global_var" " = 123" \
+	"print global_var before writing"
+    gdb_test "print global_var = 321" " = 321" \
+	"write to global_var"
+    gdb_test "print global_var" " = 321" \
+	"print global_var after writing"
+    gdb_test "print global_var = 123" " = 123" \
+	"write to global_var again"
+
+    # Check we can set a breakpoint while the process is running.  The
+    # breakpoint should hit immediately.
+    set any "\[^\r\n\]*"
+
+    gdb_test_multiple "b maybe_stop_here" "" {
+	-re "Breakpoint $decimal at $any: file $any${srcfile}, line $decimal.\r\n$gdb_prompt " {
+	    pass $gdb_test_name
+	}
+    }
+    gdb_test_multiple "" "breakpoint hits" {
+	-re "Breakpoint $decimal, maybe_stop_here \\(\\) at $any${srcfile}:$decimal\r\n" {
+	    pass "$gdb_test_name"
+	}
+    }
+}
+
+foreach non_stop { "off" "on" } {
+    set stop_mode [expr ($non_stop=="off")?"all-stop":"non-stop"]
+    with_test_prefix "$stop_mode" {
+	test $non_stop
+    }
+}
diff --git a/gdb/testsuite/gdb.threads/access-mem-running-thread-exit.c b/gdb/testsuite/gdb.threads/access-mem-running-thread-exit.c
new file mode 100644
index 00000000000..5f89d223065
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/access-mem-running-thread-exit.c
@@ -0,0 +1,123 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2021 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#define _GNU_SOURCE
+#include <assert.h>
+#include <pthread.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+
+#define THREADS 20
+
+static volatile unsigned int global_var = 123;
+
+/* Wrapper around pthread_create.   */
+
+static void
+create_thread (pthread_t *child,
+	       void *(*start_routine) (void *), void *arg)
+{
+  int rc;
+
+  while ((rc = pthread_create (child, NULL, start_routine, arg)) != 0)
+    {
+      fprintf (stderr, "unexpected error from pthread_create: %s (%d)\n",
+	       strerror (rc), rc);
+      sleep (1);
+    }
+}
+
+/* Data passed to threads on creation.  This is allocated on the heap
+   and ownership transferred from parent to child.  */
+
+struct thread_arg
+{
+  /* The thread's parent.  */
+  pthread_t parent;
+
+  /* Whether to call pthread_join on the parent.  */
+  int join_parent;
+};
+
+/* Entry point for threads.  */
+
+static void *
+thread_fn (void *arg)
+{
+  struct thread_arg *p = arg;
+
+  /* Passing no argument makes the thread exit immediately.  */
+  if (p == NULL)
+    return NULL;
+
+  if (p->join_parent)
+    assert (pthread_join (p->parent, NULL) == 0);
+
+  /* Spawn a number of threads that exit immediately, and then join
+     them.  The idea is to maximize the time window when we mostly
+     have threads exiting.  */
+  {
+    pthread_t child[THREADS];
+    int i;
+
+    /* Passing no argument makes the thread exit immediately.  */
+    for (i = 0; i < THREADS; i++)
+      create_thread (&child[i], thread_fn, NULL);
+
+    for (i = 0; i < THREADS; i++)
+      pthread_join (child[i], NULL);
+  }
+
+  /* Spawn a new thread that joins us, and exit.  The idea here is to
+     not have any thread that stays around forever.  */
+  {
+    pthread_t child;
+
+    p->parent = pthread_self ();
+    p->join_parent = 1;
+    create_thread (&child, thread_fn, p);
+  }
+
+  return NULL;
+}
+
+int
+main (void)
+{
+  int i;
+
+  for (i = 0; i < 4; i++)
+    {
+      struct thread_arg *p;
+      pthread_t child;
+
+      p = malloc (sizeof *p);
+      p->parent = pthread_self ();
+      /* Only join the parent once.  */
+      if (i == 0)
+	p->join_parent = 1;
+      else
+	p->join_parent = 0;
+      create_thread (&child, thread_fn, p);
+    }
+
+  /* Exit the leader to make sure that we can access memory with the
+     leader gone.  */
+  pthread_exit (NULL);
+}
diff --git a/gdb/testsuite/gdb.threads/access-mem-running-thread-exit.exp b/gdb/testsuite/gdb.threads/access-mem-running-thread-exit.exp
new file mode 100644
index 00000000000..ea228e4ba13
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/access-mem-running-thread-exit.exp
@@ -0,0 +1,166 @@
+# Copyright (C) 2021 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test that we can access memory while all the threads of the inferior
+# are running, and even if:
+#
+# - the leader thread exits
+# - the selected thread exits
+#
+# This test constantly spawns short lived threads to make sure that on
+# systems with debug APIs that require passing down a specific thread
+# to work with (e.g., GNU/Linux ptrace and /proc filesystem), GDB
+# copes with accessing memory just while the thread it is accessing
+# memory through exits.
+#
+# The test spawns two processes and alternates memory accesses between
+# them to force flushing per-process caches.  At the time of writing,
+# the Linux backend accesses inferior memory via /proc/<pid>/mem, and
+# keeps one such file open, as a cache.  Alternating inferiors forces
+# opening such file for a different process, which fails if GDB tries
+# to open the file for a thread that exited.  The test does ensures
+# those reopen/fail code paths are exercised.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug pthreads}] == -1} {
+    return -1
+}
+
+# The test proper.  NON_STOP indicates whether we're testing in
+# non-stop, or all-stop mode.
+
+proc test { non_stop } {
+    global binfile
+    global gdb_prompt
+    global GDBFLAGS
+
+    save_vars { GDBFLAGS } {
+      append GDBFLAGS " -ex \"set non-stop $non_stop\""
+      clean_restart ${binfile}
+    }
+
+    if ![runto_main] {
+	fail "cannot run to main"
+	return -1
+    }
+
+    # If debugging with target remote, check whether the all-stop variant
+    # of the RSP is being used.  If so, we can't run the background tests.
+    if {!$non_stop
+	&& [target_info exists gdb_protocol]
+	&& ([target_info gdb_protocol] == "remote"
+	    || [target_info gdb_protocol] == "extended-remote")} {
+
+	gdb_test_multiple "maint show target-non-stop" "" {
+	    -wrap -re "(is|currently) on.*" {
+	    }
+	    -wrap -re "(is|currently) off.*" {
+		unsupported "can't issue commands while target is running"
+		return 0
+	    }
+	}
+    }
+
+    delete_breakpoints
+
+    # Start the second inferior.
+    with_test_prefix "second inferior" {
+	gdb_test "add-inferior -no-connection" "New inferior 2.*"
+	gdb_test "inferior 2" "Switching to inferior 2 .*"
+
+	gdb_load $binfile
+
+	if ![runto_main] {
+	    fail "cannot run to main"
+	    return -1
+	}
+    }
+
+    delete_breakpoints
+
+    # These put too much noise in the logs.
+    gdb_test_no_output "set print thread-events off"
+
+    # Continue all threads of both processes.
+    gdb_test_no_output "set schedule-multiple on"
+    if {$non_stop == "off"} {
+	set cmd "continue &"
+    } else {
+	set cmd "continue -a &"
+    }
+    gdb_test_multiple $cmd "continuing" {
+	-re "Continuing\.\r\n$gdb_prompt " {
+	    pass $gdb_test_name
+	}
+    }
+
+    # Like gdb_test, but:
+    # - don't issue a pass on success.
+    # - on failure, clear the ok variable in the calling context, and
+    #   break it.
+    proc my_gdb_test {cmd pattern message} {
+	upvar inf inf
+	upvar iter iter
+	if {[gdb_test_multiple $cmd "access mem ($message, inf=$inf, iter=$iter)" {
+	    -wrap -re $pattern {
+	    }
+	}] != 0} {
+	    uplevel 1 {set ok 0}
+	    return -code break
+	}
+    }
+
+    # Hammer away for 5 seconds, alternating between inferiors.
+    set ::done 0
+    after 5000 { set ::done 1 }
+
+    set inf 1
+    set ok 1
+    set iter 0
+    while {!$::done && $ok} {
+	incr iter
+	verbose -log "xxxxx: iteration $iter"
+	gdb_test "info threads" ".*" ""
+
+	if {$inf == 1} {
+	    set inf 2
+	} else {
+	    set inf 1
+	}
+
+	my_gdb_test "inferior $inf" ".*" "inferior $inf"
+
+	my_gdb_test "print global_var = 555" " = 555" \
+	    "write to global_var"
+	my_gdb_test "print global_var" " = 555" \
+	    "print global_var after writing"
+	my_gdb_test "print global_var = 333" " = 333" \
+	    "write to global_var again"
+	my_gdb_test "print global_var" " = 333" \
+	    "print global_var after writing again"
+    }
+
+   if {$ok} {
+       pass "access mem"
+   }
+}
+
+foreach non_stop { "off" "on" } {
+    set stop_mode [expr ($non_stop=="off")?"all-stop":"non-stop"]
+    with_test_prefix "$stop_mode" {
+	test $non_stop
+    }
+}