Author: philippe
Date: Tue Mar 3 22:00:06 2015
New Revision: 14976
Log:
Fix 343173 - helgrind crash during stack unwind
This fixes a helgrind crash detected on android.
Android bionic pthread lib unmaps the stack for detached threads
before exiting.
Helgrind tries to unwind the stack to record a 'read' after
the stack unmap, just before the exit syscall.
The unwind then causes a SEGV.
The solution consists in tightening the calculation of
the stack limits, so as to stop unwinding when no valid stack
can be found.
Regression test reproduces the same problem by simulating the
bionic behaviour on linux, using asm similar to bionic lib.
Added:
trunk/helgrind/tests/stackteardown.c
trunk/helgrind/tests/stackteardown.stderr.exp
trunk/helgrind/tests/stackteardown.stdout.exp
trunk/helgrind/tests/stackteardown.vgtest
Modified:
trunk/NEWS
trunk/coregrind/m_stacks.c
trunk/helgrind/tests/Makefile.am
Modified: trunk/NEWS
==============================================================================
--- trunk/NEWS (original)
+++ trunk/NEWS Tue Mar 3 22:00:06 2015
@@ -88,6 +88,7 @@
342795 Internal glibc __GI_mempcpy call should be intercepted
343012 Unhandled syscall 319 (memfd_create)
343069 Patch updating v4l2 API support
+343173 helgrind crash during stack unwind
343303 Fix known deliberate memory leak in setenv() on Mac OS X 10.10
343306 OS X 10.10: UNKNOWN mach_msg unhandled MACH_SEND_TRAILER option
343332 Unhandled instruction 0x9E310021 (fcvtmu) on aarch64
Modified: trunk/coregrind/m_stacks.c
==============================================================================
--- trunk/coregrind/m_stacks.c (original)
+++ trunk/coregrind/m_stacks.c Tue Mar 3 22:00:06 2015
@@ -33,6 +33,7 @@
#include "pub_core_libcassert.h"
#include "pub_core_libcprint.h"
#include "pub_core_mallocfree.h"
+#include "pub_core_aspacemgr.h"
#include "pub_core_options.h"
#include "pub_core_stacks.h"
#include "pub_core_tooliface.h"
@@ -202,7 +203,7 @@
current_stack = i;
}
- VG_(debugLog)(2, "stacks", "register [%p-%p] as stack %lu\n",
+ VG_(debugLog)(2, "stacks", "register [start-end] [%p-%p] as stack %lu\n",
(void*)start, (void*)end, i->id);
return i->id;
@@ -269,11 +270,56 @@
void VG_(stack_limits)(Addr SP, Addr *start, Addr *end )
{
Stack* stack = find_stack_by_addr(SP);
+ NSegment const *stackseg = VG_(am_find_nsegment) (SP);
if (stack) {
*start = stack->start;
*end = stack->end;
}
+
+ /* SP is assumed to be in a RW segment.
+ If no such segment is found, assume we have no valid
+ stack for SP, and set *start and *end to 0.
+ Otherwise, possibly reduce the stack limits to the boundaries of the
+ RW segment containing SP. */
+ if (stackseg == NULL) {
+ VG_(debugLog)(2, "stacks",
+ "no addressable segment for SP %p\n",
+ (void*)SP);
+ *start = 0;
+ *end = 0;
+ } else if (!stackseg->hasR || !stackseg->hasW) {
+ VG_(debugLog)(2, "stacks",
+ "segment for SP %p is not Readable and/or not Writable\n",
+ (void*)SP);
+ *start = 0;
+ *end = 0;
+ } else {
+ if (*start < stackseg->start) {
+ VG_(debugLog)(2, "stacks",
+ "segment for SP %p changed stack start limit"
+ " from %p to %p\n",
+ (void*)SP, (void*)*start, (void*)stackseg->start);
+ *start = stackseg->start;
+ }
+ if (*end > stackseg->end) {
+ VG_(debugLog)(2, "stacks",
+ "segment for SP %p changed stack end limit"
+ " from %p to %p\n",
+ (void*)SP, (void*)*end, (void*)stackseg->end);
+ *end = stackseg->end;
+ }
+
+ /* If reducing start and/or end to the SP segment gives an
+ empty range, return 'empty' limits */
+ if (*start > *end) {
+ VG_(debugLog)(2, "stacks",
+ "stack for SP %p start %p after end %p\n",
+ (void*)SP, (void*)*start, (void*)end);
+ *start = 0;
+ *end = 0;
+ }
+ }
}
/* complaints_stack_switch reports that SP has changed by more than some
Modified: trunk/helgrind/tests/Makefile.am
==============================================================================
--- trunk/helgrind/tests/Makefile.am (original)
+++ trunk/helgrind/tests/Makefile.am Tue Mar 3 22:00:06 2015
@@ -49,6 +49,7 @@
pth_spinlock.vgtest pth_spinlock.stdout.exp pth_spinlock.stderr.exp \
rwlock_race.vgtest rwlock_race.stdout.exp rwlock_race.stderr.exp \
rwlock_test.vgtest rwlock_test.stdout.exp rwlock_test.stderr.exp \
+ stackteardown.vgtest stackteardown.stdout.exp stackteardown.stderr.exp \
t2t_laog.vgtest t2t_laog.stdout.exp t2t_laog.stderr.exp \
tc01_simple_race.vgtest tc01_simple_race.stdout.exp \
tc01_simple_race.stderr.exp \
@@ -124,6 +125,7 @@
locked_vs_unlocked2 \
locked_vs_unlocked3 \
pth_destroy_cond \
+ stackteardown \
t2t \
tc01_simple_race \
tc02_simple_tls \
Added: trunk/helgrind/tests/stackteardown.c
==============================================================================
--- trunk/helgrind/tests/stackteardown.c (added)
+++ trunk/helgrind/tests/stackteardown.c Tue Mar 3 22:00:06 2015
@@ -0,0 +1,104 @@
+#include <sys/mman.h>
+#include <pthread.h>
+#include <unistd.h>
+#include <assert.h>
+#include <unistd.h>
+#include <sys/syscall.h>
+#include "../../config.h"
+
+#define VG_STRINGIFZ(__str) #__str
+#define VG_STRINGIFY(__str) VG_STRINGIFZ(__str)
+
+extern void _exit_with_stack_teardown(void*, size_t);
+
+/* Below code is modified version of android bionic
+ pthread_exit: when a detached thread exits: it munmaps
+ its stack and then exits. We cannot do that in C,
+ as we cannot touch the stack after the munmap
+ and before the exit. */
+
+#if defined(VGP_x86_linux)
+asm("\n"
+ ".text\n"
+ "\t.globl _exit_with_stack_teardown\n"
+ "\t.type _exit_with_stack_teardown,@function\n"
+ "_exit_with_stack_teardown:\n"
+ // We can trash registers because this function never returns.
+ "\tmov 4(%esp), %ebx\n" // stackBase
+ "\tmov 8(%esp), %ecx\n" // stackSize
+ "\tmov $"VG_STRINGIFY(__NR_munmap)", %eax\n"
+ "\tint $0x80\n"
+ // If munmap failed, we ignore the failure and exit anyway.
+
+ "\tmov $0, %ebx\n" // status
+ "\tmovl $"VG_STRINGIFY(__NR_exit)", %eax\n"
+ "\tint $0x80\n");
+ // The exit syscall does not return.
+
+#elif defined(VGP_amd64_linux)
+asm("\n"
+ ".text\n"
+ "\t.globl _exit_with_stack_teardown\n"
+ "\t.type _exit_with_stack_teardown,@function\n"
+ "_exit_with_stack_teardown:\n"
+ "\tmov $"VG_STRINGIFY(__NR_munmap)", %eax\n"
+ "\tsyscall\n"
+ // If munmap failed, we ignore the failure and exit anyway.
+ "\tmov $0, %rdi\n"
+ "\tmov $"VG_STRINGIFY(__NR_exit)", %eax\n"
+ "\tsyscall\n");
+ // The exit syscall does not return.
+
+#elif defined(VGP_arm_linux)
+asm("\n"
+ ".text\n"
+ "\t.globl _exit_with_stack_teardown\n"
+ "\t.type _exit_with_stack_teardown,%function\n"
+ "_exit_with_stack_teardown:\n"
+ "\tldr r7, ="VG_STRINGIFY(__NR_munmap)"\n"
+ "\tswi #0\n"
+ // If munmap failed, we ignore the failure and exit anyway.
+
+ "\tmov r0, #0\n"
+ "\tldr r7, ="VG_STRINGIFY(__NR_exit)"\n"
+ "\tswi #0\n");
+ // The exit syscall does not return.
+
+#else
+void _exit_with_stack_teardown(void*stack, size_t sz)
+{
+ // asm code not done for this platform.
+ // Do nothing, just return. The thread will exit spontaneously
+}
+
+#endif
+static void *stack;
+static size_t sz = 64 * 1024;
+
+/* This one detaches, does its own thing. */
+void* child_fn ( void* arg )
+{
+ int r;
+ r= pthread_detach( pthread_self() ); assert(!r);
+ _exit_with_stack_teardown(stack, sz);
+ return NULL;
+}
+
+/* Parent creates 1 child, that will detach, and exit after destroying
+ its own stack. */
+int main ( void )
+{
+ int r;
+ pthread_attr_t attr;
+ pthread_t child;
+
+ r = pthread_attr_init(&attr); assert(!r);
+ stack = mmap(NULL, sz, PROT_READ|PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS,
+ -1, 0);
+ assert(stack != (void *)-1);
+ r = pthread_attr_setstack(&attr, stack, sz);
+ r = pthread_create(&child, &attr, child_fn, NULL); assert(!r);
+ sleep(1);
+
+ return 0;
+}
Added: trunk/helgrind/tests/stackteardown.stderr.exp
==============================================================================
--- trunk/helgrind/tests/stackteardown.stderr.exp (added)
+++ trunk/helgrind/tests/stackteardown.stderr.exp Tue Mar 3 22:00:06 2015
@@ -0,0 +1,3 @@
+
+
+ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Added: trunk/helgrind/tests/stackteardown.stdout.exp
==============================================================================
(empty)
Added: trunk/helgrind/tests/stackteardown.vgtest
==============================================================================
--- trunk/helgrind/tests/stackteardown.vgtest (added)
+++ trunk/helgrind/tests/stackteardown.vgtest Tue Mar 3 22:00:06 2015
@@ -0,0 +1 @@
+prog: stackteardown
|