Make the GDB-attach stuff thread-aware, and work (at least partially)

when running multithreaded.  Can still cause crashes (assertion failures)
when GDB exits.  I think it that's due to my use of libc's system()
call; should roll my own.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@111

diff --git a/coregrind/arch/x86-linux/vg_libpthread.c b/coregrind/arch/x86-linux/vg_libpthread.c
index f5c53616089f1b19c4514dd08c43a265fe184c10..5b18a849876747a89bab9497cbf4519eff99c11e 100644 (file)
--- a/coregrind/arch/x86-linux/vg_libpthread.c
+++ b/coregrind/arch/x86-linux/vg_libpthread.c
@@ -233,7 +233,7 @@ static int thread_specific_errno[VG_N_THREADS];
 int* __errno_location ( void )
 {
    int tid;
-   ensure_valgrind("__errno_location");
+   /* ensure_valgrind("__errno_location"); */
    VALGRIND_MAGIC_SEQUENCE(tid, 0 /* default */,
                            VG_USERREQ__PTHREAD_GET_THREADID,
                            0, 0, 0, 0);

diff --git a/coregrind/vg_errcontext.c b/coregrind/vg_errcontext.c
index 3f44821456014c8f9eee26a98dea78e32b817efc..1e3297f91548713d382268cb2c7bb906f305cf9c 100644 (file)
--- a/coregrind/vg_errcontext.c
+++ b/coregrind/vg_errcontext.c
@@ -144,6 +144,14 @@ typedef
       Bool isWriteableLack;
       /* ALL */
       ThreadId tid;
+      /* ALL */
+      /* These record %EIP, %ESP and %EBP at the error point.  They
+         are only used to make GDB-attaching convenient; there is no
+         other purpose; specifically they are not used to do
+         comparisons between errors. */
+      UInt m_eip;
+      UInt m_esp;
+      UInt m_ebp;
    } 
    ErrContext;
 
@@ -192,6 +200,9 @@ static void clear_ErrContext ( ErrContext* ec )
    clear_AddrInfo ( &ec->addrinfo );
    ec->syscall_param   = NULL;
    ec->isWriteableLack = False;
+   ec->m_eip   = 0xDEADB00F;
+   ec->m_esp   = 0xDEADBE0F;
+   ec->m_ebp   = 0xDEADB0EF;
    ec->tid     = VG_INVALID_THREADID;
 }
 
@@ -531,7 +542,8 @@ static void VG_(maybe_add_context) ( ErrContext* ec )
       vg_n_errs_shown++;
       /* Perhaps we want a GDB attach at this point? */
       if (vg_is_GDB_attach_requested()) {
-         VG_(swizzle_esp_then_start_GDB)();
+         VG_(swizzle_esp_then_start_GDB)(
+            ec->m_eip, ec->m_esp, ec->m_ebp);
       }
    } else {
       vg_n_errs_suppressed++;
@@ -562,6 +574,9 @@ void VG_(record_value_error) ( Int size )
    ec.ekind = ValueErr;
    ec.size  = size;
    ec.tid   = VG_(get_current_tid)();
+   ec.m_eip = VG_(baseBlock)[VGOFF_(m_eip)];
+   ec.m_esp = VG_(baseBlock)[VGOFF_(m_esp)];
+   ec.m_ebp = VG_(baseBlock)[VGOFF_(m_ebp)];
    VG_(maybe_add_context) ( &ec );
 }
 
@@ -585,6 +600,9 @@ void VG_(record_address_error) ( Addr a, Int size, Bool isWrite )
    ec.size    = size;
    ec.addr    = a;
    ec.tid     = VG_(get_current_tid)();
+   ec.m_eip = VG_(baseBlock)[VGOFF_(m_eip)];
+   ec.m_esp = VG_(baseBlock)[VGOFF_(m_esp)];
+   ec.m_ebp = VG_(baseBlock)[VGOFF_(m_ebp)];
    VG_(describe_addr) ( a, &ec.addrinfo );
    VG_(maybe_add_context) ( &ec );
 }
@@ -604,6 +622,9 @@ void VG_(record_free_error) ( ThreadState* tst, Addr a )
    ec.ekind   = FreeErr;
    ec.addr    = a;
    ec.tid     = tst->tid;
+   ec.m_eip   = tst->m_eip;
+   ec.m_esp   = tst->m_esp;
+   ec.m_ebp   = tst->m_ebp;
    VG_(describe_addr) ( a, &ec.addrinfo );
    VG_(maybe_add_context) ( &ec );
 }
@@ -618,6 +639,9 @@ void VG_(record_freemismatch_error) ( ThreadState* tst, Addr a )
    ec.ekind   = FreeMismatchErr;
    ec.addr    = a;
    ec.tid     = tst->tid;
