Logo AND Algorithmique Numérique Distribuée

Public GIT Repository
Separate mmalloc from xbt
authorMartin Quinson <martin.quinson@ens-rennes.fr>
Thu, 21 Jul 2022 15:39:17 +0000 (17:39 +0200)
committerMartin Quinson <martin.quinson@ens-rennes.fr>
Thu, 21 Jul 2022 16:13:49 +0000 (18:13 +0200)
This is too bad that we cannot THROW on double free anymore, but we'll
have to live with it if we want to move mmalloc to libsthread.so

16 files changed:
src/include/xbt/mmalloc.h
src/include/xbt/xbt_modinter.h
src/mc/remote/AppSide.cpp
src/mc/remote/mc_protocol.h
src/xbt/mmalloc/mfree.c
src/xbt/mmalloc/mm_legacy.c
src/xbt/mmalloc/mm_module.c
src/xbt/mmalloc/mmalloc.c
src/xbt/mmalloc/mmorecore.c
src/xbt/mmalloc/mmprivate.h
src/xbt/mmalloc/swag.c
src/xbt/mmalloc/swag.h
src/xbt/xbt_main.cpp
teshsuite/xbt/mmalloc/mmalloc_32.tesh
teshsuite/xbt/mmalloc/mmalloc_64.tesh
teshsuite/xbt/mmalloc/mmalloc_test.cpp

index f2b85ac..6a872fe 100644 (file)
 
 #include "src/internal_config.h"
 
+/** Environment variable name used to pass the communication socket.
+ *
+ * It is set by `simgrid-mc` to enable MC support in the children processes.
+ *
+ * It is placed in this file so that it's visible from mmalloc and MC without sharing anythin of xbt in mmalloc
+ */
+#define MC_ENV_SOCKET_FD "SIMGRID_MC_SOCKET_FD"
+
 #include <stdio.h>     /* for NULL */
 #include <sys/types.h> /* for size_t */
 
-#include "xbt/dict.h"
-#include "xbt/dynar.h"
-
 SG_BEGIN_DECL
 
 /* Datatype representing a separate heap. The whole point of the mmalloc module is to allow several such heaps in the
index 254f554..5913b17 100644 (file)
@@ -20,9 +20,6 @@ void xbt_log_postexit(void);
 void xbt_dict_preinit(void);
 void xbt_dict_postexit(void);
 
-xbt_mheap_t mmalloc_preinit(void);
-void mmalloc_postexit(void);
-
 extern int xbt_initialized;
 
 SG_END_DECL
index 2d4dfbe..929c97c 100644 (file)
@@ -73,7 +73,7 @@ AppSide* AppSide::initialize(xbt_dynar_t actors_addr)
   xbt_assert(errno == 0 && raise(SIGSTOP) == 0, "Could not wait for the model-checker (errno = %d: %s)", errno,
              strerror(errno));
 
-  s_mc_message_initial_addresses_t message{MessageType::INITIAL_ADDRESSES, mmalloc_preinit(),
+  s_mc_message_initial_addresses_t message{MessageType::INITIAL_ADDRESSES, mmalloc_get_current_heap(),
                                            kernel::actor::ActorImpl::get_maxpid_addr(), actors_addr};
   xbt_assert(instance_->channel_.send(message) == 0, "Could not send the initial message with addresses.");
 
index e83a281..c04acc1 100644 (file)
@@ -8,12 +8,6 @@
 
 // ***** Environment variables for passing context to the model-checked process
 
-/** Environment variable name used to pass the communication socket.
- *
- * It is set by `simgrid-mc` to enable MC support in the children processes
- */
-#define MC_ENV_SOCKET_FD "SIMGRID_MC_SOCKET_FD"
-
 #ifdef __cplusplus
 
 #include "src/kernel/actor/SimcallObserver.hpp"
