gdb: clear inferior displaced stepping state and in-line step-over info on exec

When a process does an exec, all its program space is replaced with the
newly loaded executable.  All non-main threads disappear and the main
thread starts executing at the entry point of the new executable.

Things can go wrong if a displaced step operation is in progress while
we process the exec event.

If the main thread is the one executing the displaced step: when that
thread (now executing in the new executable) stops somewhere (say, at a
breakpoint), displaced_step_fixup will run and clear up the state.  We
will execute the "fixup" phase for the instruction we single-stepped in
the old program space.  We are now in a completely different context,
so doing the fixup may corrupt the state.

If it is a non-main thread that is doing the displaced step: while
handling the exec event, GDB deletes the thread_info representing that
thread (since the thread doesn't exist in the inferior after the exec).
But inferior::displaced_step_state::step_thread will still point to it.
When handling events later, this condition, in displaced_step_fixup,
will likely never be true:

    /* Was this event for the thread we displaced?  */
    if (displaced->step_thread != event_thread)
      return 0;

... since displaced->step_thread points to a deleted thread (unless that
storage gets re-used for a new thread_info, but that wouldn't be good
either).  This effectively makes the displaced stepping buffer occupied
for ever.  When a thread in the new program space will want to do a
displaced step, it will wait for ever.

I think we simply need to reset the displaced stepping state of the
inferior on exec.  Everything execution-related that existed before the
exec is now gone.

Similarly, if a thread does an in-line step over an exec syscall
instruction, nothing clears the in-line step over info when the event is
handled.  So it the in-line step over info stays there indefinitely, and
things hang because we can never start another step over.  To fix this,
I added a call to clear_step_over_info in infrun_inferior_execd.

Add a test with a program with two threads that does an exec.  The test
includes the following axes:

- whether it's the leader thread or the other thread that does the exec.

- whether the exec'r and exec'd program have different text segment
  addresses.  This is to hopefully catch cases where the displaced
  stepping info doesn't get reset, and GDB later tries to restore bytes
  of the old address space in the new address space.  If the mapped
  addresses are different, we should get some memory error.   This
  happens without the patch applied:

  $ ./gdb -q -nx --data-directory=data-directory testsuite/outputs/gdb.threads/step-over-exec/step-over-exec-execr-thread-leader-diff-text-segs-true -ex "b main" -ex r -ex "b my_execve_syscall if 0"  -ex "set displaced-stepping on"
  ...
  Breakpoint 1, main (argc=1, argv=0x7fffffffde38) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.threads/step-over-exec.c:69
  69        argv0 = argv[0];
  Breakpoint 2 at 0x60133a: file /home/simark/src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S, line 34.
  (gdb) c
  Continuing.
  [New Thread 0x7ffff7c62640 (LWP 1455423)]
  Leader going in exec.
  Exec-ing /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-exec/step-over-exec-execr-thread-leader-diff-text-segs-true-execd
  [Thread 0x7ffff7c62640 (LWP 1455423) exited]
  process 1455418 is executing new program: /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-exec/step-over-exec-execr-thread-leader-diff-text-segs-true-execd
  Error in re-setting breakpoint 2: Function "my_execve_syscall" not defined.
  No unwaited-for children left.
  (gdb) n
  Single stepping until exit from function _start,
  which has no line number information.
  Cannot access memory at address 0x6010d2
  (gdb)

- Whether displaced stepping is allowed or not, so that we end up
  testing both displaced stepping and in-line stepping on arches that do
  support displaced stepping (otherwise, it just tests in-line stepping
  twice I suppose)

To be able to precisely put a breakpoint on the syscall instruction, I
added a small assembly file (lib/my-syscalls.S) that contains minimal
Linux syscall wrappers.  I prefer that to the strategy used in
gdb.base/step-over-syscall.exp, which is to stepi into the glibc wrapper
until we find something that looks like a syscall instruction, I find
that more predictable.

gdb/ChangeLog:

	* infrun.c (infrun_inferior_execd): New function.
	(_initialize_infrun): Attach inferior_execd observer.

gdb/testsuite/ChangeLog:

	* gdb.threads/step-over-exec.exp: New.
	* gdb.threads/step-over-exec.c: New.
	* gdb.threads/step-over-exec-execd.c: New.
	* lib/my-syscalls.S: New.
	* lib/my-syscalls.h: New.

Change-Id: I1bbc8538e683f53af5b980091849086f4fec5ff9

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d7bfae85345bf07f5931b913035d6bff4e1d58a5..8ce239f4b35237027a22590b36d1527b12aab641 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2020-12-04  Simon Marchi  <simon.marchi@efficios.com>
+
+       * infrun.c (infrun_inferior_execd): New function.
+       (_initialize_infrun): Attach inferior_execd observer.
+
 2020-12-04  Simon Marchi  <simon.marchi@efficios.com>
 
        * observable.h (inferior_execd): Declare new observable.

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 473a0fb85420c43196a2b68a8d042f2d8a1ba27c..0e0a7a691c437cee61a05942deefeba05a1d4d73 100644 (file)
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1528,6 +1528,21 @@ infrun_inferior_exit (struct inferior *inf)
   inf->displaced_step_state.reset ();
 }
 
