[Valgrind-developers] valgrind/coregrind

[Jeremy F. <je...@go...>]

CVS commit by fitzhardinge: 

Fix nasty use after free bug revealed by last munmap fix.
Unexport split_segment; it isn't needed elsewhere.
(Something still wrong with munmap.)


  M +0 -1      vg_include.h   1.163
  M +18 -11    vg_memory.c   1.47
  M +1 -2      vg_symtab2.c   1.70


--- valgrind/coregrind/vg_include.h  #1.162:1.163
@@ -1581,5 +1581,4 @@ extern Addr VG_(find_map_space)(Addr bas
 extern Segment *VG_(find_segment)(Addr a);
 extern Segment *VG_(next_segment)(Segment *);
-extern Segment *VG_(split_segment)(Addr a);
 
 extern Bool     VG_(seg_contains)(const Segment *s, Addr ptr, UInt size);

--- valgrind/coregrind/vg_memory.c  #1.46:1.47
@@ -143,4 +143,7 @@ Segment *VG_(split_segment)(Addr a)
    ns->len -= delta;
 
+   if (s->filename != NULL)
+      ns->filename = VG_(arena_strdup)(VG_AR_CORE, s->filename);
+
    if (ns->symtab != NULL)
       VG_(symtab_incref)(ns->symtab);
@@ -157,15 +160,17 @@ void VG_(unmap_range)(Addr addr, UInt le
    Segment *s;
    Segment *next;
-   static const Bool debug = False || mem_debug;
-   Addr end = addr+len;
+   static const Bool debug = True || mem_debug;
+   Addr end;
 
    if (len == 0)
       return;
 
+   len = PGROUNDUP(len);
+   vg_assert(addr == PGROUNDDN(addr));
+
    if (debug)
       VG_(printf)("unmap_range(%p, %d)\n", addr, len);
 
-   len = PGROUNDUP(addr+len)-PGROUNDDN(addr);
-   addr = PGROUNDDN(addr);
+   end = addr+len;
 
    /* Everything must be page-aligned */
@@ -183,5 +188,5 @@ void VG_(unmap_range)(Addr addr, UInt le
       if (debug)
          VG_(printf)("unmap: addr=%p-%p s=%p ->addr=%p-%p len=%d\n",
-                     addr, addr+len, s, s->addr, s->addr+s->len, s->len);
+                     addr, end, s, s->addr, seg_end, s->len);
 
       if (!VG_(seg_overlaps)(s, addr, len)) {
@@ -218,10 +223,12 @@ void VG_(unmap_range)(Addr addr, UInt le
          Int delta = (addr+len) - s->addr;
 
+         if (debug)
+            VG_(printf)("  case 3: s->addr=%p s->len=%d delta=%d\n", s->addr, s->len, delta);
+
          s->addr += delta;
          s->offset += delta;
          s->len -= delta;
 
-         if (debug)
-            VG_(printf)("  case 3: s->addr=%p s->len=%d delta=%d\n", s->addr, s->len, delta);
+         vg_assert(s->len != 0);
       } else if (addr > s->addr && end < seg_end) {
          /* [addr, addr+len) is contained within a single segment
@@ -246,6 +253,6 @@ void VG_(unmap_range)(Addr addr, UInt le
 }
 
-/* If possible, merge segment with its neighbours - some segments,
-   including s, may be destroyed in the process */
+/* Return true if two segments are adjacent and mergable (s1 is
+   assumed to have a lower ->addr than s2) */
 static inline Bool neighbours(Segment *s1, Segment *s2)
 {
@@ -274,6 +281,6 @@ static inline Bool neighbours(Segment *s
 }
 
-/* Merge segments in the address range if they're adjacent and
-   compatible */
+/* If possible, merge segment with its neighbours - some segments,
+   including s, may be destroyed in the process */
 static void merge_segments(Addr a, UInt len)
 {

--- valgrind/coregrind/vg_symtab2.c  #1.69:1.70
@@ -1209,6 +1209,5 @@ SegInfo *VG_(read_seg_symbols) ( Segment
    si->size     = seg->len;
    si->foffset  = seg->offset;
-   si->filename = VG_(arena_malloc)(VG_AR_SYMTAB, 1 + VG_(strlen)(seg->filename));
-   VG_(strcpy)(si->filename, seg->filename);
+   si->filename = VG_(arena_strdup)(VG_AR_SYMTAB, seg->filename);
 
    si->ref = 1;