From 3b7a962dec0d5d852ad5f1338add07781adef7b4 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Fri, 4 Dec 2020 16:43:52 -0500 Subject: [PATCH] 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 --- gdb/ChangeLog | 5 + gdb/infrun.c | 16 +++ gdb/testsuite/ChangeLog | 8 ++ .../gdb.threads/step-over-exec-execd.c | 30 +++++ gdb/testsuite/gdb.threads/step-over-exec.c | 90 ++++++++++++++ gdb/testsuite/gdb.threads/step-over-exec.exp | 113 ++++++++++++++++++ gdb/testsuite/lib/my-syscalls.S | 56 +++++++++ gdb/testsuite/lib/my-syscalls.h | 25 ++++ 8 files changed, 343 insertions(+) create mode 100644 gdb/testsuite/gdb.threads/step-over-exec-execd.c create mode 100644 gdb/testsuite/gdb.threads/step-over-exec.c create mode 100644 gdb/testsuite/gdb.threads/step-over-exec.exp create mode 100644 gdb/testsuite/lib/my-syscalls.S create mode 100644 gdb/testsuite/lib/my-syscalls.h diff --git a/gdb/ChangeLog b/gdb/ChangeLog index d7bfae85345..8ce239f4b35 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2020-12-04 Simon Marchi + + * infrun.c (infrun_inferior_execd): New function. + (_initialize_infrun): Attach inferior_execd observer. + 2020-12-04 Simon Marchi * observable.h (inferior_execd): Declare new observable. diff --git a/gdb/infrun.c b/gdb/infrun.c index 473a0fb8542..0e0a7a691c4 100644 --- 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 5de67b4fe9a..d3350bcc506 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,11 @@ +2020-12-04 Simon Marchi + + * 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 * 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 index 00000000000..5f3835c1b29 --- /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 . */ + +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 index 00000000000..a043e8d2535 --- /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 . */ + +#include +#include +#include +#include +#include + +#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 index 00000000000..411980cfc70 --- /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 . + +# 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 index 00000000000..f75f7ec5b59 --- /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 . */ + + +/* This file implements simple Linux syscall wrappers, to be used by tests that + need to know exactly where the syscall instructions are. */ + +#include + +/* 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 index 00000000000..8a82780e35b --- /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 . */ + +#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 */ -- 2.30.2