Logo AND Algorithmique Numérique Distribuée

Public GIT Repository
Remove the need of pthread_mutex in mmalloc, to allow its use with sthread
authorMartin Quinson <martin.quinson@ens-rennes.fr>
Thu, 28 Jul 2022 23:51:52 +0000 (01:51 +0200)
committerMartin Quinson <martin.quinson@ens-rennes.fr>
Thu, 28 Jul 2022 23:58:09 +0000 (01:58 +0200)
Historically, mmalloc was used in all case when MC was activated at
build time (even if MC was not involved at runtime), forcing this
protection against race conditions for e.g. parallel simulations.

But nowadays, mmalloc is only activated at runtime in the verified
application. In other runtime setups, mmalloc uses the libc's malloc
implem instead of itself (that's why we use dlsym in the
initialization of mmalloc, to find the replacement).

So, removing this code should have no impact whatsoever.
Only maybe speeding up the verification a tiny bit.

And now, sthread becomes usable in conjunction with Mc Simgrid!!!

Given the current poor coverage of the pthread API, we can't verify
any useful program, but at least we have a valid (and rather robust)
prototype of such a verification tool.

src/include/xbt/mmalloc.h
src/simgrid/sg_config.cpp
src/xbt/mmalloc/mfree.c
src/xbt/mmalloc/mm_legacy.c
src/xbt/mmalloc/mm_module.c
src/xbt/mmalloc/mmprivate.h

index 6a872fe..bdff2c1 100644 (file)
@@ -50,13 +50,14 @@ XBT_PUBLIC void mfree(xbt_mheap_t md, void* ptr);
 
 XBT_PUBLIC xbt_mheap_t xbt_mheap_new(void* baseaddr, int options);
 
-XBT_PUBLIC void xbt_mheap_destroy_no_free(xbt_mheap_t md);
-
 XBT_PUBLIC void* xbt_mheap_destroy(xbt_mheap_t md);
 
 /* To get the heap used when using the legacy version malloc/free/realloc and such */
 xbt_mheap_t mmalloc_get_current_heap(void);
 
+/* Returns true if we are using the internal mmalloc, and false if we are using the libc's malloc */
+XBT_PUBLIC int malloc_use_mmalloc(void);
+
 #endif
 SG_END_DECL
 
index e60efa3..10d160a 100644 (file)
@@ -10,6 +10,7 @@
 #include <xbt/config.hpp>
 
 #include "simgrid/sg_config.hpp"
+#include "src/include/xbt/mmalloc.h"
 #include "src/instr/instr_private.hpp"
 #include "src/internal_config.h"
 #include "src/kernel/context/Context.hpp"
@@ -327,9 +328,17 @@ void sg_config_init(int *argc, char **argv)
   static simgrid::config::Flag<int> cfg_context_guard_size{
       "contexts/guard-size", "Guard size for contexts stacks in memory pages", default_guard_size,
       [](int value) { simgrid::kernel::context::guard_size = value * xbt_pagesize; }};
-  static simgrid::config::Flag<int> cfg_context_nthreads{"contexts/nthreads",
-                                                         "Number of parallel threads used to execute user contexts", 1,
-                                                         &simgrid::kernel::context::set_nthreads};
+
+  static simgrid::config::Flag<int> cfg_context_nthreads{
+      "contexts/nthreads", "Number of parallel threads used to execute user contexts", 1, [](int nthreads) {
+#if HAVE_MMALLOC
+        xbt_assert(
+            nthreads == 1 || !malloc_use_mmalloc(),
+            "Parallel simulation is forbidden in the verified program, as there is no protection against race "
+            "conditions in mmalloc itself. Please don't be so greedy and show some mercy for our implementation.");
+#endif
+        simgrid::kernel::context::set_nthreads(nthreads);
+      }};
 
   /* synchronization mode for parallel user contexts */
 #if HAVE_FUTEX_H
index 652b381..c6c5a84 100644 (file)
@@ -42,13 +42,11 @@ void mfree(struct mdesc *mdp, void *ptr)
 
   switch (type) {
   case MMALLOC_TYPE_HEAPINFO:
-    UNLOCK(mdp);
     fprintf(stderr, "Asked to free a fragment in a heapinfo block. I'm confused.\n");
     abort();
     break;
 
   case MMALLOC_TYPE_FREE: /* Already free */
-    UNLOCK(mdp);
     fprintf(stderr, "Asked to free a fragment in a block that is already free. I'm puzzled.\n");
     abort();
     break;
@@ -172,7 +170,6 @@ void mfree(struct mdesc *mdp, void *ptr)
     frag_nb = RESIDUAL(ptr, BLOCKSIZE) >> type;
 
     if( mdp->heapinfo[block].busy_frag.frag_size[frag_nb] == -1){
-      UNLOCK(mdp);
       fprintf(stderr, "Asked to free a fragment that is already free. I'm puzzled\n");
       abort();
     }
index 7b946b5..ee822fd 100644 (file)
@@ -160,10 +160,7 @@ void *malloc(size_t n)
   if (!mdp)
     return NULL;
 
-  LOCK(mdp);
-  void *ret = mmalloc(mdp, n);
-  UNLOCK(mdp);
-  return ret;
+  return mmalloc(mdp, n);
 }
 
 void *calloc(size_t nmemb, size_t size)
