[Valgrind-developers] valgrind [POSSIBLY UNSAFE]

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

CVS commit by fitzhardinge: 

Fixed munmap bug - split_segment wasn't updating the lengths properly.
Turn off debug printing.


  M +15 -14    coregrind/vg_memory.c   1.48
  M +17 -5     none/tests/map_unmap.c   1.2 [POSSIBLY UNSAFE: printf,system]


--- valgrind/coregrind/vg_memory.c  #1.47:1.48
@@ -142,4 +142,5 @@ Segment *VG_(split_segment)(Addr a)
    ns->offset += delta;
    ns->len -= delta;
+   s->len = delta;
 
    if (s->filename != NULL)
@@ -160,5 +161,5 @@ void VG_(unmap_range)(Addr addr, UInt le
    Segment *s;
    Segment *next;
-   static const Bool debug = True || mem_debug;
+   static const Bool debug = False || mem_debug;
    Addr end;
 
@@ -207,22 +208,12 @@ void VG_(unmap_range)(Addr addr, UInt le
          if (debug)
             VG_(printf)("  case 1: s->len=%d\n", s->len);
-      } else if (addr <= s->addr && end >= seg_end) {
-         /* this segment is completely contained within [addr, addr+len)
-            -> delete segment
-         */
-         Segment *rs = VG_(SkipList_Remove)(&sk_segments, &s->addr);
-         vg_assert(rs == s);
-         freeseg(s);
-
-         if (debug)
-            VG_(printf)("  case 2: s==%p deleted\n", s);
       } else if (addr <= s->addr && end > s->addr && end < seg_end) {
          /* this segment's head is truncated by [addr, addr+len)
             -> truncate head
          */
-         Int delta = (addr+len) - s->addr;
+         Int delta = end - s->addr;
 
          if (debug)
-            VG_(printf)("  case 3: s->addr=%p s->len=%d delta=%d\n", s->addr, s->len, delta);
+            VG_(printf)("  case 2: s->addr=%p s->len=%d delta=%d\n", s->addr, s->len, delta);
 
          s->addr += delta;
@@ -231,4 +222,14 @@ void VG_(unmap_range)(Addr addr, UInt le
 
          vg_assert(s->len != 0);
+      } else if (addr <= s->addr && end >= seg_end) {
+         /* this segment is completely contained within [addr, addr+len)
+            -> delete segment
+         */
+         Segment *rs = VG_(SkipList_Remove)(&sk_segments, &s->addr);
+         vg_assert(rs == s);
+         freeseg(s);
+
+         if (debug)
+            VG_(printf)("  case 3: s==%p deleted\n", s);
       } else if (addr > s->addr && end < seg_end) {
          /* [addr, addr+len) is contained within a single segment
@@ -489,7 +490,7 @@ void VG_(mprotect_range)(Addr a, UInt le
 Addr VG_(find_map_space)(Addr addr, UInt len, Bool for_client)
 {
+   static const Bool debug = False || mem_debug;
    Segment *s;
    Addr ret;
-   static const Bool debug = False || mem_debug;
    Addr limit = (for_client ? VG_(client_end) : VG_(valgrind_mmap_end));
 

--- valgrind/none/tests/map_unmap.c  #1.1:1.2
@@ -27,7 +27,8 @@ static void nibblemap(void *p)
         int i;
 
-        off = (random() & ~0x1fff) % LEN;
+        off = (random() % LEN) & ~(pagesize-1);
         
         for(i = 0; i < PAGES; i++) {
+                /* printf("unmapping off=%d\n", off/pagesize); */
                 munmap((char *)p + off, pagesize);
                 off += 619*pagesize;
@@ -36,4 +37,12 @@ static void nibblemap(void *p)
 }
 
+static void prmaps()
+{
+        char buf[100];
+        sprintf(buf, "/bin/cat /proc/%d/maps", getpid());
+        system(buf);
+        exit(1);
+}
+
 int main()
 {
@@ -52,12 +62,14 @@ int main()
                 m1 = domap();
                 if (m1 != expect1) {
-                        printf("FAIL: m=%p expect=%p\n",
-                               m1, expect1);
+                        printf("FAIL i=%d: m1=%p expect1=%p\n",
+                               i, m1, expect1);
+                        prmaps();
                         return 1;
                 }
                 m2 = domap();
                 if (m2 != expect2) {
-                        printf("FAIL: m=%p expect=%p\n",
-                               m2, expect2);
+                        printf("FAIL i=%d: m2=%p expect2=%p\n",
+                               i, m2, expect2);
+                        prmaps();
                         return 1;
                 }