From cd27b318e1f55ae7fc2db3ec8b9554f803879d4e Mon Sep 17 00:00:00 2001 From: Erik Schnetter Date: Tue, 19 Feb 2008 04:45:00 +0000 Subject: CarpetLib: Make mem safer When passing in a pointer to mem, also pass the size of the pointed-to memory region, so that mem can check that there is enough space. Keep track of the number of allocated bytes in mempools. darcs-hash:20080219044528-dae7b-107edc6f696a35aad32ef6e58129b3281d00eb56.gz --- Carpet/CarpetLib/src/data.cc | 5 +++-- Carpet/CarpetLib/src/data.hh | 2 +- Carpet/CarpetLib/src/gdata.cc | 12 ++++++++---- Carpet/CarpetLib/src/gdata.hh | 2 +- Carpet/CarpetLib/src/mem.cc | 28 +++++++++++++++++++--------- Carpet/CarpetLib/src/mem.hh | 5 ++++- 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* data::make_typed (const int varindex_, template void data::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::allocate (const ibbox& extent_, if (dist::rank() == _proc) { if (vectorindex == 0) { assert (not vectorleader); - _memory = new mem (vectorlength, _size, (T*)memptr); + _memory = new mem (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 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 mem:: -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 mem:: ~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:: 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 chunks; + // Total size of all allocated chunks + size_t allocated; // Pointer to the beginning of some unused memory void * freeptr; -- cgit v1.2.3