From b66dec8383676c57d9dd6b541ada6d3f4fa624b7 Mon Sep 17 00:00:00 2001
From: Marion Guthmuller <marion.guthmuller@loria.fr>
Date: Mon, 30 Jul 2012 16:11:59 +0200
Subject: [PATCH] model-checker : cleanup on mc_checkpoint

- XBT_DEBUG instead of fprintf on stderr or XBT_INFO
- remove useless debug information
- pointer detection for comparison of data region of libsimgrid
- free the memory map
---
 src/mc/mc_checkpoint.c | 121 +++++++++++++++++++----------------------
 src/mc/mc_private.h    |   1 +
 src/mc/memory_map.c    |   9 +++
 3 files changed, 67 insertions(+), 64 deletions(-)

diff --git a/src/mc/mc_checkpoint.c b/src/mc/mc_checkpoint.c
index 60f74e5805..b2a5d0ddba 100644
--- a/src/mc/mc_checkpoint.c
+++ b/src/mc/mc_checkpoint.c
@@ -7,7 +7,6 @@
 #include "mc_private.h"
 #include "xbt/module.h"
 
-
 XBT_LOG_NEW_DEFAULT_SUBCATEGORY(mc_checkpoint, mc,
                                 "Logging specific to mc_checkpoint");
 
@@ -17,6 +16,9 @@ static void MC_region_destroy(mc_mem_region_t reg);
 
 static void MC_snapshot_add_region(mc_snapshot_t snapshot, int type, void *start_addr, size_t size);
 
+static int data_program_region_compare(void *d1, void *d2, size_t size);
+static int data_libsimgrid_region_compare(void *d1, void *d2, size_t size);
+
 static mc_mem_region_t MC_region_new(int type, void *start_addr, size_t size)
 {
   mc_mem_region_t new_reg = xbt_new0(s_mc_mem_region_t, 1);
@@ -24,8 +26,9 @@ static mc_mem_region_t MC_region_new(int type, void *start_addr, size_t size)
   new_reg->start_addr = start_addr;
   new_reg->size = size;
   new_reg->data = xbt_malloc0(size);
-  XBT_DEBUG("New reg data %p, start_addr %p, size %zu", new_reg->data, start_addr, size);
   memcpy(new_reg->data, start_addr, size);
+
+  XBT_DEBUG("New region : type : %d, data : %p, size : %zu", type, new_reg->data, size);
   
   return new_reg;
 }
@@ -35,7 +38,6 @@ static void MC_region_restore(mc_mem_region_t reg)
   /*FIXME: check if start_addr is still mapped, if it is not, then map it
     before copying the data */
  
-  XBT_DEBUG("Memcpy : dest %p, src %p, size %zu", reg->start_addr, reg->data, reg->size);
   memcpy(reg->start_addr, reg->data, reg->size);
  
   return;
@@ -49,17 +51,6 @@ static void MC_region_destroy(mc_mem_region_t reg)
 
 static void MC_snapshot_add_region(mc_snapshot_t snapshot, int type, void *start_addr, size_t size)
 {
-  switch(type){
-  case 0 : 
-    XBT_DEBUG("New region heap (%zu)", size);
-    break;
-  case 1 : 
-    XBT_DEBUG("New region libsimgrid (%zu)", size);
-    break;
-  case 2 : 
-    XBT_DEBUG("New region program data (%zu)", size);
-    break;
-  }
   mc_mem_region_t new_reg = MC_region_new(type, start_addr, size);
   snapshot->regions = xbt_realloc(snapshot->regions, (snapshot->num_reg + 1) * sizeof(mc_mem_region_t));
   snapshot->regions[snapshot->num_reg] = new_reg;
@@ -90,7 +81,7 @@ void MC_take_snapshot(mc_snapshot_t snapshot)
     i++;
   }
 
-  /* FIXME: free the memory map */
+  free_memory_map(maps);
 }
 
 void MC_take_snapshot_liveness(mc_snapshot_t snapshot)
@@ -98,29 +89,34 @@ void MC_take_snapshot_liveness(mc_snapshot_t snapshot)
   unsigned int i = 0;
   s_map_region_t reg;
   memory_map_t maps = get_memory_map();
+  int nb_reg = 0;
 
   /* Save the std heap and the writable mapped pages of libsimgrid */
