Logo AND Algorithmique Numérique Distribuée

Public GIT Repository
MC gets the addresses of variables it needs from the network (not dwarf)
authorMartin Quinson <martin.quinson@ens-rennes.fr>
Sun, 14 Mar 2021 18:23:10 +0000 (19:23 +0100)
committerMartin Quinson <martin.quinson@ens-rennes.fr>
Sun, 14 Mar 2021 20:05:02 +0000 (21:05 +0100)
This way, Dwarf info are only loaded when they are really needed (ie,
when state equality is used or for liveness properties).

Running all MC tests goes from about 70 seconds down to 10 seconds
(there is a liveness test that lasts 10 seconds). The MC tests that do
not need dwarf go from 6 or 8 seconds down to half a second.

I'm not 100% sure, but this commit may introduce a race condition when
SMPI is used with dlopen privatization. The binary is sometimes
removed from disk too early for MC to pick the dwarf informations it
needs. I saw it some times while implementing it and now it seems to
be gone although I did not explicitely fix it. This seem to be tested
by mc-umpire-comm-dup-no-free2 so I'm not sure.

12 files changed:
include/simgrid/simix.hpp
src/kernel/actor/ActorImpl.cpp
src/kernel/actor/ActorImpl.hpp
src/mc/ModelChecker.cpp
src/mc/remote/AppSide.cpp
src/mc/remote/RemoteProcess.cpp
src/mc/remote/RemoteProcess.hpp
src/mc/remote/mc_protocol.h
src/mc/sosp/Snapshot_test.cpp
src/simix/smx_global.cpp
teshsuite/mc/dwarf-expression/dwarf-expression.cpp
teshsuite/mc/dwarf/dwarf.cpp

index 76fd191..12f84f5 100644 (file)
@@ -125,6 +125,10 @@ public:
   static double next() { return simix_timers().empty() ? -1.0 : simix_timers().top().first; }
 };
 
+// In MC mode, the application sends these pointers to the MC
+void* simix_global_get_actors_addr();
+void* simix_global_get_dead_actors_addr();
+
 } // namespace simix
 } // namespace simgrid
 
index 2b03fd7..9bd5961 100644 (file)
@@ -45,6 +45,10 @@ unsigned long get_maxpid()
 {
   return maxpid;
 }
+void* get_maxpid_addr()
+{
+  return &maxpid;
+}
 ActorImpl* ActorImpl::by_PID(aid_t PID)
 {
   auto item = simix_global->process_list.find(PID);
index ee2c5b6..307527e 100644 (file)
@@ -198,6 +198,8 @@ using SynchroList =
 
 XBT_PUBLIC void create_maestro(const std::function<void()>& code);
 XBT_PUBLIC unsigned long get_maxpid();
+XBT_PUBLIC void* get_maxpid_addr(); // In MC mode, the application sends this pointers to the MC
+
 } // namespace actor
 } // namespace kernel
 } // namespace simgrid
index 198b920..32ad396 100644 (file)
@@ -68,8 +68,6 @@ void ModelChecker::start()
   if (res < 0 || not WIFSTOPPED(status) || WSTOPSIG(status) != SIGSTOP)
     xbt_die("Could not wait model-checked process");
 
-  remote_process_->init();
-
   if (not _sg_mc_dot_output_file.get().empty())
     MC_init_dot_output();
 
