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 a5019735-40e9-0310-863c-91ae7b9d1cf9
diff --git a/coregrind/m_addrinfo.c b/coregrind/m_addrinfo.c
index 072b782..af52be4 100644
--- a/coregrind/m_addrinfo.c
+++ b/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 @@
}
/* -- 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 @@
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 @@
[ 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 @@
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: {
diff --git a/include/pub_tool_addrinfo.h b/include/pub_tool_addrinfo.h
index a27a064..b07e476 100644
--- a/include/pub_tool_addrinfo.h
+++ b/include/pub_tool_addrinfo.h
@@ -75,6 +75,16 @@
}
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 @@
// 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
diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am
index 9e2760e..72dd61b 100644
--- a/memcheck/tests/Makefile.am
+++ b/memcheck/tests/Makefile.am
@@ -97,6 +97,7 @@
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 @@
clireq_nofill \
clo_redzone \
cond_ld_st \
+ descr_belowsp \
leak_cpp_interior \
custom_alloc \
custom-overlap \
@@ -402,6 +404,7 @@
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
diff --git a/memcheck/tests/descr_belowsp.c b/memcheck/tests/descr_belowsp.c
new file mode 100644
index 0000000..704c222
--- /dev/null
+++ b/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;
+}
+
diff --git a/memcheck/tests/descr_belowsp.stderr.exp b/memcheck/tests/descr_belowsp.stderr.exp
new file mode 100644
index 0000000..c80e120
--- /dev/null
+++ b/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
diff --git a/memcheck/tests/descr_belowsp.vgtest b/memcheck/tests/descr_belowsp.vgtest
new file mode 100644
index 0000000..9fa8191
--- /dev/null
+++ b/memcheck/tests/descr_belowsp.vgtest
@@ -0,0 +1,2 @@
+prog: descr_belowsp
+vgopts: -q
diff --git a/tests/filter_stderr_basic b/tests/filter_stderr_basic
index 5354c3f..c24cd5e 100755
--- a/tests/filter_stderr_basic
+++ b/tests/filter_stderr_basic
@@ -58,6 +58,9 @@
# 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' |