Logo AND Algorithmique Numérique Distribuée

Public GIT Repository
Move more of the CheckerSide creation logic to the object constructor
authorMartin Quinson <martin.quinson@ens-rennes.fr>
Sun, 19 Mar 2023 20:07:00 +0000 (21:07 +0100)
committerMartin Quinson <martin.quinson@ens-rennes.fr>
Sun, 19 Mar 2023 20:12:24 +0000 (21:12 +0100)
src/mc/api/RemoteApp.cpp
src/mc/api/RemoteApp.hpp
src/mc/remote/Channel.hpp
src/mc/remote/CheckerSide.cpp
src/mc/remote/CheckerSide.hpp

index b25e42f..c72cb85 100644 (file)
 
 #include <algorithm>
 #include <array>
-#include <boost/tokenizer.hpp>
 #include <memory>
 #include <numeric>
 #include <string>
 
-#include <fcntl.h>
-#ifdef __linux__
-#include <sys/personality.h>
-#include <sys/prctl.h>
-#endif
 #include <sys/ptrace.h>
 #include <sys/wait.h>
 
-#ifdef __linux__
-#define WAITPID_CHECKED_FLAGS __WALL
-#else
-#define WAITPID_CHECKED_FLAGS 0
-#endif
-
 XBT_LOG_NEW_DEFAULT_SUBCATEGORY(mc_Session, mc, "Model-checker session");
 XBT_LOG_EXTERNAL_CATEGORY(mc_global);
 
-static simgrid::config::Flag<std::string> _sg_mc_setenv{
-    "model-check/setenv", "Extra environment variables to pass to the child process (ex: 'AZE=aze;QWE=qwe').", "",
-    [](std::string_view value) {
-      xbt_assert(value.empty() || value.find('=', 0) != std::string_view::npos,
-                 "The 'model-check/setenv' parameter must be like 'AZE=aze', but it does not contain an equal sign.");
-    }};
-
 namespace simgrid::mc {
 
-XBT_ATTRIB_NORETURN static void run_child_process(int socket, const std::vector<char*>& args)
-{
-  /* On startup, simix_global_init() calls simgrid::mc::Client::initialize(), which checks whether the MC_ENV_SOCKET_FD
-   * env variable is set. If so, MC mode is assumed, and the client is setup from its side
-   */
-
-#ifdef __linux__
-  // Make sure we do not outlive our parent
-  sigset_t mask;
-  sigemptyset(&mask);
-  xbt_assert(sigprocmask(SIG_SETMASK, &mask, nullptr) >= 0, "Could not unblock signals");
-  xbt_assert(prctl(PR_SET_PDEATHSIG, SIGHUP) == 0, "Could not PR_SET_PDEATHSIG");
-
-  // Make sure that the application process layout is not randomized, so that the info we gather is stable over re-execs
-  if (personality(ADDR_NO_RANDOMIZE) == -1) {
-    XBT_ERROR("Could not set the NO_RANDOMIZE personality");
-    throw xbt::errno_error();
-  }
-#endif
-
-  // Remove CLOEXEC to pass the socket to the application
-  int fdflags = fcntl(socket, F_GETFD, 0);
-  xbt_assert(fdflags != -1 && fcntl(socket, F_SETFD, fdflags & ~FD_CLOEXEC) != -1,
-             "Could not remove CLOEXEC for socket");
-
-  setenv(MC_ENV_SOCKET_FD, std::to_string(socket).c_str(), 1);
-
-  /* Setup the tokenizer that parses the cfg:model-check/setenv parameter */
-  using Tokenizer = boost::tokenizer<boost::char_separator<char>>;
-  boost::char_separator<char> semicol_sep(";");
-  boost::char_separator<char> equal_sep("=");
-  Tokenizer token_vars(_sg_mc_setenv.get(), semicol_sep); /* Iterate over all FOO=foo parts */
-  for (const auto& token : token_vars) {
-    std::vector<std::string> kv;
-    Tokenizer token_kv(token, equal_sep);
-    for (const auto& t : token_kv) /* Iterate over 'FOO' and then 'foo' in that 'FOO=foo' */
-      kv.push_back(t);
-    xbt_assert(kv.size() == 2, "Parse error on 'model-check/setenv' value %s. Does it contain an equal sign?",
-               token.c_str());
-    XBT_INFO("setenv '%s'='%s'", kv[0].c_str(), kv[1].c_str());
-    setenv(kv[0].c_str(), kv[1].c_str(), 1);
-  }
-
-  /* And now, exec the child process */
-  int i = 1;
-  while (args[i] != nullptr && args[i][0] == '-')
-    i++;
-
-  xbt_assert(args[i] != nullptr,
-             "Unable to find a binary to exec on the command line. Did you only pass config flags?");
-
-  execvp(args[i], args.data() + i);
-  XBT_CRITICAL("The model-checked process failed to exec(%s): %s.\n"
-               "        Make sure that your binary exists on disk and is executable.",
-               args[i], strerror(errno));
-  if (strchr(args[i], '=') != nullptr)
-    XBT_CRITICAL("If you want to pass environment variables to the application, please use --cfg=model-check/setenv:%s",
-                 args[i]);
-
-  xbt_die("Aborting now.");
-}
-
 RemoteApp::RemoteApp(const std::vector<char*>& args)
 {
-  // Create an AF_LOCAL socketpair used for exchanging messages
-  // between the model-checker process (ourselves) and the model-checked
-  // process:
-  int sockets[2];
-  xbt_assert(socketpair(AF_LOCAL, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sockets) != -1, "Could not create socketpair");
-
-  pid_t pid = fork();
-  xbt_assert(pid >= 0, "Could not fork model-checked process");
-
-  if (pid == 0) { // Child
-    ::close(sockets[1]);
-    run_child_process(sockets[0], args);
-    DIE_IMPOSSIBLE;
-  }
-
-  // Parent (model-checker):
-  ::close(sockets[0]);
 
-  checker_side_ = std::make_unique<simgrid::mc::CheckerSide>(sockets[1], pid);
-
-  start();
+  checker_side_ = std::make_unique<simgrid::mc::CheckerSide>(args);
 
   /* Take the initial snapshot */
-  wait_for_requests();
   initial_snapshot_ = std::make_shared<simgrid::mc::Snapshot>(0, page_store_, checker_side_->get_remote_memory());
 }
 
@@ -144,32 +43,7 @@ RemoteApp::~RemoteApp()
   initial_snapshot_ = nullptr;
   shutdown();
 }