+static void
+infrun_inferior_execd (inferior *inf)
+{
+  /* If a thread was doing a displaced step in this inferior at the moment of
+     the exec, it no longer exists.  Even if the exec'ing thread was the one
+     doing a displaced step, we don't want to to any fixup nor restore displaced
+     stepping buffer bytes.  */
+  inf->displaced_step_state.reset ();
+
+  /* Since an in-line step is done with everything else stopped, if there was
+     one in progress at the time of the exec, it must have been the exec'ing
+     thread.  */
+  clear_step_over_info ();
+}
+
 /* If ON, and the architecture supports it, GDB will use displaced
    stepping to step over breakpoints.  If OFF, or if the architecture
    doesn't support it, GDB will instead use the traditional
@@ -9509,6 +9524,7 @@ enabled by default on some platforms."),
   gdb::observers::thread_stop_requested.attach (infrun_thread_stop_requested);
   gdb::observers::thread_exit.attach (infrun_thread_thread_exit);
   gdb::observers::inferior_exit.attach (infrun_inferior_exit);
+  gdb::observers::inferior_execd.attach (infrun_inferior_execd);
 
   /* Explicitly create without lookup, since that tries to create a
      value with a void typed value, and when we get here, gdbarch

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 5de67b4fe9a1f4a8a5bb968247c0f03fd9bfe95a..d3350bcc506da0546b5c63711394d9a6d3113dc3 100644 (file)
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2020-12-04  Simon Marchi  <simon.marchi@efficios.com>
+
+       * gdb.threads/step-over-exec.exp: New.
+       * gdb.threads/step-over-exec.c: New.
+       * gdb.threads/step-over-exec-execd.c: New.
+       * lib/my-syscalls.S: New.
+       * lib/my-syscalls.h: New.
+
 2020-12-04  Simon Marchi  <simon.marchi@efficios.com>
 
        * lib/dwarf.exp (declare_labels): Use name as text if text is

diff --git a/gdb/testsuite/gdb.threads/step-over-exec-execd.c b/gdb/testsuite/gdb.threads/step-over-exec-execd.c
new file mode 100644 (file)
index 0000000..5f3835c
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/step-over-exec-execd.c
@@ -0,0 +1,30 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2020 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/>.  */
+
+int a_variable_in_execd = 1212;
+
+static void
+foo (void)
+{
+}
+
+int
+main (void)
+{
+  foo ();
+  return a_variable_in_execd;
+}