@@ -154,6 +152,15 @@ bool ModelChecker::handle_message(const char* buffer, ssize_t size)
   memcpy(&base_message, buffer, sizeof(base_message));
 
   switch(base_message.type) {
+    case MessageType::INITIAL_ADDRESSES: {
+      s_mc_message_initial_addresses_t message;
+      xbt_assert(size == sizeof(message), "Broken message. Got %zd bytes instead of %zd.", size, sizeof(message));
+      memcpy(&message, buffer, sizeof(message));
+
+      get_remote_process().init(message.mmalloc_default_mdp, message.maxpid, message.actors, message.dead_actors);
+      break;
+    }
+
     case MessageType::IGNORE_HEAP: {
       s_mc_message_ignore_heap_t message;
       xbt_assert(size == sizeof(message), "Broken message");
index 7448d7c..4518895 100644 (file)
@@ -8,6 +8,7 @@
 #include "src/kernel/actor/ActorImpl.hpp"
 #include "src/mc/checker/SimcallObserver.hpp"
 #include "src/mc/remote/RemoteProcess.hpp"
+#include "src/xbt_modinter.h" /* mmalloc_preinit to get the default mmalloc arena address */
 #include <simgrid/modelchecker.h>
 
 #include <cerrno>
@@ -63,6 +64,11 @@ AppSide* AppSide::initialize()
   if (errno != 0 || raise(SIGSTOP) != 0)
     xbt_die("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(), simgrid::kernel::actor::get_maxpid_addr(),
+      simgrid::simix::simix_global_get_actors_addr(), simgrid::simix::simix_global_get_dead_actors_addr()};
+  xbt_assert(instance_->channel_.send(message) == 0, "Could not send the initial message with addresses.");
+
   instance_->handle_messages();
   return instance_.get();
 }
index 411a8e9..6f2ac48 100644 (file)
@@ -231,8 +231,13 @@ int open_vm(pid_t pid, int flags)
 
 RemoteProcess::RemoteProcess(pid_t pid) : AddressSpace(this), pid_(pid), running_(true) {}
 
-void RemoteProcess::init()
+void RemoteProcess::init(void* mmalloc_default_mdp, void* maxpid, void* actors, void* dead_actors)
 {
+  this->heap_address      = mmalloc_default_mdp;
+  this->maxpid_addr_      = maxpid;
+  this->actors_addr_      = actors;
+  this->dead_actors_addr_ = dead_actors;
+
   this->memory_map_ = simgrid::xbt::get_memory_map(this->pid_);
   this->init_memory_map_info();
 
@@ -240,12 +245,6 @@ void RemoteProcess::init()
   xbt_assert(fd >= 0, "Could not open file for process virtual address space");
   this->memory_file = fd;
 
-  // Read std_heap (is a struct mdesc*):
-  const simgrid::mc::Variable* std_heap_var = this->find_variable("__mmalloc_default_mdp");
-  xbt_assert(std_heap_var, "No heap information in the target process");
-  xbt_assert(std_heap_var->address, "No constant address for this variable");
-  this->read_bytes(&this->heap_address, sizeof(mdesc*), remote(std_heap_var->address));
-
   this->smx_actors_infos.clear();
   this->smx_dead_actors_infos.clear();
   this->unw_addr_space            = simgrid::mc::UnwindContext::createUnwindAddressSpace();
@@ -572,37 +571,15 @@ void RemoteProcess::dump_stack() const
 
 unsigned long RemoteProcess::get_maxpid() const
 {
-  static void* maxpid_addr = nullptr;
-  if (maxpid_addr == nullptr) {
-    const Variable* maxpid_var = find_variable("simgrid::kernel::actor::maxpid");
-    if (maxpid_var == nullptr)
-      maxpid_var = find_variable("maxpid"); // GCC sometimes eats the namespaces
-    maxpid_addr = maxpid_var->address;
-  }
   unsigned long maxpid;
-  this->read_bytes(&maxpid, sizeof(unsigned long), remote(maxpid_addr));
-
+  this->read_bytes(&maxpid, sizeof(unsigned long), remote(maxpid_addr_));
   return maxpid;
 }
 
 void RemoteProcess::get_actor_vectors(RemotePtr<s_xbt_dynar_t>& actors, RemotePtr<s_xbt_dynar_t>& dead_actors)
 {
-  static_assert(std::is_same<std::unique_ptr<simgrid::simix::Global>, decltype(simix_global)>::value,
-                "Unexpected type for simix_global");
-  static_assert(sizeof(simix_global) == sizeof(simgrid::simix::Global*), "Bad size for simix_global");
-
-  static RemotePtr<s_xbt_dynar_t> actors_;
-  static RemotePtr<s_xbt_dynar_t> dead_actors_;
-  static bool inited = false;
-  if (not inited) {
-    RemotePtr<simgrid::simix::Global> simix_global_p{this->read_variable<simgrid::simix::Global*>("simix_global")};
-    Remote<simgrid::simix::Global> simix_global = this->read<simgrid::simix::Global>(simix_global_p);
-
-    actors_      = remote(simix_global.get_buffer()->actors_vector);
-    dead_actors_ = remote(simix_global.get_buffer()->dead_actors_vector);
-  }
-  actors      = actors_;
-  dead_actors = dead_actors_;
+  actors      = remote(static_cast<s_xbt_dynar_t*>(actors_addr_));
+  dead_actors = remote(static_cast<s_xbt_dynar_t*>(dead_actors_addr_));
 }
 } // namespace mc
 } // namespace simgrid
index d0d53cb..2530d13 100644 (file)
@@ -75,7 +75,7 @@ private:
 public:
   explicit RemoteProcess(pid_t pid);
   ~RemoteProcess() override;
-  void init();
+  void init(void* mmalloc_default_mdp, void* maxpid, void* actors, void* dead_actors);
 
   RemoteProcess(RemoteProcess const&) = delete;
   RemoteProcess(RemoteProcess&&)      = delete;
@@ -164,7 +164,13 @@ public:
   /* ***************** */
   /* SIMIX-related API */
   /* ***************** */
+private:
+  // Cache the address of the variables we read directly in the memory of remote
+  void* maxpid_addr_;
+  void* actors_addr_;
+  void* dead_actors_addr_;
 
+public:
   std::vector<ActorInformation>& actors();
   std::vector<ActorInformation>& dead_actors();
 
