|
From: <sv...@va...> - 2015-03-03 14:56:26
|
Author: florian
Date: Tue Mar 3 14:56:17 2015
New Revision: 14974
Log:
Produce a user message in case of stack overflow.
Change VG_(extend_stack) and VG_(am_extend_into_adjacent_reservation_client)
accordingly.
Remove some redundant checking.
Add testcase.
Added:
trunk/none/tests/linux/stack-overflow.c
trunk/none/tests/linux/stack-overflow.stderr.exp
trunk/none/tests/linux/stack-overflow.vgtest
Modified:
trunk/coregrind/m_aspacemgr/aspacemgr-linux.c
trunk/coregrind/m_sigframe/sigframe-amd64-linux.c
trunk/coregrind/m_sigframe/sigframe-arm-linux.c
trunk/coregrind/m_sigframe/sigframe-arm64-linux.c
trunk/coregrind/m_sigframe/sigframe-mips32-linux.c
trunk/coregrind/m_sigframe/sigframe-mips64-linux.c
trunk/coregrind/m_sigframe/sigframe-ppc32-linux.c
trunk/coregrind/m_sigframe/sigframe-ppc64-linux.c
trunk/coregrind/m_sigframe/sigframe-s390x-linux.c
trunk/coregrind/m_sigframe/sigframe-x86-linux.c
trunk/coregrind/m_signals.c
trunk/coregrind/m_syswrap/syswrap-generic.c
trunk/coregrind/m_syswrap/syswrap-main.c
trunk/coregrind/pub_core_aspacemgr.h
trunk/coregrind/pub_core_signals.h
trunk/none/tests/linux/Makefile.am
Modified: trunk/coregrind/m_aspacemgr/aspacemgr-linux.c
==============================================================================
--- trunk/coregrind/m_aspacemgr/aspacemgr-linux.c (original)
+++ trunk/coregrind/m_aspacemgr/aspacemgr-linux.c Tue Mar 3 14:56:17 2015
@@ -2789,12 +2789,15 @@
page long. The function returns a pointer to the resized segment. */
const NSegment *VG_(am_extend_into_adjacent_reservation_client)( Addr addr,
- SSizeT delta )
+ SSizeT delta,
+ Bool *overflow)
{
Int segA, segR;
UInt prot;
SysRes sres;
+ *overflow = False;
+
segA = find_nsegment_idx(addr);
aspacem_assert(nsegments[segA].kind == SkAnonC);
@@ -2813,11 +2816,14 @@
segR = segA+1;
if (segR >= nsegments_used
|| nsegments[segR].kind != SkResvn
- || nsegments[segR].smode != SmLower
- || nsegments[segR].start != nsegments[segA].end + 1
- || delta + VKI_PAGE_SIZE
- > (nsegments[segR].end - nsegments[segR].start + 1))
- return NULL;
+ || nsegments[segR].smode != SmLower)
+ return NULL;
+
+ if (delta + VKI_PAGE_SIZE
+ > (nsegments[segR].end - nsegments[segR].start + 1)) {
+ *overflow = True;
+ return NULL;
+ }
/* Extend the kernel's mapping. */
// DDD: #warning GrP fixme MAP_FIXED can clobber memory!
@@ -2849,11 +2855,14 @@
segR = segA-1;
if (segR < 0
|| nsegments[segR].kind != SkResvn
- || nsegments[segR].smode != SmUpper
- || nsegments[segR].end + 1 != nsegments[segA].start
- || delta + VKI_PAGE_SIZE
- > (nsegments[segR].end - nsegments[segR].start + 1))
- return NULL;
+ || nsegments[segR].smode != SmUpper)
+ return NULL;
+
+ if (delta + VKI_PAGE_SIZE
+ > (nsegments[segR].end - nsegments[segR].start + 1)) {
+ *overflow = True;
+ return NULL;
+ }
/* Extend the kernel's mapping. */
// DDD: #warning GrP fixme MAP_FIXED can clobber memory!
@@ -2875,7 +2884,6 @@
nsegments[segR].end -= delta;
nsegments[segA].start -= delta;
aspacem_assert(nsegments[segR].start <= nsegments[segR].end);
-
}
AM_SANITY_CHECK;
Modified: trunk/coregrind/m_sigframe/sigframe-amd64-linux.c
==============================================================================
--- trunk/coregrind/m_sigframe/sigframe-amd64-linux.c (original)
+++ trunk/coregrind/m_sigframe/sigframe-amd64-linux.c Tue Mar 3 14:56:17 2015
@@ -381,7 +381,7 @@
ThreadId tid = tst->tid;
NSegment const* stackseg = NULL;
- if (VG_(extend_stack)(addr, tst->client_stack_szB)) {
+ if (VG_(extend_stack)(tid, addr)) {
stackseg = VG_(am_find_nsegment)(addr);
if (0 && stackseg)
VG_(printf)("frame=%#lx seg=%#lx-%#lx\n",
Modified: trunk/coregrind/m_sigframe/sigframe-arm-linux.c
==============================================================================
--- trunk/coregrind/m_sigframe/sigframe-arm-linux.c (original)
+++ trunk/coregrind/m_sigframe/sigframe-arm-linux.c Tue Mar 3 14:56:17 2015
@@ -86,7 +86,7 @@
ThreadId tid = tst->tid;
NSegment const* stackseg = NULL;
- if (VG_(extend_stack)(addr, tst->client_stack_szB)) {
+ if (VG_(extend_stack)(tid, addr)) {
stackseg = VG_(am_find_nsegment)(addr);
if (0 && stackseg)
VG_(printf)("frame=%#lx seg=%#lx-%#lx\n",
Modified: trunk/coregrind/m_sigframe/sigframe-arm64-linux.c
==============================================================================
--- trunk/coregrind/m_sigframe/sigframe-arm64-linux.c (original)
+++ trunk/coregrind/m_sigframe/sigframe-arm64-linux.c Tue Mar 3 14:56:17 2015
@@ -84,7 +84,7 @@
ThreadId tid = tst->tid;
NSegment const* stackseg = NULL;
- if (VG_(extend_stack)(addr, tst->client_stack_szB)) {
+ if (VG_(extend_stack)(tid, addr)) {
stackseg = VG_(am_find_nsegment)(addr);
if (0 && stackseg)
VG_(printf)("frame=%#lx seg=%#lx-%#lx\n",
Modified: trunk/coregrind/m_sigframe/sigframe-mips32-linux.c
==============================================================================
--- trunk/coregrind/m_sigframe/sigframe-mips32-linux.c (original)
+++ trunk/coregrind/m_sigframe/sigframe-mips32-linux.c Tue Mar 3 14:56:17 2015
@@ -83,10 +83,9 @@
ThreadId tid = tst->tid;
NSegment const* stackseg = NULL;
- if (VG_(extend_stack)(addr, tst->client_stack_szB))
- {
- stackseg = VG_(am_find_nsegment)(addr);
- }
+ if (VG_(extend_stack)(tid, addr)) {
+ stackseg = VG_(am_find_nsegment)(addr);
+ }
if (stackseg == NULL || !stackseg->hasR || !stackseg->hasW)
{
Modified: trunk/coregrind/m_sigframe/sigframe-mips64-linux.c
==============================================================================
--- trunk/coregrind/m_sigframe/sigframe-mips64-linux.c (original)
+++ trunk/coregrind/m_sigframe/sigframe-mips64-linux.c Tue Mar 3 14:56:17 2015
@@ -80,7 +80,7 @@
ThreadId tid = tst->tid;
NSegment const* stackseg = NULL;
- if (VG_(extend_stack)(addr, tst->client_stack_szB)) {
+ if (VG_(extend_stack)(tid, addr)) {
stackseg = VG_(am_find_nsegment)(addr);
}
Modified: trunk/coregrind/m_sigframe/sigframe-ppc32-linux.c
==============================================================================
--- trunk/coregrind/m_sigframe/sigframe-ppc32-linux.c (original)
+++ trunk/coregrind/m_sigframe/sigframe-ppc32-linux.c Tue Mar 3 14:56:17 2015
@@ -511,7 +511,7 @@
ThreadId tid = tst->tid;
NSegment const* stackseg = NULL;
- if (VG_(extend_stack)(addr, tst->client_stack_szB)) {
+ if (VG_(extend_stack)(tid, addr)) {
stackseg = VG_(am_find_nsegment)(addr);
if (0 && stackseg)
VG_(printf)("frame=%#lx seg=%#lx-%#lx\n",
Modified: trunk/coregrind/m_sigframe/sigframe-ppc64-linux.c
==============================================================================
--- trunk/coregrind/m_sigframe/sigframe-ppc64-linux.c (original)
+++ trunk/coregrind/m_sigframe/sigframe-ppc64-linux.c Tue Mar 3 14:56:17 2015
@@ -141,7 +141,7 @@
ThreadId tid = tst->tid;
NSegment const* stackseg = NULL;
- if (VG_(extend_stack)(addr, tst->client_stack_szB)) {
+ if (VG_(extend_stack)(tid, addr)) {
stackseg = VG_(am_find_nsegment)(addr);
if (0 && stackseg)
VG_(printf)("frame=%#lx seg=%#lx-%#lx\n",
Modified: trunk/coregrind/m_sigframe/sigframe-s390x-linux.c
==============================================================================
--- trunk/coregrind/m_sigframe/sigframe-s390x-linux.c (original)
+++ trunk/coregrind/m_sigframe/sigframe-s390x-linux.c Tue Mar 3 14:56:17 2015
@@ -267,7 +267,7 @@
ThreadId tid = tst->tid;
NSegment const* stackseg = NULL;
- if (VG_(extend_stack)(addr, tst->client_stack_szB)) {
+ if (VG_(extend_stack)(tid, addr)) {
stackseg = VG_(am_find_nsegment)(addr);
if (0 && stackseg)
VG_(printf)("frame=%#lx seg=%#lx-%#lx\n",
Modified: trunk/coregrind/m_sigframe/sigframe-x86-linux.c
==============================================================================
--- trunk/coregrind/m_sigframe/sigframe-x86-linux.c (original)
+++ trunk/coregrind/m_sigframe/sigframe-x86-linux.c Tue Mar 3 14:56:17 2015
@@ -402,7 +402,7 @@
ThreadId tid = tst->tid;
NSegment const* stackseg = NULL;
- if (VG_(extend_stack)(addr, tst->client_stack_szB)) {
+ if (VG_(extend_stack)(tid, addr)) {
stackseg = VG_(am_find_nsegment)(addr);
if (0 && stackseg)
VG_(printf)("frame=%#lx seg=%#lx-%#lx\n",
Modified: trunk/coregrind/m_signals.c
==============================================================================
--- trunk/coregrind/m_signals.c (original)
+++ trunk/coregrind/m_signals.c Tue Mar 3 14:56:17 2015
@@ -1,3 +1,4 @@
+/* -*- mode: C; c-basic-offset: 3; -*- */
/*--------------------------------------------------------------------*/
/*--- Implementation of POSIX signals. m_signals.c ---*/
@@ -1715,7 +1716,7 @@
if (tid == 1) { // main thread
Addr esp = VG_(get_SP)(tid);
Addr base = VG_PGROUNDDN(esp - VG_STACK_REDZONE_SZB);
- if (VG_(extend_stack)(base, VG_(threads)[tid].client_stack_szB)) {
+ if (VG_(extend_stack)(tid, base)) {
if (VG_(clo_trace_signals))
VG_(dmsg)(" -> extended stack base to %#lx\n",
VG_PGROUNDDN(esp));
@@ -2238,54 +2239,54 @@
"while outside of scheduler");
}
-/* Extend the stack to cover addr. maxsize is the limit the stack can grow to.
+/* Extend the stack of thread #tid to cover addr.
Returns True on success, False on failure.
Succeeds without doing anything if addr is already within a segment.
Failure could be caused by:
- - addr not below a growable segment
- - new stack size would exceed maxsize
+ - addr not below a growable segment or in a free segment
+ - new stack size would exceed the stack limit for the given thread
- mmap failed for some other reason
- */
-Bool VG_(extend_stack)(Addr addr, UInt maxsize)
+*/
+Bool VG_(extend_stack)(ThreadId tid, Addr addr)
{
SizeT udelta;
- /* Find the next Segment above addr */
- NSegment const* seg
- = VG_(am_find_nsegment)(addr);
- NSegment const* seg_next
- = seg ? VG_(am_next_nsegment)( seg, True/*fwds*/ )
- : NULL;
+ /* Get the segment containing addr. */
+ const NSegment* seg = VG_(am_find_nsegment)(addr);
+ if (seg == NULL) return False; // addr in a SkFree segment
/* TODO: the test "seg->kind == SkAnonC" is really inadequate,
because although it tests whether the segment is mapped
_somehow_, it doesn't check that it has the right permissions
(r,w, maybe x) ? */
- if (seg && seg->kind == SkAnonC)
+ if (seg->kind == SkAnonC)
/* addr is already mapped. Nothing to do. */
return True;
- /* Check that the requested new base is in a shrink-down
- reservation section which abuts an anonymous mapping that
- belongs to the client. */
- if ( ! (seg
- && seg->kind == SkResvn
- && seg->smode == SmUpper
- && seg_next
- && seg_next->kind == SkAnonC
- && seg->end+1 == seg_next->start))
- return False;
+ /* Find the next Segment above addr. This will return NULL if ADDR
+ is bogus -- which it may be. See comment at the call site in function
+ VG_(client_syscall) */
+ const NSegment* seg_next = VG_(am_next_nsegment)( seg, True/*fwds*/ );
+ if (seg_next == NULL || seg_next->kind != SkAnonC) return False;
udelta = VG_PGROUNDUP(seg_next->start - addr);
+
VG_(debugLog)(1, "signals",
"extending a stack base 0x%llx down by %lld\n",
(ULong)seg_next->start, (ULong)udelta);
+ Bool overflow;
if (! VG_(am_extend_into_adjacent_reservation_client)
- ( seg_next->start, -(SSizeT)udelta )) {
- VG_(debugLog)(1, "signals", "extending a stack base: FAILED\n");
+ ( seg_next->start, -(SSizeT)udelta, &overflow )) {
+ Addr new_stack_base = seg_next->start - udelta;
+ if (overflow)
+ VG_(umsg)("Stack overflow in thread #%d: can't grow stack to %#lx\n",
+ tid, new_stack_base);
+ else
+ VG_(umsg)("Cannot map memory to grow the stack for thread #%d "
+ "to %#lx\n", tid, new_stack_base);
return False;
}
@@ -2407,7 +2408,6 @@
Addr fault;
Addr esp;
NSegment const* seg;
- NSegment const* seg_next;
if (info->si_signo != VKI_SIGSEGV)
return False;
@@ -2415,8 +2415,6 @@
fault = (Addr)info->VKI_SIGINFO_si_addr;
esp = VG_(get_SP)(tid);
seg = VG_(am_find_nsegment)(fault);
- seg_next = seg ? VG_(am_next_nsegment)( seg, True/*fwds*/ )
- : NULL;
if (VG_(clo_trace_signals)) {
if (seg == NULL)
@@ -2431,11 +2429,6 @@
if (info->si_code == VKI_SEGV_MAPERR
&& seg
- && seg->kind == SkResvn
- && seg->smode == SmUpper
- && seg_next
- && seg_next->kind == SkAnonC
- && seg->end+1 == seg_next->start
&& fault >= fault_mask(esp - VG_STACK_REDZONE_SZB)) {
/* If the fault address is above esp but below the current known
stack segment base, and it was a fault because there was
@@ -2443,14 +2436,12 @@
then extend the stack segment.
*/
Addr base = VG_PGROUNDDN(esp - VG_STACK_REDZONE_SZB);
- if (VG_(extend_stack)(base, VG_(threads)[tid].client_stack_szB)) {
+ if (VG_(extend_stack)(tid, base)) {
if (VG_(clo_trace_signals))
VG_(dmsg)(" -> extended stack base to %#lx\n",
VG_PGROUNDDN(fault));
return True;
} else {
- VG_(umsg)("Stack overflow in thread %d: can't grow stack to %#lx\n",
- tid, fault);
return False;
}
} else {
Modified: trunk/coregrind/m_syswrap/syswrap-generic.c
==============================================================================
--- trunk/coregrind/m_syswrap/syswrap-generic.c (original)
+++ trunk/coregrind/m_syswrap/syswrap-generic.c Tue Mar 3 14:56:17 2015
@@ -1254,7 +1254,9 @@
vg_assert(delta > 0);
vg_assert(VG_IS_PAGE_ALIGNED(delta));
- if (! VG_(am_extend_into_adjacent_reservation_client)( aseg->start, delta ))
+ Bool overflow; // ignored here
+ if (! VG_(am_extend_into_adjacent_reservation_client)( aseg->start, delta,
+ &overflow))
goto bad;
VG_(brk_limit) = newbrk;
Modified: trunk/coregrind/m_syswrap/syswrap-main.c
==============================================================================
--- trunk/coregrind/m_syswrap/syswrap-main.c (original)
+++ trunk/coregrind/m_syswrap/syswrap-main.c Tue Mar 3 14:56:17 2015
@@ -1479,7 +1479,16 @@
if (tid == 1/*ROOT THREAD*/) {
Addr stackMin = VG_(get_SP)(tid) - VG_STACK_REDZONE_SZB;
- VG_(extend_stack)( stackMin, tst->client_stack_szB );
+ /* Note, that the stack pointer can be bogus at this point. This is
+ extremely rare. A legitimate testcase that exercises this is
+ none/tests/s390x/stmg.c: The stack pointer happens to be in the
+ reservation segment near the end of the addressable memory and
+ there is no SkAnonC segment above.
+
+ We could do slightly better here by not extending the stack for
+ system calls that do not access user space memory. That's busy
+ work with very little gain... */
+ VG_(extend_stack)( tid, stackMin ); // may fail
}
# endif
/* END ensure root thread's stack is suitably mapped */
Modified: trunk/coregrind/pub_core_aspacemgr.h
==============================================================================
--- trunk/coregrind/pub_core_aspacemgr.h (original)
+++ trunk/coregrind/pub_core_aspacemgr.h Tue Mar 3 14:56:17 2015
@@ -282,7 +282,7 @@
the reservation segment after the operation must be at least one
page long. The function returns a pointer to the resized segment. */
extern const NSegment *VG_(am_extend_into_adjacent_reservation_client)
- ( Addr addr, SSizeT delta );
+ ( Addr addr, SSizeT delta, /*OUT*/Bool *overflow );
/* --- --- --- resizing/move a mapping --- --- --- */
Modified: trunk/coregrind/pub_core_signals.h
==============================================================================
--- trunk/coregrind/pub_core_signals.h (original)
+++ trunk/coregrind/pub_core_signals.h Tue Mar 3 14:56:17 2015
@@ -81,7 +81,7 @@
extern void VG_(synth_sigfpe) (ThreadId tid, UInt code);
/* Extend the stack to cover addr, if possible */
-extern Bool VG_(extend_stack)(Addr addr, UInt maxsize);
+extern Bool VG_(extend_stack)(ThreadId tid, Addr addr);
/* Forces the client's signal handler to SIG_DFL - generally just
before using that signal to kill the process. */
Modified: trunk/none/tests/linux/Makefile.am
==============================================================================
--- trunk/none/tests/linux/Makefile.am (original)
+++ trunk/none/tests/linux/Makefile.am Tue Mar 3 14:56:17 2015
@@ -8,15 +8,16 @@
mremap.stderr.exp mremap.stderr.exp-glibc27 mremap.stdout.exp \
mremap.vgtest \
mremap2.stderr.exp mremap2.stdout.exp mremap2.vgtest \
- mremap3.stderr.exp mremap3.stdout.exp mremap3.vgtest
+ mremap3.stderr.exp mremap3.stdout.exp mremap3.vgtest \
+ stack-overflow.stderr.exp stack-overflow.vgtest
check_PROGRAMS = \
blockfault \
mremap \
mremap2 \
- mremap3
+ mremap3 \
+ stack-overflow
AM_CFLAGS += $(AM_FLAG_M3264_PRI)
AM_CXXFLAGS += $(AM_FLAG_M3264_PRI)
-
Added: trunk/none/tests/linux/stack-overflow.c
==============================================================================
--- trunk/none/tests/linux/stack-overflow.c (added)
+++ trunk/none/tests/linux/stack-overflow.c Tue Mar 3 14:56:17 2015
@@ -0,0 +1,9 @@
+/* There should be a user message about the overflow.
+ Wrtten in a single line so there is no confusion on what line
+ the overflow occurs. */
+
+int main(int argc, char *argv[]) \
+{ \
+ volatile int arr[1000]; \
+ return main(arr[argc%2], 0); \
+}
Added: trunk/none/tests/linux/stack-overflow.stderr.exp
==============================================================================
--- trunk/none/tests/linux/stack-overflow.stderr.exp (added)
+++ trunk/none/tests/linux/stack-overflow.stderr.exp Tue Mar 3 14:56:17 2015
@@ -0,0 +1,13 @@
+
+Stack overflow in thread #1: can't grow stack to 0x........
+
+Process terminating with default action of signal 11 (SIGSEGV)
+ Access not within mapped region at address 0x........
+Stack overflow in thread #1: can't grow stack to 0x........
+ at 0x........: main (stack-overflow.c:6)
+ If you believe this happened as a result of a stack
+ overflow in your program's main thread (unlikely but
+ possible), you can try to increase the size of the
+ main thread stack using the --main-stacksize= flag.
+ The main thread stack size used in this run was ....
+
Added: trunk/none/tests/linux/stack-overflow.vgtest
==============================================================================
--- trunk/none/tests/linux/stack-overflow.vgtest (added)
+++ trunk/none/tests/linux/stack-overflow.vgtest Tue Mar 3 14:56:17 2015
@@ -0,0 +1 @@
+prog: stack-overflow
|