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

diff --git a/vg_include.h b/vg_include.h
index 7795aa80fd58eeb8d0c83ced999649dd42cd8c2a..4ed054ff46e8d95f7cfa25f16bce02c6f6d4677c 100644 (file)
--- 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 a14558e22083561208b0f7afeff884a970a05da4..2ac975e82cb6ee2686cba1d2cf0dce06582dc960 100644 (file)
--- 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 f5c002c9aa1a98c50ca2908967cf237308925ac7..4825e4f3b12fcef7c89b225f8a0ca8ea6d1e82d9 100644 (file)
--- 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;