+   ec.m_eip   = tst->m_eip;
+   ec.m_esp   = tst->m_esp;
+   ec.m_ebp   = tst->m_ebp;
    VG_(describe_addr) ( a, &ec.addrinfo );
    VG_(maybe_add_context) ( &ec );
 }
@@ -633,6 +657,9 @@ void VG_(record_jump_error) ( ThreadState* tst, Addr a )
    ec.axskind = ExecAxs;
    ec.addr    = a;
    ec.tid     = tst->tid;
+   ec.m_eip   = tst->m_eip;
+   ec.m_esp   = tst->m_esp;
+   ec.m_ebp   = tst->m_ebp;
    VG_(describe_addr) ( a, &ec.addrinfo );
    VG_(maybe_add_context) ( &ec );
 }
@@ -648,6 +675,9 @@ void VG_(record_param_err) ( ThreadState* tst, Addr a, Bool isWriteLack,
    ec.ekind   = ParamErr;
    ec.addr    = a;
    ec.tid     = tst->tid;
+   ec.m_eip   = tst->m_eip;
+   ec.m_esp   = tst->m_esp;
+   ec.m_ebp   = tst->m_ebp;
    VG_(describe_addr) ( a, &ec.addrinfo );
    ec.syscall_param = msg;
    ec.isWriteableLack = isWriteLack;
@@ -664,6 +694,9 @@ void VG_(record_user_err) ( ThreadState* tst, Addr a, Bool isWriteLack )
    ec.ekind   = UserErr;
    ec.addr    = a;
    ec.tid     = tst->tid;
+   ec.m_eip   = tst->m_eip;
+   ec.m_esp   = tst->m_esp;
+   ec.m_ebp   = tst->m_ebp;
    VG_(describe_addr) ( a, &ec.addrinfo );
    ec.isWriteableLack = isWriteLack;
    VG_(maybe_add_context) ( &ec );

diff --git a/coregrind/vg_include.h b/coregrind/vg_include.h
index 09bafb0318dc9fd60d8e87c84c01be4d7124ce57..ee7adfae1158404be78fa2168593dc006b98a043 100644 (file)
--- a/coregrind/vg_include.h
+++ b/coregrind/vg_include.h
@@ -1490,7 +1490,9 @@ extern void VG_(do_syscall) ( void );
 extern void VG_(shutdown);
 extern void VG_(switch_to_real_CPU) ( void );
 
-extern void VG_(swizzle_esp_then_start_GDB) ( void );
+extern void VG_(swizzle_esp_then_start_GDB) ( Addr m_eip_at_error,
+                                              Addr m_esp_at_error,
+                                              Addr m_ebp_at_error );
 
 
 /* ---------------------------------------------------------------------

diff --git a/coregrind/vg_libpthread.c b/coregrind/vg_libpthread.c
index f5c53616089f1b19c4514dd08c43a265fe184c10..5b18a849876747a89bab9497cbf4519eff99c11e 100644 (file)
--- a/coregrind/vg_libpthread.c
+++ b/coregrind/vg_libpthread.c
@@ -233,7 +233,7 @@ static int thread_specific_errno[VG_N_THREADS];
 int* __errno_location ( void )
 {
    int tid;
-   ensure_valgrind("__errno_location");
+   /* ensure_valgrind("__errno_location"); */
    VALGRIND_MAGIC_SEQUENCE(tid, 0 /* default */,
                            VG_USERREQ__PTHREAD_GET_THREADID,
                            0, 0, 0, 0);

diff --git a/coregrind/vg_startup.S b/coregrind/vg_startup.S
index 1215e6870c43ec80fa5ecc487f73f087e26d5f56..1a3b60fcb0f8c08855abfe7886ea433e924cd443 100644 (file)
--- a/coregrind/vg_startup.S
+++ b/coregrind/vg_startup.S
@@ -175,6 +175,12 @@ VG_(switch_to_real_CPU):
 /*--- %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:
@@ -190,12 +196,15 @@ VG_(swizzle_esp_then_start_GDB):
        # remember the simulators current stack/frame pointers
        movl    %ebp, vg_ebp_saved_over_GDB_start
        movl    %esp, vg_esp_saved_over_GDB_start
-       
-       movl    $VG_(baseBlock), %ebx
 
-       # fetch %ESP into %esp
-       movl    VGOFF_(m_esp), %esi
-       movl    (%ebx, %esi, 4), %esp
+       # get args into regs
+       movl    44(%esp), %eax          # client %EBP
+       movl    40(%esp), %ebx          # client %ESP
+       movl    36(%esp), %ecx          # client %EIP
+
+       # Now thatn we don't 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
@@ -203,14 +212,10 @@ VG_(swizzle_esp_then_start_GDB):
        ### as if the top (currently executing) stack frame of the
        ### client is missing.
        
-       # push %EIP, via %eax.  This is a faked-up return address.
-       movl    VGOFF_(m_eip), %esi
-       movl    (%ebx, %esi, 4), %eax
-       pushl   %eax
+       # push %EIP.  This is a faked-up return address.
+       pushl   %ecx
 
-       # push %EBP, via %eax.  This is a faked %ebp-chain pointer.
-       movl    VGOFF_(m_ebp), %esi
-       movl    (%ebx, %esi, 4), %eax
+       # push %EBP.  This is a faked %ebp-chain pointer.
        pushl   %eax
 
        movl    %esp, %ebp

diff --git a/vg_errcontext.c b/vg_errcontext.c
index 3f44821456014c8f9eee26a98dea78e32b817efc..1e3297f91548713d382268cb2c7bb906f305cf9c 100644 (file)
--- a/vg_errcontext.c
+++ b/vg_errcontext.c
@@ -144,6 +144,14 @@ typedef
       Bool isWriteableLack;
       /* ALL */
       ThreadId tid;
+      /* ALL */
+      /* These record %EIP, %ESP and %EBP at the error point.  They
+         are only used to make GDB-attaching convenient; there is no
+         other purpose; specifically they are not used to do
+         comparisons between errors. */
+      UInt m_eip;
+      UInt m_esp;
+      UInt m_ebp;
    } 
    ErrContext;
 
