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 a2ff17da4fee277b82cb71de6b7ae32826de4a28..ee3a869e76bf65ebddba100522d394a614765cec 100644 (file)
--- 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 baadeb2f92bd5b9996b4041b14ac1975a12f2837..6d6b3ebf0a2febfdec6888675010c2364a31c9ad 100644 (file)
--- 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 (file)
index 0000000..aa1fc2a
--- /dev/null
+++ b/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;
+}

diff --git a/memcheck/tests/resvn_stack.stderr.exp b/memcheck/tests/resvn_stack.stderr.exp
new file mode 100644 (file)
index 0000000..ad75467
--- /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 (file)
index 0000000..7b13852
--- /dev/null
+++ b/memcheck/tests/resvn_stack.vgtest
@@ -0,0 +1,2 @@
+prog: resvn_stack
+vgopts: -q