Logo AND Algorithmique Numérique Distribuée

Public GIT Repository
Move methods not related to Memory out of RemoteProcessMemory
authorMartin Quinson <martin.quinson@ens-rennes.fr>
Sun, 19 Mar 2023 14:20:23 +0000 (15:20 +0100)
committerMartin Quinson <martin.quinson@ens-rennes.fr>
Sun, 19 Mar 2023 14:20:28 +0000 (15:20 +0100)
Now that ModelChecker is gone, it's time to move to the next step of cleanup.

The goal is that CheckerSide is in charge of the interaction with the
application process and RemoteProcessMemory is in charge of its memory.

Right now, RemoteProcessMemory does a bit more, as it stores the pid
and whether or not the application process is running.

This is bad because we want to make RemoteProcessMemory optional, only
used when we need to introspect the application memory (liveness
checking, non-progression checking, etc), so that we can run the app
in valgrind when we don't need to introspect its memory (safety
checking without non-progression checking).

I know I just moved this chunks of code from ModelChecker to
RemoteProcessMemory to now move it further, and I'm sorry for the
noise, but this code drives me nuts and I need to clean it step by step.

src/mc/remote/CheckerSide.cpp
src/mc/remote/CheckerSide.hpp
src/mc/sosp/RemoteProcessMemory.cpp
src/mc/sosp/RemoteProcessMemory.hpp

index b455df1..9d6df4d 100644 (file)
@@ -4,9 +4,13 @@
  * under the terms of the license (GNU LGPL) which comes with this package. */
 
 #include "src/mc/remote/CheckerSide.hpp"
+#include "src/mc/explo/Exploration.hpp"
+#include "src/mc/explo/LivenessChecker.hpp"
 #include "src/mc/sosp/RemoteProcessMemory.hpp"
 #include "xbt/system_error.hpp"
+
 #include <csignal>
+#include <sys/ptrace.h>
 #include <sys/wait.h>
 
 XBT_LOG_NEW_DEFAULT_SUBCATEGORY(mc_checkerside, mc, "MC communication with the application");
@@ -31,7 +35,7 @@ CheckerSide::CheckerSide(int sockfd, std::unique_ptr<RemoteProcessMemory> memory
               throw simgrid::xbt::errno_error();
           }
 