@@ -192,6 +200,9 @@ static void clear_ErrContext ( ErrContext* ec )
    clear_AddrInfo ( &ec->addrinfo );
    ec->syscall_param   = NULL;
    ec->isWriteableLack = False;
+   ec->m_eip   = 0xDEADB00F;
+   ec->m_esp   = 0xDEADBE0F;
+   ec->m_ebp   = 0xDEADB0EF;
    ec->tid     = VG_INVALID_THREADID;
 }
 
@@ -531,7 +542,8 @@ static void VG_(maybe_add_context) ( ErrContext* ec )
       vg_n_errs_shown++;
       /* Perhaps we want a GDB attach at this point? */
       if (vg_is_GDB_attach_requested()) {
-         VG_(swizzle_esp_then_start_GDB)();
+         VG_(swizzle_esp_then_start_GDB)(
+            ec->m_eip, ec->m_esp, ec->m_ebp);
       }
    } else {
       vg_n_errs_suppressed++;
@@ -562,6 +574,9 @@ void VG_(record_value_error) ( Int size )
    ec.ekind = ValueErr;
    ec.size  = size;
    ec.tid   = VG_(get_current_tid)();
+   ec.m_eip = VG_(baseBlock)[VGOFF_(m_eip)];
+   ec.m_esp = VG_(baseBlock)[VGOFF_(m_esp)];
+   ec.m_ebp = VG_(baseBlock)[VGOFF_(m_ebp)];
    VG_(maybe_add_context) ( &ec );
 }
 
@@ -585,6 +600,9 @@ void VG_(record_address_error) ( Addr a, Int size, Bool isWrite )
    ec.size    = size;
    ec.addr    = a;
    ec.tid     = VG_(get_current_tid)();
+   ec.m_eip = VG_(baseBlock)[VGOFF_(m_eip)];
+   ec.m_esp = VG_(baseBlock)[VGOFF_(m_esp)];
+   ec.m_ebp = VG_(baseBlock)[VGOFF_(m_ebp)];
    VG_(describe_addr) ( a, &ec.addrinfo );
    VG_(maybe_add_context) ( &ec );
 }
@@ -604,6 +622,9 @@ void VG_(record_free_error) ( ThreadState* tst, Addr a )
    ec.ekind   = FreeErr;
    ec.addr    = a;
    ec.tid     = tst->tid;
+   ec.m_eip   = tst->m_eip;
+   ec.m_esp   = tst->m_esp;
+   ec.m_ebp   = tst->m_ebp;
    VG_(describe_addr) ( a, &ec.addrinfo );
    VG_(maybe_add_context) ( &ec );
 }
@@ -618,6 +639,9 @@ void VG_(record_freemismatch_error) ( ThreadState* tst, Addr a )
    ec.ekind   = FreeMismatchErr;
    ec.addr    = a;
    ec.tid     = tst->tid;
+   ec.m_eip   = tst->m_eip;
+   ec.m_esp   = tst->m_esp;
+   ec.m_ebp   = tst->m_ebp;
    VG_(describe_addr) ( a, &ec.addrinfo );
    VG_(maybe_add_context) ( &ec );
 }
@@ -633,6 +657,9 @@ void VG_(record_jump_error) ( ThreadState* tst, Addr a )
    ec.axskind = ExecAxs;
    ec.addr    = a;
    ec.tid     = tst->tid;
