From 0450cc3993fd6914ee4c9c47ee8261a250b58cd7 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Sun, 19 Mar 2023 15:20:23 +0100 Subject: [PATCH] Move methods not related to Memory out of RemoteProcessMemory 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 | 136 +++++++++++++++++++++++++++- src/mc/remote/CheckerSide.hpp | 3 + src/mc/sosp/RemoteProcessMemory.cpp | 130 -------------------------- src/mc/sosp/RemoteProcessMemory.hpp | 2 - 4 files changed, 137 insertions(+), 134 deletions(-) diff --git a/src/mc/remote/CheckerSide.cpp b/src/mc/remote/CheckerSide.cpp index b455df1546..9d6df4dd8f 100644 --- a/src/mc/remote/CheckerSide.cpp +++ b/src/mc/remote/CheckerSide.cpp @@ -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 +#include #include XBT_LOG_NEW_DEFAULT_SUBCATEGORY(mc_checkerside, mc, "MC communication with the application"); @@ -31,7 +35,7 @@ CheckerSide::CheckerSide(int sockfd, std::unique_ptr 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 memory auto checker = static_cast(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(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 diff --git a/src/mc/remote/CheckerSide.hpp b/src/mc/remote/CheckerSide.hpp index 0c3294885c..8f99b4844b 100644 --- a/src/mc/remote/CheckerSide.hpp +++ b/src/mc/remote/CheckerSide.hpp @@ -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 diff --git a/src/mc/sosp/RemoteProcessMemory.cpp b/src/mc/sosp/RemoteProcessMemory.cpp index 0bfb964fea..0051ddfa42 100644 --- a/src/mc/sosp/RemoteProcessMemory.cpp +++ b/src/mc/sosp/RemoteProcessMemory.cpp @@ -25,8 +25,6 @@ #include #include #include -#include -#include 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(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 diff --git a/src/mc/sosp/RemoteProcessMemory.hpp b/src/mc/sosp/RemoteProcessMemory.hpp index b0eaeb640b..40aef1849a 100644 --- a/src/mc/sosp/RemoteProcessMemory.hpp +++ b/src/mc/sosp/RemoteProcessMemory.hpp @@ -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 */ -- 2.20.1