@@ -182,9 +179,7 @@ void *calloc(size_t nmemb, size_t size)
   if (!mdp)
     return NULL;
 
-  LOCK(mdp);
   void *ret = mmalloc(mdp, nmemb*size);
-  UNLOCK(mdp);
   // This was already done in the callee:
   if(!(mdp->options & XBT_MHEAP_OPTION_MEMSET)) {
     memset(ret, 0, nmemb * size);
@@ -208,10 +203,7 @@ void *realloc(void *p, size_t s)
   if (!mdp)
     return NULL;
 
-  LOCK(mdp);
-  void* ret = mrealloc(mdp, p, s);
-  UNLOCK(mdp);
-  return ret;
+  return mrealloc(mdp, p, s);
 }
 
 void free(void *p)
@@ -231,7 +223,5 @@ void free(void *p)
     return;
 
   xbt_mheap_t mdp = GET_HEAP();
-  LOCK(mdp);
   mfree(mdp, p);
-  UNLOCK(mdp);
 }
index 56091d2..05d59b1 100644 (file)
@@ -68,7 +68,6 @@ xbt_mheap_t xbt_mheap_new(void* baseaddr, int options)
   mdp->next_mdesc = NULL;
   mdp->options = options;
 
-  pthread_mutex_init(&mdp->mutex, NULL);
   /* If we have not been passed a valid open file descriptor for the file
      to map to, then open /dev/zero and use that to map to. */
 
@@ -89,28 +88,12 @@ xbt_mheap_t xbt_mheap_new(void* baseaddr, int options)
     while(mdp->next_mdesc)
       mdp = mdp->next_mdesc;
 
-    LOCK(mdp);
     mdp->next_mdesc = (struct mdesc *)mbase;
-    UNLOCK(mdp);
   }
 
   return mbase;
 }
 
-
-
-/** Terminate access to a mmalloc managed region, but do not free its content.
- *
- * This is for example useful for the base region where ldl stores its data
- *   because it leaves the place after us.
- */
-void xbt_mheap_destroy_no_free(xbt_mheap_t md)
-{
-  struct mdesc *mdp = md;
-
-  pthread_mutex_destroy(&mdp->mutex);
-}
-
 /** Terminate access to a mmalloc managed region by unmapping all memory pages associated with the region, and closing
  *  the file descriptor if it is one that we opened.
 
@@ -132,7 +115,6 @@ void *xbt_mheap_destroy(xbt_mheap_t mdp)
 
     mdptemp->next_mdesc = mdp->next_mdesc;
 
-    xbt_mheap_destroy_no_free(mdp);
     struct mdesc mtemp = *mdp;
 
     /* Now unmap all the pages associated with this region by asking for a
@@ -158,7 +140,6 @@ static void mmalloc_fork_prepare(void)
   xbt_mheap_t mdp = NULL;
   if ((mdp =__mmalloc_default_mdp)){
     while(mdp){
-      LOCK(mdp);
       mdp = mdp->next_mdesc;
     }
   }
@@ -169,7 +150,6 @@ static void mmalloc_fork_parent(void)
   xbt_mheap_t mdp = NULL;
   if ((mdp =__mmalloc_default_mdp)){
     while(mdp){
-      UNLOCK(mdp);
       mdp = mdp->next_mdesc;
     }
   }
@@ -180,7 +160,6 @@ static void mmalloc_fork_child(void)
   struct mdesc* mdp = NULL;
   if ((mdp =__mmalloc_default_mdp)){
     while(mdp){
-      UNLOCK(mdp);
       mdp = mdp->next_mdesc;
     }
   }
index cddfd15..451c1e6 100644 (file)
@@ -174,9 +174,6 @@ typedef struct {
  * if such a file exists.
  * */
 struct mdesc {
-  /** @brief Mutex locking the access to the heap */
-  pthread_mutex_t mutex;
-
   /** @brief Chained lists of mdescs */
   struct mdesc *next_mdesc;
 
@@ -261,17 +258,6 @@ XBT_PUBLIC_DATA struct mdesc* __mmalloc_default_mdp;
 
 XBT_PUBLIC void* mmorecore(struct mdesc* mdp, ssize_t size);
 
-/** Thread-safety (if the mutex is already created)
- *
- * This is mandatory in the case where the user runs a parallel simulation
- * in a model-checking enabled tree. Without this protection, our malloc
- * implementation will not like multi-threading AT ALL.
- */
-#define LOCK(mdp) pthread_mutex_lock(&(mdp)->mutex)
-#define UNLOCK(mdp) pthread_mutex_unlock(&(mdp)->mutex)
-
-XBT_PRIVATE int malloc_use_mmalloc(void);
-
 XBT_PRIVATE size_t mmalloc_get_bytes_used_remote(size_t heaplimit, const malloc_info* heapinfo);
 
 /* We call dlsym during mmalloc initialization, but dlsym uses malloc.