From: Rhys Kidd Date: Sat, 7 Mar 2015 05:22:12 +0000 (+0000) Subject: Fix stack traces missing penultimate frame X-Git-Tag: svn/VALGRIND_3_11_0~607 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=2424a2d34767c39a96e9fa28394b75a01f91e086;p=thirdparty%2Fvalgrind.git 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 6abecc9bb8..b7d2680e18 100644 --- 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 45594ae94b..617f26c5b0 100644 --- 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 4aa0a03d23..db534043d2 100644 --- 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 f722f3768a..49dbc1ee84 100644 --- 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 5b216b6c71..b3d2508ac9 100644 --- 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 index 0000000000..9f12683708 --- /dev/null +++ b/memcheck/tests/darwin/deep_badparam.c @@ -0,0 +1,41 @@ +#include +#include +#include + +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 0000000000..a25bf282f3 --- /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 0000000000..4e4e493570 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 index 0000000000..86f5ee93c9 --- /dev/null +++ b/memcheck/tests/darwin/deep_badparam.vgtest @@ -0,0 +1,2 @@ +prog: deep_badparam +vgopts: -q