From 3159bc49c8ef4dca85ae3502d7bdc1c4f7ffb4b3 Mon Sep 17 00:00:00 2001 From: Philippe Waroquiers Date: Sun, 31 Aug 2014 22:27:19 +0000 Subject: [PATCH] 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 | 108 ++++++++++++++- include/pub_tool_addrinfo.h | 16 +++ memcheck/tests/Makefile.am | 3 + memcheck/tests/descr_belowsp.c | 175 ++++++++++++++++++++++++ memcheck/tests/descr_belowsp.stderr.exp | 26 ++++ memcheck/tests/descr_belowsp.vgtest | 2 + tests/filter_stderr_basic | 3 + 7 files changed, 332 insertions(+), 1 deletion(-) create mode 100644 memcheck/tests/descr_belowsp.c create mode 100644 memcheck/tests/descr_belowsp.stderr.exp create mode 100644 memcheck/tests/descr_belowsp.vgtest diff --git a/coregrind/m_addrinfo.c b/coregrind/m_addrinfo.c index 072b7823a5..af52be4ac3 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 @@ 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: { diff --git a/include/pub_tool_addrinfo.h b/include/pub_tool_addrinfo.h index a27a0649fe..b07e476123 100644 --- a/include/pub_tool_addrinfo.h +++ b/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 diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am index 9e2760eff6..72dd61bfb9 100644 --- a/memcheck/tests/Makefile.am +++ b/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 diff --git a/memcheck/tests/descr_belowsp.c b/memcheck/tests/descr_belowsp.c new file mode 100644 index 0000000000..704c222917 --- /dev/null +++ b/memcheck/tests/descr_belowsp.c @@ -0,0 +1,175 @@ +#include "../../config.h" + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include +#ifdef HAVE_GETPAGESIZE +#include +#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 0000000000..c80e120373 --- /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 0000000000..9fa8191f5c --- /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 5354c3f532..c24cd5ea29 100755 --- a/tests/filter_stderr_basic +++ b/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' | -- 2.47.3