In VG_(get_changed_segments) use dynamic memory allocation rather than

static memory allocation to avoid hardwiring an upper limit on CSS_SIZE.



git-svn-id: svn://svn.valgrind.org/valgrind/trunk@10374 a5019735-40e9-0310-863c-91ae7b9d1cf9

3 files changed, 53 insertions, 31 deletions

diff --git a/coregrind/m_aspacemgr/aspacemgr-linux.c b/coregrind/m_aspacemgr/aspacemgr-linux.c
index dc547e6b5..fc54f1938 100644
--- a/coregrind/m_aspacemgr/aspacemgr-linux.c
+++ b/coregrind/m_aspacemgr/aspacemgr-linux.c
@@ -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
diff --git a/coregrind/m_syswrap/syswrap-darwin.c b/coregrind/m_syswrap/syswrap-darwin.c
index 147e34b7a..ae69cd24e 100644
--- a/coregrind/m_syswrap/syswrap-darwin.c
+++ b/coregrind/m_syswrap/syswrap-darwin.c
@@ -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);
 }
 
 /* ---------------------------------------------------------------------
diff --git a/coregrind/pub_core_aspacemgr.h b/coregrind/pub_core_aspacemgr.h
index 372b9237a..b41801fb6 100644
--- a/coregrind/pub_core_aspacemgr.h
+++ b/coregrind/pub_core_aspacemgr.h
@@ -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