Cleanups in class mc::Region

- former name was mc::RegionSnapshot
- please sonar: mark fields private, and kill copy constructor
- mc::Region cannot be of type Unknown anymore
- and other cleanups

diff --git a/src/mc/compare.cpp b/src/mc/compare.cpp
index d91e7fc..dda78ad 100644 (file)
--- a/src/mc/compare.cpp
+++ b/src/mc/compare.cpp
@@ -268,7 +268,7 @@ int StateComparator::initHeapInformation(xbt_mheap_t heap1, xbt_mheap_t heap2,
 }
 
 // TODO, have a robust way to find it in O(1)
-static inline RegionSnapshot* MC_get_heap_region(Snapshot* snapshot)
+static inline Region* MC_get_heap_region(Snapshot* snapshot)
 {
   for (auto const& region : snapshot->snapshot_regions_)
     if (region->region_type() == simgrid::mc::RegionType::Heap)
@@ -294,8 +294,8 @@ int mmalloc_compare_heap(
   malloc_info heapinfo_temp2;
   malloc_info heapinfo_temp2b;
 
-  simgrid::mc::RegionSnapshot* heap_region1 = MC_get_heap_region(snapshot1);
-  simgrid::mc::RegionSnapshot* heap_region2 = MC_get_heap_region(snapshot2);
+  simgrid::mc::Region* heap_region1 = MC_get_heap_region(snapshot1);
+  simgrid::mc::Region* heap_region2 = MC_get_heap_region(snapshot2);
 
   // This is the address of std_heap->heapinfo in the application process:
   void* heapinfo_address = &((xbt_mheap_t) process->heap_address)->heapinfo;
@@ -546,8 +546,8 @@ static int compare_heap_area_without_type(simgrid::mc::StateComparator& state, c
                                           int check_ignore)
 {
   simgrid::mc::RemoteClient* process = &mc_model_checker->process();
-  simgrid::mc::RegionSnapshot* heap_region1 = MC_get_heap_region(snapshot1);
-  simgrid::mc::RegionSnapshot* heap_region2 = MC_get_heap_region(snapshot2);
+  simgrid::mc::Region* heap_region1  = MC_get_heap_region(snapshot1);
+  simgrid::mc::Region* heap_region2  = MC_get_heap_region(snapshot2);
 
   for (int i = 0; i < size; ) {
 
@@ -649,8 +649,8 @@ static int compare_heap_area_with_type(simgrid::mc::StateComparator& state, cons
     const void* addr_pointed1;
     const void* addr_pointed2;
 
-    simgrid::mc::RegionSnapshot* heap_region1 = MC_get_heap_region(snapshot1);
-    simgrid::mc::RegionSnapshot* heap_region2 = MC_get_heap_region(snapshot2);
+    simgrid::mc::Region* heap_region1 = MC_get_heap_region(snapshot1);
+    simgrid::mc::Region* heap_region2 = MC_get_heap_region(snapshot2);
 
     switch (type->type) {
       case DW_TAG_unspecified_type:
@@ -935,8 +935,8 @@ static int compare_heap_area(simgrid::mc::StateComparator& state, const void* ar
 
   }
 
-  simgrid::mc::RegionSnapshot* heap_region1 = MC_get_heap_region(snapshot1);
-  simgrid::mc::RegionSnapshot* heap_region2 = MC_get_heap_region(snapshot2);
+  simgrid::mc::Region* heap_region1 = MC_get_heap_region(snapshot1);
+  simgrid::mc::Region* heap_region2 = MC_get_heap_region(snapshot2);
 
   const malloc_info* heapinfo1 = (const malloc_info*) MC_region_read(
     heap_region1, &heapinfo_temp1, &heapinfos1[block1], sizeof(malloc_info));
@@ -1148,9 +1148,9 @@ static int compare_heap_area(simgrid::mc::StateComparator& state, const void* ar
 /******************************************************************************/
 
 static int compare_areas_with_type(simgrid::mc::StateComparator& state, void* real_area1,
-                                   simgrid::mc::Snapshot* snapshot1, simgrid::mc::RegionSnapshot* region1,
-                                   void* real_area2, simgrid::mc::Snapshot* snapshot2,
-                                   simgrid::mc::RegionSnapshot* region2, simgrid::mc::Type* type, int pointer_level)
+                                   simgrid::mc::Snapshot* snapshot1, simgrid::mc::Region* region1, void* real_area2,
+                                   simgrid::mc::Snapshot* snapshot2, simgrid::mc::Region* region2,
+                                   simgrid::mc::Type* type, int pointer_level)
 {
   simgrid::mc::RemoteClient* process = &mc_model_checker->process();
 
@@ -1264,8 +1264,8 @@ static int compare_areas_with_type(simgrid::mc::StateComparator& state, void* re
         for (simgrid::mc::Member& member : type->members) {
           void* member1 = simgrid::dwarf::resolve_member(real_area1, type, &member, snapshot1);
           void* member2 = simgrid::dwarf::resolve_member(real_area2, type, &member, snapshot2);
-          simgrid::mc::RegionSnapshot* subregion1 = snapshot1->get_region(member1, region1); // region1 is hinted
-          simgrid::mc::RegionSnapshot* subregion2 = snapshot2->get_region(member2, region2); // region2 is hinted
+          simgrid::mc::Region* subregion1 = snapshot1->get_region(member1, region1); // region1 is hinted
+          simgrid::mc::Region* subregion2 = snapshot2->get_region(member2, region2); // region2 is hinted
           res = compare_areas_with_type(state, member1, snapshot1, subregion1, member2, snapshot2, subregion2,
                                         member.type, pointer_level);
           if (res == 1)
@@ -1284,8 +1284,8 @@ static int compare_areas_with_type(simgrid::mc::StateComparator& state, void* re
 }
 
 static int compare_global_variables(simgrid::mc::StateComparator& state, simgrid::mc::ObjectInformation* object_info,
-                                    simgrid::mc::RegionSnapshot* r1, simgrid::mc::RegionSnapshot* r2,
-                                    simgrid::mc::Snapshot* snapshot1, simgrid::mc::Snapshot* snapshot2)
+                                    simgrid::mc::Region* r1, simgrid::mc::Region* r2, simgrid::mc::Snapshot* snapshot1,
+                                    simgrid::mc::Snapshot* snapshot2)
 {
   xbt_assert(r1 && r2, "Missing region.");
 
@@ -1471,8 +1471,8 @@ int snapshot_compare(Snapshot* s1, Snapshot* s2)
   xbt_assert(regions_count == s2->snapshot_regions_.size());
 
   for (size_t k = 0; k != regions_count; ++k) {
-    RegionSnapshot* region1 = s1->snapshot_regions_[k].get();
-    RegionSnapshot* region2 = s2->snapshot_regions_[k].get();
+    Region* region1 = s1->snapshot_regions_[k].get();
+    Region* region2 = s2->snapshot_regions_[k].get();
 
     // Preconditions:
     if (region1->region_type() != RegionType::Data)

diff --git a/src/mc/sosp/Region.cpp b/src/mc/sosp/Region.cpp
index b9c4107..b7dbbca 100644 (file)
--- a/src/mc/sosp/Region.cpp
+++ b/src/mc/sosp/Region.cpp
@@ -21,21 +21,20 @@ XBT_LOG_NEW_DEFAULT_SUBCATEGORY(mc_RegionSnaphot, mc, "Logging specific to regio
 namespace simgrid {
 namespace mc {
 
-RegionSnapshot::RegionSnapshot(RegionType region_type, void* start_addr, size_t size)
+Region::Region(RegionType region_type, void* start_addr, size_t size)
     : region_type_(region_type), start_addr_(start_addr), size_(size)
 {
-  simgrid::mc::RemoteClient* process = &mc_model_checker->process();
-
   xbt_assert((((uintptr_t)start_addr) & (xbt_pagesize - 1)) == 0, "Start address not at the beginning of a page");
 
-  chunks_ = ChunkedData(mc_model_checker->page_store(), *process, RemotePtr<void>(start_addr), mmu::chunk_count(size));
+  chunks_ = ChunkedData(mc_model_checker->page_store(), mc_model_checker->process(), RemotePtr<void>(start_addr),
+                        mmu::chunk_count(size));
 }
 
 /** @brief Restore a region from a snapshot
  *
  *  @param region     Target region
  */
-void RegionSnapshot::restore()
+void Region::restore()
 {
   xbt_assert(((start().address()) & (xbt_pagesize - 1)) == 0, "Not at the beginning of a page");
   xbt_assert(simgrid::mc::mmu::chunk_count(size()) == get_chunks().page_count());

diff --git a/src/mc/sosp/Region.hpp b/src/mc/sosp/Region.hpp
index a151b6c..3079060 100644 (file)
--- a/src/mc/sosp/Region.hpp
+++ b/src/mc/sosp/Region.hpp
@@ -3,8 +3,8 @@
 /* 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. */
 
-#ifndef SIMGRID_MC_REGION_SNAPSHOT_HPP
-#define SIMGRID_MC_REGION_SNAPSHOT_HPP
+#ifndef SIMGRID_MC_SOSP_REGION_HPP
+#define SIMGRID_MC_SOSP_REGION_HPP
 
 #include "src/mc/remote/RemotePtr.hpp"
 #include "src/mc/sosp/ChunkedData.hpp"
@@ -15,17 +15,16 @@
 namespace simgrid {
 namespace mc {
 
-enum class RegionType { Unknown = 0, Heap = 1, Data = 2 };
+enum class RegionType { Heap = 1, Data = 2 };
 
 /** A copy/snapshot of a given memory region, where identical pages are stored only once */
-class RegionSnapshot {
+class Region {
 public:
-  static const RegionType UnknownRegion = RegionType::Unknown;
   static const RegionType HeapRegion    = RegionType::Heap;
   static const RegionType DataRegion    = RegionType::Data;
 
-protected:
-  RegionType region_type_                      = UnknownRegion;
+private:
+  RegionType region_type_;
   simgrid::mc::ObjectInformation* object_info_ = nullptr;
 
   /** @brief  Virtual address of the region in the simulated process */
@@ -37,41 +36,15 @@ protected:
   ChunkedData chunks_;
 
 public:
-  RegionSnapshot(RegionType type, void* start_addr, size_t size);
-  ~RegionSnapshot()                     = default;
-  RegionSnapshot(RegionSnapshot const&) = delete;
-  RegionSnapshot& operator=(RegionSnapshot const&) = delete;
-  RegionSnapshot(RegionSnapshot&& that)
-      : region_type_(that.region_type_)
-      , object_info_(that.object_info_)
-      , start_addr_(that.start_addr_)
-      , size_(that.size_)
-      , chunks_(std::move(that.chunks_))
-  {
-    that.clear();
-  }
-  RegionSnapshot& operator=(RegionSnapshot&& that)
-  {
-    region_type_ = that.region_type_;
-    object_info_ = that.object_info_;
-    start_addr_  = that.start_addr_;
-    size_        = that.size_;
-    chunks_      = std::move(that.chunks_);
-    that.clear();
-    return *this;
-  }
+  Region(RegionType type, void* start_addr, size_t size);
+  ~Region()             = default;
+  Region(Region const&) = delete;
+  Region& operator=(Region const&) = delete;
+  Region(Region&& that)            = delete;
+  Region& operator=(Region&& that) = delete;
 
   // Data
 
-  void clear()
-  {
-    region_type_ = UnknownRegion;
-    chunks_.clear();
-    object_info_ = nullptr;
-    start_addr_  = nullptr;
-    size_        = 0;
-  }
-
   ChunkedData const& get_chunks() const { return chunks_; }
 
   simgrid::mc::ObjectInformation* object_info() const { return object_info_; }

diff --git a/src/mc/sosp/Snapshot.cpp b/src/mc/sosp/Snapshot.cpp
index a3d6c79..6a2716e 100644 (file)
--- a/src/mc/sosp/Snapshot.cpp
+++ b/src/mc/sosp/Snapshot.cpp
@@ -20,7 +20,7 @@ XBT_LOG_NEW_DEFAULT_SUBCATEGORY(mc_snapshot, mc, "Taking and restoring snapshots
  *  @param size    Size of the data to read in bytes
  *  @return Pointer where the data is located (target buffer of original location)
  */
-const void* MC_region_read_fragmented(simgrid::mc::RegionSnapshot* region, void* target, const void* addr, size_t size)
+const void* MC_region_read_fragmented(simgrid::mc::Region* region, void* target, const void* addr, size_t size)
 {
   // Last byte of the memory area:
   void* end = (char*)addr + size - 1;
@@ -62,8 +62,8 @@ const void* MC_region_read_fragmented(simgrid::mc::RegionSnapshot* region, void*
  * @param region2 Region of the address in the second snapshot
  * @return same semantic as memcmp
  */
-int MC_snapshot_region_memcmp(const void* addr1, simgrid::mc::RegionSnapshot* region1, const void* addr2,
-                              simgrid::mc::RegionSnapshot* region2, size_t size)
+int MC_snapshot_region_memcmp(const void* addr1, simgrid::mc::Region* region1, const void* addr2,
+                              simgrid::mc::Region* region2, size_t size)
 {
   // Using alloca() for large allocations may trigger stack overflow:
   // use malloc if the buffer is too big.
@@ -304,14 +304,14 @@ void Snapshot::add_region(RegionType type, ObjectInformation* object_info, void*
   else if (type == simgrid::mc::RegionType::Heap)
     xbt_assert(not object_info, "Unexpected object info for heap region.");
 
-  simgrid::mc::RegionSnapshot* region = new RegionSnapshot(type, start_addr, size);
+  simgrid::mc::Region* region = new Region(type, start_addr, size);
   region->object_info(object_info);
-  snapshot_regions_.push_back(std::unique_ptr<simgrid::mc::RegionSnapshot>(std::move(region)));
+  snapshot_regions_.push_back(std::unique_ptr<simgrid::mc::Region>(std::move(region)));
 }
 
 const void* Snapshot::read_bytes(void* buffer, std::size_t size, RemotePtr<void> address, ReadOptions options) const
 {
-  RegionSnapshot* region = this->get_region((void*)address.address());
+  Region* region = this->get_region((void*)address.address());
   if (region) {
     const void* res = MC_region_read(region, buffer, (void*)address.address(), size);
     if (buffer == res || options & ReadOptions::lazy())
@@ -327,11 +327,11 @@ const void* Snapshot::read_bytes(void* buffer, std::size_t size, RemotePtr<void>
  *
  *  @param addr     Pointer
  * */
-RegionSnapshot* Snapshot::get_region(const void* addr) const
+Region* Snapshot::get_region(const void* addr) const
 {
   size_t n = snapshot_regions_.size();
   for (size_t i = 0; i != n; ++i) {
-    RegionSnapshot* region = snapshot_regions_[i].get();
+    Region* region = snapshot_regions_[i].get();
     if (not(region && region->contain(simgrid::mc::remote(addr))))
       continue;
 
@@ -342,7 +342,7 @@ RegionSnapshot* Snapshot::get_region(const void* addr) const
 }
 
 /** @brief Find the snapshoted region from a pointer, with a hinted_region */
-RegionSnapshot* Snapshot::get_region(const void* addr, RegionSnapshot* hinted_region) const
+Region* Snapshot::get_region(const void* addr, Region* hinted_region) const
 {
   if (hinted_region->contain(simgrid::mc::remote(addr)))
     return hinted_region;
@@ -355,7 +355,7 @@ void Snapshot::restore(RemoteClient* process)
   XBT_DEBUG("Restore snapshot %i", num_state_);
 
   // Restore regions
-  for (std::unique_ptr<simgrid::mc::RegionSnapshot> const& region : snapshot_regions_) {
+  for (std::unique_ptr<simgrid::mc::Region> const& region : snapshot_regions_) {
     if (region) // privatized variables are not snapshoted
       region.get()->restore();
   }

diff --git a/src/mc/sosp/Snapshot.hpp b/src/mc/sosp/Snapshot.hpp
index 8e24f6c..4bdb153 100644 (file)
--- a/src/mc/sosp/Snapshot.hpp
+++ b/src/mc/sosp/Snapshot.hpp
@@ -13,7 +13,7 @@
 
 // ***** Snapshot region
 
-static XBT_ALWAYS_INLINE void* mc_translate_address_region(uintptr_t addr, simgrid::mc::RegionSnapshot* region)
+static XBT_ALWAYS_INLINE void* mc_translate_address_region(uintptr_t addr, simgrid::mc::Region* region)
 {
   auto split                = simgrid::mc::mmu::split(addr - region->start().address());
   auto pageno               = split.first;
@@ -76,14 +76,14 @@ public:
   /* Regular use */
   const void* read_bytes(void* buffer, std::size_t size, RemotePtr<void> address,
                          ReadOptions options = ReadOptions::none()) const override;
-  RegionSnapshot* get_region(const void* addr) const;
-  RegionSnapshot* get_region(const void* addr, RegionSnapshot* hinted_region) const;
+  Region* get_region(const void* addr) const;
+  Region* get_region(const void* addr, Region* hinted_region) const;
   void restore(RemoteClient* process);
 
   // To be private
   int num_state_;
   std::size_t heap_bytes_used_;
-  std::vector<std::unique_ptr<RegionSnapshot>> snapshot_regions_;
+  std::vector<std::unique_ptr<Region>> snapshot_regions_;
   std::set<pid_t> enabled_processes_;
   std::vector<std::size_t> stack_sizes_;
   std::vector<s_mc_snapshot_stack_t> stacks_;
@@ -101,11 +101,10 @@ private:
 
 static const void* mc_snapshot_get_heap_end(simgrid::mc::Snapshot* snapshot);
 
-const void* MC_region_read_fragmented(simgrid::mc::RegionSnapshot* region, void* target, const void* addr,
-                                      std::size_t size);
+const void* MC_region_read_fragmented(simgrid::mc::Region* region, void* target, const void* addr, std::size_t size);
 
-int MC_snapshot_region_memcmp(const void* addr1, simgrid::mc::RegionSnapshot* region1, const void* addr2,
-                              simgrid::mc::RegionSnapshot* region2, std::size_t size);
+int MC_snapshot_region_memcmp(const void* addr1, simgrid::mc::Region* region1, const void* addr2,
+                              simgrid::mc::Region* region2, std::size_t size);
 
 static XBT_ALWAYS_INLINE const void* mc_snapshot_get_heap_end(simgrid::mc::Snapshot* snapshot)
 {
@@ -122,7 +121,7 @@ static XBT_ALWAYS_INLINE const void* mc_snapshot_get_heap_end(simgrid::mc::Snaps
  *  @param size    Size of the data to read in bytes
  *  @return Pointer where the data is located (target buffer of original location)
  */
-static XBT_ALWAYS_INLINE const void* MC_region_read(simgrid::mc::RegionSnapshot* region, void* target, const void* addr,
+static XBT_ALWAYS_INLINE const void* MC_region_read(simgrid::mc::Region* region, void* target, const void* addr,
                                                     std::size_t size)
 {
   xbt_assert(region);
@@ -139,7 +138,7 @@ static XBT_ALWAYS_INLINE const void* MC_region_read(simgrid::mc::RegionSnapshot*
   return MC_region_read_fragmented(region, target, addr, size);
 }
 
-static XBT_ALWAYS_INLINE void* MC_region_read_pointer(simgrid::mc::RegionSnapshot* region, const void* addr)
+static XBT_ALWAYS_INLINE void* MC_region_read_pointer(simgrid::mc::Region* region, const void* addr)
 {
   void* res;
   return *(void**)MC_region_read(region, &res, addr, sizeof(void*));

diff --git a/src/mc/sosp/Snapshot_test.cpp b/src/mc/sosp/Snapshot_test.cpp
index 8c55285..4b30fc9 100644 (file)
--- a/src/mc/sosp/Snapshot_test.cpp
+++ b/src/mc/sosp/Snapshot_test.cpp
@@ -12,7 +12,7 @@
 #include <sys/mman.h>
 
 /**************** Class BOOST_tests *************************/
-using simgrid::mc::RegionSnapshot;
+using simgrid::mc::Region;
 class snap_test_helper {
 public:
   static void init_memory(void* mem, size_t size);
@@ -21,8 +21,8 @@ public:
     size_t size;
     void* src;
     void* dstn;
-    RegionSnapshot* region0;
-    RegionSnapshot* region;
+    Region* region0;
+    Region* region;
   } prologue_return;
   static prologue_return prologue(int n); // common to the below 5 fxs
   static void read_whole_region();
@@ -73,13 +73,11 @@ snap_test_helper::prologue_return snap_test_helper::prologue(int n)
 
   // Init memory and take snapshots:
   init_memory(source, byte_size);
-  simgrid::mc::RegionSnapshot* region0 =
-      new simgrid::mc::RegionSnapshot(simgrid::mc::RegionType::Unknown, source, byte_size);
+  simgrid::mc::Region* region0 = new simgrid::mc::Region(simgrid::mc::RegionType::Data, source, byte_size);
   for (int i = 0; i < n; i += 2) {
     init_memory((char*)source + i * xbt_pagesize, xbt_pagesize);
   }
-  simgrid::mc::RegionSnapshot* region =
-      new simgrid::mc::RegionSnapshot(simgrid::mc::RegionType::Unknown, source, byte_size);
+  simgrid::mc::Region* region = new simgrid::mc::Region(simgrid::mc::RegionType::Data, source, byte_size);
 
   void* destination = mmap(nullptr, byte_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
   INFO("Could not allocate destination memory");
@@ -170,8 +168,7 @@ void snap_test_helper::read_pointer()
 
   prologue_return ret = prologue(1);
   memcpy(ret.src, &mc_model_checker, sizeof(void*));
-  simgrid::mc::RegionSnapshot* region2 =
-      new simgrid::mc::RegionSnapshot(simgrid::mc::RegionType::Unknown, ret.src, ret.size);
+  simgrid::mc::Region* region2 = new simgrid::mc::Region(simgrid::mc::RegionType::Data, ret.src, ret.size);
   INFO("Mismtach in MC_region_read_pointer()");
   REQUIRE(MC_region_read_pointer(region2, ret.src) == mc_model_checker);