]> git.ipfire.org Git - thirdparty/valgrind.git/commitdiff
In VG_(get_changed_segments) use dynamic memory allocation rather than
authorNicholas Nethercote <njn@valgrind.org>
Wed, 24 Jun 2009 08:32:42 +0000 (08:32 +0000)
committerNicholas Nethercote <njn@valgrind.org>
Wed, 24 Jun 2009 08:32:42 +0000 (08:32 +0000)
static memory allocation to avoid hardwiring an upper limit on CSS_SIZE.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@10374

coregrind/m_aspacemgr/aspacemgr-linux.c
coregrind/m_syswrap/syswrap-darwin.c
coregrind/pub_core_aspacemgr.h

index dc547e6b5c395fc05861c46ed93e54ae1341f198..fc54f1938ef956eec17ffa8377490225a5957155 100644 (file)
@@ -3342,6 +3342,7 @@ static void parse_procselfmaps (
       (*record_gap)(last, (Addr)-1 - last);
 }
 
+Bool        css_overflowed;
 ChangedSeg* css_local;
 Int         css_size_local;
 Int         css_used_local;
@@ -3376,15 +3377,16 @@ static void add_mapping_callback(Addr addr, SizeT len, UInt prot,
       else if (nsegments[i].kind == SkFree || nsegments[i].kind == SkResvn) {
           /* Add mapping for SkResvn regions */
          ChangedSeg* cs = &css_local[css_used_local];
-         // If this assert fails, the css_size arg passed to
-         // VG_(get_changed_segments) needs to be increased.
-         aspacem_assert(css_used_local < css_size_local);
-         cs->is_added = True;
-         cs->start    = addr;
-         cs->end      = addr + len - 1;
-         cs->prot     = prot;
-         cs->offset   = offset;
-         css_used_local++;
+         if (css_used_local < css_size_local) {
+            cs->is_added = True;
+            cs->start    = addr;
+            cs->end      = addr + len - 1;
+            cs->prot     = prot;
+            cs->offset   = offset;
+            css_used_local++;
+         } else {
+            css_overflowed = True;
+         }
          return;
 
       } else if (nsegments[i].kind == SkAnonC ||
@@ -3437,22 +3439,24 @@ static void remove_mapping_callback(Addr addr, SizeT len)
       if (nsegments[i].kind != SkFree  &&  nsegments[i].kind != SkResvn) {
          // V has a mapping, kernel doesn't
          ChangedSeg* cs = &css_local[css_used_local];
-         // If this assert fails, the css_size arg passed to
-         // VG_(get_changed_segments) needs to be increased.
-         aspacem_assert(css_used_local < css_size_local);
-         cs->is_added = False;
-         cs->start    = nsegments[i].start;
-         cs->end      = nsegments[i].end;
-         cs->prot     = 0;
-         cs->offset   = 0;
-         css_used_local++;
+         if (css_used_local < css_size_local) {
+            cs->is_added = False;
+            cs->start    = nsegments[i].start;
+            cs->end      = nsegments[i].end;
+            cs->prot     = 0;
+            cs->offset   = 0;
+            css_used_local++;
+         } else {
+            css_overflowed = True;
+         }
          return;
       }
    }
 }
 
 
-void VG_(get_changed_segments)(
+// Returns False if 'css' wasn't big enough.
+Bool VG_(get_changed_segments)(
       const HChar* when, const HChar* where, /*OUT*/ChangedSeg* css,
       Int css_size, /*OUT*/Int* css_used)
 {
@@ -3465,6 +3469,7 @@ void VG_(get_changed_segments)(
          stats_synccalls++, stats_machcalls, when, where
       );
 
+   css_overflowed = False;
    css_local = css;
    css_size_local = css_size;
    css_used_local = 0;
@@ -3473,6 +3478,12 @@ void VG_(get_changed_segments)(
    parse_procselfmaps(&add_mapping_callback, &remove_mapping_callback);
 
    *css_used = css_used_local;
+
+   if (css_overflowed) {
+      aspacem_assert(css_used_local == css_size_local);
+   }
+
+   return !css_overflowed;
 }
 
 #endif // HAVE_PROC
index 147e34b7ad4c8f5c0d4498a4bcb9b76cf223d9b5..ae69cd24e8d5e4dece71223179d1ee778ba07f4b 100644 (file)
@@ -595,17 +595,26 @@ void VG_(show_open_ports)(void)
 static void sync_mappings(const HChar *when, const HChar *where, Int num)
 {
    // Usually the number of segments added/removed in a single calls is very
-   // small e.g. 1.  But the limit was 20 at one point, and that wasn't enough
-   // for at least one invocation of Firefox.  If we need to go much bigger,
-   // should probably make VG_(get_changed_segments) fail if the size isn't
-   // big enough, and repeatedly redo it with progressively bigger dynamically
-   // allocated buffers until it succeeds.
-   #define CSS_SIZE     100
-   ChangedSeg css[CSS_SIZE];
-   Int        css_used;
-   Int        i;
-
-   VG_(get_changed_segments)(when, where, css, CSS_SIZE, &css_used);
+   // small e.g. 1.  But it sometimes gets up to at least 100 or so (eg. for
+   // Quicktime).  So we use a repeat-with-bigger-buffers-until-success model,
+   // because we can't do dynamic allocation within VG_(get_changed_segments),
+   // because it's in m_aspacemgr.
+   ChangedSeg* css = NULL;
+   Int         css_size;
+   Int         css_used;
+   Int         i;
+   Bool        ok;
+
+   // 16 is enough for most cases, but small enough that overflow happens
+   // occasionally and thus the overflow path gets some test coverage.
+   css_size = 16;
+   ok = False;
+   while (!ok) {
+      VG_(free)(css);   // css is NULL on first iteration;  that's ok.
+      css = VG_(malloc)("sys_wrap.sync_mappings", css_size*sizeof(ChangedSeg));
+      ok = VG_(get_changed_segments)(when, where, css, css_size, &css_used);
+      css_size *= 2;
+   } 
 
    // Now add/remove them.
    for (i = 0; i < css_used; i++) {
@@ -629,6 +638,8 @@ static void sync_mappings(const HChar *when, const HChar *where, Int num)
                         action, cs->start, cs->end + 1, where, when);
       }
    }
+
+   VG_(free)(css);
 }
 
 /* ---------------------------------------------------------------------
index 372b9237a50a6e21ed56d94d1490b1183eb1d75a..b41801fb624551e664e050d474c81663ae23c0d8 100644 (file)
@@ -413,7 +413,7 @@ typedef
    }
    ChangedSeg;
 
-extern void VG_(get_changed_segments)(
+extern Bool VG_(get_changed_segments)(
       const HChar* when, const HChar* where, /*OUT*/ChangedSeg* css,
       Int css_size, /*OUT*/Int* css_used);
 #endif