-void RemoteApp::start()
-{
-  XBT_DEBUG("Waiting for the model-checked process");
-  int status;
-
-  // The model-checked process SIGSTOP itself to signal it's ready:
-  const pid_t pid = checker_side_->get_pid();
-
-  xbt_assert(waitpid(pid, &status, WAITPID_CHECKED_FLAGS) == pid && WIFSTOPPED(status) && WSTOPSIG(status) == SIGSTOP,
-             "Could not wait model-checked process");
-
-  errno = 0;
-#ifdef __linux__
-  ptrace(PTRACE_SETOPTIONS, pid, nullptr, PTRACE_O_TRACEEXIT);
-  ptrace(PTRACE_CONT, pid, 0, 0);
-#elif defined BSD
-  ptrace(PT_CONTINUE, pid, (caddr_t)1, 0);
-#else
-#error "no ptrace equivalent coded for this platform"
-#endif
-  xbt_assert(errno == 0,
-             "Ptrace does not seem to be usable in your setup (errno: %d). "
-             "If you run from within a docker, adding `--cap-add SYS_PTRACE` to the docker line may help. "
-             "If it does not help, please report this bug.",
-             errno);
-}
+
 void RemoteApp::restore_initial_state() const
 {
   this->initial_snapshot_->restore(checker_side_->get_remote_memory());
@@ -293,13 +167,7 @@ void RemoteApp::check_deadlock() const
 
 void RemoteApp::wait_for_requests()
 {
-  /* Resume the application */
-  if (checker_side_->get_channel().send(MessageType::CONTINUE) != 0)
-    throw xbt::errno_error();
-  checker_side_->clear_memory_cache();
-
-  if (checker_side_->running())
-    checker_side_->dispatch_events();
+  checker_side_->wait_for_requests();
 }
 
 void RemoteApp::shutdown()
index 62758d5..d6e5eee 100644 (file)
@@ -46,7 +46,6 @@ public:
 
   ~RemoteApp();
 
-  void start();
   void restore_initial_state() const;
   void wait_for_requests();
 
index da1f649..fa270b1 100644 (file)
@@ -22,6 +22,7 @@ class Channel {
   template <class M> static constexpr bool messageType() { return std::is_class_v<M> && std::is_trivial_v<M>; }
 
 public:
+  Channel() = default;
   explicit Channel(int sock) : socket_(sock) {}
   ~Channel();
 
@@ -50,6 +51,7 @@ public:
   }
 
   int get_socket() const { return socket_; }
+  void reset_socket(int socket) { socket_ = socket; }
 };
 } // namespace simgrid::mc
 
