Add an expensive sanity check to make sure that the Segment list is consistent with the contents of /proc/self/maps at all times.es coregrind/core.h | 6 coregrind/vg_main.c | 44 ++++++ coregrind/vg_memory.c | 278 ++++++++++++++++++++++++++++++++++++++++++-- coregrind/vg_procselfmaps.c | 3 4 files changed, 315 insertions(+), 16 deletions(-) diff -puN coregrind/vg_memory.c~segment-sanity coregrind/vg_memory.c --- valgrind/coregrind/vg_memory.c~segment-sanity 2005-01-13 16:01:40.000000000 -0800 +++ valgrind-jeremy/coregrind/vg_memory.c 2005-01-13 17:49:27.000000000 -0800 @@ -113,6 +113,15 @@ static void freeseg(Segment *s) VG_(SkipNode_Free)(&sk_segments, s); } +static inline Segment *allocseg() +{ + Segment *s; + + s = VG_(SkipNode_Alloc)(&sk_segments); + + return s; +} + /* Split a segment at address a, returning the new segment */ Segment *VG_(split_segment)(Addr a) { @@ -132,7 +141,7 @@ Segment *VG_(split_segment)(Addr a) vg_assert(a > s->addr && a < (s->addr+s->len)); - ns = VG_(SkipNode_Alloc)(&sk_segments); + ns = allocseg(); *ns = *s; @@ -142,6 +151,10 @@ Segment *VG_(split_segment)(Addr a) ns->len -= delta; s->len = delta; + vg_assert(ns->len > 0); + vg_assert(s->len > 0); + vg_assert(ns->addr == (s->addr+s->len)); + if (s->filename != NULL) ns->filename = VG_(arena_strdup)(VG_AR_CORE, s->filename); @@ -204,6 +217,8 @@ void VG_(unmap_range)(Addr addr, SizeT l */ s->len = addr - s->addr; + vg_assert(s->len > 0); + if (debug) VG_(printf)(" case 1: s->len=%d\n", s->len); } else if (addr <= s->addr && end > s->addr && end < seg_end) { @@ -219,7 +234,7 @@ void VG_(unmap_range)(Addr addr, SizeT l s->offset += delta; s->len -= delta; - vg_assert(s->len != 0); + vg_assert(s->len > 0); } else if (addr <= s->addr && end >= seg_end) { /* this segment is completely contained within [addr, addr+len) -> delete segment @@ -239,7 +254,8 @@ void VG_(unmap_range)(Addr addr, SizeT l middle = VG_(split_segment)(addr); VG_(split_segment)(addr+len); - vg_assert(middle->addr == addr); + vg_assert(middle->addr == addr); + vg_assert(middle->len == len); rs = VG_(SkipList_Remove)(&sk_segments, &addr); vg_assert(rs == middle); @@ -250,6 +266,8 @@ void VG_(unmap_range)(Addr addr, SizeT l addr, addr+len); } } + + //VG_(sanity_check_memory)(); } /* Return true if two segments are adjacent and mergable (s1 is @@ -326,6 +344,9 @@ void VG_(map_file_segment)(Addr addr, Si VG_(printf)("map_file_segment(%p, %llu, %x, %x, %4x, %d, %ld, %s)\n", addr, (ULong)len, prot, flags, dev, ino, off, filename); + if (len == 0) + return; + /* Everything must be page-aligned */ vg_assert((addr & (VKI_PAGE_SIZE-1)) == 0); len = PGROUNDUP(len); @@ -352,7 +373,7 @@ void VG_(map_file_segment)(Addr addr, Si recycled = False; VG_(unmap_range)(addr, len); - s = VG_(SkipNode_Alloc)(&sk_segments); + s = allocseg(); s->addr = addr; s->len = len; @@ -462,25 +483,42 @@ void VG_(mprotect_range)(Addr a, SizeT l { Segment *s, *next; static const Bool debug = False || mem_debug; + Segment *s1, *s2; if (debug) VG_(printf)("mprotect_range(%p, %d, %x)\n", a, len, prot); + if (len == 0) + return; + /* Everything must be page-aligned */ vg_assert((a & (VKI_PAGE_SIZE-1)) == 0); len = PGROUNDUP(len); - VG_(split_segment)(a); - VG_(split_segment)(a+len); + s1 = VG_(split_segment)(a); + s2 = VG_(split_segment)(a+len); - for(s = VG_(SkipList_Find)(&sk_segments, &a); + if (debug) { + s = VG_(find_segment)(a); + VG_(printf)(" split: s1=%p-%p s2=%p-%p s(%p)=%p-%p\n", + s1 ? s1->addr : 0, s1 ? (s1->addr+s1->len) : 0, + s2 ? s2->addr : 0, s2 ? (s2->addr+s2->len) : 0, + a, + s ? s->addr : 0, s ? (s->addr+s->len) : 0); + } + + for(s = VG_(find_segment)(a); s != NULL && s->addr < a+len; s = next) { - next = VG_(SkipNode_Next)(&sk_segments, s); + next = VG_(next_segment)(s); if (s->addr < a) continue; + if (debug) + VG_(printf)(" setting s->%p - %p to prot %x\n", + s->addr, s->addr+s->len, prot); + s->prot = prot; } @@ -845,12 +883,19 @@ void *VG_(shadow_alloc)(UInt size) size = PGROUNDUP(size); - if (shadow_alloc == 0) + if (shadow_alloc == 0) { shadow_alloc = VG_(shadow_base); + vg_assert(VG_(is_addressable)(VG_(shadow_base), VG_(shadow_end)-VG_(shadow_base), + VKI_PROT_NONE)); + } if (shadow_alloc >= VG_(shadow_end)) return 0; + if (0) + VG_(printf)("shadow_base=%p shadow_alloc=%p alloc %d\n", + VG_(shadow_base), shadow_alloc, size); + ret = (void *)shadow_alloc; VG_(mprotect)(ret, size, VKI_PROT_READ|VKI_PROT_WRITE); @@ -860,6 +905,221 @@ void *VG_(shadow_alloc)(UInt size) } /*--------------------------------------------------------------------*/ +/*--- Sanity checking ---*/ +/*--------------------------------------------------------------------*/ + +static Segment *next_segment; +static Bool segment_maps_ok; +static Addr prevmapstart, prevmapend; + +static void check_segment_maps(Addr addr, SizeT len, Char rr, Char ww, Char xx, + UInt dev, UInt ino, ULong foff, const UChar *filename) +{ + Addr segend; + Addr end = addr+len; + Segment *seg, *next, *prev; + UInt prot; + + if (addr >= VG_(valgrind_last)) + return; /* don't care */ + + if (addr < prevmapend) { + VG_(printf)("KERNEL BUG: mapping %p-%p <= prevmap %p-%p !?\n", + addr, end, prevmapstart, prevmapend); + return; + } + prevmapstart = addr; + prevmapend = end; + + prot = 0; + prot |= rr == 'r' ? VKI_PROT_READ : 0; + prot |= ww == 'w' ? VKI_PROT_WRITE: 0; + prot |= xx == 'x' ? VKI_PROT_EXEC : 0; + + if (next_segment == NULL) + seg = VG_(first_segment)(); + else + seg = next_segment; + + if (seg == NULL) + return; /* no segments yet */ + + segend = seg->addr + seg->len; + + /* + The invariants here are: + 1. all mappings from /proc/self/maps must be completely covered by Segments + - there may be multiple Segments per map + 2. a Segment may never overlap the ends of a map + 3. all Segments must be backed by mappings + 4. Segment permissions must not be a superset of mapping permissions (subset OK) + */ + + /* + Check invariant 3: this segment must have the same start address + as the mapping. If we find a floating Segment (ie, has no + mapping backing it), then skip forward through list until we + find a Segment which could overlap this mapping. + + And inv 2: if it doesn't start at the beginnging of the mapping, + it should be some completely floating segment; it must not + overlap the mapping. + */ + prev = NULL; + while (seg && seg->addr < addr) { + VG_(printf)("INV 3 FAILED: seg %p-%p %4x doesn't start at beginning of mapping %p-%p\n", + seg->addr, segend, seg->flags, addr, end); + if (VG_(seg_overlaps)(seg, addr, len)) + VG_(printf)(" 2 FAILED: seg %p-%p overlaps of mapping %p-%p\n", + seg->addr, segend, addr, end); + segment_maps_ok = False; + prev = seg; + seg = VG_(next_segment)(seg); + } + + if (seg == NULL) + return; /* no more segments */ + + /* Iterate over all Segments covering the mapping */ + next = VG_(next_segment)(seg); + + while(seg && (seg->addr < end)) { + prev = seg; + segend = seg->addr + seg->len; + + if (0) + VG_(printf)("seg: %p-%p (%c%c%c) %4x mapping %p-%p %x (%c%c%c) %s%s %s\n", + seg->addr, segend, + seg->prot & VKI_PROT_READ ? 'r' : '-', + seg->prot & VKI_PROT_WRITE? 'w' : '-', + seg->prot & VKI_PROT_EXEC ? 'x' : '-', + seg->flags, + addr, end, prot, rr, ww, xx, + (addr == seg->addr && segend == end) ? "" : " !!!", + (seg->prot & prot) != seg->prot ? " ???" : "", + seg->filename ? seg->filename : (const Char *)""); + + vg_assert(VG_(seg_overlaps)(seg, addr, len)); + + if ((seg->addr < addr) || (segend > end)) { + VG_(printf)("INV 2 FAILED: seg %p-%p crosses boundaries of mapping %p-%p\n", + seg->addr, segend, + addr, end); + segment_maps_ok = False; + } + + if ((seg->prot & prot) != seg->prot) { + VG_(printf)("INV 4 FAILED: seg %p-%p permissions %c%c%c not subset of mapping %p-%p permissions %c%c%c\n", + seg->addr, segend, + seg->prot & VKI_PROT_READ ? 'r' : '-', + seg->prot & VKI_PROT_WRITE? 'w' : '-', + seg->prot & VKI_PROT_EXEC ? 'x' : '-', + addr, end, + rr, ww, xx); + segment_maps_ok = False; + } + + if (filename != NULL) { + Int delta; + + if (!(seg->flags & SF_FILE)) { + VG_(printf)("seg %p-%p flags %x does't have SF_FILE\n", + seg->addr, segend, seg->flags); + segment_maps_ok = False; + } + + delta = seg->addr - addr; + if ((seg->offset - delta) != foff) { + VG_(printf)("seg %p-%p file %s delta %d offset %lld != (%p-%p) %lld %s\n", + seg->addr, segend, seg->filename, delta, seg->offset, + addr, end, foff, filename); + segment_maps_ok = False; + } + } + + next = VG_(next_segment)(seg); + + /* segments covering a map must be contigious */ + if (next == NULL || next->addr != segend) + break; + + prev = seg; + seg = next; + } + + segend = seg->addr + seg->len; + + if (0) + VG_(printf)("loop done seg=%p-%p next=%p-%p map=%p-%p\n", + seg ? seg->addr : 0, seg ? (seg->addr+seg->len) : 0, + next ? next->addr : 0, next ? (next->addr+next->len) : 0, + addr, end); + + /* otherwise the loop exited too early */ + //vg_assert(seg == NULL || segend <= end); + vg_assert(next == NULL || next->addr >= end); + + if ((prev->addr+prev->len) != end) { + VG_(printf)("INV 1 FAILED: seg -%p does not end at mapping %p-%p end\n", + prev->addr, prev->addr+prev->len, addr, end); + segment_maps_ok = False; + } + + next_segment = next; +} + +Bool VG_(sanity_check_memory)(void) +{ + Segment *seg; + Addr prev; + Bool ok = True; + + /* First, walk the segment list and make sure it looks internally OK */ + prev = 0; + for(seg = VG_(first_segment)(); + seg != NULL; + seg = VG_(next_segment)(seg)) { + Addr end = seg->addr + seg->len; + + if (end <= seg->addr) { + ok = False; + VG_(printf)("Segment %p: %p-%p has bad size %d\n", + seg, seg->addr, end, seg->len); + } + + if (seg->addr < prev) { + ok = False; + VG_(printf)("Segment %p: %p-%p is before previous seg (ending at %p)\n", + seg, seg->addr, end, prev); + } + + if (!!(seg->flags & SF_VALGRIND) != (seg->addr >= VG_(client_end))) { + ok = False; + VG_(printf)("Segment %p: %p-%p has flags (%x) inconsistent with client_end=%p\n", + seg, seg->addr, seg->addr+seg->len, seg->flags, VG_(client_end)); + } + + if (prev < end) + prev = end; + } + + /* Now compare with /proc/self/maps */ + VG_(read_procselfmaps)(); + + next_segment = NULL; + segment_maps_ok = True; + prevmapstart = prevmapend = 0; + VG_(parse_procselfmaps)(check_segment_maps); + + ok = ok && segment_maps_ok; + + if (0 && ok) + VG_(printf)("(memory segments OK)\n"); + + return ok && segment_maps_ok; +} + +/*--------------------------------------------------------------------*/ /*--- end vg_memory.c ---*/ /*--------------------------------------------------------------------*/ diff -puN coregrind/core.h~segment-sanity coregrind/core.h --- valgrind/coregrind/core.h~segment-sanity 2005-01-13 16:01:40.000000000 -0800 +++ valgrind-jeremy/coregrind/core.h 2005-01-13 18:39:29.000000000 -0800 @@ -113,8 +113,8 @@ typedef struct _ThreadState ThreadState; /* Size of a buffer used for creating messages. */ #define M_VG_MSGBUF 10000 -/* Size of a smallish table used to read /proc/self/map entries. */ -#define M_PROCMAP_BUF 50000 +/* Size of a big enough table used to read /proc/self/map entries. */ +#define M_PROCMAP_BUF 200000 /* Max length of pathname to a .so/executable file. */ #define M_VG_LIBNAMESTR 100 @@ -1252,6 +1252,8 @@ extern void VG_(unpad_address_space)(voi extern REGPARM(1) void VG_(unknown_SP_update) ( Addr new_SP ); +extern Bool VG_(sanity_check_memory)(void); + /* --------------------------------------------------------------------- Exports of vg_syscalls.c ------------------------------------------------------------------ */ diff -puN coregrind/vg_main.c~segment-sanity coregrind/vg_main.c --- valgrind/coregrind/vg_main.c~segment-sanity 2005-01-13 16:01:40.000000000 -0800 +++ valgrind-jeremy/coregrind/vg_main.c 2005-01-13 17:45:19.000000000 -0800 @@ -2320,7 +2320,7 @@ static void build_valgrind_map_callback symbols. This is so we know where the free space is before we start allocating more memory (note: heap is OK, it's just mmap which is the problem here). */ - if (start >= VG_(valgrind_base) && (start+size-1) <= VG_(valgrind_last)) { + if (start >= VG_(client_end) && (start+size-1) <= VG_(valgrind_last)) { flags |= SF_VALGRIND; VG_(map_file_segment)(start, size, prot, flags, dev, ino, foffset, filename); } @@ -2329,6 +2329,16 @@ static void build_valgrind_map_callback // Global var used to pass local data to callback Addr sp_at_startup___global_arg = 0; +/* + This second pass adds in client mappings, and loads symbol tables + for all interesting mappings. The trouble is that things can + change as we go, because we're calling the Tool to track memory as + we find it. + + So for Valgrind mappings, we don't replace any mappings which + aren't still identical (which will include the .so mappings, so we + will load their symtabs)> + */ static void build_segment_map_callback ( Addr start, SizeT size, Char rr, Char ww, Char xx, UInt dev, UInt ino, ULong foffset, const UChar* filename ) @@ -2341,6 +2351,10 @@ static void build_segment_map_callback is_stack_segment = (start == VG_(clstk_base) && (start+size) == VG_(clstk_end)); + if (0) + VG_(printf)("init: %p-%p prot %c%c%c stack=%d\n", + start, start+size, rr, ww, xx, is_stack_segment); + if (rr == 'r') prot |= VKI_PROT_READ; if (ww == 'w') prot |= VKI_PROT_WRITE; if (xx == 'x') prot |= VKI_PROT_EXEC; @@ -2353,10 +2367,27 @@ static void build_segment_map_callback if (filename != NULL) flags |= SF_FILE; - if (start >= VG_(valgrind_base) && (start+size-1) <= VG_(valgrind_last)) - flags |= SF_VALGRIND; + if (start >= VG_(client_end) && (start+size-1) <= VG_(valgrind_last)) { + Segment *s = VG_(find_segment)(start); - VG_(map_file_segment)(start, size, prot, flags, dev, ino, foffset, filename); + /* We have to be a bit careful about inserting new mappings into + the Valgrind part of the address space. We're actively + changing things as we parse these mappings, particularly in + shadow memory, and so we don't want to overwrite those + changes. Therefore, we only insert/update a mapping if it is + mapped from a file or it exactly matches an existing mapping. + + NOTE: we're only talking about the Segment list mapping + metadata; this doesn't actually mmap anything more. */ + if (filename || (s && s->addr == start && s->len == size)) { + flags |= SF_VALGRIND; + VG_(map_file_segment)(start, size, prot, flags, dev, ino, foffset, filename); + } else { + /* assert range is already mapped */ + vg_assert(VG_(is_addressable)(start, size, VKI_PROT_NONE)); + } + } else + VG_(map_file_segment)(start, size, prot, flags, dev, ino, foffset, filename); if (VG_(is_client_addr)(start) && VG_(is_client_addr)(start+size-1)) VG_TRACK( new_mem_startup, start, size, rr=='r', ww=='w', xx=='x' ); @@ -2422,6 +2453,9 @@ void VG_(sanity_check_general) ( Bool fo vg_assert(SK_(expensive_sanity_check)()); VGP_POPCC(VgpSkinExpensiveSanity); } + + vg_assert(VG_(sanity_check_memory)()); + /* if ((sanity_fast_count % 500) == 0) VG_(mallocSanityCheckAll)(); */ @@ -2685,10 +2719,12 @@ int main(int argc, char **argv, char **e //-------------------------------------------------------------- // Build segment map (all segments) + // p: shadow/redzone segments // p: setup_client_stack() [for 'sp_at_startup'] // p: init tool [for 'new_mem_startup'] //-------------------------------------------------------------- sp_at_startup___global_arg = sp_at_startup; + VG_(read_procselfmaps)(); VG_(parse_procselfmaps) ( build_segment_map_callback ); /* everything */ sp_at_startup___global_arg = 0; diff -puN coregrind/vg_procselfmaps.c~segment-sanity coregrind/vg_procselfmaps.c --- valgrind/coregrind/vg_procselfmaps.c~segment-sanity 2005-01-13 16:01:40.000000000 -0800 +++ valgrind-jeremy/coregrind/vg_procselfmaps.c 2005-01-13 18:41:44.000000000 -0800 @@ -100,8 +100,9 @@ void VG_(read_procselfmaps)(void) } buf_n_tot = 0; do { + Int want = M_PROCMAP_BUF - buf_n_tot; n_chunk = VG_(read) ( fd, &procmap_buf[buf_n_tot], - M_PROCMAP_BUF - buf_n_tot ); + want > 4000 ? 4000 : want ); buf_n_tot += n_chunk; } while ( n_chunk > 0 && buf_n_tot < M_PROCMAP_BUF ); VG_(close)(fd); _