-  while (i < maps->mapsize) {
+  while (i < maps->mapsize && nb_reg < 3) {
     reg = maps->regions[i];
     if ((reg.prot & PROT_WRITE)){
       if (maps->regions[i].pathname == NULL){
         if (reg.start_addr == std_heap){ // only save the std heap (and not the raw one)
           MC_snapshot_add_region(snapshot, 0, reg.start_addr, (char*)reg.end_addr - (char*)reg.start_addr);
+          nb_reg++;
         }
       } else {
         if (!memcmp(basename(maps->regions[i].pathname), "libsimgrid", 10)){
           MC_snapshot_add_region(snapshot, 1, reg.start_addr, (char*)reg.end_addr - (char*)reg.start_addr);
+          nb_reg++;
         } else {
           if (!memcmp(basename(maps->regions[i].pathname), basename(xbt_binary_name), strlen(basename(xbt_binary_name)))){
             MC_snapshot_add_region(snapshot, 2, reg.start_addr, (char*)reg.end_addr - (char*)reg.start_addr);
-          } 
+            nb_reg++;
+          }
         }
       }
     }
     i++;
   }
+  
+  free_memory_map(maps);
 
-  /* FIXME: free the memory map */
 }
 
 void MC_restore_snapshot(mc_snapshot_t snapshot)
@@ -128,18 +124,6 @@ void MC_restore_snapshot(mc_snapshot_t snapshot)
   unsigned int i;
   for(i=0; i < snapshot->num_reg; i++){
     MC_region_restore(snapshot->regions[i]);
-    switch(snapshot->regions[i]->type){
-    case 0 : 
-      XBT_DEBUG("heap restored");
-      break;
-    case 1:
-      XBT_DEBUG("libsimgrid (data) restored");
-      break;
-    case 2:
-      XBT_DEBUG("data program restored");
-      break;
-    }
-
   }
 
 }
@@ -153,108 +137,117 @@ void MC_free_snapshot(mc_snapshot_t snapshot)
   xbt_free(snapshot);
 }
 
-static int data_program_region_compare(void *d1, void *d2, size_t size);
-static int data_libsimgrid_region_compare(void *d1, void *d2, size_t size);
-
 static int data_program_region_compare(void *d1, void *d2, size_t size){
   int distance = 0;
-  int i;
+  size_t i = 0;
   
   for(i=0; i<size; i++){
     if(memcmp(((char *)d1) + i, ((char *)d2) + i, 1) != 0){
-      fprintf(stderr,"Different byte (offset=%d) (%p - %p) in data program region\n", i, (char *)d1 + i, (char *)d2 + i);
+      XBT_DEBUG("Different byte (offset=%zu) (%p - %p) in data program region", i, (char *)d1 + i, (char *)d2 + i);
       distance++;
     }
   }
   
-  fprintf(stderr, "Hamming distance between data program regions : %d\n", distance);
+  XBT_DEBUG("Hamming distance between data program regions : %d", distance);
 
   return distance;
 }
 