index 10c47ff..4863878 100644 (file)
@@ -11,7 +11,6 @@
    Heavily modified Mar 1992 by Fred Fish.  (fnf@cygnus.com) */
 
 #include "mmprivate.h"
-#include "xbt/ex.h"
 #include "mc/mc.h"
 
 /* Return memory to the heap.
@@ -39,12 +38,14 @@ void mfree(struct mdesc *mdp, void *ptr)
   switch (type) {
   case MMALLOC_TYPE_HEAPINFO:
     UNLOCK(mdp);
-    THROW("Asked to free a fragment in a heapinfo block. I'm confused.\n");
+    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);
-    THROW("Asked to free a fragment in a block that is already free. I'm puzzled.\n");
+    fprintf(stderr, "Asked to free a fragment in a block that is already free. I'm puzzled.\n");
+    abort();
     break;
 
   case MMALLOC_TYPE_UNFRAGMENTED:
@@ -167,7 +168,8 @@ void mfree(struct mdesc *mdp, void *ptr)
 
     if( mdp->heapinfo[block].busy_frag.frag_size[frag_nb] == -1){
       UNLOCK(mdp);
-      THROW("Asked to free a fragment that is already free. I'm puzzled\n");
+      fprintf(stderr, "Asked to free a fragment that is already free. I'm puzzled\n");
+      abort();
     }
 
     if (MC_is_active() && mdp->heapinfo[block].busy_frag.ignore[frag_nb] > 0)
index 3228074..84a7eb4 100644 (file)
@@ -1,5 +1,4 @@
-/* Copyright (c) 2010-2022. The SimGrid Team.
- * All rights reserved.                                                     */
+/* Copyright (c) 2010-2022. The SimGrid Team. All rights reserved.          */
 
 /* This program is free software; you can redistribute it and/or modify it
  * under the terms of the license (GNU LGPL) which comes with this package. */
@@ -7,19 +6,16 @@
 /* Redefine the classical malloc/free/realloc functions so that they fit well in the mmalloc framework */
 #define _GNU_SOURCE
 
-#include <stdlib.h>
+#include "mmprivate.h"
 
 #include <dlfcn.h>
-
-#include "mmprivate.h"
-#include "src/internal_config.h"
-#include "src/mc/remote/mc_protocol.h"
-#include "xbt/xbt_modinter.h"
 #include <math.h>
+#include <stdlib.h>
 
 /* ***** Whether to use `mmalloc` of the underlying malloc ***** */
 
 static int __malloc_use_mmalloc;
+int mmalloc_pagesize = 0;
 
 int malloc_use_mmalloc(void)
 {
@@ -43,8 +39,6 @@ xbt_mheap_t mmalloc_get_current_heap(void)
 }
 
 /* Override the malloc-like functions if MC is activated at compile time */
-#if SIMGRID_HAVE_MC
-
 /* ***** Temporary allocator
  *
  * This is used before we have found the real malloc implementation with dlsym.
@@ -132,6 +126,8 @@ XBT_ATTRIB_CONSTRUCTOR(101) static void mm_legacy_constructor()
     mm_real_calloc   = dlsym(RTLD_NEXT, "calloc");
 #endif
   }
+  mmalloc_pagesize = getpagesize();
+
   mm_initializing = 0;
   mm_initialized = 1;
 }
@@ -238,4 +234,3 @@ void free(void *p)
   mfree(mdp, p);
   UNLOCK(mdp);
 }
-#endif /* SIMGRID_HAVE_MC */
index 8358d43..fc721ea 100644 (file)
@@ -33,8 +33,6 @@
 #include <sys/stat.h>
 #include <string.h>
 #include "mmprivate.h"
-#include "xbt/ex.h"
-#include "xbt/xbt_modinter.h" /* declarations of mmalloc_preinit and friends that live here */
 
 /* Initialize access to a mmalloc managed region.
 
@@ -49,7 +47,7 @@
    so that users of the package don't have to worry about the actual
    implementation details.
 
-   On failure returns NULL. */
+   On failure, returns NULL. */
 
 xbt_mheap_t xbt_mheap_new(void* baseaddr, int options)
 {
@@ -189,32 +187,28 @@ static void mmalloc_fork_child(void)
   }
 }
 