-          if (not checker->get_remote_memory().handle_message(buffer.data(), size))
+          if (not checker->handle_message(buffer.data(), size))
             checker->break_loop();
         } else {
           xbt_die("Unexpected event");
@@ -47,7 +51,7 @@ CheckerSide::CheckerSide(int sockfd, std::unique_ptr<RemoteProcessMemory> memory
         auto checker = static_cast<simgrid::mc::CheckerSide*>(arg);
         if (events == EV_SIGNAL) {
           if (sig == SIGCHLD)
-            checker->get_remote_memory().handle_waitpid();
+            checker->handle_waitpid();
           else
             xbt_die("Unexpected signal: %d", sig);
         } else {
@@ -69,4 +73,132 @@ void CheckerSide::break_loop() const
   event_base_loopbreak(base_.get());
 }
 
+bool CheckerSide::handle_message(const char* buffer, ssize_t size)
+{
+  s_mc_message_t base_message;
+  xbt_assert(size >= (ssize_t)sizeof(base_message), "Broken message");
+  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 %d bytes instead of %d.", (int)size,
+                 (int)sizeof(message));
+      memcpy(&message, buffer, sizeof(message));
+
+      get_remote_memory().init(message.mmalloc_default_mdp);
+      break;
+    }
+
+    case MessageType::IGNORE_HEAP: {
+      s_mc_message_ignore_heap_t message;
+      xbt_assert(size == sizeof(message), "Broken message");
+      memcpy(&message, buffer, sizeof(message));
+
+      IgnoredHeapRegion region;
+      region.block    = message.block;
+      region.fragment = message.fragment;
+      region.address  = message.address;
+      region.size     = message.size;
+      get_remote_memory().ignore_heap(region);
+      break;
+    }
+
+    case MessageType::UNIGNORE_HEAP: {
+      s_mc_message_ignore_memory_t message;
+      xbt_assert(size == sizeof(message), "Broken message");
+      memcpy(&message, buffer, sizeof(message));
+      get_remote_memory().unignore_heap((void*)(std::uintptr_t)message.addr, message.size);
+      break;
+    }
+
+    case MessageType::IGNORE_MEMORY: {
+      s_mc_message_ignore_memory_t message;
+      xbt_assert(size == sizeof(message), "Broken message");
+      memcpy(&message, buffer, sizeof(message));
+      get_remote_memory().ignore_region(message.addr, message.size);
+      break;
+    }
+
+    case MessageType::STACK_REGION: {
+      s_mc_message_stack_region_t message;
+      xbt_assert(size == sizeof(message), "Broken message");
+      memcpy(&message, buffer, sizeof(message));
+      get_remote_memory().stack_areas().push_back(message.stack_region);
+    } break;
+
+    case MessageType::REGISTER_SYMBOL: {
+      s_mc_message_register_symbol_t message;
+      xbt_assert(size == sizeof(message), "Broken message");
+      memcpy(&message, buffer, sizeof(message));
+      xbt_assert(not message.callback, "Support for client-side function proposition is not implemented.");
+      XBT_DEBUG("Received symbol: %s", message.name.data());
+
+      LivenessChecker::automaton_register_symbol(get_remote_memory(), message.name.data(), remote((int*)message.data));
+      break;
+    }
+
+    case MessageType::WAITING:
+      return false;
+
+    case MessageType::ASSERTION_FAILED:
+      Exploration::get_instance()->report_assertion_failure();
+      break;
+
+    default:
+      xbt_die("Unexpected message from model-checked application");
+  }
+  return true;
+}
+
+void CheckerSide::handle_waitpid()
+{
+  XBT_DEBUG("Check for wait event");
+  int status;
+  pid_t pid;
+  while ((pid = waitpid(-1, &status, WNOHANG)) != 0) {
+    if (pid == -1) {
+      if (errno == ECHILD) { // No more children:
+        xbt_assert(not get_remote_memory().running(), "Inconsistent state");
+        break;
+      } else {
+        XBT_ERROR("Could not wait for pid");
+        throw simgrid::xbt::errno_error();
+      }
+    }
+
+    if (pid == get_remote_memory().pid()) {
+      // From PTRACE_O_TRACEEXIT:
+#ifdef __linux__
+      if (status >> 8 == (SIGTRAP | (PTRACE_EVENT_EXIT << 8))) {
+        unsigned long eventmsg;
+        xbt_assert(ptrace(PTRACE_GETEVENTMSG, pid, 0, &eventmsg) != -1, "Could not get exit status");
+        status = static_cast<int>(eventmsg);
+        if (WIFSIGNALED(status))
+          Exploration::get_instance()->report_crash(status);
+      }
+#endif
+
+      // We don't care about non-lethal signals, just reinject them:
+      if (WIFSTOPPED(status)) {
+        XBT_DEBUG("Stopped with signal %i", (int)WSTOPSIG(status));
+        errno = 0;
+#ifdef __linux__
+        ptrace(PTRACE_CONT, pid, 0, WSTOPSIG(status));
+#elif defined BSD
+        ptrace(PT_CONTINUE, pid, (caddr_t)1, WSTOPSIG(status));
+#endif
+        xbt_assert(errno == 0, "Could not PTRACE_CONT");
+      }
+
+      else if (WIFSIGNALED(status)) {
+        Exploration::get_instance()->report_crash(status);
+      } else if (WIFEXITED(status)) {
+        XBT_DEBUG("Child process is over");
+        get_remote_memory().terminate();
+      }
+    }
+  }
+}
+
 } // namespace simgrid::mc
index 0c32948..8f99b48 100644 (file)
@@ -36,6 +36,9 @@ public:
 
   void dispatch_events() const;
   void break_loop() const;
+
+  void handle_waitpid();
+  bool handle_message(const char* buffer, ssize_t size);
 };
 
 } // namespace simgrid::mc
index 0bfb964..0051ddf 100644 (file)
@@ -25,8 +25,6 @@
 #include <mutex>
 #include <string>
 #include <string_view>
-#include <sys/ptrace.h>
-#include <sys/wait.h>
 
 XBT_LOG_NEW_DEFAULT_SUBCATEGORY(mc_process, mc, "MC process information");
 
@@ -456,132 +454,4 @@ void RemoteProcessMemory::dump_stack() const
   unw_destroy_addr_space(as);
 }
 
