Fix for bug 91162: cope with jumps to bogus addresses when there is a SEGV

signal handler present -- previously, Valgrind would abort unnecessarily on
this case.

Added a regression test for it.

MERGE TO STABLE

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

diff --git a/coregrind/core.h b/coregrind/core.h
index 0084d8daa51e4afd2fc8daab9990777a2ddff587..bc75b09f7441ff37dcfb46f4128685edd2f12fc9 100644 (file)
--- a/coregrind/core.h
+++ b/coregrind/core.h
@@ -1128,7 +1128,7 @@ struct _UCodeBlock {
    Int     nextTemp;
 };
 
-extern void VG_(translate)  ( ThreadId tid, Addr orig_addr, Bool debugging );
+extern Bool VG_(translate)  ( ThreadId tid, Addr orig_addr, Bool debugging );
 
 extern void VG_(sanity_check_UInstr) ( UInt n, UInstr* u );
 

diff --git a/coregrind/vg_scheduler.c b/coregrind/vg_scheduler.c
index 7c8acdcaca07122f2b161f549d053b8e41276a32..7de3f220fc0a46b59e15249f364c9da13dbcf48f 100644 (file)
--- a/coregrind/vg_scheduler.c
+++ b/coregrind/vg_scheduler.c
@@ -857,20 +857,29 @@ VgSchedReturnCode do_scheduler ( Int* exitcode, ThreadId* last_run_tid )
             thread. */
 
          if (trc == VG_TRC_INNER_FASTMISS) {
+            Addr ip = ARCH_INSTR_PTR(VG_(threads)[tid].arch);
+
             vg_assert(VG_(dispatch_ctr) > 0);
 
             /* Trivial event.  Miss in the fast-cache.  Do a full
                lookup for it. */
-            trans_addr = VG_(search_transtab) 
-                              ( ARCH_INSTR_PTR(VG_(threads)[tid].arch) );
+            trans_addr = VG_(search_transtab)( ip );
             if (trans_addr == (Addr)0) {
                /* Not found; we need to request a translation. */
-               VG_(translate)( tid, ARCH_INSTR_PTR(VG_(threads)[tid].arch),
-                               /*debug*/False ); 
-               trans_addr = VG_(search_transtab) 
-                                 ( ARCH_INSTR_PTR(VG_(threads)[tid].arch) ); 
-               if (trans_addr == (Addr)0)
-                  VG_(core_panic)("VG_TRC_INNER_FASTMISS: missing tt_fast entry");
+               if (VG_(translate)( tid, ip, /*debug*/False )) {
+                  trans_addr = VG_(search_transtab)( ip ); 
+                  if (trans_addr == (Addr)0)
+                     VG_(core_panic)("VG_TRC_INNER_FASTMISS: missing tt_fast entry");
+               } else {
+                  // If VG_(translate)() fails, it's because it had to throw
+                  // a signal because the client jumped to a bad address.
+                  // This means VG_(deliver_signal)() will have been called
+                  // by now, and the program counter will now be pointing to
+                  // the start of the signal handler (if there is no
+                  // handler, things would have been aborted by now), so do
+                  // nothing, and things will work out next time around the
+                  // scheduler loop.
+               }
             }
             continue; /* with this thread */
          }

diff --git a/coregrind/vg_translate.c b/coregrind/vg_translate.c
index b88c1f8a27b40c5d48034bebc026b60c58aa0bca..54918b3ca996769a0104c6a3e13cd4678cf80aaa 100644 (file)
--- a/coregrind/vg_translate.c
+++ b/coregrind/vg_translate.c
@@ -2428,7 +2428,7 @@ static void vg_realreg_liveness_analysis ( UCodeBlock* cb )
 
    'tid' is the identity of the thread needing this block.
 */