-/* Initialize the default malloc descriptor. */
+/* Initialize the default malloc descriptor.
+ *
+ * There is no malloc_postexit() destroying the default mdp, because it would break ldl trying to free its memory
+ */
 xbt_mheap_t mmalloc_preinit(void)
 {
   if (__mmalloc_default_mdp == NULL) {
-    if(!xbt_pagesize)
-      xbt_pagesize = getpagesize();
-    unsigned long mask = ~((unsigned long)xbt_pagesize - 1);
+    if (!mmalloc_pagesize)
+      mmalloc_pagesize = getpagesize();
+    unsigned long mask    = ~((unsigned long)mmalloc_pagesize - 1);
     void *addr = (void*)(((unsigned long)sbrk(0) + HEAP_OFFSET) & mask);
     __mmalloc_default_mdp = xbt_mheap_new(addr, XBT_MHEAP_OPTION_MEMSET);
 
     // atfork mandated at least on FreeBSD, or simgrid-mc will fail to fork the verified app
     int res = pthread_atfork(mmalloc_fork_prepare, mmalloc_fork_parent, mmalloc_fork_child);
-    xbt_assert(res == 0, "pthread_atfork() failed: return value %d", res);
+    mmalloc_assert(res == 0, "pthread_atfork() failed: return value %d", res);
   }
-  xbt_assert(__mmalloc_default_mdp != NULL);
+  mmalloc_assert(__mmalloc_default_mdp != NULL, "__mmalloc_default_mdp cannot be NULL");
 
   return __mmalloc_default_mdp;
 }
 
-void mmalloc_postexit(void)
-{
-  /* Do not destroy the default mdp or ldl won't be able to free the memory it
-   * allocated since we're in memory */
-  // xbt_mheap_destroy_no_free(__mmalloc_default_mdp)
-}
-
 // This is the underlying implementation of mmalloc_get_bytes_used_remote.
 // Is it used directly in order to evaluate the bytes used from a different
 // process.
index e1db0d4..be0070f 100644 (file)
@@ -51,7 +51,7 @@ static void* mmalloc_aligned(struct mdesc* mdp, size_t size)
 static void initialize_heapinfo_heapinfo(const s_xbt_mheap_t* mdp)
 {
   // Update heapinfo about the heapinfo pages (!):
-  xbt_assert((uintptr_t) mdp->heapinfo % BLOCKSIZE == 0);
+  mmalloc_assert((uintptr_t)mdp->heapinfo % BLOCKSIZE == 0, "Failed assert in initialize_heapinfo_heapinfo()");
   size_t block   = BLOCK(mdp->heapinfo);
   size_t nblocks = mdp->heapsize * sizeof(malloc_info) / BLOCKSIZE;
   // Mark them as free:
@@ -214,8 +214,9 @@ void *mmalloc_no_memset(xbt_mheap_t mdp, size_t size)
       for (candidate_frag=0;candidate_frag<(size_t) (BLOCKSIZE >> log);candidate_frag++)
         if (candidate_info->busy_frag.frag_size[candidate_frag] == -1)
           break;
-      xbt_assert(candidate_frag < (size_t) (BLOCKSIZE >> log),
-          "Block %zu was registered as containing free fragments of type %zu, but I can't find any",candidate_block,log);
+      mmalloc_assert(candidate_frag < (size_t)(BLOCKSIZE >> log),
+                     "Block %zu was registered as containing free fragments of type %zu, but I can't find any",
+                     candidate_block, log);
 
       result = (void*) (((char*)ADDRESS(candidate_block)) + (candidate_frag << log));
 
index 828e2f5..0e7ce96 100644 (file)
@@ -23,8 +23,7 @@
 #define MAP_ANONYMOUS MAP_ANON
 #endif
 
-#define PAGE_ALIGN(addr) (void*) (((long)(addr) + xbt_pagesize - 1) &   \
-                                  ~((long)xbt_pagesize - 1))
+#define PAGE_ALIGN(addr) (void*)(((long)(addr) + mmalloc_pagesize - 1) & ~((long)mmalloc_pagesize - 1))
 
 /** @brief Add memory to this heap
  *
@@ -49,6 +48,10 @@ void *mmorecore(struct mdesc *mdp, ssize_t size)
     return mdp->breakval;
   }
 
+  if (mmalloc_pagesize == 0) { // Not initialized yet
+    mmalloc_pagesize = (int)sysconf(_SC_PAGESIZE);
+  }
+
   if (size < 0) {
     /* We are deallocating memory.  If the amount requested would cause us to try to deallocate back past the base of
      * the mmap'd region then die verbosely.  Otherwise, deallocate the memory and return the old break value. */
