From 1e251d6d5fb36bcc96f7a3a92314cf54ec03e35a Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Sun, 19 Mar 2023 21:07:00 +0100 Subject: [PATCH] Move more of the CheckerSide creation logic to the object constructor --- src/mc/api/RemoteApp.cpp | 138 +------------------------------ src/mc/api/RemoteApp.hpp | 1 - src/mc/remote/Channel.hpp | 2 + src/mc/remote/CheckerSide.cpp | 147 +++++++++++++++++++++++++++++++++- src/mc/remote/CheckerSide.hpp | 5 +- 5 files changed, 155 insertions(+), 138 deletions(-) diff --git a/src/mc/api/RemoteApp.cpp b/src/mc/api/RemoteApp.cpp index b25e42f2d5..c72cb851b0 100644 --- a/src/mc/api/RemoteApp.cpp +++ b/src/mc/api/RemoteApp.cpp @@ -17,125 +17,24 @@ #include #include -#include #include #include #include -#include -#ifdef __linux__ -#include -#include -#endif #include #include -#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 _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& 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 semicol_sep(";"); - boost::char_separator 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 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& 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(sockets[1], pid); - - start(); + checker_side_ = std::make_unique(args); /* Take the initial snapshot */ - wait_for_requests(); initial_snapshot_ = std::make_shared(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() diff --git a/src/mc/api/RemoteApp.hpp b/src/mc/api/RemoteApp.hpp index 62758d5721..d6e5eee825 100644 --- a/src/mc/api/RemoteApp.hpp +++ b/src/mc/api/RemoteApp.hpp @@ -46,7 +46,6 @@ public: ~RemoteApp(); - void start(); void restore_initial_state() const; void wait_for_requests(); diff --git a/src/mc/remote/Channel.hpp b/src/mc/remote/Channel.hpp index da1f649ae1..fa270b1670 100644 --- a/src/mc/remote/Channel.hpp +++ b/src/mc/remote/Channel.hpp @@ -22,6 +22,7 @@ class Channel { template static constexpr bool messageType() { return std::is_class_v && std::is_trivial_v; } 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 diff --git a/src/mc/remote/CheckerSide.cpp b/src/mc/remote/CheckerSide.cpp index 9c04307ab4..bd7b16b367 100644 --- a/src/mc/remote/CheckerSide.cpp +++ b/src/mc/remote/CheckerSide.cpp @@ -9,14 +9,122 @@ #include "src/mc/sosp/RemoteProcessMemory.hpp" #include "xbt/system_error.hpp" +#ifdef __linux__ +#include +#include +#endif + +#include #include +#include #include #include +#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 _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& 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 semicol_sep(";"); + boost::char_separator 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 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& 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_) diff --git a/src/mc/remote/CheckerSide.hpp b/src/mc/remote/CheckerSide.hpp index d3e571ba65..b385ae036a 100644 --- a/src/mc/remote/CheckerSide.hpp +++ b/src/mc/remote/CheckerSide.hpp @@ -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& 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_; } -- 2.20.1