diff --git a/gdb/testsuite/gdb.threads/step-over-exec.c b/gdb/testsuite/gdb.threads/step-over-exec.c
new file mode 100644 (file)
index 0000000..a043e8d
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/step-over-exec.c
@@ -0,0 +1,90 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2020 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>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <pthread.h>
+
+#include "../lib/my-syscalls.h"
+
+#if (!defined(LEADER_DOES_EXEC) && !defined(OTHER_DOES_EXEC) \
+     || defined(LEADER_DOES_EXEC) && defined(OTHER_DOES_EXEC))
+# error "Exactly one of LEADER_DOES_EXEC and OTHER_DOES_EXEC must be defined."
+#endif
+
+
+static char *argv0;
+static pthread_barrier_t barrier;
+
+static void
+do_the_exec (void)
+{
+  char *execd_path = (char *) malloc (strlen (argv0) + sizeof ("-execd"));
+  sprintf (execd_path, "%s-execd", argv0);
+  char *argv[] = { execd_path, NULL };
+
+  printf ("Exec-ing %s\n", execd_path);
+
+  extern char **environ;
+  my_execve (execd_path, argv, environ);
+
+  printf ("Exec failed :(\n");
+  abort ();
+}
+
+static void *
+thread_func (void *arg)
+{
+  pthread_barrier_wait (&barrier);
+#ifdef OTHER_DOES_EXEC
+  printf ("Other going in exec.\n");
+  do_the_exec ();
+#endif
+
+  /* Just make sure the thread does not exit when the leader does the exec.  */
+  pthread_barrier_wait (&barrier);
+
+  return NULL;
+}
+
+int
+main (int argc, char *argv[])
+{
+  argv0 = argv[0];
+
+  int ret = pthread_barrier_init (&barrier, NULL, 2);
+  if (ret != 0)
+    abort ();
+
+  pthread_t thread;
+  ret = pthread_create (&thread, NULL, thread_func, argv[0]);
+  if (ret != 0)
+    abort ();
+
+  pthread_barrier_wait (&barrier);
+
+#ifdef LEADER_DOES_EXEC
+  printf ("Leader going in exec.\n");
+  do_the_exec ();
+#endif
+
+  pthread_join (thread, NULL);
+
+  return 0;
+}

diff --git a/gdb/testsuite/gdb.threads/step-over-exec.exp b/gdb/testsuite/gdb.threads/step-over-exec.exp
new file mode 100644 (file)
index 0000000..411980c
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/step-over-exec.exp
@@ -0,0 +1,113 @@
+# Copyright 2020 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 stepping over an exec syscall instruction in a multi-threaded program.
+
+standard_testfile .c -execd.c
+
+set syscalls_src $srcdir/lib/my-syscalls.S
+
+# EXECR_THREAD is "leader" or "other", and decides which thread does the exec.
+#
+# If DIFFERENT_TEXT_SEGMENTS is true, the exec'er and exec'd program are
+# compiled with different, explicit text segment addresses.  This makes it so
+# the address of the displaced stepping buffer in the old executable is likely
+# not accessible in the new executable.  This might catch cases where GDB tries
+# (wrongfully) to restore the bytes saved from the old executable in the new
+# executable.
+#
+# DISPLACED_STEPPING is "auto" or "off" and controls the value of "set
+# displaced-stepping".
+
+proc do_test { execr_thread different_text_segments displaced_stepping } {
+    global srcdir subdir srcfile srcfile2 binfile
+    global syscalls_src
+    global decimal hex
+
+    set execr_srcs [list $srcdir/$subdir/$srcfile $syscalls_src]
+    set execd_srcs [list $srcdir/$subdir/$srcfile2]
+
+    # Generate unique filenames for each case.
+    set execr_binfile $binfile-execr-thread-$execr_thread-diff-text-segs-$different_text_segments
+    set execd_binfile $execr_binfile-execd
+
+    set execr_opts [list debug]
+    set execd_opts [list debug]
+
+    if { $different_text_segments } {
+       lappend execr_opts "ldflags=-Wl,-Ttext-segment=0x600000"
+       lappend execd_opts "ldflags=-Wl,-Ttext-segment=0x800000"
+    }
+
+    if { $execr_thread == "leader" } {
+       lappend execr_opts "additional_flags=-DLEADER_DOES_EXEC"
+    } elseif { $execr_thread == "other" } {
+       lappend execr_opts "additional_flags=-DOTHER_DOES_EXEC"
+    } else {
+       error "Invalid execr_thread value: $execr_thread."
+    }
+
+    # Compile execr binary (the one that does the exec).
+    if {[gdb_compile_pthreads $execr_srcs $execr_binfile executable $execr_opts] != "" } {
+       return -1
+    }
+
+    # Compile the second binary (the one that gets exec'd).
+    if {[gdb_compile $execd_srcs $execd_binfile executable $execd_opts] != "" } {
+       return -1
+    }
+
+    clean_restart ${execr_binfile}
+
+    gdb_test_no_output "set displaced-stepping $displaced_stepping"
+
+    if ![runto_main] {
+       return
+    }
+
+    # Leave breakpoint main inserted, we expect to hit it after exec.
+
+    # This breakpoint will be stepped by whatever thread does the exec.
+    gdb_test "break my_execve_syscall if 0" "Breakpoint $decimal at $hex.*"
+
+    # Continue across exec to main.
+    if { [target_is_gdbserver] } {
+       setup_kfail gdb/27020 "*-*-*"
+    }
+    set failed [gdb_test "continue" \
+                   "process $decimal is executing new program: .* hit Breakpoint $decimal, main .*" \
+                   "continue across exec"]
+    if { $failed } {
+       return
+    }
+
+    # Just to confirm we are indeed in the execd program.
+    gdb_test "print a_variable_in_execd" " = 1212"
+
+    # Continue execution to make sure we can step over the breakpoint on main.
+    # It would be nice to use gdb_continue_to_end to ensure the program can
+    # exit properly, but it hangs due to PR gdb/26995.
+    gdb_breakpoint foo
+    gdb_test "continue" "Breakpoint $decimal, foo .*" \
+       "continue to foo"
+}
+
+foreach_with_prefix displaced_stepping {auto off} {
+    foreach_with_prefix different_text_segments {true false} {
+       foreach_with_prefix execr_thread {leader other} {
+           do_test $execr_thread $different_text_segments $displaced_stepping
+       }
+    }
+}