index 9c04307..bd7b16b 100644 (file)
 #include "src/mc/sosp/RemoteProcessMemory.hpp"
 #include "xbt/system_error.hpp"
 
+#ifdef __linux__
+#include <sys/personality.h>
+#include <sys/prctl.h>
+#endif
+
+#include <boost/tokenizer.hpp>
 #include <csignal>
+#include <fcntl.h>
 #include <sys/ptrace.h>
 #include <sys/wait.h>
 
+#ifdef __linux__
+#define WAITPID_CHECKED_FLAGS __WALL
+#else
+#define WAITPID_CHECKED_FLAGS 0
+#endif
+
 XBT_LOG_NEW_DEFAULT_SUBCATEGORY(mc_checkerside, mc, "MC communication with the application");
 
+static simgrid::config::Flag<std::string> _sg_mc_setenv{
+    "model-check/setenv", "Extra environment variables to pass to the child process (ex: 'AZE=aze;QWE=qwe').", "",
+    [](std::string_view value) {
+      xbt_assert(value.empty() || value.find('=', 0) != std::string_view::npos,
+                 "The 'model-check/setenv' parameter must be like 'AZE=aze', but it does not contain an equal sign.");
+    }};
+
 namespace simgrid::mc {
-CheckerSide::CheckerSide(int sockfd, pid_t pid) : channel_(sockfd), running_(true), pid_(pid)
+
+XBT_ATTRIB_NORETURN static void run_child_process(int socket, const std::vector<char*>& args)
+{
+  /* On startup, simix_global_init() calls simgrid::mc::Client::initialize(), which checks whether the MC_ENV_SOCKET_FD
+   * env variable is set. If so, MC mode is assumed, and the client is setup from its side
+   */
+
+#ifdef __linux__
+  // Make sure we do not outlive our parent
+  sigset_t mask;
+  sigemptyset(&mask);
+  xbt_assert(sigprocmask(SIG_SETMASK, &mask, nullptr) >= 0, "Could not unblock signals");
+  xbt_assert(prctl(PR_SET_PDEATHSIG, SIGHUP) == 0, "Could not PR_SET_PDEATHSIG");
+
+  // Make sure that the application process layout is not randomized, so that the info we gather is stable over re-execs
+  if (personality(ADDR_NO_RANDOMIZE) == -1) {
+    XBT_ERROR("Could not set the NO_RANDOMIZE personality");
+    throw xbt::errno_error();
+  }
+#endif
+
+  // Remove CLOEXEC to pass the socket to the application
+  int fdflags = fcntl(socket, F_GETFD, 0);
+  xbt_assert(fdflags != -1 && fcntl(socket, F_SETFD, fdflags & ~FD_CLOEXEC) != -1,
+             "Could not remove CLOEXEC for socket");
+
+  setenv(MC_ENV_SOCKET_FD, std::to_string(socket).c_str(), 1);
+
+  /* Setup the tokenizer that parses the cfg:model-check/setenv parameter */
+  using Tokenizer = boost::tokenizer<boost::char_separator<char>>;
+  boost::char_separator<char> semicol_sep(";");
+  boost::char_separator<char> equal_sep("=");
+  Tokenizer token_vars(_sg_mc_setenv.get(), semicol_sep); /* Iterate over all FOO=foo parts */
+  for (const auto& token : token_vars) {
+    std::vector<std::string> kv;
+    Tokenizer token_kv(token, equal_sep);
+    for (const auto& t : token_kv) /* Iterate over 'FOO' and then 'foo' in that 'FOO=foo' */
+      kv.push_back(t);
+    xbt_assert(kv.size() == 2, "Parse error on 'model-check/setenv' value %s. Does it contain an equal sign?",
+               token.c_str());
+    XBT_INFO("setenv '%s'='%s'", kv[0].c_str(), kv[1].c_str());
+    setenv(kv[0].c_str(), kv[1].c_str(), 1);
+  }
+
+  /* And now, exec the child process */
+  int i = 1;
+  while (args[i] != nullptr && args[i][0] == '-')
+    i++;
+
+  xbt_assert(args[i] != nullptr,
+             "Unable to find a binary to exec on the command line. Did you only pass config flags?");
+
+  execvp(args[i], args.data() + i);
+  XBT_CRITICAL("The model-checked process failed to exec(%s): %s.\n"
+               "        Make sure that your binary exists on disk and is executable.",
+               args[i], strerror(errno));
+  if (strchr(args[i], '=') != nullptr)
+    XBT_CRITICAL("If you want to pass environment variables to the application, please use --cfg=model-check/setenv:%s",
+                 args[i]);
+
+  xbt_die("Aborting now.");
+}
+
+static void wait_application_process(pid_t pid)
+{
+  XBT_DEBUG("Waiting for the model-checked process");
+  int status;
+
+  // The model-checked process SIGSTOP itself to signal it's ready:
+  xbt_assert(waitpid(pid, &status, WAITPID_CHECKED_FLAGS) == pid && WIFSTOPPED(status) && WSTOPSIG(status) == SIGSTOP,
+             "Could not wait model-checked process");
+
+  errno = 0;
+#ifdef __linux__
+  ptrace(PTRACE_SETOPTIONS, pid, nullptr, PTRACE_O_TRACEEXIT);
+  ptrace(PTRACE_CONT, pid, 0, 0);
+#elif defined BSD
+  ptrace(PT_CONTINUE, pid, (caddr_t)1, 0);
+#else
+#error "no ptrace equivalent coded for this platform"
+#endif
+  xbt_assert(errno == 0,
+             "Ptrace does not seem to be usable in your setup (errno: %d). "
+             "If you run from within a docker, adding `--cap-add SYS_PTRACE` to the docker line may help. "
+             "If it does not help, please report this bug.",
+             errno);
+}
+
+void CheckerSide::setup_events()
 {
   auto* base = event_base_new();
   base_.reset(base);
@@ -62,6 +170,32 @@ CheckerSide::CheckerSide(int sockfd, pid_t pid) : channel_(sockfd), running_(tru
   signal_event_.reset(signal_event);
 }
 
+CheckerSide::CheckerSide(const std::vector<char*>& args) : running_(true)
+{
+  // Create an AF_LOCAL socketpair used for exchanging messages between the model-checker process (ancestor)
+  // and the application process (child)
+  int sockets[2];
+  xbt_assert(socketpair(AF_LOCAL, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sockets) != -1, "Could not create socketpair");
+
+  pid_ = fork();
+  xbt_assert(pid_ >= 0, "Could not fork model-checked process");
+
+  if (pid_ == 0) { // Child
+    ::close(sockets[1]);
+    run_child_process(sockets[0], args);
+    DIE_IMPOSSIBLE;
+  }
+
+  // Parent (model-checker):
+  ::close(sockets[0]);
+  channel_.reset_socket(sockets[1]);
+
+  setup_events();
+  wait_application_process(pid_);
+
+  wait_for_requests();
+}
+
 void CheckerSide::dispatch_events() const
 {
   event_base_dispatch(base_.get());
@@ -150,6 +284,17 @@ bool CheckerSide::handle_message(const char* buffer, ssize_t size)
   return true;
 }
 
+void CheckerSide::wait_for_requests()
+{
+  /* Resume the application */
+  if (get_channel().send(MessageType::CONTINUE) != 0)
+    throw xbt::errno_error();
+  clear_memory_cache();
+
+  if (running())
+    dispatch_events();
+}
+
 void CheckerSide::clear_memory_cache()
 {
   if (remote_memory_)
index d3e571b..b385ae0 100644 (file)
@@ -27,8 +27,10 @@ class CheckerSide {
   bool running_ = false;
   pid_t pid_;
 
+  void setup_events(); // Part of the initialization
+
 public:
-  explicit CheckerSide(int sockfd, pid_t pid);
+  explicit CheckerSide(const std::vector<char*>& args);
 
   // No copy:
   CheckerSide(CheckerSide const&) = delete;
@@ -42,6 +44,7 @@ public:
   bool handle_message(const char* buffer, ssize_t size);
   void dispatch_events() const;
   void break_loop() const;
+  void wait_for_requests();
 
   /* Interacting with the application process */
   pid_t get_pid() const { return pid_; }