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
This commit is contained in:
Simon Marchi
2020-12-04 16:43:52 -05:00
parent 42a4fec513
commit 3b7a962dec
8 changed files with 343 additions and 0 deletions

View File

@@ -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