fix n-i-bz false positive leaks due to aspacemgr merging non heap segments with heap...

aspace mgr provides VG_(am_mmap_client_heap) that mmaps memory and
marks it as being client heap memory. Marking superblock segments used
for malloc/free as heap is critical for correct leak search: segments
mmap-ed for malloc/free cannot be considered as part of the root set.
On the other hand, other mmap-ed segments cannot be marked as client
heap, otherwise these segments will not be part of the root set, and
will not be scanned.

aspacemgr merges adjacent segments when they have the same characteristics
e.g. kind, RWX and isCH (is client heap) must be the same (see function
maybe_merge_nsegments).

However, VG_(am_mmap_client_heap) has a bug:
  * it first mmaps a normal segment (not marked as heap) using
     VG_(am_mmap_anon_float_client)
  * it then searches the segment that contains the just mmap-ed address and
    marks it as heap.

The problem is that VG_(am_mmap_anon_float_client) has already
possibly merged the new segment with a neighbour segment, without
taking the to be marked isCH into account, as the newly allocated memory
has not yet been marked as Client Heap.  So, this results in some memory being
marked as client heap, while it in fact is not client heap. This
memory will then not be scanned by the leak search.

The fix consists in having VG_(am_mmap_anon_float_client) and
VG_(am_mmap_client_heap) calling a new function
am_mmap_anon_float_client, which will mark (or not) the new segment as
client heap *before* trying to merge it with neighbouring segments.
Then the new (heap) segment will only be merged with neighbours that are also
client heap segments.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@15759

diff --git a/NEWS b/NEWS
index d9a18c535ba2c9463ea8787a8814d0b9e6bde7f5..07c750efc253f4a1193575149d39aff66e770ed8 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -62,6 +62,7 @@ where XXXXXX is the bug number as listed below.
 
 n-i-bz Fix incorrect (or infinite loop) unwind on RHEL7 x86 32 bits
 n-i-bz massif --pages-as-heap=yes does not report peak caused by mmap+munmap
+n-i-bz false positive leaks due to aspacemgr merging non heap segments with heap segments.
 
 Release 3.11.0 (22 September 2015)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

diff --git a/coregrind/m_aspacemgr/aspacemgr-linux.c b/coregrind/m_aspacemgr/aspacemgr-linux.c
index 7ce3c996eadbd8f79d8e64cf46e226a9ead5ec47..0a8f6753d3b2d28ceacedfe69963d3199edafcee 100644 (file)
--- a/coregrind/m_aspacemgr/aspacemgr-linux.c
+++ b/coregrind/m_aspacemgr/aspacemgr-linux.c
@@ -2476,7 +2476,7 @@ SysRes VG_(am_mmap_anon_fixed_client) ( Addr start, SizeT length, UInt prot )
 /* Map anonymously at an unconstrained address for the client, and
    update the segment array accordingly.  */
 
-SysRes VG_(am_mmap_anon_float_client) ( SizeT length, Int prot )
+static SysRes am_mmap_anon_float_client ( SizeT length, Int prot, Bool isCH )
 {
    SysRes     sres;
    NSegment   seg;
@@ -2524,12 +2524,17 @@ SysRes VG_(am_mmap_anon_float_client) ( SizeT length, Int prot )
    seg.hasR  = toBool(prot & VKI_PROT_READ);
    seg.hasW  = toBool(prot & VKI_PROT_WRITE);
    seg.hasX  = toBool(prot & VKI_PROT_EXEC);
+   seg.isCH  = isCH;
    add_segment( &seg );
 
    AM_SANITY_CHECK;
    return sres;
 }
 
+SysRes VG_(am_mmap_anon_float_client) ( SizeT length, Int prot )
+{
+   return am_mmap_anon_float_client (length, prot, False /* isCH */);
+}
 
 /* Map anonymously at an unconstrained address for V, and update the
    segment array accordingly.  This is fundamentally how V allocates
@@ -2738,21 +2743,13 @@ SysRes VG_(am_shared_mmap_file_float_valgrind)
                                                   fd, offset );
 }
 
-/* Convenience wrapper around VG_(am_mmap_anon_float_client) which also
+/* Similar to VG_(am_mmap_anon_float_client) but also
    marks the segment as containing the client heap. This is for the benefit
    of the leak checker which needs to be able to identify such segments
    so as not to use them as sources of roots during leak checks. */
 SysRes VG_(am_mmap_client_heap) ( SizeT length, Int prot )
 {
-   SysRes res = VG_(am_mmap_anon_float_client)(length, prot);
-
-   if (! sr_isError(res)) {
-      Addr addr = sr_Res(res);
-      Int ix = find_nsegment_idx(addr);
-
-      nsegments[ix].isCH = True;
-   }
-   return res;
+   return am_mmap_anon_float_client (length, prot, True /* isCH */);
 }
 
 /* --- --- munmap helper --- --- */

diff --git a/coregrind/pub_core_aspacemgr.h b/coregrind/pub_core_aspacemgr.h
index 003d94e44b14a0bb84543f59b19f4955be7b42d7..2e846724e9ff819ead6bb20a7a897f8b70b6e95d 100644 (file)
--- a/coregrind/pub_core_aspacemgr.h
+++ b/coregrind/pub_core_aspacemgr.h
@@ -254,7 +254,7 @@ extern SysRes VG_(am_mmap_file_float_valgrind)
 extern SysRes VG_(am_shared_mmap_file_float_valgrind)
    ( SizeT length, UInt prot, Int fd, Off64T offset );
 
-/* Convenience wrapper around VG_(am_mmap_anon_float_client) which also
+/* Similar to VG_(am_mmap_anon_float_client) but also
    marks the segment as containing the client heap. */
 extern SysRes VG_(am_mmap_client_heap) ( SizeT length, Int prot );