diff options
author | florian <florian@a5019735-40e9-0310-863c-91ae7b9d1cf9> | 2015-02-20 14:00:23 +0000 |
---|---|---|
committer | florian <florian@a5019735-40e9-0310-863c-91ae7b9d1cf9> | 2015-02-20 14:00:23 +0000 |
commit | ea8a88c21eecdd9e91c75ccbd3c864e708e2f41b (patch) | |
tree | 51957f2c74840efda048901755f12bd68d88a7b0 | |
parent | 426e6a2ca4c94127fdbb0d0f9e89a831695f6cdb (diff) | |
download | valgrind-ea8a88c21eecdd9e91c75ccbd3c864e708e2f41b.tar.gz |
Pass in a mask of segment kinds to VG_(get_segment_starts)
and VG_(am_get_segment_starts) to indicate which segments
should be collected. That should solve the following problem:
in m_main.c we used to:
seg_starts = VG_(get_segment_starts)( &n_seg_starts );
for (i = 0; i < n_seg_starts; i++) {
Word j, n;
NSegment const* seg
= VG_(am_find_nsegment)( seg_starts[i] );
vg_assert(seg);
if (seg->kind == SkFileC || seg->kind == SkAnonC) {
...
// ... dynamic memory allocation for valgrind
...
}
This caused the vassert(seg) to fire because the dynamic memory
allocation further down the loop changed segments such that a
valgrind segment which used to be non-SkFree suddenly became
SkFree and hence VG_(am_find_nsegment) returned NULL. Whoom.
With this revision we only collect the segments we're really
interested in. For the example above that is all client segments.
So if V allocates memory -- fine. That will not change the layout
of client segments.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@14949 a5019735-40e9-0310-863c-91ae7b9d1cf9
-rw-r--r-- | coregrind/m_aspacehl.c | 6 | ||||
-rw-r--r-- | coregrind/m_aspacemgr/aspacemgr-linux.c | 16 | ||||
-rw-r--r-- | coregrind/m_coredump/coredump-elf.c | 5 | ||||
-rw-r--r-- | coregrind/m_main.c | 36 | ||||
-rw-r--r-- | include/pub_tool_aspacehl.h | 4 | ||||
-rw-r--r-- | include/pub_tool_aspacemgr.h | 23 | ||||
-rw-r--r-- | memcheck/mc_leakcheck.c | 6 |
7 files changed, 43 insertions, 53 deletions
diff --git a/coregrind/m_aspacehl.c b/coregrind/m_aspacehl.c index 36fb49d1d..17eccce2a 100644 --- a/coregrind/m_aspacehl.c +++ b/coregrind/m_aspacehl.c @@ -38,7 +38,9 @@ // addresses. The vector is dynamically allocated and should be freed // by the caller when done. REQUIRES m_mallocfree to be running. // Writes the number of addresses required into *n_acquired. -Addr* VG_(get_segment_starts) ( /*OUT*/Int* n_acquired ) +// Only those segments are considered whose kind matches any of the kinds +// given in KIND_MASK. +Addr* VG_(get_segment_starts) ( UInt kind_mask, /*OUT*/Int* n_acquired ) { Addr* starts; Int n_starts, r = 0; @@ -46,7 +48,7 @@ Addr* VG_(get_segment_starts) ( /*OUT*/Int* n_acquired ) n_starts = 1; while (True) { starts = VG_(malloc)( "main.gss.1", n_starts * sizeof(Addr) ); - r = VG_(am_get_segment_starts)( starts, n_starts ); + r = VG_(am_get_segment_starts)( kind_mask, starts, n_starts ); if (r >= 0) break; VG_(free)(starts); diff --git a/coregrind/m_aspacemgr/aspacemgr-linux.c b/coregrind/m_aspacemgr/aspacemgr-linux.c index f3eb7ceb5..25f304958 100644 --- a/coregrind/m_aspacemgr/aspacemgr-linux.c +++ b/coregrind/m_aspacemgr/aspacemgr-linux.c @@ -624,7 +624,8 @@ const HChar* VG_(am_get_filename)( NSegment const * seg ) return (i < 0) ? NULL : segnames + i; } -/* Collect up the start addresses of all non-free, non-resvn segments. +/* Collect up the start addresses of segments whose kind matches one of + the kinds specified in kind_mask. The interface is a bit strange in order to avoid potential segment-creation races caused by dynamic allocation of the result buffer *starts. @@ -638,7 +639,7 @@ const HChar* VG_(am_get_filename)( NSegment const * seg ) Correct use of this function may mean calling it multiple times in order to establish a suitably-sized buffer. */ -Int VG_(am_get_segment_starts)( Addr* starts, Int nStarts ) +Int VG_(am_get_segment_starts)( UInt kind_mask, Addr* starts, Int nStarts ) { Int i, j, nSegs; @@ -647,9 +648,8 @@ Int VG_(am_get_segment_starts)( Addr* starts, Int nStarts ) nSegs = 0; for (i = 0; i < nsegments_used; i++) { - if (nsegments[i].kind == SkFree || nsegments[i].kind == SkResvn) - continue; - nSegs++; + if ((nsegments[i].kind & kind_mask) != 0) + nSegs++; } if (nSegs > nStarts) { @@ -663,10 +663,8 @@ Int VG_(am_get_segment_starts)( Addr* starts, Int nStarts ) j = 0; for (i = 0; i < nsegments_used; i++) { - if (nsegments[i].kind == SkFree || nsegments[i].kind == SkResvn) - continue; - starts[j] = nsegments[i].start; - j++; + if ((nsegments[i].kind & kind_mask) != 0) + starts[j++] = nsegments[i].start; } aspacem_assert(j == nSegs); /* this should not fail */ diff --git a/coregrind/m_coredump/coredump-elf.c b/coregrind/m_coredump/coredump-elf.c index f0034ccac..2c59bba94 100644 --- a/coregrind/m_coredump/coredump-elf.c +++ b/coregrind/m_coredump/coredump-elf.c @@ -635,8 +635,9 @@ void make_elf_coredump(ThreadId tid, const vki_siginfo_t *si, ULong max_size) return; /* can't create file */ } - /* Get the segments */ - seg_starts = VG_(get_segment_starts)(&n_seg_starts); + /* Get the client segments */ + seg_starts = VG_(get_segment_starts)(SkFileC | SkAnonC | SkShmC, + &n_seg_starts); /* First, count how many memory segments to dump */ num_phdrs = 1; /* start with notes */ diff --git a/coregrind/m_main.c b/coregrind/m_main.c index b956ae848..82aa00a7f 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -2185,7 +2185,7 @@ Int valgrind_main ( Int argc, HChar **argv, HChar **envp ) Int n_seg_starts; Addr_n_ULong anu; - seg_starts = VG_(get_segment_starts)( &n_seg_starts ); + seg_starts = VG_(get_segment_starts)( SkFileC | SkFileV, &n_seg_starts ); vg_assert(seg_starts && n_seg_starts >= 0); /* show them all to the debug info reader. allow_SkFileV has to @@ -2207,7 +2207,7 @@ Int valgrind_main ( Int argc, HChar **argv, HChar **envp ) # elif defined(VGO_darwin) { Addr* seg_starts; Int n_seg_starts; - seg_starts = VG_(get_segment_starts)( &n_seg_starts ); + seg_starts = VG_(get_segment_starts)( SkFileC, &n_seg_starts ); vg_assert(seg_starts && n_seg_starts >= 0); /* show them all to the debug info reader. @@ -2291,38 +2291,20 @@ Int valgrind_main ( Int argc, HChar **argv, HChar **envp ) vg_assert(VG_(running_tid) == VG_INVALID_THREADID); VG_(running_tid) = tid_main; - seg_starts = VG_(get_segment_starts)( &n_seg_starts ); + seg_starts = VG_(get_segment_starts)( SkFileC | SkAnonC | SkShmC, + &n_seg_starts ); vg_assert(seg_starts && n_seg_starts >= 0); - /* show interesting ones to the tool */ + /* Show client segments to the tool */ for (i = 0; i < n_seg_starts; i++) { Word j, n; NSegment const* seg = VG_(am_find_nsegment)( seg_starts[i] ); vg_assert(seg); - if (seg->kind == SkFileC || seg->kind == SkAnonC) { - /* This next assertion is tricky. If it is placed - immediately before this 'if', it very occasionally fails. - Why? Because previous iterations of the loop may have - caused tools (via the new_mem_startup calls) to do - dynamic memory allocation, and that may affect the mapped - segments; in particular it may cause segment merging to - happen. Hence we cannot assume that seg_starts[i], which - reflects the state of the world before we started this - loop, is the same as seg->start, as the latter reflects - the state of the world (viz, mappings) at this particular - iteration of the loop. - - Why does moving it inside the 'if' make it safe? Because - any dynamic memory allocation done by the tools will - affect only the state of Valgrind-owned segments, not of - Client-owned segments. And the 'if' guards against that - -- we only get in here for Client-owned segments. - - In other words: the loop may change the state of - Valgrind-owned segments as it proceeds. But it should - not cause the Client-owned segments to change. */ - vg_assert(seg->start == seg_starts[i]); + vg_assert(seg->kind == SkFileC || seg->kind == SkAnonC || + seg->kind == SkShmC); + vg_assert(seg->start == seg_starts[i]); + { VG_(debugLog)(2, "main", "tell tool about %010lx-%010lx %c%c%c\n", seg->start, seg->end, diff --git a/include/pub_tool_aspacehl.h b/include/pub_tool_aspacehl.h index 00709c930..acc05a701 100644 --- a/include/pub_tool_aspacehl.h +++ b/include/pub_tool_aspacehl.h @@ -37,7 +37,9 @@ // addresses. The vector is dynamically allocated and should be freed // by the caller when done. REQUIRES m_mallocfree to be running. // Writes the number of addresses required into *n_acquired. -extern Addr* VG_(get_segment_starts) ( /*OUT*/Int* n_acquired ); +// Only those segments are considered whose kind matches any of the kinds +// given in KIND_MASK. +extern Addr* VG_(get_segment_starts)( UInt kind_mask, /*OUT*/Int* n_acquired ); #endif // __PUB_TOOL_ASPACEHL_H diff --git a/include/pub_tool_aspacemgr.h b/include/pub_tool_aspacemgr.h index 5edcb030f..75d9f9f32 100644 --- a/include/pub_tool_aspacemgr.h +++ b/include/pub_tool_aspacemgr.h @@ -36,16 +36,17 @@ //-------------------------------------------------------------- // Definition of address-space segments -/* Describes segment kinds. */ +/* Describes segment kinds. Enumerators are one-hot encoded so they + can be or'ed together. */ typedef enum { - SkFree, // unmapped space - SkAnonC, // anonymous mapping belonging to the client - SkAnonV, // anonymous mapping belonging to valgrind - SkFileC, // file mapping belonging to the client - SkFileV, // file mapping belonging to valgrind - SkShmC, // shared memory segment belonging to the client - SkResvn // reservation + SkFree = 0x01, // unmapped space + SkAnonC = 0x02, // anonymous mapping belonging to the client + SkAnonV = 0x04, // anonymous mapping belonging to valgrind + SkFileC = 0x08, // file mapping belonging to the client + SkFileV = 0x10, // file mapping belonging to valgrind + SkShmC = 0x20, // shared memory segment belonging to the client + SkResvn = 0x40 // reservation } SegKind; @@ -115,7 +116,8 @@ typedef NSegment; -/* Collect up the start addresses of all non-free, non-resvn segments. +/* Collect up the start addresses of segments whose kind matches one of + the kinds specified in kind_mask. The interface is a bit strange in order to avoid potential segment-creation races caused by dynamic allocation of the result buffer *starts. @@ -128,7 +130,8 @@ typedef Correct use of this function may mean calling it multiple times in order to establish a suitably-sized buffer. */ -extern Int VG_(am_get_segment_starts)( Addr* starts, Int nStarts ); +extern Int VG_(am_get_segment_starts)( UInt kind_mask, Addr* starts, + Int nStarts ); /* Finds the segment containing 'a'. Only returns file/anon/resvn segments. This returns a 'NSegment const *' - a pointer to diff --git a/memcheck/mc_leakcheck.c b/memcheck/mc_leakcheck.c index 4e6e28e67..671fff5eb 100644 --- a/memcheck/mc_leakcheck.c +++ b/memcheck/mc_leakcheck.c @@ -1612,7 +1612,8 @@ static void scan_memory_root_set(Addr searched, SizeT szB) { Int i; Int n_seg_starts; - Addr* seg_starts = VG_(get_segment_starts)( &n_seg_starts ); + Addr* seg_starts = VG_(get_segment_starts)( SkFileC | SkAnonC | SkShmC, + &n_seg_starts ); tl_assert(seg_starts && n_seg_starts > 0); @@ -1624,8 +1625,9 @@ static void scan_memory_root_set(Addr searched, SizeT szB) SizeT seg_size; NSegment const* seg = VG_(am_find_nsegment)( seg_starts[i] ); tl_assert(seg); + tl_assert(seg->kind == SkFileC || seg->kind == SkAnonC || + seg->kind == SkShmC); - if (seg->kind != SkFileC && seg->kind != SkAnonC) continue; if (!(seg->hasR && seg->hasW)) continue; if (seg->isCH) continue; |