-void VG_(translate) ( ThreadId tid, Addr orig_addr,
+Bool VG_(translate) ( ThreadId tid, Addr orig_addr,
                       Bool debugging_translation )
 {
    Addr        trans_addr, redir, orig_addr0 = orig_addr;
@@ -2484,7 +2484,7 @@ void VG_(translate) ( ThreadId tid, Addr orig_addr,
       } else
         VG_(synth_fault_mapping)(tid, orig_addr);
 
-      return;
+      return False;
    } else
       seg->flags |= SF_CODE;   /* contains cached code */
 
@@ -2583,6 +2583,8 @@ void VG_(translate) ( ThreadId tid, Addr orig_addr,
    VG_(arena_free)( VG_AR_JITTER, (void*)trans_addr );
 
    VGP_POPCC(VgpTranslate);
+
+   return True;
 }
 
 

diff --git a/memcheck/tests/.cvsignore b/memcheck/tests/.cvsignore
index 9a654e8cac3c0439474ef86002c3ed2544df0d1e..8f5de619ddb1084f7893fa07f317c635ea38cde1 100644 (file)
--- a/memcheck/tests/.cvsignore
+++ b/memcheck/tests/.cvsignore
@@ -3,6 +3,7 @@ Makefile
 badaddrvalue
 badfree
 badjump
+badjump2
 badloop
 buflen_check
 clientperm

diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am
index a8fccb8c5520b057ce4330ad2bf62bde7b94f1ae..7c2727e16e19780c2e2d757b1d91ef1f1720559a 100644 (file)
--- a/memcheck/tests/Makefile.am
+++ b/memcheck/tests/Makefile.am
@@ -15,6 +15,7 @@ EXTRA_DIST = $(noinst_SCRIPTS) \
        badfree-2trace.stderr.exp badfree-2trace.vgtest \
        badfree.stderr.exp badfree.vgtest \
        badjump.stderr.exp badjump.vgtest \
+       badjump2.stderr.exp badjump2.vgtest \
        badloop.stderr.exp badloop.vgtest \
        badrw.stderr.exp badrw.vgtest \
        brk.stderr.exp brk.vgtest \
@@ -78,7 +79,8 @@ EXTRA_DIST = $(noinst_SCRIPTS) \
        zeropage.stderr.exp zeropage.vgtest
 
 check_PROGRAMS = \
-       badaddrvalue badfree badjump badloop badrw brk brk2 buflen_check \
+       badaddrvalue badfree badjump badjump2 \
+       badloop badrw brk brk2 buflen_check \
        clientperm custom_alloc \
        doublefree error_counts errs1 exitprog execve execve2 \
        fpeflags fprw fwrite inits inline \
@@ -98,6 +100,7 @@ AM_CXXFLAGS = $(AM_CFLAGS)
 badaddrvalue_SOURCES   = badaddrvalue.c
 badfree_SOURCES        = badfree.c
 badjump_SOURCES        = badjump.c
+badjump2_SOURCES       = badjump2.c
 badloop_SOURCES        = badloop.c
 badrw_SOURCES          = badrw.c
 brk_SOURCES            = brk.c

diff --git a/memcheck/tests/badjump2.c b/memcheck/tests/badjump2.c
new file mode 100644 (file)
index 0000000..361966b
--- /dev/null
+++ b/memcheck/tests/badjump2.c
@@ -0,0 +1,45 @@
+#include <assert.h>
+#include <setjmp.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+// Regression test for bug 91162:  if a client had a SEGV signal handler,
+// and jumped to a bogus address, Valgrind would abort.  With the fix,
+// the following test runs to completion correctly.
+
+static jmp_buf myjmpbuf;
+
+static
+void SIGSEGV_handler(int signum)
+{
+   __builtin_longjmp(myjmpbuf, 1);
+}
+
+int main(void)
+{
+   struct sigaction sigsegv_new, sigsegv_saved;
+   int res;
+
+   /* Install own SIGSEGV handler */
+   sigsegv_new.sa_handler  = SIGSEGV_handler;
+   sigsegv_new.sa_flags    = 0;
+   sigsegv_new.sa_restorer = NULL;
+   res = sigemptyset( &sigsegv_new.sa_mask );
+   assert(res == 0);
+
+   res = sigaction( SIGSEGV, &sigsegv_new, &sigsegv_saved );
+   assert(res == 0);
+
+   if (__builtin_setjmp(myjmpbuf) == 0) {
+      // Jump to zero; will cause seg fault
+      void (*fn)(void) = 0;
+      fn();
+      fprintf(stderr, "Got here??\n");
+   } else  {
+      fprintf(stderr, "Signal caught, as expected\n");
+   }
+
+   return 0;
+}
+

diff --git a/memcheck/tests/badjump2.stderr.exp b/memcheck/tests/badjump2.stderr.exp
new file mode 100644 (file)
index 0000000..04db2d9
--- /dev/null
+++ b/memcheck/tests/badjump2.stderr.exp
@@ -0,0 +1,6 @@
+Jump to the invalid address stated on the next line
+   at 0x........: ???
+   by 0x........: __libc_start_main (...libc...)
+   by 0x........: ...
+ Address 0x........ is not stack'd, malloc'd or (recently) free'd
+Signal caught, as expected

diff --git a/memcheck/tests/badjump2.vgtest b/memcheck/tests/badjump2.vgtest
new file mode 100644 (file)
index 0000000..4256086
--- /dev/null
+++ b/memcheck/tests/badjump2.vgtest
@@ -0,0 +1,2 @@
+prog: badjump2
+vgopts: -q