@@ -76,7 +79,8 @@ void *mmorecore(struct mdesc *mdp, ssize_t size)
 
     if (mapto == MAP_FAILED) {
       char buff[1024];
-      fprintf(stderr, "Internal error: mmap returned MAP_FAILED! error: %s\n", strerror(errno));
+      fprintf(stderr, "Internal error: mmap returned MAP_FAILED! pagesize:%d error: %s\n", mmalloc_pagesize,
+              strerror(errno));
       snprintf(buff, 1024, "cat /proc/%d/maps", getpid());
       int status = system(buff);
       if (status == -1 || !(WIFEXITED(status) && WEXITSTATUS(status) == 0))
index 648bd1f..33912ab 100644 (file)
 #ifndef XBT_MMPRIVATE_H
 #define XBT_MMPRIVATE_H 1
 
-#include <xbt/base.h>
-#include <xbt/misc.h>
-
 #include "swag.h"
 #include "src/internal_config.h"
 #include "xbt/mmalloc.h"
-#include "xbt/ex.h"
-#include "xbt/dynar.h"
 
+#include <limits.h>
 #include <pthread.h>
 #include <stdint.h>
-
-#include <limits.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+// This macro is veery similar to xbt_assert, but with no dependency on XBT
+#define mmalloc_assert(cond, ...)                                                                                      \
+  do {                                                                                                                 \
+    if (!(cond)) {                                                                                                     \
+      fprintf(stderr, __VA_ARGS__);                                                                                    \
+      abort();                                                                                                         \
+    }                                                                                                                  \
+  } while (0)
+
+XBT_PUBLIC_DATA int mmalloc_pagesize;
+XBT_PRIVATE xbt_mheap_t mmalloc_preinit(void);
 
 #define MMALLOC_MAGIC    "mmalloc"       /* Mapped file magic number */
 #define MMALLOC_MAGIC_SIZE  8       /* Size of magic number buf */
index a3cdcb9..550b89b 100644 (file)
@@ -1,5 +1,4 @@
-/* Copyright (c) 2004-2022. The SimGrid Team.
- * All rights reserved.                                                     */
+/* Copyright (c) 2004-2022. The SimGrid Team. All rights reserved.          */
 
 /* This program is free software; you can redistribute it and/or modify it
  * under the terms of the license (GNU LGPL) which comes with this package. */
@@ -11,7 +10,7 @@
 /* This type should be added to a type that is to be used in such a swag */
 
 #include "swag.h"
-#include "xbt/asserts.h"
+#include "mmprivate.h" // mmalloc_assert
 
 typedef s_xbt_swag_hookup_t *xbt_swag_hookup_t;
 typedef struct xbt_swag* xbt_swag_t;
@@ -63,11 +62,12 @@ static inline void xbt_swag_init(xbt_swag_t swag, size_t offset)
  */
 static inline void xbt_swag_insert(void *obj, xbt_swag_t swag)
 {
-  xbt_assert(!xbt_swag_belongs(obj, swag) || swag->tail,
-             "This object belongs to an empty swag! Did you correctly initialize the object's hookup?");
+
+  mmalloc_assert(!xbt_swag_belongs(obj, swag) || swag->tail,
+                 "This object belongs to an empty swag! Did you correctly initialize the object's hookup?");
 
   if (!swag->head) {
-    xbt_assert(!(swag->tail), "Inconsistent swag.");
+    mmalloc_assert(!(swag->tail), "Inconsistent swag.");
     swag->head = obj;
     swag->tail = obj;
     swag->count++;
index c88a996..032c741 100644 (file)
@@ -9,8 +9,6 @@
 #ifndef XBT_SWAG_H
 #define XBT_SWAG_H
 
-#include "xbt/sysdep.h" /* size_t */
-
 /*
  * XBT_swag: a O(1) set based on linked lists
  *
index 8ee72a4..15a5959 100644 (file)
@@ -115,9 +115,6 @@ static void xbt_postexit()
   xbt_initialized--;
   xbt_dict_postexit();
   xbt_log_postexit();
-#if SIMGRID_HAVE_MC
-  mmalloc_postexit();
-#endif
 }
 
 /** @brief Initialize the xbt mechanisms. */
index 06bc543..8bb9bac 100644 (file)
@@ -104,7 +104,6 @@ $ ${bindir:=.}/mmalloc_test --log=root.fmt:%m%n
 > All blocks were correctly allocated. Free every second block
 > Memset every second block to zero (yeah, they are not currently allocated :)
 > Re-allocate every second block
-> free all blocks (each one twice, to check that double free are correctly caught)
-> free again all blocks (to really check that double free are correctly caught)
+> free all blocks
 > Let's try different codepaths for mrealloc
 > Damnit, I cannot break mmalloc this time. That's SO disappointing.
index 17356ee..ec276c3 100644 (file)
@@ -104,7 +104,6 @@ $ ${bindir:=.}/mmalloc_test --log=root.fmt:%m%n
 > All blocks were correctly allocated. Free every second block
 > Memset every second block to zero (yeah, they are not currently allocated :)
 > Re-allocate every second block
-> free all blocks (each one twice, to check that double free are correctly caught)
-> free again all blocks (to really check that double free are correctly caught)
+> free all blocks
 > Let's try different codepaths for mrealloc
 > Damnit, I cannot break mmalloc this time. That's SO disappointing.
index 22ec933..6f798f0 100644 (file)
@@ -70,28 +70,9 @@ int main(int argc, char**argv)
     pointers[i] = mmalloc(heapA, size);
   }
 
-  XBT_INFO("free all blocks (each one twice, to check that double free are correctly caught)");
-  for (i = 0; i < TESTSIZE; i++) {
-    bool gotit = false;
+  XBT_INFO("free all blocks");
+  for (i = 0; i < TESTSIZE; i++)
     mfree(heapA, pointers[i]);
-    try {
-      mfree(heapA, pointers[i]);
-    } catch (const simgrid::Exception&) {
-      gotit = true;
-    }
-    xbt_assert(gotit, "FAIL: A double-free went undetected (for size:%d)", size_of_block(i));
-  }
-
-  XBT_INFO("free again all blocks (to really check that double free are correctly caught)");
-  for (i = 0; i < TESTSIZE; i++) {
-    bool gotit = false;
-    try {
-      mfree(heapA, pointers[i]);
-    } catch (const simgrid::Exception&) {
-      gotit = true;
-    }
-    xbt_assert(gotit, "FAIL: A double-free went undetected (for size:%d)", size_of_block(i));
-  }
 
   XBT_INFO("Let's try different codepaths for mrealloc");
   for (i = 0; i < TESTSIZE; i++) {