-bool RemoteProcessMemory::handle_message(const char* buffer, ssize_t size)
-{
-  s_mc_message_t base_message;
-  xbt_assert(size >= (ssize_t)sizeof(base_message), "Broken message");
-  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 %d bytes instead of %d.", (int)size,
-                 (int)sizeof(message));
-      memcpy(&message, buffer, sizeof(message));
-
-      this->init(message.mmalloc_default_mdp);
-      break;
-    }
-
-    case MessageType::IGNORE_HEAP: {
-      s_mc_message_ignore_heap_t message;
-      xbt_assert(size == sizeof(message), "Broken message");
-      memcpy(&message, buffer, sizeof(message));
-
-      IgnoredHeapRegion region;
-      region.block    = message.block;
-      region.fragment = message.fragment;
-      region.address  = message.address;
-      region.size     = message.size;
-      this->ignore_heap(region);
-      break;
-    }
-
-    case MessageType::UNIGNORE_HEAP: {
-      s_mc_message_ignore_memory_t message;
-      xbt_assert(size == sizeof(message), "Broken message");
-      memcpy(&message, buffer, sizeof(message));
-      this->unignore_heap((void*)(std::uintptr_t)message.addr, message.size);
-      break;
-    }
-
-    case MessageType::IGNORE_MEMORY: {
-      s_mc_message_ignore_memory_t message;
-      xbt_assert(size == sizeof(message), "Broken message");
-      memcpy(&message, buffer, sizeof(message));
-      this->ignore_region(message.addr, message.size);
-      break;
-    }
-
-    case MessageType::STACK_REGION: {
-      s_mc_message_stack_region_t message;
-      xbt_assert(size == sizeof(message), "Broken message");
-      memcpy(&message, buffer, sizeof(message));
-      this->stack_areas().push_back(message.stack_region);
-    } break;
-
-    case MessageType::REGISTER_SYMBOL: {
-      s_mc_message_register_symbol_t message;
-      xbt_assert(size == sizeof(message), "Broken message");
-      memcpy(&message, buffer, sizeof(message));
-      xbt_assert(not message.callback, "Support for client-side function proposition is not implemented.");
-      XBT_DEBUG("Received symbol: %s", message.name.data());
-
-      LivenessChecker::automaton_register_symbol(*this, message.name.data(), remote((int*)message.data));
-      break;
-    }
-
-    case MessageType::WAITING:
-      return false;
-
-    case MessageType::ASSERTION_FAILED:
-      Exploration::get_instance()->report_assertion_failure();
-      break;
-
-    default:
-      xbt_die("Unexpected message from model-checked application");
-  }
-  return true;
-}
-
-void RemoteProcessMemory::handle_waitpid()
-{
-  XBT_DEBUG("Check for wait event");
-  int status;
-  pid_t pid;
-  while ((pid = waitpid(-1, &status, WNOHANG)) != 0) {
-    if (pid == -1) {
-      if (errno == ECHILD) { // No more children:
-        xbt_assert(not running(), "Inconsistent state");
-        break;
-      } else {
-        XBT_ERROR("Could not wait for pid");
-        throw simgrid::xbt::errno_error();
-      }
-    }
-
-    if (pid == this->pid()) {
-      // From PTRACE_O_TRACEEXIT:
-#ifdef __linux__
-      if (status >> 8 == (SIGTRAP | (PTRACE_EVENT_EXIT << 8))) {
-        unsigned long eventmsg;
-        xbt_assert(ptrace(PTRACE_GETEVENTMSG, pid, 0, &eventmsg) != -1, "Could not get exit status");
-        status = static_cast<int>(eventmsg);
-        if (WIFSIGNALED(status))
-          Exploration::get_instance()->report_crash(status);
-      }
-#endif
-
-      // We don't care about non-lethal signals, just reinject them:
-      if (WIFSTOPPED(status)) {
-        XBT_DEBUG("Stopped with signal %i", (int)WSTOPSIG(status));
-        errno = 0;
-#ifdef __linux__
-        ptrace(PTRACE_CONT, pid, 0, WSTOPSIG(status));
-#elif defined BSD
-        ptrace(PT_CONTINUE, pid, (caddr_t)1, WSTOPSIG(status));
-#endif
-        xbt_assert(errno == 0, "Could not PTRACE_CONT");
-      }
-
-      else if (WIFSIGNALED(status)) {
-        Exploration::get_instance()->report_crash(status);
-      } else if (WIFEXITED(status)) {
-        XBT_DEBUG("Child process is over");
-        terminate();
-      }
-    }
-  }
-}
-
 } // namespace simgrid::mc
index b0eaeb6..40aef18 100644 (file)
@@ -62,8 +62,6 @@ public:
   pid_t pid() const { return pid_; }
   bool running() const { return running_; }
   void terminate() { running_ = false; }
-  void handle_waitpid();
-  bool handle_message(const char* buffer, ssize_t size);
 
   /* ************* */
   /* Low-level API */