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
This commit is contained in:
Florian Krohm 2015-02-20 14:00:23 +00:00
parent 38adfae0a0
commit 00ffec50e5
7 changed files with 43 additions and 53 deletions

View File

@ -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);

View File

@ -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 */

View File

@ -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 */

View File

@ -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,

View File

@ -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

View File

@ -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

View File

@ -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;