mem leaks (#92)

* delete mem allocated for arguments
* delete temp arrays
* adding nullptr checks

Author: fschlimb

src/CollComm.cpp

@@ -5,6 +5,8 @@
 namespace SHARPY {
 
 void bufferize(NDArray::ptr_type a_ptr, void *outPtr) {
+  if (!outPtr)
+    return;
   dispatch(a_ptr->dtype(), a_ptr->data(), [&a_ptr, outPtr](auto *ptr) {
     auto buff = static_cast<decltype(ptr)>(outPtr);
     auto shp = a_ptr->local_shape();

src/NDArray.cpp

@@ -57,6 +57,8 @@ NDArray::NDArray(id_type guid_, DTypeId dtype_, const shape_type &shp,
   }
   _lData = DynMemRef(nds, allocated, allocated, 0, sizes, strides);
   assert(team() == 0 || transceiver() == getTransceiver());
+  delete[] sizes;
+  delete[] strides;
 }
 
 // incomplete, useful for computing meta information
@@ -232,37 +234,6 @@ int64_t NDArray::__int__() const {
   return res;
 }
 
-void NDArray::add_to_args(std::vector<void *> &args) const {
-  int ndims = this->ndims();
-  auto storeMR = [ndims](const DynMemRef &mr) -> intptr_t * {
-    intptr_t *buff = new intptr_t[memref_sz(ndims)];
-    buff[0] = reinterpret_cast<intptr_t>(mr._allocated);
-    buff[1] = reinterpret_cast<intptr_t>(mr._aligned);
-    buff[2] = static_cast<intptr_t>(mr._offset);
-    memcpy(buff + 3, mr._sizes, ndims * sizeof(intptr_t));
-    memcpy(buff + 3 + ndims, mr._strides, ndims * sizeof(intptr_t));
-    return buff;
-  }; // FIXME memory leak?
-
-  if (team() == 0 || ndims == 0) {
-    // no-dist-mode
-    args.push_back(storeMR(_lData));
-  } else {
-    args.push_back(storeMR(_lhsHalo));
-    args.push_back(storeMR(_lData));
-    args.push_back(storeMR(_rhsHalo));
-    // local offsets last
-    auto buff = new intptr_t[memref_sz(1)];
-    assert(5 == memref_sz(1));
-    buff[0] = reinterpret_cast<intptr_t>(_lOffsets.data());
-    buff[1] = buff[0];
-    buff[2] = 0;
-    buff[3] = ndims;
-    buff[4] = 1;
-    args.push_back(buff);
-  }
-}
-
 void NDArray::replicate() {
   if (is_replicated())
     return;

src/idtr.cpp

@@ -29,6 +29,9 @@ inline SHARPY::id_type get_guid() { return ++_nguid; }
 // Transceiver * theTransceiver = MPITransceiver();
 
 template <typename T> T *mr_to_ptr(void *ptr, intptr_t offset) {
+  if (!ptr) {
+    throw std::runtime_error("Fatal: cannot handle offset on nullptr");
+  }
   return reinterpret_cast<T *>(ptr) + offset;
 }
 
@@ -91,7 +94,7 @@ uint64_t idtr_nprocs(SHARPY::Transceiver *tc) {
   initMPIRuntime();
   tc = SHARPY::getTransceiver();
 #endif
-  return tc->nranks();
+  return tc ? tc->nranks() : 1;
 }
 #pragma weak _idtr_nprocs = idtr_nprocs
 #pragma weak _mlir_ciface__idtr_nprocs = idtr_nprocs
@@ -102,7 +105,7 @@ uint64_t idtr_prank(SHARPY::Transceiver *tc) {
   initMPIRuntime();
   tc = SHARPY::getTransceiver();
 #endif
-  return tc->rank();
+  return tc ? tc->rank() : 0;
 }
 #pragma weak _idtr_prank = idtr_prank
 #pragma weak _mlir_ciface__idtr_prank = idtr_prank
@@ -226,6 +229,9 @@ mlir2sharpy(const ::imex::ndarray::DType dt) {
 void bufferize(void *cptr, SHARPY::DTypeId dtype, const int64_t *sizes,
                const int64_t *strides, const int64_t *tStarts,
                const int64_t *tSizes, uint64_t nd, uint64_t N, void *out) {
+  if (!cptr || !sizes || !strides || !tStarts || !tSizes) {
+    return;
+  }
   dispatch(dtype, cptr,
            [sizes, strides, tStarts, tSizes, nd, N, out](auto *ptr) {
              auto buff = static_cast<decltype(ptr)>(out);
@@ -252,6 +258,9 @@ void bufferize(void *cptr, SHARPY::DTypeId dtype, const int64_t *sizes,
 void unpack(void *in, SHARPY::DTypeId dtype, const int64_t *sizes,
             const int64_t *strides, const int64_t *tStarts,
             const int64_t *tSizes, uint64_t nd, uint64_t N, void *out) {
+  if (!in || !sizes || !strides || !tStarts || !tSizes || !out) {
+    return;
+  }
   dispatch(dtype, out, [sizes, strides, tStarts, tSizes, nd, N, in](auto *ptr) {
     auto buff = static_cast<decltype(ptr)>(in);
 
@@ -276,6 +285,9 @@ template <typename T>
 void copy_(uint64_t d, uint64_t &pos, T *cptr, const int64_t *sizes,
            const int64_t *strides, const uint64_t *chunks, uint64_t nd,
            uint64_t start, uint64_t end, T *&out) {
+  if (!cptr || !sizes || !strides || !chunks || !out) {
+    return;
+  }
   auto stride = strides[d];
   uint64_t sz = sizes[d];
   uint64_t chunk = chunks[d];
@@ -311,6 +323,9 @@ void copy_(uint64_t d, uint64_t &pos, T *cptr, const int64_t *sizes,
 void bufferizeN(void *cptr, SHARPY::DTypeId dtype, const int64_t *sizes,
                 const int64_t *strides, const int64_t *tStarts,
                 const int64_t *tEnds, uint64_t nd, uint64_t N, void *out) {
+  if (!cptr || !sizes || !strides || !tStarts || !tEnds || !out) {
+    return;
+  }
   std::vector<uint64_t> chunks(nd);
   chunks[nd - 1] = 1;
   for (uint64_t i = 1; i < nd; ++i) {
@@ -377,6 +392,10 @@ void _idtr_reshape(SHARPY::DTypeId sharpytype, int64_t lRank,
   initMPIRuntime();
   tc = SHARPY::getTransceiver();
 #endif
+  if (!gShapePtr || !lDataPtr || !lShapePtr || !lStridesPtr || !lOffsPtr ||
+      !oGShapePtr || !oDataPtr || !oShapePtr || !oOffsPtr || !tc) {
+    throw std::runtime_error("Fatal: received nullptr in reshape");
+  }
 
   assert(std::accumulate(&gShapePtr[0], &gShapePtr[lRank], 1,
                          std::multiplies<int64_t>()) ==
@@ -730,6 +749,13 @@ void *_idtr_update_halo(SHARPY::DTypeId sharpytype, int64_t ndims,
   initMPIRuntime();
   tc = SHARPY::getTransceiver();
 #endif
+
+  if (!ownedOff || !ownedShape || !ownedStride || !bbOff || !bbShape ||
+      !ownedData || !leftHaloShape || !leftHaloStride || !leftHaloData ||
+      !rightHaloShape || !rightHaloStride || !rightHaloData || !tc) {
+    throw std::runtime_error("Fatal error: received nullptr in update_halo.");
+  }
+
   auto nworkers = tc->nranks();
   if (nworkers <= 1 || getenv("SHARPY_SKIP_COMM"))
     return nullptr;
@@ -824,6 +850,11 @@ void *_idtr_update_halo(SHARPY::Transceiver *tc, int64_t gShapeRank,
                         void *bbShapeDescr, int64_t lHaloRank, void *lHaloDescr,
                         int64_t rHaloRank, void *rHaloDescr, int64_t key) {
 
+  if (!gShapeDescr || !oOffDescr || !oDataDescr || !bbOffDescr ||
+      !bbShapeDescr || !lHaloDescr || !rHaloDescr) {
+    throw std::runtime_error("Fatal error: received nullptr in update_halo.");
+  }
+
   auto sharpytype = SHARPY::DTYPE<T>::value;
 
   // Construct unranked memrefs for metadata and data

src/include/sharpy/NDArray.hpp

@@ -187,10 +187,6 @@ class NDArray : public array_i, protected ArrayMeta {
   /// @return size of one element in number of bytes
   virtual int item_size() const override { return sizeof_dtype(_dtype); }
 
-  /// add array to list of args in the format expected by MLIR
-  /// assuming array has ndims dims.
-  virtual void add_to_args(std::vector<void *> &args) const override;
-
   /// @return locally owned data as DynMemref
   const DynMemRef &owned_data() const { return _lData; }
   /// @return left halo as DynMemref

src/include/sharpy/array_i.hpp

@@ -122,10 +122,6 @@ class array_i {
 
   // size of a single element (in bytes)
   virtual int item_size() const = 0;
-  // store array information in form of corresponding
-  // jit::JIT::DistMemRefDescriptor
-  // @return stored size in number of intptr_t
-  virtual void add_to_args(std::vector<void *> &args) const = 0;
   // mark as deallocated
   virtual void markDeallocated() = 0;
   virtual bool isAllocated() = 0;

src/jit/mlir.cpp

@@ -238,6 +238,10 @@ void DepManager::finalizeAndRun() {
   if (osz > 0 || !input.empty()) {
     // compile and run the module
     auto output = _jit.run(_module, _fname, input, osz);
+    // we assume we only store memrefdescriptors, e.g. arrays of inptr_t
+    for (auto p : input) {
+      delete[] reinterpret_cast<intptr_t *>(p);
+    }
     if (output.size() != osz)
       throw std::runtime_error("failed running jit");