From 901f09137597064bab9349e26b7d882181ee3bdd Mon Sep 17 00:00:00 2001 From: Gabriel Corona Date: Tue, 23 Feb 2016 11:03:32 +0100 Subject: [PATCH] [mc] Partial cleanup of the initialisation of the different MC objects namely Process and ModelChecker. This fixes the testall test. --- src/mc/ModelChecker.cpp | 28 +++---- src/mc/ModelChecker.hpp | 5 +- src/mc/Process.cpp | 78 +++++++++---------- src/mc/Process.hpp | 23 +++--- src/mc/mc_snapshot.cpp | 6 +- src/mc/simgrid_mc.cpp | 4 +- teshsuite/mc/dwarf/dwarf.cpp | 38 ++++----- .../mc/dwarf_expression/dwarf_expression.cpp | 1 + 8 files changed, 88 insertions(+), 95 deletions(-) diff --git a/src/mc/ModelChecker.cpp b/src/mc/ModelChecker.cpp index 4411458d65..ca9a1b2ae4 100644 --- a/src/mc/ModelChecker.cpp +++ b/src/mc/ModelChecker.cpp @@ -48,8 +48,8 @@ using simgrid::mc::remote; namespace simgrid { namespace mc { -ModelChecker::ModelChecker(pid_t pid, int socket) : - pid_(pid), socket_(socket), +ModelChecker::ModelChecker(std::unique_ptr process) : + process_(std::move(process)), hostnames_(xbt_dict_new()), page_store_(500), parent_snapshot_(nullptr) @@ -74,16 +74,10 @@ const char* ModelChecker::get_host_name(const char* hostname) return elt->key; } -// HACK, for the unit test only -void ModelChecker::init_process() -{ - // TODO, avoid direct dependency on sg_cfg - process_ = std::unique_ptr(new Process(pid_, socket_)); - process_->privatized(sg_cfg_get_boolean("smpi/privatize_global_variables")); -} - void ModelChecker::start() { + const pid_t pid = process_->pid(); + // Block SIGCHLD (this will be handled with accept/signalfd): sigset_t set; sigemptyset(&set); @@ -97,7 +91,7 @@ void ModelChecker::start() // Prepare data for poll: struct pollfd* socket_pollfd = &fds_[SOCKET_FD_INDEX]; - socket_pollfd->fd = socket_; + socket_pollfd->fd = process_->socket();; socket_pollfd->events = POLLIN; socket_pollfd->revents = 0; @@ -114,11 +108,11 @@ void ModelChecker::start() int status; // The model-checked process SIGSTOP itself to signal it's ready: - pid_t res = waitpid(pid_, &status, __WALL); + pid_t res = waitpid(pid, &status, __WALL); if (res < 0 || !WIFSTOPPED(status) || WSTOPSIG(status) != SIGSTOP) xbt_die("Could not wait model-checked process"); - this->init_process(); + process_->init(); /* Initialize statistics */ mc_stats = xbt_new0(s_mc_stats_t, 1); @@ -132,8 +126,8 @@ void ModelChecker::start() setup_ignore(); - ptrace(PTRACE_SETOPTIONS, pid_, nullptr, PTRACE_O_TRACEEXIT); - ptrace(PTRACE_CONT, pid_, 0, 0); + ptrace(PTRACE_SETOPTIONS, pid, nullptr, PTRACE_O_TRACEEXIT); + ptrace(PTRACE_CONT, pid, 0, 0); } static const std::pair ignored_local_variables[] = { @@ -386,7 +380,7 @@ void ModelChecker::handle_waitpid() // From PTRACE_O_TRACEEXIT: if (status>>8 == (SIGTRAP | (PTRACE_EVENT_EXIT<<8))) { - if (ptrace(PTRACE_GETEVENTMSG, pid_, 0, &status) == -1) + if (ptrace(PTRACE_GETEVENTMSG, this->process().pid(), 0, &status) == -1) xbt_die("Could not get exit status"); if (WIFSIGNALED(status)) { MC_report_crash(status); @@ -397,7 +391,7 @@ void ModelChecker::handle_waitpid() // We don't care about signals, just reinject them: if (WIFSTOPPED(status)) { XBT_DEBUG("Stopped with signal %i", (int) WSTOPSIG(status)); - if (ptrace(PTRACE_CONT, pid_, 0, WSTOPSIG(status)) == -1) + if (ptrace(PTRACE_CONT, this->process().pid(), 0, WSTOPSIG(status)) == -1) xbt_die("Could not PTRACE_CONT"); } diff --git a/src/mc/ModelChecker.hpp b/src/mc/ModelChecker.hpp index a90e1b4137..7f770c3c82 100644 --- a/src/mc/ModelChecker.hpp +++ b/src/mc/ModelChecker.hpp @@ -28,8 +28,6 @@ namespace mc { /** State of the model-checker (global variables for the model checker) */ class ModelChecker { - pid_t pid_; - int socket_; struct pollfd fds_[2]; /** String pool for host names */ // TODO, use std::unordered_set with heterogeneous comparison lookup (C++14) @@ -43,7 +41,7 @@ public: public: ModelChecker(ModelChecker const&) = delete; ModelChecker& operator=(ModelChecker const&) = delete; - ModelChecker(pid_t pid, int socket); + ModelChecker(std::unique_ptr process); ~ModelChecker(); Process& process() @@ -62,7 +60,6 @@ public: } void start(); - void init_process(); void shutdown(); void resume(simgrid::mc::Process& process); void loop(); diff --git a/src/mc/Process.cpp b/src/mc/Process.cpp index 3ffcd23653..7b5a0178cf 100644 --- a/src/mc/Process.cpp +++ b/src/mc/Process.cpp @@ -206,76 +206,68 @@ int open_vm(pid_t pid, int flags) namespace simgrid { namespace mc { -Process::Process(pid_t pid, int sockfd) : AddressSpace(this) +Process::Process(pid_t pid, int sockfd) : + AddressSpace(this), socket_(sockfd), pid_(pid), running_(true) +{} + +void Process::init() { - Process* process = this; - process->socket_ = sockfd; - process->pid_ = pid; - process->running_ = true; - process->memory_map_ = simgrid::xbt::get_memory_map(pid); - process->cache_flags = MC_PROCESS_CACHE_FLAG_NONE; - process->init_memory_map_info(); - process->clear_refs_fd_ = -1; - process->pagemap_fd_ = -1; - process->privatized_ = false; - - int fd = open_vm(process->pid_, O_RDWR); + this->memory_map_ = simgrid::xbt::get_memory_map(this->pid_); + this->init_memory_map_info(); + + int fd = open_vm(this->pid_, O_RDWR); if (fd<0) xbt_die("Could not open file for process virtual address space"); - process->memory_file = fd; + this->memory_file = fd; // Read std_heap (is a struct mdesc*): - simgrid::mc::Variable* std_heap_var = process->find_variable("__mmalloc_default_mdp"); + simgrid::mc::Variable* std_heap_var = this->find_variable("__mmalloc_default_mdp"); if (!std_heap_var) xbt_die("No heap information in the target process"); if(!std_heap_var->address) xbt_die("No constant address for this variable"); - process->read_bytes(&process->heap_address, sizeof(struct mdesc*), + this->read_bytes(&this->heap_address, sizeof(struct mdesc*), remote(std_heap_var->address), simgrid::mc::ProcessIndexDisabled); - process->smx_process_infos = MC_smx_process_info_list_new(); - process->smx_old_process_infos = MC_smx_process_info_list_new(); - process->unw_addr_space = unw_create_addr_space(&mc_unw_accessors , __BYTE_ORDER); - process->unw_underlying_addr_space = unw_create_addr_space(&mc_unw_vmread_accessors, __BYTE_ORDER); - process->unw_underlying_context = _UPT_create(pid); + this->smx_process_infos = MC_smx_process_info_list_new(); + this->smx_old_process_infos = MC_smx_process_info_list_new(); + this->unw_addr_space = unw_create_addr_space(&mc_unw_accessors , __BYTE_ORDER); + this->unw_underlying_addr_space = unw_create_addr_space(&mc_unw_vmread_accessors, __BYTE_ORDER); + this->unw_underlying_context = _UPT_create(this->pid_); } Process::~Process() { - Process* process = this; - if (this->socket_ >= 0 && close(this->socket_) < 0) xbt_die("Could not close communication socket"); - process->pid_ = 0; - - process->maestro_stack_start_ = nullptr; - process->maestro_stack_end_ = nullptr; + this->maestro_stack_start_ = nullptr; + this->maestro_stack_end_ = nullptr; - xbt_dynar_free(&process->smx_process_infos); - xbt_dynar_free(&process->smx_old_process_infos); + xbt_dynar_free(&this->smx_process_infos); + xbt_dynar_free(&this->smx_old_process_infos); - if (process->memory_file >= 0) { - close(process->memory_file); + if (this->memory_file >= 0) { + close(this->memory_file); } - if (process->unw_underlying_addr_space != unw_local_addr_space) { - unw_destroy_addr_space(process->unw_underlying_addr_space); - _UPT_destroy(process->unw_underlying_context); + if (this->unw_underlying_addr_space != unw_local_addr_space) { + unw_destroy_addr_space(this->unw_underlying_addr_space); + _UPT_destroy(this->unw_underlying_context); } - process->unw_underlying_context = NULL; - process->unw_underlying_addr_space = NULL; + this->unw_underlying_context = NULL; + this->unw_underlying_addr_space = NULL; - unw_destroy_addr_space(process->unw_addr_space); - process->unw_addr_space = NULL; + unw_destroy_addr_space(this->unw_addr_space); + this->unw_addr_space = NULL; - process->cache_flags = MC_PROCESS_CACHE_FLAG_NONE; + this->cache_flags = MC_PROCESS_CACHE_FLAG_NONE; - if (process->clear_refs_fd_ >= 0) - close(process->clear_refs_fd_); - if (process->pagemap_fd_ >= 0) - close(process->pagemap_fd_); + if (this->clear_refs_fd_ >= 0) + close(this->clear_refs_fd_); + if (this->pagemap_fd_ >= 0) + close(this->pagemap_fd_); } /** Refresh the information about the process diff --git a/src/mc/Process.hpp b/src/mc/Process.hpp index 8780e3cc5a..6ecad0e666 100644 --- a/src/mc/Process.hpp +++ b/src/mc/Process.hpp @@ -66,6 +66,7 @@ class Process final : public AddressSpace { public: Process(pid_t pid, int sockfd); ~Process(); + void init(); Process(Process const&) = delete; Process(Process &&) = delete; @@ -191,22 +192,24 @@ public: void unignore_heap(void *address, size_t size); void ignore_local_variable(const char *var_name, const char *frame_name); + int socket() { return socket_; } private: void init_memory_map_info(); void refresh_heap(); void refresh_malloc_info(); + private: - pid_t pid_; - int socket_; - bool running_; + pid_t pid_ = -1; + int socket_ = -1; + bool running_ = false; std::vector memory_map_; remote_ptr maestro_stack_start_, maestro_stack_end_; - int memory_file; + int memory_file = -1; std::vector ignored_regions_; - int clear_refs_fd_; - int pagemap_fd_; - bool privatized_; + int clear_refs_fd_ = -1; + int pagemap_fd_ = -1; + bool privatized_ = false; std::vector stack_areas_; std::vector ignored_heap_; @@ -221,16 +224,16 @@ public: // Copies of MCed SMX data structures * * See mc_smx.c. */ - xbt_dynar_t smx_process_infos; + xbt_dynar_t smx_process_infos = nullptr; /** Copy of `simix_global->process_to_destroy` * * See mc_smx.c. */ - xbt_dynar_t smx_old_process_infos; + xbt_dynar_t smx_old_process_infos = nullptr; /** State of the cache (which variables are up to date) */ - mc_process_cache_flags_t cache_flags; + mc_process_cache_flags_t cache_flags = MC_PROCESS_CACHE_FLAG_NONE; /** Address of the heap structure in the MCed process. */ void* heap_address; diff --git a/src/mc/mc_snapshot.cpp b/src/mc/mc_snapshot.cpp index ec1bd479ba..fdcae06bcb 100644 --- a/src/mc/mc_snapshot.cpp +++ b/src/mc/mc_snapshot.cpp @@ -232,8 +232,10 @@ static void test_snapshot(bool sparse_checkpoint) { _sg_mc_sparse_checkpoint = sparse_checkpoint; xbt_assert(xbt_pagesize == getpagesize()); xbt_assert(1 << xbt_pagebits == xbt_pagesize); - mc_model_checker = new ::simgrid::mc::ModelChecker(getpid(), -1); - mc_model_checker->init_process(); + + std::unique_ptr process(new simgrid::mc::Process(getpid(), -1)); + process->init(); + mc_model_checker = new ::simgrid::mc::ModelChecker(std::move(process)); for(int n=1; n!=256; ++n) { diff --git a/src/mc/simgrid_mc.cpp b/src/mc/simgrid_mc.cpp index 4b307b34b2..e0f61709de 100644 --- a/src/mc/simgrid_mc.cpp +++ b/src/mc/simgrid_mc.cpp @@ -99,7 +99,9 @@ static int do_parent(int socket, pid_t child) xbt_die("MC server already present"); try { mc_mode = MC_MODE_SERVER; - mc_model_checker = new simgrid::mc::ModelChecker(child, socket); + std::unique_ptr process(new simgrid::mc::Process(child, socket)); + process->privatized(sg_cfg_get_boolean("smpi/privatize_global_variables")); + mc_model_checker = new simgrid::mc::ModelChecker(std::move(process)); mc_model_checker->start(); int res = 0; if (_sg_mc_comms_determinism || _sg_mc_send_determinism) diff --git a/teshsuite/mc/dwarf/dwarf.cpp b/teshsuite/mc/dwarf/dwarf.cpp index 91df00c073..7e1a2e519d 100644 --- a/teshsuite/mc/dwarf/dwarf.cpp +++ b/teshsuite/mc/dwarf/dwarf.cpp @@ -80,8 +80,10 @@ static void test_local_variable(simgrid::mc::ObjectInformation* info, const char } -static simgrid::mc::Variable* test_global_variable(simgrid::mc::Process* process, simgrid::mc::ObjectInformation* info, const char* name, void* address, long byte_size) { - +static simgrid::mc::Variable* test_global_variable( + simgrid::mc::Process& process, simgrid::mc::ObjectInformation* info, + const char* name, void* address, long byte_size) +{ simgrid::mc::Variable* variable = info->find_variable(name); xbt_assert(variable, "Global variable %s was not found", name); xbt_assert(variable->name == name, @@ -91,8 +93,8 @@ static simgrid::mc::Variable* test_global_variable(simgrid::mc::Process* process "Address mismatch for %s : %p expected but %p found", name, address, variable->address); - auto i = process->binary_info->types.find(variable->type_id); - xbt_assert(i != process->binary_info->types.end(), "Missing type for %s", name); + auto i = process.binary_info->types.find(variable->type_id); + xbt_assert(i != process.binary_info->types.end(), "Missing type for %s", name); simgrid::mc::Type* type = &i->second; xbt_assert(type->byte_size = byte_size, "Byte size mismatch for %s", name); return variable; @@ -110,11 +112,11 @@ int some_local_variable = 0; typedef struct foo {int i;} s_foo; -static void test_type_by_name(simgrid::mc::Process* process, s_foo my_foo) +static void test_type_by_name(simgrid::mc::Process& process, s_foo my_foo) { assert( - process->binary_info->full_types_by_name.find("struct foo") != - process->binary_info->full_types_by_name.end()); + process.binary_info->full_types_by_name.find("struct foo") != + process.binary_info->full_types_by_name.end()); } int main(int argc, char** argv) @@ -124,25 +126,25 @@ int main(int argc, char** argv) simgrid::mc::Variable* var; simgrid::mc::Type* type; - simgrid::mc::Process p(getpid(), -1); - simgrid::mc::Process* process = &p; + simgrid::mc::Process process(getpid(), -1); + process.init(); - test_global_variable(process, process->binary_info.get(), + test_global_variable(process, process.binary_info.get(), "some_local_variable", &some_local_variable, sizeof(int)); - var = test_global_variable(process, process->binary_info.get(), + var = test_global_variable(process, process.binary_info.get(), "test_some_array", &test_some_array, sizeof(test_some_array)); - auto i = process->binary_info->types.find(var->type_id); - xbt_assert(i != process->binary_info->types.end(), "Missing type"); + auto i = process.binary_info->types.find(var->type_id); + xbt_assert(i != process.binary_info->types.end(), "Missing type"); type = &i->second; xbt_assert(type->element_count == 6*5*4, "element_count mismatch in test_some_array : %i / %i", type->element_count, 6*5*4); - var = test_global_variable(process, process->binary_info.get(), + var = test_global_variable(process, process.binary_info.get(), "test_some_struct", &test_some_struct, sizeof(test_some_struct)); - i = process->binary_info->types.find(var->type_id); - xbt_assert(i != process->binary_info->types.end(), "Missing type"); + i = process.binary_info->types.find(var->type_id); + xbt_assert(i != process.binary_info->types.end(), "Missing type"); type = &i->second; assert(type); @@ -155,11 +157,11 @@ int main(int argc, char** argv) unw_getcontext(&context); unw_init_local(&cursor, &context); - test_local_variable(process->binary_info.get(), "main", "argc", &argc, &cursor); + test_local_variable(process.binary_info.get(), "main", "argc", &argc, &cursor); { int lexical_block_variable = 50; - test_local_variable(process->binary_info.get(), "main", + test_local_variable(process.binary_info.get(), "main", "lexical_block_variable", &lexical_block_variable, &cursor); } diff --git a/teshsuite/mc/dwarf_expression/dwarf_expression.cpp b/teshsuite/mc/dwarf_expression/dwarf_expression.cpp index 4a1b3428e4..fbe890e8ae 100644 --- a/teshsuite/mc/dwarf_expression/dwarf_expression.cpp +++ b/teshsuite/mc/dwarf_expression/dwarf_expression.cpp @@ -151,6 +151,7 @@ void test_deref(simgrid::dwarf::ExpressionContext const& state) { int main(int argc, char** argv) { process = new simgrid::mc::Process(getpid(), -1); + process->init(); simgrid::dwarf::ExpressionContext state; state.address_space = (simgrid::mc::AddressSpace*) process; -- 2.20.1