Improve description of an address that is on a stack but below sp.

An address below the sp will be described as being on a stack, but below sp.

The stack for such an address is found in the registered stacks.

Also, if there is a guard page at the end of the stack (lowest address)
an address in this page will be described as being in thread guard page.
A guard page is recognised as being a page not readable/writable/executable.



git-svn-id: svn://svn.valgrind.org/valgrind/trunk@14399

coregrind/m_addrinfo.c

@@ -40,11 +40,53 @@
 #include "pub_core_mallocfree.h"
 #include "pub_core_machine.h"
 #include "pub_core_options.h"
+#include "pub_core_threadstate.h"
 #include "pub_core_stacktrace.h"
+#include "pub_core_stacks.h"
+#include "pub_core_aspacemgr.h"
+
+/* Returns the tid whose stack includes the address a.
+   If not found, returns VG_INVALID_THREADID. */
+static ThreadId find_tid_with_stack_containing (Addr a)
+{
+   ThreadId tid;
+   Addr start, end;
+
+   start = 0;
+   end = 0;
+   VG_(stack_limits)(a, &start, &end);
+   if (start == end) {
+      // No stack found
+      vg_assert (start == 0 && end == 0);
+      return VG_INVALID_THREADID;
+   }
+
+   /* Stack limits found. Search the tid to which this stack belongs. */
+   vg_assert (start <= a);
+   vg_assert (a <= end);
+
+   /* The stack end (highest accessible byte) is for sure inside the 'active'
+      part of the stack of the searched tid.
+      So, scan all 'active' stacks with VG_(thread_stack_reset_iter) ... */
+   {
+      Addr       stack_min, stack_max;
+
+      VG_(thread_stack_reset_iter)(&tid);
+      while ( VG_(thread_stack_next)(&tid, &stack_min, &stack_max) ) {
+         if (stack_min <= end && end <= stack_max)
+            return tid;
+      }
+   }
+
+   /* We can arrive here if a stack was registered with wrong bounds
+      (e.g. end above the highest addressable byte)
+      and/or if the thread for the registered stack is dead, but
+      the stack was not unregistered. */
+   return VG_INVALID_THREADID;
+}
 
 void VG_(describe_addr) ( Addr a, /*OUT*/AddrInfo* ai )
 {
-   ThreadId   tid;
    VgSectKind sect;
 
    /* -- Perhaps the variable type/location data describes it? -- */
@@ -95,6 +137,7 @@ void VG_(describe_addr) ( Addr a, /*OUT*/AddrInfo* ai )
    }
    /* -- Perhaps it's on a thread's stack? -- */
    {
+      ThreadId   tid;
       Addr       stack_min, stack_max;
       VG_(thread_stack_reset_iter)(&tid);
       while ( VG_(thread_stack_next)(&tid, &stack_min, &stack_max) ) {
@@ -109,6 +152,8 @@ void VG_(describe_addr) ( Addr a, /*OUT*/AddrInfo* ai )
             ai->Addr.Stack.tinfo.tid = tid;
             ai->Addr.Stack.IP = 0;
             ai->Addr.Stack.frameNo = -1;
+            ai->Addr.Stack.stackPos = StackPos_stacked;
+            ai->Addr.Stack.spoffset = 0; // Unused.
             /* It is on thread tid stack. Build a stacktrace, and
                find the frame sp[f] .. sp[f+1] where the address is.
                Store the found frameNo and the corresponding IP in
@@ -174,6 +219,50 @@ void VG_(describe_addr) ( Addr a, /*OUT*/AddrInfo* ai )
                     [ sizeof(ai->Addr.SectKind.objname)-1 ] == 0);
       return;
    }
