Fix stack traces missing penultimate frame

bz#344560
- Also fixes memcheck/tests/badpoll test on OS X
- Problem occurs because the guest stack seen in a system call pre or post
  function happens to not have a correct topmost stack frame, as Darwin system
  call stubs do not start with the usual function prolog.
- New regression test case added.
- Thanks to Greg Banks for research, patch and test case.

Before:

== 587 tests, 240 stderr failures, 22 stdout failures, 0 stderrB failures, 0 stdoutB failures, 31 post failures ==

After:

== 588 tests, 239 stderr failures, 22 stdout failures, 0 stderrB failures, 0 stdoutB failures, 31 post failures ==

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

diff --git a/NEWS b/NEWS
index 6abecc9bb856077307954196a383aff805a43906..b7d2680e1833a5c07705614c48305441a3bc584b 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -115,6 +115,7 @@ where XXXXXX is the bug number as listed below.
 344499  Fix compilation for Linux kernel >= 4. With this, also require
         a Linux kernel >= 2.6 as 2.4 is mostly untested and might trigger
         obvious and non-obvious issues
+344560  Fix stack traces missing penultimate frame on OS X
 344621  Fix memcheck/tests/err_disable4 test on OS X
 344686  Fix suppression for pthread_rwlock_init on OS X 10.10
 344702  Fix missing libobjc suppressions on OS X 10.10

diff --git a/coregrind/m_stacktrace.c b/coregrind/m_stacktrace.c
index 45594ae94b2d506fc6b25f64cdb0b7fddbf9a0dd..617f26c5b0afbb8986370eb90e82f0ac72ef506d 100644 (file)
--- a/coregrind/m_stacktrace.c
+++ b/coregrind/m_stacktrace.c
@@ -515,6 +515,23 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
    if (fps) fps[0] = uregs.xbp;
    i = 1;
 
+#  if defined(VGO_darwin)
+   if (VG_(is_valid_tid)(tid_if_known) &&
+      VG_(is_in_syscall)(tid_if_known) &&
+      i < max_n_ips) {
+      /* On Darwin, all the system call stubs have no function
+       * prolog.  So instead of top of the stack being a new
+       * frame comprising a saved BP and a return address, we
+       * just have the return address in the caller's frame.
+       * Adjust for this by recording the return address.
+       */
+      ips[i] = *(Addr *)uregs.xsp - 1;
+      if (sps) sps[i] = uregs.xsp;
+      if (fps) fps[i] = uregs.xbp;
+      i++;
+   }
+#  endif
+       
    /* Loop unwinding the stack. Note that the IP value we get on
     * each pass (whether from CFI info or a stack frame) is a
     * return address so is actually after the calling instruction

diff --git a/coregrind/m_syswrap/syswrap-main.c b/coregrind/m_syswrap/syswrap-main.c
index 4aa0a03d2320c300c1fa1ae54c9d48e84283941d..db534043d21f9c2b44e0efb2a505a710cc71ee91 100644 (file)
--- a/coregrind/m_syswrap/syswrap-main.c
+++ b/coregrind/m_syswrap/syswrap-main.c
@@ -1377,6 +1377,12 @@ void VG_(clear_syscallInfo) ( Int tid )
    syscallInfo[tid].status.what = SsIdle;
 }
 
+Bool VG_(is_in_syscall) ( Int tid )
+{
+   vg_assert(tid >= 0 && tid < VG_N_THREADS);
+   return (syscallInfo[tid].status.what != SsIdle);
+}
+
 static void ensure_initialised ( void )
 {
    Int i;

diff --git a/coregrind/pub_core_syswrap.h b/coregrind/pub_core_syswrap.h
index f722f3768a2aace456147ce21a64500c2ee52502..49dbc1ee848d09f026f040cf91593a9031a4c122 100644 (file)
--- a/coregrind/pub_core_syswrap.h
+++ b/coregrind/pub_core_syswrap.h
@@ -50,6 +50,9 @@ extern void VG_(post_syscall)   ( ThreadId tid );
 /* Clear this module's private state for thread 'tid' */
 extern void VG_(clear_syscallInfo) ( Int tid );
 
+// Returns True if the given thread is currently in a system call
+extern Bool VG_(is_in_syscall) ( Int tid );
+
 // Fix up a thread's state when syscall is interrupted by a signal.
 extern void VG_(fixup_guest_state_after_syscall_interrupted)(
                ThreadId tid,

diff --git a/memcheck/tests/darwin/Makefile.am b/memcheck/tests/darwin/Makefile.am
index 5b216b6c71801344782f13232646813323282a48..b3d2508ac9b4d40250a1f48aa5a5cc69d14278a8 100644 (file)
--- a/memcheck/tests/darwin/Makefile.am
+++ b/memcheck/tests/darwin/Makefile.am
@@ -7,6 +7,7 @@ noinst_HEADERS = scalar.h
 
 EXTRA_DIST = \
        aio.stderr.exp aio.vgtest \
+       deep_badparam.stderr.exp deep_badparam.stdout.exp deep_badparam.vgtest \
        env.stderr.exp env.vgtest \
        pth-supp.stderr.exp pth-supp.vgtest \
        scalar.stderr.exp scalar.vgtest \
@@ -16,6 +17,7 @@ EXTRA_DIST = \
 
 check_PROGRAMS = \
        aio \
+       deep_badparam \
        env \
        pth-supp \
        scalar \

diff --git a/memcheck/tests/darwin/deep_badparam.c b/memcheck/tests/darwin/deep_badparam.c
new file mode 100644 (file)
index 0000000..9f12683
--- /dev/null
+++ b/memcheck/tests/darwin/deep_badparam.c
@@ -0,0 +1,41 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+int func_six(int x)
+{
+    char b[32];
+    int r = write(1, b, sizeof(b));
+    return x;
+}
+
+int func_five(int x)
+{
+    return func_six(x + 5);
+}
+
+int func_four(int x)
+{
+    return func_five(x + 4);
+}
+
+int func_three(int x)
+{
+    return func_four(x + 3);
+}
+
+int func_two(int x)
+{
+    return func_three(x + 2);
+}
+
+int func_one(int x)
+{
+    return func_two(x + 1);
+}
+
+int main(void)
+{
+    func_one(10);
+    return 0;
+}

diff --git a/memcheck/tests/darwin/deep_badparam.stderr.exp b/memcheck/tests/darwin/deep_badparam.stderr.exp
new file mode 100644 (file)
index 0000000..a25bf28
--- /dev/null
+++ b/memcheck/tests/darwin/deep_badparam.stderr.exp
@@ -0,0 +1,12 @@
+Syscall param write(buf) points to uninitialised byte(s)
+   ...
+   by 0x........: func_six (in ./deep_badparam)
+   by 0x........: func_five (in ./deep_badparam)
+   by 0x........: func_four (in ./deep_badparam)
+   by 0x........: func_three (in ./deep_badparam)
+   by 0x........: func_two (in ./deep_badparam)
+   by 0x........: func_one (in ./deep_badparam)
+   by 0x........: main (in ./deep_badparam)
+ Address 0x........ is on thread 1's stack
+ in frame #1, created by func_six (???:)
+

diff --git a/memcheck/tests/darwin/deep_badparam.stdout.exp b/memcheck/tests/darwin/deep_badparam.stdout.exp
new file mode 100644 (file)
index 0000000..4e4e493
Binary files /dev/null and b/memcheck/tests/darwin/deep_badparam.stdout.exp differ

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