From e3dc4ae49f23e7c04c18dea4e2095323d33e4ff3 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 30 Aug 2002 11:05:33 +0000 Subject: [PATCH] Fixed this bug: valgrind: vg_cachesim.c:389 (get_BBCC): Assertion `((Bool)0) == remove' failed. Problem was that when an exe segment was unloaded, cachesim_notify_munmap() was being called after symbols were unloaded. But it needs the symbols to do the lookup required to remove the BBCCs. It was only working some of the time for exe segments that didn't have any symbols(!) Fix: now invalidate translations first, unload symbols second. This required adding VG_(is_munmap_exe)() to determine if an unloaded segment is executable. git-svn-id: svn://svn.valgrind.org/valgrind/branches/VALGRIND_1_0_BRANCH@839 --- vg_include.h | 4 +++- vg_symtab2.c | 53 +++++++++++++++++++++++++++++++++--------------- vg_syscall_mem.c | 12 +++++++---- 3 files changed, 48 insertions(+), 21 deletions(-) diff --git a/vg_include.h b/vg_include.h index 7795aa80fd..4ed054ff46 100644 --- a/vg_include.h +++ b/vg_include.h @@ -1453,7 +1453,9 @@ extern Bool VG_(what_line_is_this) ( Addr a, extern Bool VG_(what_fn_is_this) ( Bool no_demangle, Addr a, Char* fn_name, Int n_fn_name); -extern Bool VG_(symtab_notify_munmap) ( Addr start, UInt length ); +extern Bool VG_(is_munmap_exe) ( Addr start, UInt length ); + +extern void VG_(symtab_notify_munmap) ( Addr start, UInt length ); /* --------------------------------------------------------------------- diff --git a/vg_symtab2.c b/vg_symtab2.c index a14558e220..2ac975e82c 100644 --- a/vg_symtab2.c +++ b/vg_symtab2.c @@ -39,12 +39,18 @@ Stabs reader greatly improved by Nick Nethercode, Apr 02. - 16 May 02: when notified about munmap, return a Bool indicating - whether or not the area being munmapped had executable permissions. - This is then used to determine whether or not - VG_(invalid_translations) should be called for that area. In order - that this work even if --instrument=no, in this case we still keep - track of the mapped executable segments, but do not load any debug + NJN 30 Aug 2002: when notified about munmap, use VG_(is_munmap_exe) to + return a Bool indicating whether or not the area being munmapped had + executable permissions. This is then used to determine whether or not + VG_(invalidate_translations) and VG_(symtab_notify_munmap) should be + called for that area. + + Note that for Cachegrind to work, VG_(invalidate_translations) *must* be + called before VG_(symtab_notify_munmap), because Cachegrind uses the + symbols during the invalidation step. + + In order that this work even if --instrument=no, in this case we still + keep track of the mapped executable segments, but do not load any debug info or symbols. */ @@ -1761,30 +1767,47 @@ void VG_(read_symbols) ( void ) } +/* Determine if the munmapped segment is executable, so we know if we have + to unload symbols and invalidate translations. */ +Bool VG_(is_munmap_exe) ( Addr start, UInt length ) +{ + SegInfo *prev, *curr; + + prev = NULL; + curr = segInfo; + while (True) { + if (curr == NULL) return False; + if (start == curr->start) break; + prev = curr; + curr = curr->next; + } + + vg_assert(prev == NULL || prev->next == curr); + return True; +} + + /* When an munmap() call happens, check to see whether it corresponds to a segment for a .so, and if so discard the relevant SegInfo. This might not be a very clever idea from the point of view of accuracy of error messages, but we need to do it in order to maintain the no-overlapping invariant. - - 16 May 02: Returns a Bool indicating whether or not the discarded - range falls inside a known executable segment. See comment at top - of file for why. */ -Bool VG_(symtab_notify_munmap) ( Addr start, UInt length ) +void VG_(symtab_notify_munmap) ( Addr start, UInt length ) { SegInfo *prev, *curr; prev = NULL; curr = segInfo; while (True) { - if (curr == NULL) break; + /* VG_(is_munmap_exe) does exactly the same search, and this function + * is only called if it succeeds, so this search should not fail */ + vg_assert(curr != NULL); + if (start == curr->start) break; prev = curr; curr = curr->next; } - if (curr == NULL) - return False; VG_(message)(Vg_UserMsg, "discard syms in %s due to munmap()", @@ -1797,9 +1820,7 @@ Bool VG_(symtab_notify_munmap) ( Addr start, UInt length ) } else { prev->next = curr->next; } - freeSegInfo(curr); - return True; } diff --git a/vg_syscall_mem.c b/vg_syscall_mem.c index f5c002c9aa..4825e4f3b1 100644 --- a/vg_syscall_mem.c +++ b/vg_syscall_mem.c @@ -648,9 +648,11 @@ void VG_(perform_assumed_nonblocking_syscall) ( ThreadId tid ) while ((start % VKI_BYTES_PER_PAGE) > 0) { start--; length++; } while (((start+length) % VKI_BYTES_PER_PAGE) > 0) { length++; } make_noaccess( start, length ); - munmap_exe = VG_(symtab_notify_munmap) ( start, length ); - if (munmap_exe) + munmap_exe = VG_(is_munmap_exe) ( start, length ); + if (munmap_exe) { + VG_(symtab_notify_munmap) ( start, length ); VG_(invalidate_translations) ( start, length ); + } approximate_mmap_permissions( (Addr)res, arg3, arg4 ); } break; @@ -2298,9 +2300,11 @@ void VG_(perform_assumed_nonblocking_syscall) ( ThreadId tid ) /* Tell our symbol table machinery about this, so that if this happens to be a .so being unloaded, the relevant symbols are removed too. */ - munmap_exe = VG_(symtab_notify_munmap) ( start, length ); - if (munmap_exe) + munmap_exe = VG_(is_munmap_exe) ( start, length ); + if (munmap_exe) { VG_(invalidate_translations) ( start, length ); + VG_(symtab_notify_munmap) ( start, length ); + } } break; -- 2.47.2