diff --git a/gdb/testsuite/lib/my-syscalls.S b/gdb/testsuite/lib/my-syscalls.S
new file mode 100644 (file)
index 0000000..f75f7ec
--- /dev/null
+++ b/gdb/testsuite/lib/my-syscalls.S
@@ -0,0 +1,56 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2020 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/>.  */
+
+
+/* This file implements simple Linux syscall wrappers, to be used by tests that
+   need to know exactly where the syscall instructions are.  */
+
+#include <asm/unistd.h>
+
+/* int my_execve (const char *file, char *argv[], char *envp[]);  */
+
+.global my_execve
+my_execve:
+
+#if defined(__x86_64__)
+
+       mov $__NR_execve, %rax
+       /* rdi, rsi and rdx already contain the right arguments.  */
+my_execve_syscall:
+       syscall
+       ret
+
+#elif defined(__i386__)
+
+       mov $__NR_execve, %eax
+       mov 4(%esp), %ebx
+       mov 8(%esp), %ecx
+       mov 12(%esp), %edx
+my_execve_syscall:
+       int $0x80
+       ret
+
+#elif defined(__aarch64__)
+
+       mov x8, #__NR_execve
+       /* x0, x1 and x2 already contain the right arguments.  */
+my_execve_syscall:
+       svc #0
+
+#else
+# error "Unsupported architecture"
+#endif

diff --git a/gdb/testsuite/lib/my-syscalls.h b/gdb/testsuite/lib/my-syscalls.h
new file mode 100644 (file)
index 0000000..8a82780
--- /dev/null
+++ b/gdb/testsuite/lib/my-syscalls.h
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2020 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/>.  */
+
+#ifndef MY_SYSCALLS_H
+#define MY_SYSCALLS_H
+
+/* Declarations for syscall wrappers implemented in my-syscalls.S.  */
+
+int my_execve (const char *file, char *argv[], char *envp[]);
+
+#endif /* MY_SYSCALLS_H */