CarpetLib: Make mem<T> safer

When passing in a pointer to mem<T>, also pass the size of the
pointed-to memory region, so that mem<T> can check that there is
enough space.

Keep track of the number of allocated bytes in mempools.

darcs-hash:20080219044528-dae7b-107edc6f696a35aad32ef6e58129b3281d00eb56.gz

6 files changed, 36 insertions, 18 deletions

diff --git a/Carpet/CarpetLib/src/data.cc b/Carpet/CarpetLib/src/data.cc
index 2c334da5a..8be299a21 100644
--- a/Carpet/CarpetLib/src/data.cc
+++ b/Carpet/CarpetLib/src/data.cc
@@ -262,7 +262,8 @@ data<T>* data<T>::make_typed (const int varindex_,
 template<typename T>
 void data<T>::allocate (const ibbox& extent_,
                         const int proc_,
-                        void* const memptr)
+                        void* const memptr,
+                        size_t const memsize)
 {
   assert (not _has_storage);
   _has_storage = true;
@@ -284,7 +285,7 @@ void data<T>::allocate (const ibbox& extent_,
   if (dist::rank() == _proc) {
     if (vectorindex == 0) {
       assert (not vectorleader);
-      _memory = new mem<T> (vectorlength, _size, (T*)memptr);
+      _memory = new mem<T> (vectorlength, _size, (T*)memptr, memsize);
     } else {
       assert (vectorleader);
       _memory = vectorleader->_memory;
diff --git a/Carpet/CarpetLib/src/data.hh b/Carpet/CarpetLib/src/data.hh
index be30a96c7..6b7a774e1 100644
--- a/Carpet/CarpetLib/src/data.hh
+++ b/Carpet/CarpetLib/src/data.hh
@@ -67,7 +67,7 @@ public:
 
   // Storage management
   virtual void allocate (const ibbox& extent, const int proc,
-			 void* const memptr = NULL);
+			 void* const memptr = NULL, size_t const memsize = 0);
   virtual void free ();
   virtual size_t allocsize (const ibbox& extent, const int proc) const;
   
diff --git a/Carpet/CarpetLib/src/gdata.cc b/Carpet/CarpetLib/src/gdata.cc
index f5b31f6ac..4f8244d32 100644
--- a/Carpet/CarpetLib/src/gdata.cc
+++ b/Carpet/CarpetLib/src/gdata.cc
@@ -154,22 +154,24 @@ transfer_from (comm_state & state,
     if (proc() != src->proc()) {
       if (src->proc() == dist::rank()) {
         if (interp_on_src) {
+          size_t const sendbufsize = c_datatype_size() * dstbox.size();
           void * const sendbuf =
             state.send_buffer (c_datatype(), proc(), dstbox.size());
           gdata * const buf =
             make_typed (varindex, cent, transport_operator, tag);
-          buf->allocate (dstbox, src->proc(), sendbuf);
+          buf->allocate (dstbox, src->proc(), sendbuf, sendbufsize);
           buf->transfer_from_innerloop
             (srcs, times, dstbox, time, order_space, order_time);
           delete buf;
           state.commit_send_space (c_datatype(), proc(), dstbox.size());
         } else {
           for (int tl = timelevel0; tl < timelevel0 + ntimelevels; ++ tl) {
+            size_t const sendbufsize = c_datatype_size() * srcbox.size();
             void * const sendbuf =
               state.send_buffer (c_datatype(), proc(), srcbox.size());
             gdata * const buf =
               make_typed (varindex, cent, transport_operator, tag);
-            buf->allocate (srcbox, src->proc(), sendbuf);
+            buf->allocate (srcbox, src->proc(), sendbuf, sendbufsize);
             buf->copy_from_innerloop (srcs.AT(tl), srcbox);
             delete buf;
             state.commit_send_space (c_datatype(), proc(), srcbox.size());
@@ -195,11 +197,12 @@ transfer_from (comm_state & state,
     if (proc() != src->proc()) {
       if (proc() == dist::rank()) {
         if (interp_on_src) {
+          size_t const recvbufsize = c_datatype_size() * dstbox.size();
           void * const recvbuf =
             state.recv_buffer (c_datatype(), src->proc(), dstbox.size());
           gdata * const buf =
             make_typed (varindex, cent, transport_operator, tag);
-          buf->allocate (dstbox, proc(), recvbuf);
+          buf->allocate (dstbox, proc(), recvbuf, recvbufsize);
           state.commit_recv_space (c_datatype(), src->proc(), dstbox.size());
           copy_from_innerloop (buf, dstbox);
           delete buf;
@@ -207,11 +210,12 @@ transfer_from (comm_state & state,
           gdata const * const null = 0;
           vector <gdata const *> bufs (timelevel0 + ntimelevels, null);
           for (int tl = timelevel0; tl < timelevel0 + ntimelevels; ++ tl) {
+            size_t const recvbufsize = c_datatype_size() * srcbox.size();
             void * const recvbuf =
               state.recv_buffer (c_datatype(), src->proc(), srcbox.size());
             gdata * const buf =
               make_typed (varindex, cent, transport_operator, tag);
-            buf->allocate (srcbox, proc(), recvbuf);
+            buf->allocate (srcbox, proc(), recvbuf, recvbufsize);
             state.commit_recv_space (c_datatype(), src->proc(), srcbox.size());
             bufs.AT(tl) = buf;
           }
diff --git a/Carpet/CarpetLib/src/gdata.hh b/Carpet/CarpetLib/src/gdata.hh
index 76c1ab6fe..09622fb34 100644
--- a/Carpet/CarpetLib/src/gdata.hh
+++ b/Carpet/CarpetLib/src/gdata.hh
@@ -74,7 +74,7 @@ public:
   
   // Storage management
   virtual void allocate (const ibbox& extent, const int proc,
-			 void* const mem=0) = 0;
+			 void* const memptr = NULL, size_t const memsize = 0) = 0;
   virtual void free () = 0;
   virtual size_t allocsize (const ibbox& extent, const int proc) const = 0;
   
diff --git a/Carpet/CarpetLib/src/mem.cc b/Carpet/CarpetLib/src/mem.cc
index d8603591b..081c9d8bc 100644
--- a/Carpet/CarpetLib/src/mem.cc
+++ b/Carpet/CarpetLib/src/mem.cc
@@ -58,7 +58,8 @@ static double max_allocated_objects = 0;
 
 template<typename T>
 mem<T>::
-mem (size_t const vectorlength, size_t const nelems, T * const memptr)
+mem (size_t const vectorlength, size_t const nelems,
+     T * const memptr, size_t const memsize)
   : storage_ (memptr),
     nelems_ (nelems),
     vectorlength_ (vectorlength),
@@ -85,9 +86,6 @@ mem (size_t const vectorlength, size_t const nelems, T * const memptr)
     try {
       storage_ = new T [vectorlength * nelems];
       owns_storage_ = true;
-      if (poison_new_memory) {
-        memset (storage_, poison_value, vectorlength * nelems * sizeof (T));
-      }
     } catch (...) {
       T Tdummy;
       CCTK_VWarn (0, __LINE__, __FILE__, CCTK_THORNSTRING,
@@ -100,6 +98,15 @@ mem (size_t const vectorlength, size_t const nelems, T * const memptr)
     }
     total_allocated_bytes += nbytes;
     max_allocated_bytes = max (max_allocated_bytes, total_allocated_bytes);
+    if (poison_new_memory) {
+      memset (storage_, poison_value, vectorlength * nelems * sizeof (T));
+    }
+  } else {
+    assert (memsize >= vectorlength * nelems * sizeof (T));
+    // Don't poison the memory.  Passing in a pointer allows the
+    // pointer to be re-interpreted as a mem object, keeping the
+    // previous content.  This is e.g. used to turn communication
+    // buffers into mem objects.
   }
   ++ total_allocated_objects;
   max_allocated_objects = max (max_allocated_objects, total_allocated_objects);
@@ -109,7 +116,7 @@ template<typename T>
 mem<T>::
 ~mem ()
 {
-  assert (! has_clients());
+  assert (not has_clients());
   if (owns_storage_) {
     delete [] storage_;
     total_allocated_bytes -= vectorlength_ * nelems_ * sizeof (T);
@@ -124,7 +131,7 @@ void mem<T>::
 register_client (size_t const vectorindex)
 {
   assert (vectorindex < vectorlength_);
-  assert (! clients_.AT(vectorindex));
+  assert (not clients_.AT(vectorindex));
   clients_.AT(vectorindex) = true;
   ++ num_clients_;
 }
@@ -174,7 +181,7 @@ size_t const mempool::align;
 
 mempool::
 mempool ()
-  : freeptr (0), freesize (0)
+  : allocated (0), freeptr (0), freesize (0)
 {
 }
 
@@ -203,6 +210,7 @@ alloc (size_t nbytes)
     // Allocate the usual chunk size, or more if more is requested
     freesize = max (chunksize, nbytes);
     freeptr = malloc (freesize);
+    allocated += freesize;
     if (not freeptr) {
       CCTK_VWarn (CCTK_WARN_ABORT, __LINE__, __FILE__, CCTK_THORNSTRING,
                   "Failed to allocate %.3f MB of memory",
@@ -234,7 +242,7 @@ memory ()
     memoryof (chunks) +
     memoryof (freeptr) +
     memoryof (freesize) +
-    (chunksize * chunks.size());
+    memoryof (allocated);
 }
 
 
@@ -298,7 +306,9 @@ void CarpetLib_printmemstats (CCTK_ARGUMENTS)
     mybuf.max_bytes     = max_allocated_bytes;
     mybuf.max_objects   = max_allocated_objects;
 #ifdef HAVE_MALLINFO
-    struct mallinfo minfo = mallinfo ();
+    // NOTE: struct mallinfo returns byte-counts as int, which can
+    // overflow.  In this case, the information is incorrect.
+    struct mallinfo const minfo = mallinfo ();
     mybuf.malloc_used_bytes = minfo.uordblks;
     mybuf.malloc_free_bytes = minfo.fordblks;
 #else
diff --git a/Carpet/CarpetLib/src/mem.hh b/Carpet/CarpetLib/src/mem.hh
index 084383104..dbd976c21 100644
--- a/Carpet/CarpetLib/src/mem.hh
+++ b/Carpet/CarpetLib/src/mem.hh
@@ -20,7 +20,8 @@ class mem
   size_t num_clients_;
   
 public:
-  mem (size_t vectorlength, size_t nelems, T * memptr = NULL);
+  mem (size_t vectorlength, size_t nelems,
+       T * memptr = NULL, size_t memsize = 0);
   ~mem ();
   
   T * storage (size_t vectorindex) const
@@ -61,6 +62,8 @@ class mempool
   // List of all allocated chunks.  When the mempool is destroyed,
   // these pointers need to be freed.
   stack <void *> chunks;
+  // Total size of all allocated chunks
+  size_t allocated;
   
   // Pointer to the beginning of some unused memory
   void * freeptr;