-int data_libsimgrid_region_compare(void *d1, void *d2, size_t size){
+static int data_libsimgrid_region_compare(void *d1, void *d2, size_t size){
   int distance = 0;
-  int i;
-  
+  size_t i = 0;
+  int pointer_align;
+  void *addr_pointed1 = NULL, *addr_pointed2 = NULL;
+
   for(i=0; i<size; i++){
     if(memcmp(((char *)d1) + i, ((char *)d2) + i, 1) != 0){
-      fprintf(stderr, "Different byte (offset=%d) (%p - %p) in data libsimgrid region\n", i, (char *)d1 + i, (char *)d2 + i);
+      XBT_DEBUG("Different byte (offset=%zu) (%p - %p) in data libsimgrid region", i, (char *)d1 + i, (char *)d2 + i);
+      pointer_align = (i / sizeof(void*)) * sizeof(void*);
+      addr_pointed1 = *((void **)((char *)d1 + pointer_align));
+      addr_pointed2 = *((void **)((char *)d2 + pointer_align));
+      XBT_DEBUG("Addresses pointed : %p - %p", addr_pointed1, addr_pointed2);
       distance++;
     }
   }
   
-  fprintf(stderr, "Hamming distance between data libsimgrid regions : %d\n", distance);
+  XBT_DEBUG("Hamming distance between data libsimgrid regions : %d", distance); fflush(NULL);
   
   return distance;
 }
 
 int snapshot_compare(mc_snapshot_t s1, mc_snapshot_t s2){
 
+  int errors = 0, i;
   
   if(s1->num_reg != s2->num_reg){
-    XBT_INFO("Different num_reg (s1 = %u, s2 = %u)", s1->num_reg, s2->num_reg);
+    XBT_DEBUG("Different num_reg (s1 = %u, s2 = %u)", s1->num_reg, s2->num_reg);
     return 1;
   }
 
-  int i;
-  int errors = 0;
-
   for(i=0 ; i< s1->num_reg ; i++){
-
+    
     if(s1->regions[i]->type != s2->regions[i]->type){
       XBT_INFO("Different type of region");
       errors++;
     }
-
+    
     switch(s1->regions[i]->type){
-    case 0:
+    case 0 :
+      /* Compare heapregion */
       if(s1->regions[i]->size != s2->regions[i]->size){
-        XBT_INFO("Different size of heap (s1 = %zu, s2 = %zu)", s1->regions[i]->size, s2->regions[i]->size);
+        XBT_DEBUG("Different size of heap (s1 = %zu, s2 = %zu)", s1->regions[i]->size, s2->regions[i]->size);
         errors++;
       }
       if(s1->regions[i]->start_addr != s2->regions[i]->start_addr){
-        XBT_INFO("Different start addr of heap (s1 = %p, s2 = %p)", s1->regions[i]->start_addr, s2->regions[i]->start_addr);
+        XBT_DEBUG("Different start addr of heap (s1 = %p, s2 = %p)", s1->regions[i]->start_addr, s2->regions[i]->start_addr);
         errors++;
       }
-      if(mmalloc_compare_heap(s1->regions[i]->data, s2->regions[i]->data)){
-        XBT_INFO("Different heap (mmalloc_compare)");
+      if(mmalloc_compare_heap((xbt_mheap_t)s1->regions[i]->data, (xbt_mheap_t)s2->regions[i]->data)){
+        XBT_DEBUG("Different heap (mmalloc_compare)");
         errors++; 
       }
       break;
     case 1 :
+      /* Compare data libsimgrid region */
       if(s1->regions[i]->size != s2->regions[i]->size){
-        XBT_INFO("Different size of libsimgrid (s1 = %zu, s2 = %zu)", s1->regions[i]->size, s2->regions[i]->size);
+        XBT_DEBUG("Different size of libsimgrid (data) (s1 = %zu, s2 = %zu)", s1->regions[i]->size, s2->regions[i]->size);
         errors++;
       }
       if(s1->regions[i]->start_addr != s2->regions[i]->start_addr){
-        XBT_INFO("Different start addr of libsimgrid (s1 = %p, s2 = %p)", s1->regions[i]->start_addr, s2->regions[i]->start_addr);
+        XBT_DEBUG("Different start addr of libsimgrid (data) (s1 = %p, s2 = %p)", s1->regions[i]->start_addr, s2->regions[i]->start_addr);
         errors++;
       }
       if(data_libsimgrid_region_compare(s1->regions[i]->data, s2->regions[i]->data, s1->regions[i]->size) != 0){
-        XBT_INFO("Different memcmp for data in libsimgrid");
+        XBT_DEBUG("Different memcmp for data in libsimgrid");
         errors++;
       }
       break;
+
     case 2 :
+       /* Compare data program region */
       if(s1->regions[i]->size != s2->regions[i]->size){
-        XBT_INFO("Different size of data program (s1 = %zu, s2 = %zu)", s1->regions[i]->size, s2->regions[i]->size);
-        errors++;
+        XBT_DEBUG("Different size of data program (s1 = %zu, s2 = %zu)", s1->regions[i]->size, s2->regions[i]->size);
+        //errors++;
+        return 1;
       }
       if(s1->regions[i]->start_addr != s2->regions[i]->start_addr){
-        XBT_INFO("Different start addr of data program (s1 = %p, s2 = %p)", s1->regions[i]->start_addr, s2->regions[i]->start_addr);
-        errors++;
+        XBT_DEBUG("Different start addr of data program (s1 = %p, s2 = %p)", s1->regions[i]->start_addr, s2->regions[i]->start_addr);
+        //errors++;
+        return 1;
       }
       if(data_program_region_compare(s1->regions[i]->data, s2->regions[i]->data, s1->regions[i]->size) != 0){
-        XBT_INFO("Different memcmp for data in program");
-        errors++;
+        XBT_DEBUG("Different memcmp for data in program");
+        //errors++;
+        return 1;
       }
       break;
-    default:
-      break;
+ 
     }
+
   }
 
-  return (errors > 0);
+
+  return errors > 0;
   
 }
 
diff --git a/src/mc/mc_private.h b/src/mc/mc_private.h
index 30d01cb060..3f968a2e67 100644
--- a/src/mc/mc_private.h
+++ b/src/mc/mc_private.h
@@ -183,6 +183,7 @@ typedef struct s_memory_map {
 } s_memory_map_t, *memory_map_t;
 
 memory_map_t get_memory_map(void);
+void free_memory_map(memory_map_t map);
 
 
 /********************************** DPOR for safety  **************************************/
diff --git a/src/mc/memory_map.c b/src/mc/memory_map.c
index 97f8bb81e9..25a49ca511 100644
--- a/src/mc/memory_map.c
+++ b/src/mc/memory_map.c
@@ -152,3 +152,12 @@ memory_map_t get_memory_map(void)
 
   return ret;
 }
+
+void free_memory_map(memory_map_t map){
+
+  int i;
+  for(i=0; i< map->mapsize; i++){
+    xbt_free(map->regions[i].pathname);
+  }
+  xbt_free(map);
+}
-- 
2.20.1