+
+   /* -- and yet another last ditch attempt at classification -- */
+   /* If the address is in a stack between the stack bottom (highest byte)
+      and the current stack ptr, it will have been already described above.
+      But maybe it is in a stack, but below the stack ptr (typical
+      for a 'use after return' or in the stack guard page (thread stack
+      too small). */
+   {
+      ThreadId   tid;
+      StackPos stackPos;
+
+      // First try to find a tid with stack containing a
+      tid = find_tid_with_stack_containing (a);
+      if (tid != VG_INVALID_THREADID) {
+         /* Should be below stack pointer, as if it is >= SP, it
+            will have been described as StackPos_stacked above. */
+         stackPos = StackPos_below_stack_ptr;
+      } else {
+         /* Try to find a stack with guard page containing a.
+            For this, check if a is in a page mapped without r, w and x. */
+         const NSegment *seg = VG_(am_find_nsegment) (a);
+         if (seg != NULL && seg->kind == SkAnonC
+             && !seg->hasR && !seg->hasW && !seg->hasX) {
+            /* This looks a plausible guard page. Check if a is close to
+               the start of stack (lowest byte). */
+            tid = find_tid_with_stack_containing (VG_PGROUNDUP(a+1));
+            if (tid != VG_INVALID_THREADID)
+               stackPos = StackPos_guard_page;
+         }
+      }
+
+      if (tid != VG_INVALID_THREADID) {
+         ai->tag  = Addr_Stack;
+         VG_(initThreadInfo)(&ai->Addr.Stack.tinfo);
+         ai->Addr.Stack.tinfo.tid = tid;
+         ai->Addr.Stack.IP = 0;
+         ai->Addr.Stack.frameNo = -1;
+         ai->Addr.Stack.stackPos = stackPos;
+         vg_assert (a < VG_(get_SP)(tid));
+         ai->Addr.Stack.spoffset = a - VG_(get_SP)(tid);
+         return;
+      }
+   }
+
    /* -- Clueless ... -- */
    ai->tag = Addr_Unknown;
    return;
@@ -318,6 +407,23 @@ static void pp_addrinfo_WRK ( Addr a, AddrInfo* ai, Bool mc, Bool maybe_gcc )
                           xpost );
 #undef      FLEN
          }
+         switch (ai->Addr.Stack.stackPos) {
+            case StackPos_stacked: break; // nothing more to say
+
+            case StackPos_below_stack_ptr:
+            case StackPos_guard_page:
+                VG_(emit)("%s%s%ld bytes below stack pointer%s\n",
+                          xpre, 
+                          ai->Addr.Stack.stackPos == StackPos_guard_page ?
+                          "In stack guard protected page, " : "",
+                          - ai->Addr.Stack.spoffset,
+                          xpost);
+                // Note: we change the sign of spoffset as the message speaks
+                // about the nr of bytes below stack pointer.
+                break;
+
+            default: vg_assert(0);
+         }
          break;
 
       case Addr_Block: {

include/pub_tool_addrinfo.h

@@ -75,6 +75,16 @@ typedef
    }
    AddrTag;
 
