[PATCH 15/18] gdbserver:prepare_access_memory: pick another thread

Pedro Alves
2015-10-14T15:28:33Z

Say GDB wants to access the inferior process's memory. The current
remote general thread is 3, but GDB's switched to thread 2. Because
both threads are of the same process, GDB skips making the remote
thread be thread 2 as well (sending an Hg packet) before accessing
memory (remote.c:set_general_process). However, if thread 3 has
exited meanwhile, thread 3 no longer exists on the server and
gdbserver points current_thread to NULL. The result is the memory
access fails, even through the process still exists.

Fix this by making prepare_to_access memory select the thread to
access memory through.

gdb/gdbserver/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>

* mem-break.c (check_gdb_bp_preconditions): Remove current_thread
check.
(set_gdb_breakpoint): If prepare_to_access_memory fails, set *ERR
to -1.
* target.c (struct thread_search): New structure.
(thread_search_callback): New function.
(prev_general_thread): New global.
(prepare_to_access_memory, done_accessing_memory): New functions.
* target.h (prepare_to_access_memory, done_accessing_memory):
Replace macros with function declarations.
---
gdb/gdbserver/mem-break.c | 17 +++-----
gdb/gdbserver/target.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++
gdb/gdbserver/target.h | 14 ++----
3 files changed, 118 insertions(+), 20 deletions(-)

diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index f077497..97d53b5 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -1011,13 +1011,8 @@ check_gdb_bp_preconditions (char z_type, int *err)
*err = 1;
return 0;
}
- else if (current_thread == NULL)
- {
- *err = -1;
- return 0;
- }
- else
- return 1;
+
+ return 1;
}

/* See mem-break.h. This is a wrapper for set_gdb_breakpoint_1 that
@@ -1035,9 +1030,11 @@ set_gdb_breakpoint (char z_type, CORE_ADDR addr, int size, int *err)
access memory. */
if (z_type == Z_PACKET_SW_BP)
{
- *err = prepare_to_access_memory ();
- if (*err != 0)
- return NULL;
+ if (prepare_to_access_memory () != 0)
+ {
+ *err = -1;
+ return NULL;
+ }
}

bp = set_gdb_breakpoint_1 (z_type, addr, size, err);
diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index 301f90e..a59bcb0 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -37,6 +37,113 @@ set_desired_thread (int use_general)
return (current_thread != NULL);
}

+/* Structure used to look up a thread to use as current when accessing
+ memory. */
+
+struct thread_search
+{
+ /* The PTID of the current general thread. This is an input
+ parameter. */
+ ptid_t current_gen_ptid;
+
+ /* The first thread found. */
+ struct thread_info *first;
+
+ /* The first stopped thread found. */
+ struct thread_info *stopped;
+
+ /* The current general thread, if found. */
+ struct thread_info *current;
+};
+
+/* Callback for find_inferior. Search for a thread to use as current
+ when accessing memory. */
+
+static int
+thread_search_callback (struct inferior_list_entry *entry, void *args)
+{
+ struct thread_info *thread = (struct thread_info *) entry;
+ struct thread_search *s = (struct thread_search *) args;
+
+ if (ptid_get_pid (entry->id) == ptid_get_pid (s->current_gen_ptid)
+ && mythread_alive (ptid_of (thread)))
+ {
+ if (s->stopped == NULL && thread_stopped (thread))
+ s->stopped = thread;
+
+ if (s->first == NULL)
+ s->first = thread;
+
+ if (s->current == NULL && ptid_equal (s->current_gen_ptid, entry->id))
+ s->current = thread;
+ }
+
+ return 0;
+}
+
+/* The thread that was current before prepare_to_access_memory was
+ called. done_accessing_memory uses this to restore the previous
+ selected thread. */
+static ptid_t prev_general_thread;
+
+/* See target.h. */
+
+int
+prepare_to_access_memory (void)
+{
+ struct thread_search search;
+ struct thread_info *thread;
+
+ memset (&search, 0, sizeof (search));
+ search.current_gen_ptid = general_thread;
+ prev_general_thread = general_thread;
+
+ if (the_target->prepare_to_access_memory != NULL)
+ {
+ int res;
+
+ res = the_target->prepare_to_access_memory ();
+ if (res != 0)
+ return res;
+ }
+
+ find_inferior (&all_threads, thread_search_callback, &search);
+
+ /* Prefer a stopped thread. If none is found, try the current
+ thread. Otherwise, take the first thread in the process. If
+ none is found, undo the effects of
+ target->prepare_to_access_memory() and return error. */
+ if (search.stopped != NULL)
+ thread = search.stopped;
+ else if (search.current != NULL)
+ thread = search.current;
+ else if (search.first != NULL)
+ thread = search.first;
+ else
+ {
+ done_accessing_memory ();
+ return 1;
+ }
+
+ current_thread = thread;
+ general_thread = ptid_of (thread);
+
+ return 0;
+}
+
+/* See target.h. */
+
+void
+done_accessing_memory (void)
+{
+ if (the_target->done_accessing_memory != NULL)
+ the_target->done_accessing_memory ();
+
+ /* Restore the previous selected thread. */
+ general_thread = prev_general_thread;
+ current_thread = find_thread_ptid (general_thread);
+}
+
int
read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len)
{
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index a2842b4..aad8903 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -630,17 +630,11 @@ int start_non_stop (int nonstop);
ptid_t mywait (ptid_t ptid, struct target_waitstatus *ourstatus, int options,
int connected_wait);

-#define prepare_to_access_memory() \
- (the_target->prepare_to_access_memory \
- ? (*the_target->prepare_to_access_memory) () \
- : 0)
+/* Prepare to read or write memory from the inferior process. See the
+ corresponding target_ops methods for more details. */

-#define done_accessing_memory() \
- do \
- { \
- if (the_target->done_accessing_memory) \
- (*the_target->done_accessing_memory) (); \
- } while (0)
+int prepare_to_access_memory (void);
+void done_accessing_memory (void);

#define target_core_of_thread(ptid) \
(the_target->core_of_thread ? (*the_target->core_of_thread) (ptid) \
--
1.9.3