This is the mail archive of the gdb-cvs@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[binutils-gdb] Multi-arch exec, more register reading avoidance


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=cbd2b4e316370ab740433b06ed65786831232c8f

commit cbd2b4e316370ab740433b06ed65786831232c8f
Author: Pedro Alves <palves@redhat.com>
Date:   Mon Oct 9 18:11:01 2017 +0100

    Multi-arch exec, more register reading avoidance
    
    As mentioned in commit bf93d7ba9931 ("Add thread after updating
    gdbarch when exec'ing"), we should avoid doing register reads after a
    process does an exec and before we've updated that inferior's gdbarch.
    Otherwise, we may interpret the registers using the wrong
    architecture.
    
    There's still (at least) one case where we still read registers
    post-exec with the pre-exec architecture.  That's when infrun decides
    it needs to switch context to the exec'ing thread.  I.e., if the exec
    event is processed at a time when the current thread is not already
    the exec'ing thread, then we get (with the test added by this commit):
    
      continue
      Continuing.
      Truncated register 50 in remote 'g' packet
      Truncated register 50 in remote 'g' packet
      (gdb) FAIL: gdb.multi/multi-arch-exec.exp: selected_thread=2: follow_exec_mode=same: continue across exec that changes architecture
    
    The fix is to avoid reading registers when switching context in this
    case.
    
    (I'd be nice to get rid of the constant stop_pc reading when switching
    threads, but that'd be a deeper change.)
    
    gdb/ChangeLog:
    2017-10-09  Pedro Alves  <palves@redhat.com>
    
    	* infrun.c (handle_inferior_event_1) <TARGET_WAITKIND_EXECD>: Skip
    	reading registers when switching context.
    
    gdb/testsuite/ChangeLog:
    2017-10-09  Pedro Alves  <palves@redhat.com>
    
    	* gdb.multi/multi-arch-exec.c: Include <pthread.h> and <assert.h>.
    	(barrier): New.
    	(thread_start, all_started): New functions.
    	(main): Spawn new thread and wait until it is scheduled.
    	* gdb.multi/multi-arch-exec.exp: Build $srcfile1 with the pthreads
    	option.
    	(do_test): Add 'selected_thread' parameter.  Run to all_started
    	instead of main.  Explicitly set the breakpoint at main.  Switch
    	to the SELECTED_THREAD thread.
    	(top level): Test handling the exec event with either the main
    	thread or the second thread selected.

Diff:
---
 gdb/ChangeLog                               |  5 +++++
 gdb/infrun.c                                |  5 ++++-
 gdb/testsuite/ChangeLog                     | 14 ++++++++++++++
 gdb/testsuite/gdb.multi/multi-arch-exec.c   | 27 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.multi/multi-arch-exec.exp | 30 ++++++++++++++++++++++-------
 5 files changed, 73 insertions(+), 8 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b04da8b..f1fd73c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2017-10-09  Pedro Alves  <palves@redhat.com>
+
+	* infrun.c (handle_inferior_event_1) <TARGET_WAITKIND_EXECD>: Skip
+	reading registers when switching context.
+
 2017-10-09  John Baldwin  <jhb@FreeBSD.org>
 
 	* fbsd-nat.c (fbsd_siginfo_size): Use gdbarch_long_bit.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index e82f61f..113add6 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -5307,8 +5307,11 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
       if (debug_infrun)
         fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_EXECD\n");
 
+      /* Note we can't read registers yet (the stop_pc), because we
+	 don't yet know the inferior's post-exec architecture.
+	 'stop_pc' is explicitly read below instead.  */
       if (!ptid_equal (ecs->ptid, inferior_ptid))
-	context_switch (ecs->ptid);
+	switch_to_thread_no_regs (ecs->event_thread);
 
       /* Do whatever is necessary to the parent branch of the vfork.  */
       handle_vfork_child_exec_or_exit (1);
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index bb62d22..e7b63ff 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,19 @@
 2017-10-09  Pedro Alves  <palves@redhat.com>
 
+	* gdb.multi/multi-arch-exec.c: Include <pthread.h> and <assert.h>.
+	(barrier): New.
+	(thread_start, all_started): New functions.
+	(main): Spawn new thread and wait until it is scheduled.
+	* gdb.multi/multi-arch-exec.exp: Build $srcfile1 with the pthreads
+	option.
+	(do_test): Add 'selected_thread' parameter.  Run to all_started
+	instead of main.  Explicitly set the breakpoint at main.  Switch
+	to the SELECTED_THREAD thread.
+	(top level): Test handling the exec event with either the main
+	thread or the second thread selected.
+
+2017-10-09  Pedro Alves  <palves@redhat.com>
+
 	* gdb.base/print-file-var-main.c: Fix get_version_2 value check
 	logic.  Move STOP marker after the value checks.
 	* gdb.base/print-file-var.exp (continue to STOP marker): Tighten
diff --git a/gdb/testsuite/gdb.multi/multi-arch-exec.c b/gdb/testsuite/gdb.multi/multi-arch-exec.c
index 1a443de..f43a5a8 100644
--- a/gdb/testsuite/gdb.multi/multi-arch-exec.c
+++ b/gdb/testsuite/gdb.multi/multi-arch-exec.c
@@ -20,11 +20,32 @@
 #include <unistd.h>
 #include <limits.h>
 #include <string.h>
+#include <pthread.h>
+
+#define NUM_THREADS 1
+
+static pthread_barrier_t barrier;
+
+static void *
+thread_start (void *arg)
+{
+  pthread_barrier_wait (&barrier);
+
+  while (1)
+    sleep (1);
+  return NULL;
+}
+
+static void
+all_started (void)
+{
+}
 
 int
 main (int argc, char ** argv)
 {
   char prog[PATH_MAX];
+  pthread_t thread;
   int len;
 
   strcpy (prog, argv[0]);
@@ -33,6 +54,12 @@ main (int argc, char ** argv)
   memcpy (prog + len - 15, "multi-arch-exec-hello", 21);
   prog[len + 6] = 0;
 
+  pthread_barrier_init (&barrier, NULL, NUM_THREADS + 1);
+  pthread_create (&thread, NULL, thread_start, NULL);
+
+  pthread_barrier_wait (&barrier);
+  all_started ();
+
   execl (prog,
          prog,
          (char *) NULL);
diff --git a/gdb/testsuite/gdb.multi/multi-arch-exec.exp b/gdb/testsuite/gdb.multi/multi-arch-exec.exp
index 27a0d60..3196cf3 100644
--- a/gdb/testsuite/gdb.multi/multi-arch-exec.exp
+++ b/gdb/testsuite/gdb.multi/multi-arch-exec.exp
@@ -53,7 +53,7 @@ if [istarget "s390*-*-*"] {
 }
 
 if { [prepare_for_testing "failed to prepare" ${exec1} "${srcfile1}" \
-	  [list debug \
+	  [list debug pthreads \
 	       additional_flags=${march1}]] } {
     return -1
 }
@@ -76,22 +76,38 @@ if { [prepare_for_testing "failed to prepare" ${exec2} "${srcfile2}" \
     return -1
 }
 
-proc do_test { mode } {
+proc do_test { mode selected_thread } {
 	global exec1
 
 	clean_restart ${exec1}
-	if ![runto_main] then {
-	    fail "couldn't run to main"
+	if ![runto all_started] then {
+	    fail "couldn't run to all_started"
 	    return -1
 	}
 
+	# Delete the breakpoint at 'all_started' otherwise GDB may
+	# switch context back to thread 1 to step over the breakpoint.
+	delete_breakpoints
+
+	# A location for this breakpoint should be found in the new
+	# post-exec image too.
+	gdb_breakpoint main
+
+	gdb_test "thread $selected_thread" "Switching to thread $selected_thread .*"
+
 	gdb_test_no_output "set follow-exec-mode $mode"
 
 	# Test that GDB updates the target description / arch successfuly
 	# after the exec.
-	gdb_test "continue" "Breakpoint 1, main.*" "continue across exec that changes architecture"
+	gdb_test "continue" "Breakpoint 2, main.*" "continue across exec that changes architecture"
 }
 
-foreach follow_exec_mode {"same" "new"} {
-    do_test $follow_exec_mode
+# Test handling the exec event with either the main thread or the
+# second thread selected.  This tries to ensure that GDB doesn't read
+# registers off of the execing thread before figuring out its
+# architecture.
+foreach_with_prefix selected_thread {1 2} {
+    foreach_with_prefix follow_exec_mode {"same" "new"} {
+	do_test $follow_exec_mode $selected_thread
+    }
 }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]