+/* For an address in a stack, gives the address position in this stack. */
+typedef 
+   enum {
+      StackPos_stacked,         // Addressable and 'active' stack zone.
+      StackPos_below_stack_ptr, // Below stack ptr
+      StackPos_guard_page       // In the guard page
+   }
+   StackPos;
+
+
 /* Note about ThreadInfo tid and tnr in various parts of _Addrinfo:
    A tid is an index in the VG_(threads)[] array. The entries
    in  VG_(threads) array are re-used, so the tid in an 'old' _Addrinfo
@@ -116,10 +126,16 @@ struct _AddrInfo {
       // stack address was. 0 if not found.
       // frameNo is the frame nr of the call where the stack address was.
       // -1 if not found.
+      // stackPos describes the address 'position' in the stack.
+      // If stackPos is StackPos_below_stack_ptr or StackPos_guard_page,
+      // spoffset gives the offset from the thread stack pointer.
+      // (spoffset will be negative, as stacks are assumed growing down).
       struct {
          ThreadInfo tinfo;
          Addr     IP;
          Int      frameNo;
+         StackPos stackPos;
+         PtrdiffT spoffset;
       } Stack;
 
       // This covers heap blocks (normal and from mempools), user-defined

memcheck/tests/Makefile.am

@@ -97,6 +97,7 @@ EXTRA_DIST = \
 	deep_templates.stdout.exp deep_templates.stderr.exp \
 	demangle.stderr.exp demangle.vgtest \
 	describe-block.stderr.exp describe-block.vgtest \
+	descr_belowsp.vgtest descr_belowsp.stderr.exp \
 	doublefree.stderr.exp doublefree.vgtest \
 	dw4.vgtest dw4.stderr.exp dw4.stdout.exp \
 	err_disable1.vgtest err_disable1.stderr.exp \
@@ -299,6 +300,7 @@ check_PROGRAMS = \
 	clireq_nofill \
 	clo_redzone \
 	cond_ld_st \
+	descr_belowsp \
 	leak_cpp_interior \
 	custom_alloc \
 	custom-overlap \
@@ -402,6 +404,7 @@ demangle_SOURCES = demangle.cpp
 
 dw4_CFLAGS		= $(AM_CFLAGS) -gdwarf-4 -fdebug-types-section
 
+descr_belowsp_LDADD     = -lpthread
 err_disable3_LDADD 	= -lpthread
 err_disable4_LDADD 	= -lpthread
 reach_thread_register_CFLAGS	= $(AM_CFLAGS) -O2

memcheck/tests/descr_belowsp.c

@@ -0,0 +1,175 @@
+#include "../../config.h"
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <pthread.h>
+#include <string.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include <assert.h>
+#include <setjmp.h>
+#include <signal.h>
+#ifdef HAVE_GETPAGESIZE
+#include <unistd.h>
+#endif
+#include "../../include/valgrind.h"
+#include "../memcheck.h"
+
+typedef  unsigned long   UWord;
+typedef  UWord           Addr;
+#define VG_ROUNDDN(p, a)   ((Addr)(p) & ~((Addr)(a)-1))
+#define VG_ROUNDUP(p, a)   VG_ROUNDDN((p)+(a)-1, (a))
+
+static pthread_t children;
+
+// If != 0, will test addr description does not explode with
+// wrong stack registration.
+static int shake_with_wrong_registration = 0;
+
+/* Do whatever to have the stack grown enough that
+   we can access below sp relatively safely */
+static void grow_the_stack(void)
+{
+   int i;
+   char m[5000];
+   for (i = 0; i < sizeof(m); i++)
+      m[i] = i;
+   sprintf(m, "do whatever %d", i);
+   if (strlen(m) > 1000)
+      fprintf(stderr, "something went wrong with %s\n", m);
+}
+
+static char s[1000];
+static void describe (char* what, void* a)
+{
+   fprintf(stderr, "describing %p %s\n", a, what);
+   sprintf(s, "v.info location %p", a);
+   VALGRIND_MONITOR_COMMAND(s);
+}
+
+static void bad_things_below_sp (void)
+{
+   int i;
+   char *p = (char*)&i;
+   describe ("1500 bytes below a local var", p-1500);
+}
+
+
+static volatile char *lowest_j;
+static jmp_buf goback;
+
+static void sigsegv_handler(int signr)
+{
+   longjmp(goback, 1);
+}
+
+static void bad_things_till_guard_page(void)
+{
+   char j = 0;
+   char *p = &j;
+
+   for (;;) {
+      j = j + *p;
+      p = p - 400;
+      lowest_j = p;
+   }
+}
+
+static int guess_pagesize(void)
+{
+#ifdef HAVE_GETPAGESIZE
+   const int pagesize = getpagesize();
+#else
+   const int pagesize = 4096; // let's say ?
+#endif
+   return pagesize;
+}
+
+static void describe_many(void)
+{
+   const int pagesize = guess_pagesize();
+   describe ("discovered address giving SEGV in thread stack",
+             (void*)lowest_j);
+   describe ("byte just above highest guardpage byte",
+             (void*) VG_ROUNDUP(lowest_j, pagesize));
+   describe ("highest guardpage byte",
+             (void*) VG_ROUNDUP(lowest_j, pagesize)-1);
+   describe ("lowest guardpage byte",
+             (void*) VG_ROUNDDN(lowest_j, pagesize));
+   /* Cannot test the next byte, as we cannot predict how
+      this byte will be described. */
+}
+
+static void* child_fn_0 ( void* arg )
+{
+   grow_the_stack();
+   bad_things_below_sp();
+
+   if (setjmp(goback)) {
+      describe_many();
+   } else
+      bad_things_till_guard_page();
+
+   if (shake_with_wrong_registration) {
+      // Do whatever stupid things we could imagine
+      // with stack registration and see no explosion happens
+      // Note: this is executed only if an arg is given to the program.
+      // 
+      
+      const int pgsz = guess_pagesize();
+      int stackid;
+
+      fprintf(stderr, "\n\nShaking after unregistering stack\n");
+      // Assuming our first stack was automatically registered as nr 1
+      VALGRIND_STACK_DEREGISTER(1);
+      // Test with no stack registered
+      describe_many();
+
+      fprintf(stderr, "\n\nShaking with small stack\n");
+      stackid = VALGRIND_STACK_REGISTER((void*) VG_ROUNDDN(&stackid, pgsz),
+                                        (void*) VG_ROUNDUP(&stackid, pgsz));
+      describe_many();
+      VALGRIND_STACK_DEREGISTER(stackid);
+
+      fprintf(stderr, "\n\nShaking with huge stack\n");
+      stackid = VALGRIND_STACK_REGISTER((void*) 0x0,
+                                        (void*) VG_ROUNDUP(&stackid, 2<<20));
+      describe_many();
+      VALGRIND_STACK_DEREGISTER(stackid);
+
+
+   }
+
+   return NULL;
+}
+
+int main(int argc, const char** argv)
+{
+   struct sigaction sa;
+   int r;
+
+   shake_with_wrong_registration = argc > 1;
+
+   /* We will discover the thread guard page using SEGV.
+      So, prepare an handler. */
+   sa.sa_handler = sigsegv_handler;
+   sigemptyset(&sa.sa_mask);
+   sa.sa_flags = 0;
+
+   if (sigaction (SIGSEGV, &sa, NULL) != 0)
+      perror("sigaction");
+
+   grow_the_stack();
+   bad_things_below_sp();
+   
+   r = pthread_create(&children, NULL, child_fn_0, NULL);
+   assert(!r);
+   
+   r = pthread_join(children, NULL);
+   assert(!r);
+   
+   
+   return 0;
+}
+