index 824b8bb..f03cf3f 100644 (file)
@@ -26,8 +26,8 @@
 namespace simgrid {
 namespace mc {
 
-XBT_DECLARE_ENUM_CLASS(MessageType, NONE, CONTINUE, IGNORE_HEAP, UNIGNORE_HEAP, IGNORE_MEMORY, STACK_REGION,
-                       REGISTER_SYMBOL, DEADLOCK_CHECK, DEADLOCK_CHECK_REPLY, WAITING, SIMCALL_HANDLE,
+XBT_DECLARE_ENUM_CLASS(MessageType, NONE, INITIAL_ADDRESSES, CONTINUE, IGNORE_HEAP, UNIGNORE_HEAP, IGNORE_MEMORY,
+                       STACK_REGION, REGISTER_SYMBOL, DEADLOCK_CHECK, DEADLOCK_CHECK_REPLY, WAITING, SIMCALL_HANDLE,
                        SIMCALL_IS_VISIBLE, SIMCALL_IS_VISIBLE_ANSWER, SIMCALL_TO_STRING, SIMCALL_TO_STRING_ANSWER,
                        SIMCALL_DOT_LABEL, ASSERTION_FAILED, ACTOR_ENABLED, ACTOR_ENABLED_REPLY);
 
@@ -57,6 +57,14 @@ struct s_mc_message_int_t {
 };
 
 /* Client->Server */
+struct s_mc_message_initial_addresses_t {
+  simgrid::mc::MessageType type;
+  void* mmalloc_default_mdp;
+  void* maxpid;
+  void* actors;
+  void* dead_actors;
+};
+
 struct s_mc_message_ignore_heap_t {
   simgrid::mc::MessageType type;
   int block;
index 47350be..a2af058 100644 (file)
@@ -58,7 +58,7 @@ void snap_test_helper::Init()
   REQUIRE(1 << xbt_pagebits == xbt_pagesize);
 
   process = std::make_unique<simgrid::mc::RemoteProcess>(getpid());
-  process->init();
+  process->init(nullptr, nullptr, nullptr, nullptr);
   mc_model_checker = new ::simgrid::mc::ModelChecker(std::move(process), -1);
 }
 
index 50fb970..4b4a165 100644 (file)
@@ -38,6 +38,24 @@ void (*SMPI_switch_data_segment)(simgrid::s4u::ActorPtr) = nullptr;
 namespace simgrid {
 namespace simix {
 config::Flag<bool> cfg_verbose_exit{"debug/verbose-exit", "Display the actor status at exit", true};
+
+void* simix_global_get_actors_addr()
+{
+#if SIMGRID_HAVE_MC
+  return simix_global->actors_vector;
+#else
+  xbt_die("This function is intended to be used when compiling with MC");
+#endif
+}
+void* simix_global_get_dead_actors_addr()
+{
+#if SIMGRID_HAVE_MC
+  return simix_global->dead_actors_vector;
+#else
+  xbt_die("This function is intended to be used when compiling with MC");
+#endif
+}
+
 } // namespace simix
 } // namespace simgrid
 
@@ -267,22 +285,19 @@ void SIMIX_set_maestro(void (*code)(void*), void* data)
   maestro_code = std::bind(code, data);
 }
 
-/**
- * @ingroup SIMIX_API
- * @brief Initialize SIMIX internal data.
- */
 void SIMIX_global_init(int* argc, char** argv)
 {
+  if (simix_global == nullptr) {
+    simix_global = std::make_unique<simgrid::simix::Global>();
+
 #if SIMGRID_HAVE_MC
-  // The communication initialization is done ASAP.
-  // We need to communicate  initialization of the different layers to the model-checker.
-  simgrid::mc::AppSide::initialize();
+    // The communication initialization is done ASAP, as we need to get some init parameters from the MC for different layers.
+    // But simix_global needs to be created, as we send the address of some of its fields to the MC that wants to read them directly.
+    simgrid::mc::AppSide::initialize();
 #endif
 
-  if (simix_global == nullptr) {
     surf_init(argc, argv); /* Initialize SURF structures */
 
-    simix_global           = std::make_unique<simgrid::simix::Global>();
     simix_global->maestro_ = nullptr;
     SIMIX_context_mod_init();
 
index 23b7b3d..febdea1 100644 (file)
@@ -146,7 +146,7 @@ static void test_deref(simgrid::dwarf::ExpressionContext const& state)
 int main()
 {
   auto* process = new simgrid::mc::RemoteProcess(getpid());
-  process->init();
+  process->init(nullptr, nullptr, nullptr, nullptr);
 
   simgrid::dwarf::ExpressionContext state;
   state.address_space = (simgrid::mc::AddressSpace*) process;
index 39739ca..b3fe240 100644 (file)
@@ -117,7 +117,7 @@ int main(int argc, char** argv)
   simgrid::mc::Type* type;
 
   simgrid::mc::RemoteProcess process(getpid());
-  process.init();
+  process.init(nullptr, nullptr, nullptr, nullptr);
 
   test_global_variable(process, process.binary_info.get(), "some_local_variable", &some_local_variable, sizeof(int));