+   ec.m_eip   = tst->m_eip;
+   ec.m_esp   = tst->m_esp;
+   ec.m_ebp   = tst->m_ebp;
    VG_(describe_addr) ( a, &ec.addrinfo );
    VG_(maybe_add_context) ( &ec );
 }
@@ -648,6 +675,9 @@ void VG_(record_param_err) ( ThreadState* tst, Addr a, Bool isWriteLack,
    ec.ekind   = ParamErr;
    ec.addr    = a;
    ec.tid     = tst->tid;
+   ec.m_eip   = tst->m_eip;
+   ec.m_esp   = tst->m_esp;
+   ec.m_ebp   = tst->m_ebp;
    VG_(describe_addr) ( a, &ec.addrinfo );
    ec.syscall_param = msg;
    ec.isWriteableLack = isWriteLack;
@@ -664,6 +694,9 @@ void VG_(record_user_err) ( ThreadState* tst, Addr a, Bool isWriteLack )
    ec.ekind   = UserErr;
    ec.addr    = a;
    ec.tid     = tst->tid;
+   ec.m_eip   = tst->m_eip;
+   ec.m_esp   = tst->m_esp;
+   ec.m_ebp   = tst->m_ebp;
    VG_(describe_addr) ( a, &ec.addrinfo );
    ec.isWriteableLack = isWriteLack;
    VG_(maybe_add_context) ( &ec );

diff --git a/vg_include.h b/vg_include.h
index 09bafb0318dc9fd60d8e87c84c01be4d7124ce57..ee7adfae1158404be78fa2168593dc006b98a043 100644 (file)
--- a/vg_include.h
+++ b/vg_include.h
@@ -1490,7 +1490,9 @@ extern void VG_(do_syscall) ( void );
 extern void VG_(shutdown);
 extern void VG_(switch_to_real_CPU) ( void );
 
-extern void VG_(swizzle_esp_then_start_GDB) ( void );
+extern void VG_(swizzle_esp_then_start_GDB) ( Addr m_eip_at_error,
+                                              Addr m_esp_at_error,
+                                              Addr m_ebp_at_error );
 
 
 /* ---------------------------------------------------------------------

diff --git a/vg_libpthread.c b/vg_libpthread.c
index f5c53616089f1b19c4514dd08c43a265fe184c10..5b18a849876747a89bab9497cbf4519eff99c11e 100644 (file)
--- a/vg_libpthread.c
+++ b/vg_libpthread.c
@@ -233,7 +233,7 @@ static int thread_specific_errno[VG_N_THREADS];
 int* __errno_location ( void )
 {
    int tid;
-   ensure_valgrind("__errno_location");
+   /* ensure_valgrind("__errno_location"); */
    VALGRIND_MAGIC_SEQUENCE(tid, 0 /* default */,
                            VG_USERREQ__PTHREAD_GET_THREADID,
                            0, 0, 0, 0);

diff --git a/vg_startup.S b/vg_startup.S
index 1215e6870c43ec80fa5ecc487f73f087e26d5f56..1a3b60fcb0f8c08855abfe7886ea433e924cd443 100644 (file)
--- a/vg_startup.S
+++ b/vg_startup.S
@@ -175,6 +175,12 @@ VG_(switch_to_real_CPU):
 /*--- %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:
@@ -190,12 +196,15 @@ VG_(swizzle_esp_then_start_GDB):
        # remember the simulators current stack/frame pointers
        movl    %ebp, vg_ebp_saved_over_GDB_start
        movl    %esp, vg_esp_saved_over_GDB_start
-       
-       movl    $VG_(baseBlock), %ebx
 
-       # fetch %ESP into %esp
-       movl    VGOFF_(m_esp), %esi
-       movl    (%ebx, %esi, 4), %esp
+       # get args into regs
+       movl    44(%esp), %eax          # client %EBP
+       movl    40(%esp), %ebx          # client %ESP
+       movl    36(%esp), %ecx          # client %EIP
+
+       # Now thatn we don't 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
@@ -203,14 +212,10 @@ VG_(swizzle_esp_then_start_GDB):
        ### as if the top (currently executing) stack frame of the
        ### client is missing.
        
-       # push %EIP, via %eax.  This is a faked-up return address.
-       movl    VGOFF_(m_eip), %esi
-       movl    (%ebx, %esi, 4), %eax
-       pushl   %eax
+       # push %EIP.  This is a faked-up return address.
+       pushl   %ecx
 
-       # push %EBP, via %eax.  This is a faked %ebp-chain pointer.
-       movl    VGOFF_(m_ebp), %esi
-       movl    (%ebx, %esi, 4), %eax
+       # push %EBP.  This is a faked %ebp-chain pointer.
        pushl   %eax
 
        movl    %esp, %ebp