From: Philippe Waroquiers Date: Sat, 28 Mar 2015 12:52:23 +0000 (+0000) Subject: Extensible main thread stack is tricky :(. X-Git-Tag: svn/VALGRIND_3_11_0~546 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=275c1897dc94ecaa63200c987a11e2677cfa5aec;p=thirdparty%2Fvalgrind.git Extensible main thread stack is tricky :(. Revision 14976 causes a regression : stacktrace produced when the stack has not yet been extended to cover SP will only contain one element, as the stack limits are considered to be the limits of the resvn segment. This patch fixes that, by taking Resvn/SmUpper segment into account to properly compute the limits. It also contains a new regtest that fails with the trunk (only one function in the stacktrace) and succeeds with this patch (the 2 expected functions). git-svn-id: svn://svn.valgrind.org/valgrind/trunk@15046 --- diff --git a/coregrind/m_stacks.c b/coregrind/m_stacks.c index a2ff17da4f..ee3a869e76 100644 --- a/coregrind/m_stacks.c +++ b/coregrind/m_stacks.c @@ -277,49 +277,72 @@ void VG_(stack_limits)(Addr SP, Addr *start, Addr *end ) *end = stack->end; } - /* SP is assumed to be in a RW segment. + /* SP is assumed to be in a RW segment or in the SkResvn segment of an + extensible stack (normally, only the main thread has an extensible + stack 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. */ + Otherwise, possibly reduce the stack limits using the boundaries of + the RW segment/SkResvn segments 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) { + return; + } + + if ((!stackseg->hasR || !stackseg->hasW) + && (stackseg->kind != SkResvn || stackseg->smode != SmUpper)) { VG_(debugLog)(2, "stacks", - "segment for SP %p is not Readable and/or not Writable\n", + "segment for SP %p is not RW or not a SmUpper Resvn\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; - } + return; + } + + // SP is in a RW segment, or in the SkResvn of an extensible stack. + 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 reducing start and/or end to the SP segment gives an - empty range, return 'empty' limits */ - if (*start > *end) { + if (stackseg->kind == SkResvn) { + stackseg = VG_(am_next_nsegment)(stackseg, /*forward*/ True); + if (!stackseg || !stackseg->hasR || !stackseg->hasW + || stackseg->kind != SkAnonC) { VG_(debugLog)(2, "stacks", - "stack for SP %p start %p after end %p\n", - (void*)SP, (void*)*start, (void*)end); + "Next forward segment for SP %p Resvn segment" + " is not RW or not AnonC\n", + (void*)SP); *start = 0; *end = 0; + return; } } + + 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 diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am index baadeb2f92..6d6b3ebf0a 100644 --- a/memcheck/tests/Makefile.am +++ b/memcheck/tests/Makefile.am @@ -214,6 +214,7 @@ EXTRA_DIST = \ realloc2.stderr.exp realloc2.vgtest \ realloc3.stderr.exp realloc3.vgtest \ recursive-merge.stderr.exp recursive-merge.vgtest \ + resvn_stack.stderr.exp resvn_stack.vgtest \ sbfragment.stdout.exp sbfragment.stderr.exp sbfragment.vgtest \ sem.stderr.exp sem.vgtest \ sendmsg.stderr.exp sendmsg.vgtest \ @@ -340,6 +341,7 @@ check_PROGRAMS = \ post-syscall \ realloc1 realloc2 realloc3 \ recursive-merge \ + resvn_stack \ sbfragment \ sendmsg \ sh-mem sh-mem-random \ diff --git a/memcheck/tests/resvn_stack.c b/memcheck/tests/resvn_stack.c new file mode 100644 index 0000000000..aa1fc2af84 --- /dev/null +++ b/memcheck/tests/resvn_stack.c @@ -0,0 +1,23 @@ +#include + +__attribute__((noinline)) void big(void) +{ + /* The below ensures the stack grows a lot. However, we hope the stack + extension is not done yet, as no memory has been read/written. */ + volatile char c[200000]; + + /* Access only the higher part of the stack, to avoid mapping SP */ + /* The below 2 printfs should produce deterministic output, whatever + the random value of c[]. */ + if (c[200000 - 1]) + fprintf(stderr, "Accessing fresh %s\n", "stack"); + else + fprintf(stderr, "Accessing %s stack\n", "fresh"); + +} + +int main(void ) +{ + big(); + return 0; +} diff --git a/memcheck/tests/resvn_stack.stderr.exp b/memcheck/tests/resvn_stack.stderr.exp new file mode 100644 index 0000000000..ad75467ae1 --- /dev/null +++ b/memcheck/tests/resvn_stack.stderr.exp @@ -0,0 +1,5 @@ +Conditional jump or move depends on uninitialised value(s) + at 0x........: big (resvn_stack.c:12) + by 0x........: main (resvn_stack.c:21) + +Accessing fresh stack diff --git a/memcheck/tests/resvn_stack.vgtest b/memcheck/tests/resvn_stack.vgtest new file mode 100644 index 0000000000..7b13852976 --- /dev/null +++ b/memcheck/tests/resvn_stack.vgtest @@ -0,0 +1,2 @@ +prog: resvn_stack +vgopts: -q