From: Florian Krohm Date: Wed, 25 Feb 2015 10:06:06 +0000 (+0000) Subject: Change VG_(am_extend_map_client) as follows: X-Git-Tag: svn/VALGRIND_3_11_0~629 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=d59ebddc38b81889af84f2a82bb1e56d11c62456;p=thirdparty%2Fvalgrind.git Change VG_(am_extend_map_client) as follows: - Tighten up on asserts - Simplify; as the function grows memory into a free segment, there cannot possibly be any translations to be discarded. Free segments do not have translations. sane_NSegment will make sure. - Change the prototype to take in the start address of the mapping and return a pointer to the resized segment. Previously, the code ok = VG_(am_extend_map_client)( &d, old_seg, needL ); if (!ok) goto eNOMEM; VG_TRACK( new_mem_mmap, needA, needL, old_seg->hasR, old_seg->hasW, old_seg->hasX, was examining old_seg->hasR etc even though VG_(am_extend_map_client) stated that *old_seg was invalid after the function returned. That wasn't exactly a problem, but clearly looked wrong. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@14963 --- diff --git a/coregrind/m_aspacemgr/aspacemgr-linux.c b/coregrind/m_aspacemgr/aspacemgr-linux.c index 41fb3e43e8..32ac8424ae 100644 --- a/coregrind/m_aspacemgr/aspacemgr-linux.c +++ b/coregrind/m_aspacemgr/aspacemgr-linux.c @@ -2892,38 +2892,38 @@ Bool VG_(am_extend_into_adjacent_reservation_client) ( const NSegment* seg, #if HAVE_MREMAP -/* Let SEG be a client mapping (anonymous or file). This fn extends - the mapping forwards only by DELTA bytes, and trashes whatever was - in the new area. Fails if SEG is not a single client mapping or if - the new area is not accessible to the client. Fails if DELTA is - not page aligned. *seg is invalid after a successful return. If - *need_discard is True after a successful return, the caller should - immediately discard translations from the new area. */ - -Bool VG_(am_extend_map_client)( /*OUT*/Bool* need_discard, - const NSegment* seg, SizeT delta ) +/* This function grows a client mapping in place into an adjacent free segment. + ADDR is the client mapping's start address and DELTA, which must be page + aligned, is the growth amount. The function returns a pointer to the + resized segment. The function is used in support of mremap. */ +const NSegment *VG_(am_extend_map_client)( Addr addr, SizeT delta ) { Addr xStart; SysRes sres; - NSegment seg_copy = *seg; - SizeT seg_old_len = seg->end + 1 - seg->start; if (0) VG_(am_show_nsegments)(0, "VG_(am_extend_map_client) BEFORE"); - if (seg->kind != SkFileC && seg->kind != SkAnonC) - return False; + /* Get the client segment */ + Int ix = find_nsegment_idx(addr); + aspacem_assert(ix >= 0 && ix < nsegments_used); - if (delta == 0 || !VG_IS_PAGE_ALIGNED(delta)) - return False; + NSegment *seg = nsegments + ix; + + aspacem_assert(seg->kind == SkFileC || seg->kind == SkAnonC); + aspacem_assert(delta > 0 && VG_IS_PAGE_ALIGNED(delta)) ; xStart = seg->end+1; - if (xStart + delta < delta) - return False; + aspacem_assert(xStart + delta >= delta); // no wrap-around - if (!VG_(am_is_valid_for_client_or_free_or_resvn)( xStart, delta, - VKI_PROT_NONE )) - return False; + /* The segment following the client segment must be a free segment and + it must be large enough to cover the additional memory. */ + NSegment *segf = seg + 1; + aspacem_assert(segf->kind == SkFree); + aspacem_assert(segf->start == xStart); + aspacem_assert(xStart + delta - 1 <= segf->end); + + SizeT seg_old_len = seg->end + 1 - seg->start; AM_SANITY_CHECK; sres = ML_(am_do_extend_mapping_NO_NOTIFY)( seg->start, @@ -2931,14 +2931,13 @@ Bool VG_(am_extend_map_client)( /*OUT*/Bool* need_discard, seg_old_len + delta ); if (sr_isError(sres)) { AM_SANITY_CHECK; - return False; + return NULL; } else { /* the area must not have moved */ aspacem_assert(sr_Res(sres) == seg->start); } - *need_discard = any_Ts_in_range( seg_copy.end+1, delta ); - + NSegment seg_copy = *seg; seg_copy.end += delta; add_segment( &seg_copy ); @@ -2946,7 +2945,7 @@ Bool VG_(am_extend_map_client)( /*OUT*/Bool* need_discard, VG_(am_show_nsegments)(0, "VG_(am_extend_map_client) AFTER"); AM_SANITY_CHECK; - return True; + return nsegments + find_nsegment_idx(addr); } diff --git a/coregrind/m_syswrap/syswrap-generic.c b/coregrind/m_syswrap/syswrap-generic.c index 42d134c0c5..eaf29d0e76 100644 --- a/coregrind/m_syswrap/syswrap-generic.c +++ b/coregrind/m_syswrap/syswrap-generic.c @@ -435,14 +435,12 @@ SysRes do_mremap( Addr old_addr, SizeT old_len, ok = VG_(am_covered_by_single_free_segment) ( needA, needL ); } if (ok && advised == needA) { - ok = VG_(am_extend_map_client)( &d, old_seg, needL ); - if (ok) { + const NSegment *new_seg = VG_(am_extend_map_client)( old_addr, needL ); + if (new_seg) { VG_TRACK( new_mem_mmap, needA, needL, - old_seg->hasR, - old_seg->hasW, old_seg->hasX, + new_seg->hasR, + new_seg->hasW, new_seg->hasX, 0/*di_handle*/ ); - if (d) - VG_(discard_translations)( needA, needL, "do_remap(3)" ); return VG_(mk_SysRes_Success)( old_addr ); } } @@ -490,14 +488,13 @@ SysRes do_mremap( Addr old_addr, SizeT old_len, } if (!ok || advised != needA) goto eNOMEM; - ok = VG_(am_extend_map_client)( &d, old_seg, needL ); - if (!ok) + const NSegment *new_seg = VG_(am_extend_map_client)( old_addr, needL ); + if (!new_seg) goto eNOMEM; VG_TRACK( new_mem_mmap, needA, needL, - old_seg->hasR, old_seg->hasW, old_seg->hasX, + new_seg->hasR, new_seg->hasW, new_seg->hasX, 0/*di_handle*/ ); - if (d) - VG_(discard_translations)( needA, needL, "do_remap(6)" ); + return VG_(mk_SysRes_Success)( old_addr ); } /*NOTREACHED*/ vg_assert(0); diff --git a/coregrind/pub_core_aspacemgr.h b/coregrind/pub_core_aspacemgr.h index 915c37a147..7db41415b3 100644 --- a/coregrind/pub_core_aspacemgr.h +++ b/coregrind/pub_core_aspacemgr.h @@ -286,15 +286,11 @@ extern Bool VG_(am_extend_into_adjacent_reservation_client) /* --- --- --- resizing/move a mapping --- --- --- */ -/* Let SEG be a client mapping (anonymous or file). This fn extends - the mapping forwards only by DELTA bytes, and trashes whatever was - in the new area. Fails if SEG is not a single client mapping or if - the new area is not accessible to the client. Fails if DELTA is - not page aligned. *seg is invalid after a successful return. If - *need_discard is True after a successful return, the caller should - immediately discard translations from the new area. */ -extern Bool VG_(am_extend_map_client)( /*OUT*/Bool* need_discard, - const NSegment* seg, SizeT delta ); +/* This function grows a client mapping in place into an adjacent free segment. + ADDR is the client mapping's start address and DELTA, which must be page + aligned, is the growth amount. The function returns a pointer to the + resized segment. The function is used in support of mremap. */ +extern const NSegment *VG_(am_extend_map_client)( Addr addr, SizeT delta ); /* Remap the old address range to the new address range. Fails if any parameter is not page aligned, if the either size is zero, if any