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 a5019735-40e9-0310-863c-91ae7b9d1cf9
diff --git a/NEWS b/NEWS
index 6abecc9..b7d2680 100644
--- a/NEWS
+++ b/NEWS
@@ -115,6 +115,7 @@
 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 45594ae..617f26c 100644
--- a/coregrind/m_stacktrace.c
+++ b/coregrind/m_stacktrace.c
@@ -515,6 +515,23 @@
    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 4aa0a03..db53404 100644
--- a/coregrind/m_syswrap/syswrap-main.c
+++ b/coregrind/m_syswrap/syswrap-main.c
@@ -1377,6 +1377,12 @@
    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 f722f37..49dbc1e 100644
--- a/coregrind/pub_core_syswrap.h
+++ b/coregrind/pub_core_syswrap.h
@@ -50,6 +50,9 @@
 /* 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 5b216b6..b3d2508 100644
--- a/memcheck/tests/darwin/Makefile.am
+++ b/memcheck/tests/darwin/Makefile.am
@@ -7,6 +7,7 @@
 
 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 @@
 
 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
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
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
index 0000000..4e4e493
--- /dev/null
+++ b/memcheck/tests/darwin/deep_badparam.stdout.exp
Binary files differ
diff --git a/memcheck/tests/darwin/deep_badparam.vgtest b/memcheck/tests/darwin/deep_badparam.vgtest
new file mode 100644
index 0000000..86f5ee9
--- /dev/null
+++ b/memcheck/tests/darwin/deep_badparam.vgtest
@@ -0,0 +1,2 @@
+prog: deep_badparam
+vgopts: -q