Make --gdb-attach=yes work properly; rather than trying to attach gdb
to itself, Valgrind forks a child, and uses ptrace to manipulate its
state into what the client state would be at that point, and attaches
gdb to that. In addition to giving gdb clean state to inspect, it
also stops mistakes in gdb (eg, continuing) from killing your target.
It also makes gdb strictly read-only; any state changes made from within
gdb will not be reflected in the running client. Patch from Tom Hughes.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@2187 a5019735-40e9-0310-863c-91ae7b9d1cf9
diff --git a/coregrind/vg_errcontext.c b/coregrind/vg_errcontext.c
index c1935ef..892502f 100644
--- a/coregrind/vg_errcontext.c
+++ b/coregrind/vg_errcontext.c
@@ -253,21 +253,8 @@
if (allow_GDB_attach &&
VG_(is_action_requested)( "Attach to GDB", & VG_(clo_GDB_attach) ))
{
- Addr m_eip, m_esp, m_ebp;
-
- if (VG_(is_running_thread)( err->tid )) {
- m_eip = VG_(baseBlock)[VGOFF_(m_eip)];
- m_esp = VG_(baseBlock)[VGOFF_(m_esp)];
- m_ebp = VG_(baseBlock)[VGOFF_(m_ebp)];
- } else {
- ThreadState* tst = & VG_(threads)[ err->tid ];
- m_eip = tst->m_eip;
- m_esp = tst->m_esp;
- m_ebp = tst->m_ebp;
- }
- VG_(printf)("starting gdb with eip=%p esp=%p ebp=%p\n",
- m_eip, m_esp, m_ebp);
- VG_(swizzle_esp_then_start_GDB)( m_eip, m_esp, m_ebp );
+ VG_(printf)("starting gdb\n");
+ VG_(start_GDB)( err->tid );
}
/* Or maybe we want to generate the error's suppression? */
if (VG_(is_action_requested)( "Print suppression",
diff --git a/coregrind/vg_include.h b/coregrind/vg_include.h
index 4dc5b8c..36fc10a 100644
--- a/coregrind/vg_include.h
+++ b/coregrind/vg_include.h
@@ -1450,11 +1450,8 @@
which match pattern. */
extern void VG_(mash_colon_env)(Char *varp, const Char *pattern);
-/* Something of a function looking for a home ... start up GDB. This
- is called from VG_(swizzle_esp_then_start_GDB) and so runs on the
- *client's* stack. This is necessary to give GDB the illusion that
- the client program really was running on the real cpu. */
-extern void VG_(start_GDB_whilst_on_client_stack) ( void );
+/* Something of a function looking for a home ... start up GDB. */
+extern void VG_(start_GDB) ( Int tid );
/* VG_(bbs_done) in include/vg_skin.h */
@@ -1682,11 +1679,6 @@
extern void VG_(switch_to_real_CPU) ( void );
-extern void VG_(swizzle_esp_then_start_GDB) ( Addr m_eip_at_error,
- Addr m_esp_at_error,
- Addr m_ebp_at_error );
-
-
/* ---------------------------------------------------------------------
Exports of vg_dispatch.S
------------------------------------------------------------------ */
diff --git a/coregrind/vg_main.c b/coregrind/vg_main.c
index ee26d20..b1a74a4 100644
--- a/coregrind/vg_main.c
+++ b/coregrind/vg_main.c
@@ -31,6 +31,13 @@
#include "vg_include.h"
+#include <stdlib.h>
+#include <sys/ptrace.h>
+#include <sys/signal.h>
+#include <sys/user.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
/* ---------------------------------------------------------------------
Compute offsets into baseBlock. See comments in vg_include.h.
------------------------------------------------------------------ */
@@ -1745,27 +1752,86 @@
*output++ = '\0';
}
-/* RUNS ON THE CLIENT'S STACK, but on the real CPU. Start GDB and get
- it to attach to this process. Called if the user requests this
- service after an error has been shown, so she can poke around and
- look at parameters, memory, etc. You can't meaningfully get GDB to
- continue the program, though; to continue, quit GDB. */
-void VG_(start_GDB_whilst_on_client_stack) ( void )
+/* Start GDB and get it to attach to this process. Called if the user
+ requests this service after an error has been shown, so she can
+ poke around and look at parameters, memory, etc. You can't
+ meaningfully get GDB to continue the program, though; to continue,
+ quit GDB. */
+void VG_(start_GDB) ( Int tid )
{
- Int res;
- UChar buf[100];
+ Int pid;
- VG_(sprintf)(buf, "%s -nw /proc/%d/fd/%d %d",
- VG_(clo_GDB_path), VG_(getpid)(), VG_(clexecfd), VG_(getpid)());
- VG_(message)(Vg_UserMsg, "starting GDB with cmd: %s", buf);
- res = VG_(system)(buf);
- if (res == 0) {
- VG_(message)(Vg_UserMsg, "");
- VG_(message)(Vg_UserMsg,
- "GDB has detached. Valgrind regains control. We continue.");
- } else {
- VG_(message)(Vg_UserMsg, "Apparently failed!");
- VG_(message)(Vg_UserMsg, "");
+ if ((pid = fork()) == 0)
+ {
+ ptrace(PTRACE_TRACEME, 0, NULL, NULL);
+ VG_(kkill)(VG_(getpid)(), VKI_SIGSTOP);
+ }
+ else if (pid > 0)
+ {
+ struct user_regs_struct regs;
+ Int status;
+ Int res;
+
+ if (VG_(is_running_thread)( tid )) {
+ regs.xcs = VG_(baseBlock)[VGOFF_(m_cs)];
+ regs.xss = VG_(baseBlock)[VGOFF_(m_ss)];
+ regs.xds = VG_(baseBlock)[VGOFF_(m_ds)];
+ regs.xes = VG_(baseBlock)[VGOFF_(m_es)];
+ regs.xfs = VG_(baseBlock)[VGOFF_(m_fs)];
+ regs.xgs = VG_(baseBlock)[VGOFF_(m_gs)];
+ regs.eax = VG_(baseBlock)[VGOFF_(m_eax)];
+ regs.ebx = VG_(baseBlock)[VGOFF_(m_ebx)];
+ regs.ecx = VG_(baseBlock)[VGOFF_(m_ecx)];
+ regs.edx = VG_(baseBlock)[VGOFF_(m_edx)];
+ regs.esi = VG_(baseBlock)[VGOFF_(m_esi)];
+ regs.edi = VG_(baseBlock)[VGOFF_(m_edi)];
+ regs.ebp = VG_(baseBlock)[VGOFF_(m_ebp)];
+ regs.esp = VG_(baseBlock)[VGOFF_(m_esp)];
+ regs.eflags = VG_(baseBlock)[VGOFF_(m_eflags)];
+ regs.eip = VG_(baseBlock)[VGOFF_(m_eip)];
+ } else {
+ ThreadState* tst = & VG_(threads)[ tid ];
+
+ regs.xcs = tst->m_cs;
+ regs.xss = tst->m_ss;
+ regs.xds = tst->m_ds;
+ regs.xes = tst->m_es;
+ regs.xfs = tst->m_fs;
+ regs.xgs = tst->m_gs;
+ regs.eax = tst->m_eax;
+ regs.ebx = tst->m_ebx;
+ regs.ecx = tst->m_ecx;
+ regs.edx = tst->m_edx;
+ regs.esi = tst->m_esi;
+ regs.edi = tst->m_edi;
+ regs.ebp = tst->m_ebp;
+ regs.esp = tst->m_esp;
+ regs.eflags = tst->m_eflags;
+ regs.eip = tst->m_eip;
+ }
+
+ if ((res = VG_(waitpid)(pid, &status, 0)) == pid &&
+ WIFSTOPPED(status) && WSTOPSIG(status) == SIGSTOP &&
+ ptrace(PTRACE_SETREGS, pid, NULL, ®s) == 0 &&
+ ptrace(PTRACE_DETACH, pid, NULL, SIGSTOP) == 0) {
+ UChar buf[VG_(strlen)(VG_(clo_GDB_path)) + 100];
+
+ VG_(sprintf)(buf, "%s -nw /proc/%d/fd/%d %d",
+ VG_(clo_GDB_path), VG_(main_pid), VG_(clexecfd), pid);
+ VG_(message)(Vg_UserMsg, "starting GDB with cmd: %s", buf);
+ res = VG_(system)(buf);
+ if (res == 0) {
+ VG_(message)(Vg_UserMsg, "");
+ VG_(message)(Vg_UserMsg,
+ "GDB has detached. Valgrind regains control. We continue.");
+ } else {
+ VG_(message)(Vg_UserMsg, "Apparently failed!");
+ VG_(message)(Vg_UserMsg, "");
+ }
+ }
+
+ VG_(kkill)(pid, VKI_SIGKILL);
+ VG_(waitpid)(pid, &status, 0);
}
}
diff --git a/coregrind/vg_signals.c b/coregrind/vg_signals.c
index f8b510c..204ebea 100644
--- a/coregrind/vg_signals.c
+++ b/coregrind/vg_signals.c
@@ -1392,8 +1392,7 @@
}
if (VG_(is_action_requested)( "Attach to GDB", & VG_(clo_GDB_attach) )) {
- ThreadState* tst = & VG_(threads)[ tid ];
- VG_(swizzle_esp_then_start_GDB)( tst->m_eip, tst->m_esp, tst->m_ebp );
+ VG_(start_GDB)( tid );
}
if (VG_(fatal_signal_set)) {
diff --git a/coregrind/vg_startup.S b/coregrind/vg_startup.S
index b96c915..c2d60f6 100644
--- a/coregrind/vg_startup.S
+++ b/coregrind/vg_startup.S
@@ -80,88 +80,6 @@
-/*------------------------------------------------------------*/
-/*--- A function to temporarily copy %ESP/%EBP into ---*/
-/*--- %esp/%ebp and then start up GDB. ---*/
-/*------------------------------------------------------------*/
-
-/*
-extern void VG_(swizzle_esp_then_start_GDB) ( Addr m_eip_at_error,
- Addr m_esp_at_error,
- Addr m_ebp_at_error );
-*/
-
-/*--- This is clearly not re-entrant! ---*/
-.data
-vg_ebp_saved_over_GDB_start:
- .long 0
-vg_esp_saved_over_GDB_start:
- .long 0
-.text
-
-.type VG_(swizzle_esp_then_start_GDB),@function
-.global VG_(swizzle_esp_then_start_GDB)
-VG_(swizzle_esp_then_start_GDB):
-#ifdef HAVE_GAS_CFI
- .cfi_startproc
-#endif
- pushal
-
- # remember the simulators current stack/frame pointers
- movl %ebp, vg_ebp_saved_over_GDB_start
- movl %esp, vg_esp_saved_over_GDB_start
-
- # get args into regs
- movl 44(%esp), %eax # client %EBP
- movl 40(%esp), %ebx # client %ESP
- movl 36(%esp), %ecx # client %EIP
-
- # Now that we dont need to refer to simulators stack any more,
- # put %ESP into %esp
- movl %ebx, %esp
-
- ### %esp now refers to clients stack
- ### mess with the clients stack to make it look as if it
- ### called this procedure, since otherwise it will look to gdb
- ### as if the top (currently executing) stack frame of the
- ### client is missing.
-
- # push %EIP. This is a faked-up return address.
- pushl %ecx
-
- # push %EBP. This is a faked %ebp-chain pointer.
- pushl %eax
-#ifdef HAVE_GAS_CFI
- .cfi_adjust_cfa_offset 0x4
-#endif
-
- movl %esp, %ebp
-#ifdef HAVE_GAS_CFI
- .cfi_def_cfa_register ebp
-#endif
-
- call VG_(start_GDB_whilst_on_client_stack)
-
- # restore the simulators stack/frame pointer
- movl vg_ebp_saved_over_GDB_start, %ebp
- movl vg_esp_saved_over_GDB_start, %esp
-#ifdef HAVE_GAS_CFI
- .cfi_adjust_cfa_offset -0x4
-#endif
-
- popal
- ret
-#ifdef HAVE_GAS_CFI
- .cfi_endproc
-#endif
-
-# gcc puts this construction at the end of every function. I think it
-# allows the linker to figure out the size of the function. So we do
-# the same, in the vague hope that it might help GDBs navigation.
-.Lend_of_swizzle:
- .size VG_(swizzle_esp_then_start_GDB), .Lend_of_swizzle-VG_(swizzle_esp_then_start_GDB)
-
-
##--------------------------------------------------------------------##
##--- end vg_startup.S ---##
##--------------------------------------------------------------------##