memcheck/tests/descr_belowsp.stderr.exp

@@ -0,0 +1,26 @@
+describing 0x........ 1500 bytes below a local var
+ Address 0x........ is on thread 1's stack
+ .... bytes below stack pointer
+describing 0x........ 1500 bytes below a local var
+ Address 0x........ is on thread 2's stack
+ .... bytes below stack pointer
+Thread 2:
+Invalid read of size 1
+   at 0x........: bad_things_till_guard_page (descr_belowsp.c:73)
+   by 0x........: child_fn_0 (descr_belowsp.c:112)
+   ...
+ Address 0x........ is on thread 2's stack
+ .... bytes below stack pointer
+
+describing 0x........ discovered address giving SEGV in thread stack
+ Address 0x........ is on thread 2's stack
+ In stack guard protected page, .... bytes below stack pointer
+describing 0x........ byte just above highest guardpage byte
+ Address 0x........ is on thread 2's stack
+ .... bytes below stack pointer
+describing 0x........ highest guardpage byte
+ Address 0x........ is on thread 2's stack
+ In stack guard protected page, .... bytes below stack pointer
+describing 0x........ lowest guardpage byte
+ Address 0x........ is on thread 2's stack
+ In stack guard protected page, .... bytes below stack pointer

memcheck/tests/descr_belowsp.vgtest

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

tests/filter_stderr_basic

@@ -58,6 +58,9 @@ sed "s/\(signal [0-9]* (SIG[A-Z]*)\): dumping core/\1/" |
 # Remove the size in "The main thread stack size..." message.
 sed "s/The main thread stack size used in this run was [0-9]*/The main thread stack size used in this run was .../" |
 
+# Remove the size in "10482464 bytes below stack pointer" message.
+sed "s/[0-9][0-9]* bytes below stack pointer/.... bytes below stack pointer/" |
+
 # Suppress warnings from incompatible debug info
 sed '/warning: the debug information found in "[^"]*" does not match/d' |