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

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;
+   } 
 
-      /* If reducing start and/or end to the SP segment gives an
-         empty range, return 'empty' limits */
-      if (*start > *end) {
+   // 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 (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

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 \

memcheck/tests/resvn_stack.c

@@ -0,0 +1,23 @@
+#include <stdio.h>
+
+__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;
+}

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

memcheck/tests/resvn_stack.vgtest

@@ -0,0 +1,2 @@
+prog: resvn_stack
+vgopts: -q