Author: philippe
Date: Thu Dec 10 22:37:59 2015
New Revision: 15745
Log:
Fix massif --pages-as-heap=yes does not report peak caused by mmap+munmap
ms_unrecord_page_mem was wrongly taking the (possible) peak snapshot
when unrecording the last block.
But the peak snapshot will be detected when unrecording the first block
of an munmap, not when unrecording the last block.
Added:
trunk/massif/tests/mmapunmap.c
trunk/massif/tests/mmapunmap.post.exp
trunk/massif/tests/mmapunmap.stderr.exp
trunk/massif/tests/mmapunmap.vgtest
Modified:
trunk/NEWS
trunk/massif/ms_main.c
trunk/massif/tests/Makefile.am
Modified: trunk/NEWS
==============================================================================
--- trunk/NEWS (original)
+++ trunk/NEWS Thu Dec 10 22:37:59 2015
@@ -52,11 +52,12 @@
354797 Added vbit tester support for PPC 64 isa 2.07 iops
354933 Fix documentation of --kernel-variant=android-no-hw-tls option
355188 valgrind should intercept all malloc related global functions
-355455 expected stderr of test cases wrapmalloc and wrapmallocstatic overconstrained
+355455 stderr.exp of test cases wrapmalloc and wrapmallocstatic overconstrained
355454 do not intercept malloc related symbols from the runtime linker
356044 Dwarf line info reader misinterprets is_stmt register
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
Release 3.11.0 (22 September 2015)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Modified: trunk/massif/ms_main.c
==============================================================================
--- trunk/massif/ms_main.c (original)
+++ trunk/massif/ms_main.c Thu Dec 10 22:37:59 2015
@@ -1816,10 +1816,13 @@
Addr end;
tl_assert(VG_IS_PAGE_ALIGNED(len));
tl_assert(len >= VKI_PAGE_SIZE);
+ // Unrecord the first page. This might be the peak, so do a snapshot.
+ unrecord_block((void*)a, /*maybe_snapshot*/True);
+ a += VKI_PAGE_SIZE;
+ // Then unrecord the remaining pages, but without snapshots.
for (end = a + len - VKI_PAGE_SIZE; a < end; a += VKI_PAGE_SIZE) {
unrecord_block((void*)a, /*maybe_snapshot*/False);
}
- unrecord_block((void*)a, /*maybe_snapshot*/True);
}
//------------------------------------------------------------//
Modified: trunk/massif/tests/Makefile.am
==============================================================================
--- trunk/massif/tests/Makefile.am (original)
+++ trunk/massif/tests/Makefile.am Thu Dec 10 22:37:59 2015
@@ -24,6 +24,7 @@
long-names.post.exp long-names.stderr.exp long-names.vgtest \
long-time.post.exp long-time.stderr.exp long-time.vgtest \
malloc_usable.stderr.exp malloc_usable.vgtest \
+ mmapunmap.post.exp mmapunmap.stderr.exp mmapunmap.vgtest \
new-cpp.post.exp new-cpp.stderr.exp new-cpp.vgtest \
no-stack-no-heap.post.exp no-stack-no-heap.stderr.exp no-stack-no-heap.vgtest \
null.post.exp null.stderr.exp null.vgtest \
@@ -61,6 +62,7 @@
insig \
long-names \
long-time \
+ mmapunmap \
malloc_usable \
new-cpp \
null \
Added: trunk/massif/tests/mmapunmap.c
==============================================================================
--- trunk/massif/tests/mmapunmap.c (added)
+++ trunk/massif/tests/mmapunmap.c Thu Dec 10 22:37:59 2015
@@ -0,0 +1,16 @@
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include "tests/sys_mman.h"
+
+int main()
+{
+ void *m;
+
+ m = mmap(NULL, 80 * 1000 * 1024,
+ PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS,
+ -1, 0);
+ munmap(m, 80 * 1000 * 1024);
+ return 0;
+}
Added: trunk/massif/tests/mmapunmap.post.exp
==============================================================================
--- trunk/massif/tests/mmapunmap.post.exp (added)
+++ trunk/massif/tests/mmapunmap.post.exp Thu Dec 10 22:37:59 2015
@@ -0,0 +1 @@
+ n0: 81920000 0x........: main (mmapunmap.c:11)
Added: trunk/massif/tests/mmapunmap.stderr.exp
==============================================================================
(empty)
Added: trunk/massif/tests/mmapunmap.vgtest
==============================================================================
--- trunk/massif/tests/mmapunmap.vgtest (added)
+++ trunk/massif/tests/mmapunmap.vgtest Thu Dec 10 22:37:59 2015
@@ -0,0 +1,6 @@
+prog: mmapunmap
+vgopts: --pages-as-heap=yes --threshold=30.0 -q
+vgopts: --stacks=no --time-unit=B --depth=8 --massif-out-file=massif.out
+vgopts: --ignore-fn=__part_load_locale --ignore-fn=__time_load_locale --ignore-fn=dwarf2_unwind_dyld_add_image_hook --ignore-fn=get_or_create_key_element
+post: grep -A3 -e =peak massif.out | grep -e 'main (mmapunmap.c:11)' | ../../tests/filter_addresses
+# cleanup: rm massif.out
|
2015-12-10 23:38 GMT+01:00 <sv...@va...>:
> Author: philippe
> Date: Thu Dec 10 22:37:59 2015
> New Revision: 15745
>
> ...
>
> Added: trunk/massif/tests/mmapunmap.post.exp
>
> ==============================================================================
> --- trunk/massif/tests/mmapunmap.post.exp (added)
> +++ trunk/massif/tests/mmapunmap.post.exp Thu Dec 10 22:37:59 2015
> @@ -0,0 +1 @@
> + n0: 81920000 0x........: main (mmapunmap.c:11)
>
Hello Philippe,
Thank you for a new massif test case!
When porting this test case to Solaris, I noticed that stack trace differs
from Linux.
Linux gives (with --show-below-main=yes):
n2: 88260608 (page allocation syscalls) mmap/mremap/brk, --alloc-fns, etc.
n1: 81920000 0x4D29729: mmap (syscall-template.S:81)
n1: 81920000 0x400714: main (mmapunmap.c:11)
n1: 81920000 0x4C48A3E: __libc_start_main (libc-start.c:289)
n1: 81920000 0x400547: _start (in
/var/tmp/valgrind/massif/tests/mmapunmap)
while Solaris gives:
n2: 93609984 (page allocation syscalls) mmap/mremap/brk, --alloc-fns, etc.
n1: 81920000 0x7FFB4ACD9: mmap (in /lib/amd64/libc.so.1)
n0: 81920000 0x400E61: _start (in
/export/home/tester1/valgrind/massif/tests/mmapunmap)
Function mmap() does not contain proper function prologue (push %rbp; mov
%rsp, %rbp is missing)
on both Solaris and Linux.
I think the difference stems from the fact that Solaris libc currently
contains
no DWARF CFI. So that the unwinder which takes a stack trace gets just %rip
for mmap()
and by following the %rbp chain it gets also a return address from %rbp+8
(which points at _start() where it calls main()).
I wonder if I can adjust the test case by introducing an intermediate
function
so that the stack trace will look like (--show-below-main=no):
n2: (page allocation syscalls) mmap/mremap/brk, --alloc-fns, etc.
n1: mmap (syscall-template.S:81)
n1: f (mmapunmap.c:11)
n1: main (mmapunmap.c:20)
or
n2: (page allocation syscalls) mmap/mremap/brk, --alloc-fns, etc.
n1: mmap (in /lib/amd64/libc.so.1)
n1: main (mmapunmap.c:20)
and grep in mmapunmap.vgtest just for "mmapunmap.c". Would such test still
cover
the bug you fixed?
===================
...
int f(void)
{
void *m;
m = mmap(NULL, 80 * 1000 * 1024,
PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS,
-1, 0);
munmap(m, 80 * 1000 * 1024);
return 0;
}
int main()
{
return f();
}
===================
I tried to get stack traces also with gdb. It gives "correct" stack traces
on
Solaris with original sources, such as:
mmap (in /lib/amd64/libc.so.1)
main (mmapunmap.c:11)
It cannot use DWARF CFI for libc as there is none. And following %rbp chain
just gives
mmap() and _start(), but not main(). Do you have any idea what gdb does
behind the scenes?
Kind regards,
I.
|
On Sat, 2015-12-12 at 21:41 +0100, Ivo Raisr wrote: > I wonder if I can adjust the test case by introducing an intermediate > function > so that the stack trace will look like (--show-below-main=no): > n2: (page allocation syscalls) mmap/mremap/brk, --alloc-fns, etc. > n1: mmap (syscall-template.S:81) > n1: f (mmapunmap.c:11) > n1: main (mmapunmap.c:20) > > or > n2: (page allocation syscalls) mmap/mremap/brk, --alloc-fns, etc. > > n1: mmap (in /lib/amd64/libc.so.1) > n1: main (mmapunmap.c:20) > > > > and grep in mmapunmap.vgtest just for "mmapunmap.c". Would such test > still cover > the bug you fixed? What the test checks is that we have in the peak snapshot a (unique) part of the stacktrace that does the 'big mmap' (to check that the peak is properly computed). So, introducing a function f, that is only in the stacktrace leading to the big mmap of 80Mb will be good enough to precisely check the (fixed) bug (if needed, you can have f calling g calling mmap (80Mb). > > I tried to get stack traces also with gdb. It gives "correct" stack > traces on > Solaris with original sources, such as: > mmap (in /lib/amd64/libc.so.1) > main (mmapunmap.c:11) > It cannot use DWARF CFI for libc as there is none. And following %rbp > chain just gives > mmap() and _start(), but not main(). Do you have any idea what gdb > does behind the scenes? Humph, no clear idea. gdb unwinder is very sophisticated, it has a bunch of heuristics to unwind e.g. in function prologues or epilogues, even without dwarf unwind info. Thanks for the massif test failure analysis, and upcoming fix :). Philippe |
2015-12-13 14:25 GMT+01:00 Philippe Waroquiers < phi...@sk...>: > So, introducing a function f, that is only in the stacktrace leading to > the big mmap of 80Mb will be good enough to precisely check the (fixed) > bug (if needed, you can have f calling g calling mmap (80Mb). > Thank you, Philippe! I committed this change under r15749. > > > Do you have any idea what gdb does behind the scenes? > Humph, no clear idea. > gdb unwinder is very sophisticated, it has a bunch of heuristics > to unwind e.g. in function prologues or epilogues, even without > dwarf unwind info. > Yes, that sounds really smart. Perhaps examining the place where return address points to if it really could have invoked that particular function. Or inspecting function prologue and epilogue and guessing where the proper return address could lie on the stack... I. |