From 1679193978528bbb628e0f3a7853cddf4f3cfe0d Mon Sep 17 00:00:00 2001 From: Gabriel Corona Date: Mon, 23 May 2016 14:17:27 +0200 Subject: [PATCH] [mc] Fix remote access to the new Process structure --- include/xbt/string.hpp | 89 ++++++++++++---------- src/mc/AddressSpace.hpp | 59 ++++++++------ src/mc/CommunicationDeterminismChecker.cpp | 15 ++-- src/mc/LivenessChecker.cpp | 4 +- src/mc/Process.cpp | 3 +- src/mc/Process.hpp | 20 ++--- src/mc/SafetyChecker.cpp | 8 +- src/mc/mc_base.cpp | 4 +- src/mc/mc_checkpoint.cpp | 2 +- src/mc/mc_global.cpp | 2 +- src/mc/mc_request.cpp | 10 +-- src/mc/mc_smx.cpp | 25 +++--- src/mc/mc_state.cpp | 6 +- src/simix/smx_process.cpp | 2 +- src/simix/smx_process_private.h | 3 +- 15 files changed, 139 insertions(+), 113 deletions(-) diff --git a/include/xbt/string.hpp b/include/xbt/string.hpp index 1a2619a462..6bcf4c72ea 100644 --- a/include/xbt/string.hpp +++ b/include/xbt/string.hpp @@ -124,12 +124,12 @@ public: assign(s, s == nullptr ? 0 : std::strlen(s)); return *this; } - string& operator=(string& s) + string& operator=(string const& s) { assign(s.c_str(), s.size()); return *this; } - string& operator=(std::string& s) + string& operator=(std::string const& s) { assign(s.c_str(), s.size()); return *this; @@ -188,71 +188,78 @@ public: string_data::data = (char*) &NUL; } - // Compare - int compare(string const& that) const + bool equals(const char* data, std::size_t len) const { - size_t n = std::min(this->size(), that.size()); - int res = memcmp(this->c_str(), that.c_str(), n); - if (res != 0) - return res; - else if (this->size() == that.size()) - return 0; - else if (this->size() < that.size()) - return -1; - else - return 1; + return this->size() == len + && std::memcmp(this->c_str(), data, len) == 0; } + bool operator==(string const& that) const { - return this->size() == that.size() - && std::memcmp(this->c_str(), that.c_str(), this->size()) == 0; + return this->equals(that.c_str(), that.size()); } - bool operator!=(string const& that) const + bool operator==(std::string const& that) const { - return !(*this == that); + return this->equals(that.c_str(), that.size()); } - bool operator<(string const& that) const + bool operator==(const char* that) const { - return compare(that) < 0; + return this->equals(that, std::strlen(that)); } - bool operator<=(string const& that) const + + template + bool operator!=(X const& that) const { - return compare(that) <= 0; + return !((*this) == that); } - bool operator>(string const& that) const + + // Compare: + int compare(const char* data, std::size_t len) const { - return compare(that) > 0; + size_t n = std::min(this->size(), len); + int res = memcmp(this->c_str(), data, n); + if (res != 0) + return res; + else if (this->size() == len) + return 0; + else if (this->size() < len) + return -1; + else + return 1; } - bool operator>=(string const& that) const + int compare(string const& that) const { - return compare(that) >= 0; + return this->compare(that.c_str(), that.size()); } - - // Compare with std::string - bool operator==(std::string const& that) const + int compare(std::string const& that) const { - return this->size() == that.size() - && std::memcmp(this->c_str(), that.c_str(), this->size()) == 0; + return this->compare(that.c_str(), that.size()); } - bool operator!=(std::string const& that) const + int compare(const char* that) const { - return !(*this == that); + return this->compare(that, std::strlen(that)); } - bool operator<(std::string const& that) const + + // Define < <= >= > in term of compare(): + template + bool operator<(X const& that) const { - return compare(that) < 0; + return this->compare(that) < 0; } - bool operator<=(std::string const& that) const + template + bool operator<=(X const& that) const { - return compare(that) <= 0; + return this->compare(that) <= 0; } - bool operator>(std::string const& that) const + template + bool operator>(X const& that) const { - return compare(that) > 0; + return this->compare(that) > 0; } - bool operator>=(std::string const& that) const + template + bool operator>=(X const& that) const { - return compare(that) >= 0; + return this->compare(that) >= 0; } }; diff --git a/src/mc/AddressSpace.hpp b/src/mc/AddressSpace.hpp index 18969aaa0a..02db2c4b86 100644 --- a/src/mc/AddressSpace.hpp +++ b/src/mc/AddressSpace.hpp @@ -13,6 +13,9 @@ #include #include +#include +#include + #include "src/mc/mc_forward.hpp" #include "src/mc/RemotePtr.hpp" @@ -92,7 +95,7 @@ public: static constexpr ReadOptions lazy() { return ReadOptions(1); } }; -/** A value from another process +/** HACK, A value from another process * * This represents a value from another process: * @@ -109,29 +112,31 @@ public: * cross-architecture (such as 32-bit/64-bit access). */ template -class Remote { +union Remote { private: - // If we use a union, it won't work with abstract types: - char buffer[sizeof(T)]; + T buffer; public: - // HACK, some code currently cast this to T* which is **not** legal. - void* data() { return buffer; } - const void* data() const { return buffer; } - constexpr std::size_t size() const { return sizeof(T); } - operator T() const { - static_assert(std::is_trivial::value, "Cannot convert non trivial type"); - T res; - std::memcpy(&res, buffer, sizeof(T)); - return res; - } Remote() {} - Remote(T const& x) + ~Remote() {} + Remote(Remote const& that) { - std::memcpy(&x, buffer, sizeof(T)); + std::memcpy(&buffer, &that.buffer, sizeof(buffer)); } - Remote& operator=(T const& x) + Remote& operator=(Remote const& that) { - std::memcpy(&x, buffer, sizeof(T)); + std::memcpy(&buffer, &that.buffer, sizeof(buffer)); + return *this; + } + T* getBuffer() { return &buffer; } + const T* getBuffer() const { return &buffer; } + std::size_t getBufferSize() const { return sizeof(T); } + operator T() const { + static_assert(std::is_trivial::value, "Cannot convert non trivial type"); + return buffer; + } + void clear() + { + std::memset(static_cast(&buffer), 0, sizeof(T)); } }; @@ -166,25 +171,35 @@ public: /** Read a given data structure from the address space */ template inline - void read(T *buffer, RemotePtr ptr, int process_index = ProcessIndexAny) + void read(T *buffer, RemotePtr ptr, int process_index = ProcessIndexAny) const { this->read_bytes(buffer, sizeof(T), ptr, process_index); } template inline - void read(Remote& buffer, RemotePtr ptr, int process_index = ProcessIndexAny) + void read(Remote& buffer, RemotePtr ptr, int process_index = ProcessIndexAny) const { - this->read_bytes(buffer.data(), sizeof(T), ptr, process_index); + this->read_bytes(buffer.getBuffer(), sizeof(T), ptr, process_index); } /** Read a given data structure from the address space */ template inline - Remote read(RemotePtr ptr, int process_index = ProcessIndexMissing) + Remote read(RemotePtr ptr, int process_index = ProcessIndexMissing) const { Remote res; this->read_bytes(&res, sizeof(T), ptr, process_index); return res; } + + std::string read_string(RemotePtr address, std::size_t len) const + { + // TODO, use std::vector with .data() in C++17 to avoid useless copies + std::vector buffer(len); + buffer[len] = '\0'; + this->read_bytes(buffer.data(), len, address); + return std::string(buffer.data(), buffer.size()); + } + }; } diff --git a/src/mc/CommunicationDeterminismChecker.cpp b/src/mc/CommunicationDeterminismChecker.cpp index dc3ac5f7ad..87088ed178 100644 --- a/src/mc/CommunicationDeterminismChecker.cpp +++ b/src/mc/CommunicationDeterminismChecker.cpp @@ -101,7 +101,7 @@ static void update_comm_pattern( simgrid::mc::Remote temp_comm; mc_model_checker->process().read(temp_comm, comm_addr); simgrid::simix::Comm* comm = - static_cast(temp_comm.data()); + static_cast(temp_comm.getBuffer()); smx_process_t src_proc = mc_model_checker->process().resolveProcess( simgrid::mc::remote(comm->src_proc)); @@ -199,7 +199,7 @@ void CommunicationDeterminismChecker::get_comm_pattern(xbt_dynar_t list, smx_sim mc_model_checker->process().read(temp_synchro, remote( static_cast(pattern->comm_addr))); simgrid::simix::Comm* synchro = - static_cast(temp_synchro.data()); + static_cast(temp_synchro.getBuffer()); char* remote_name = mc_model_checker->process().read( (std::uint64_t)(synchro->mbox ? &synchro->mbox->name : &synchro->mbox_cpy->name)); @@ -247,8 +247,7 @@ void CommunicationDeterminismChecker::get_comm_pattern(xbt_dynar_t list, smx_sim simgrid::mc::Remote temp_comm; mc_model_checker->process().read(temp_comm, remote( static_cast(pattern->comm_addr))); - simgrid::simix::Comm* comm = - static_cast(temp_comm.data()); + simgrid::simix::Comm* comm = temp_comm.getBuffer(); char* remote_name; mc_model_checker->process().read(&remote_name, @@ -393,8 +392,8 @@ void CommunicationDeterminismChecker::prepare() /* Get an enabled process and insert it in the interleave set of the initial state */ for (auto& p : mc_model_checker->process().simix_processes()) - if (simgrid::mc::process_is_enabled(&p.copy)) - initial_state->interleave(&p.copy); + if (simgrid::mc::process_is_enabled(p.copy.getBuffer())) + initial_state->interleave(p.copy.getBuffer()); stack_.push_back(std::move(initial_state)); } @@ -532,8 +531,8 @@ int CommunicationDeterminismChecker::main(void) /* Get enabled processes and insert them in the interleave set of the next state */ for (auto& p : mc_model_checker->process().simix_processes()) - if (simgrid::mc::process_is_enabled(&p.copy)) - next_state->interleave(&p.copy); + if (simgrid::mc::process_is_enabled(p.copy.getBuffer())) + next_state->interleave(p.copy.getBuffer()); if (dot_output != nullptr) fprintf(dot_output, "\"%d\" -> \"%d\" [%s];\n", diff --git a/src/mc/LivenessChecker.cpp b/src/mc/LivenessChecker.cpp index 8dae669664..3853e2b4f9 100644 --- a/src/mc/LivenessChecker.cpp +++ b/src/mc/LivenessChecker.cpp @@ -445,8 +445,8 @@ std::shared_ptr LivenessChecker::newPair(Pair* current_pair, xbt_automaton next_pair->depth = 1; /* Get enabled processes and insert them in the interleave set of the next graph_state */ for (auto& p : mc_model_checker->process().simix_processes()) - if (simgrid::mc::process_is_enabled(&p.copy)) - next_pair->graph_state->interleave(&p.copy); + if (simgrid::mc::process_is_enabled(p.copy.getBuffer())) + next_pair->graph_state->interleave(p.copy.getBuffer()); next_pair->requests = next_pair->graph_state->interleaveSize(); /* FIXME : get search_cycle value for each acceptant state */ if (next_pair->automaton_state->type == 1 || diff --git a/src/mc/Process.cpp b/src/mc/Process.cpp index 597710f838..0f4a0b0d87 100644 --- a/src/mc/Process.cpp +++ b/src/mc/Process.cpp @@ -425,11 +425,12 @@ void Process::read_variable(const char* name, void* target, size_t size) const this->read_bytes(target, size, remote(var->address)); } -std::string Process::read_string(RemotePtr address) const +std::string Process::read_string(RemotePtr address) const { if (!address) return {}; + // TODO, use std::vector with .data() in C++17 to avoid useless copies std::vector res(128); off_t off = 0; diff --git a/src/mc/Process.hpp b/src/mc/Process.hpp index 8b96b0cbbc..d5ff78233f 100644 --- a/src/mc/Process.hpp +++ b/src/mc/Process.hpp @@ -45,11 +45,9 @@ namespace mc { class SimixProcessInformation { public: /** MCed address of the process */ - RemotePtr address = nullptr; - union { - /** (Flat) Copy of the process data structure */ - struct s_smx_process copy; - }; + RemotePtr address = nullptr; + Remote copy; + /** Hostname (owned by `mc_modelchecker->hostnames`) */ const char* hostname = nullptr; std::string name; @@ -120,7 +118,11 @@ public: read_variable(name, &res, sizeof(T)); return res; } - std::string read_string(RemotePtr address) const; + std::string read_string(RemotePtr address) const; + std::string read_string(RemotePtr address, std::size_t len) const + { + return AddressSpace::read_string(address, len); + } // Write memory: void write_bytes(const void* buffer, size_t len, RemotePtr address); @@ -217,7 +219,7 @@ public: /** Get a local description of a remote SIMIX process */ simgrid::mc::SimixProcessInformation* resolveProcessInfo( - simgrid::mc::RemotePtr process) + simgrid::mc::RemotePtr process) { xbt_assert(mc_model_checker != nullptr); if (!process) @@ -233,12 +235,12 @@ public: } /** Get a local copy of the SIMIX process structure */ - smx_process_t resolveProcess(simgrid::mc::RemotePtr process) + simgrid::simix::Process* resolveProcess(simgrid::mc::RemotePtr process) { simgrid::mc::SimixProcessInformation* process_info = this->resolveProcessInfo(process); if (process_info) - return &process_info->copy; + return process_info->copy.getBuffer(); else return nullptr; } diff --git a/src/mc/SafetyChecker.cpp b/src/mc/SafetyChecker.cpp index a92efbe70b..285330b32b 100644 --- a/src/mc/SafetyChecker.cpp +++ b/src/mc/SafetyChecker.cpp @@ -154,8 +154,8 @@ int SafetyChecker::run() /* Get an enabled process and insert it in the interleave set of the next state */ for (auto& p : mc_model_checker->process().simix_processes()) - if (simgrid::mc::process_is_enabled(&p.copy)) { - next_state->interleave(&p.copy); + if (simgrid::mc::process_is_enabled(p.copy.getBuffer())) { + next_state->interleave(p.copy.getBuffer()); if (reductionMode_ != simgrid::mc::ReductionMode::none) break; } @@ -329,8 +329,8 @@ void SafetyChecker::init() /* Get an enabled process and insert it in the interleave set of the initial state */ for (auto& p : mc_model_checker->process().simix_processes()) - if (simgrid::mc::process_is_enabled(&p.copy)) { - initial_state->interleave(&p.copy); + if (simgrid::mc::process_is_enabled(p.copy.getBuffer())) { + initial_state->interleave(p.copy.getBuffer()); if (reductionMode_ != simgrid::mc::ReductionMode::none) break; } diff --git a/src/mc/mc_base.cpp b/src/mc/mc_base.cpp index df6234ac63..708f3c5d4c 100644 --- a/src/mc/mc_base.cpp +++ b/src/mc/mc_base.cpp @@ -96,7 +96,7 @@ bool request_is_enabled(smx_simcall_t req) simgrid::mc::Remote temp_comm; if (mc_model_checker != nullptr) { mc_model_checker->process().read(temp_comm, remote(act)); - act = static_cast(temp_comm.data()); + act = static_cast(temp_comm.getBuffer()); } #endif @@ -147,7 +147,7 @@ bool request_is_enabled(smx_simcall_t req) if (mc_model_checker != nullptr) { memcpy(&act, buffer + comms->elmsize * index, sizeof(act)); mc_model_checker->process().read(temp_comm, remote(act)); - act = static_cast(temp_comm.data()); + act = static_cast(temp_comm.getBuffer()); } else #endif diff --git a/src/mc/mc_checkpoint.cpp b/src/mc/mc_checkpoint.cpp index 20d72488fe..20754d3f1e 100644 --- a/src/mc/mc_checkpoint.cpp +++ b/src/mc/mc_checkpoint.cpp @@ -569,7 +569,7 @@ std::shared_ptr take_snapshot(int num_state) snapshot->num_state = num_state; for (auto& p : mc_model_checker->process().simix_processes()) - snapshot->enabled_processes.insert(p.copy.pid); + snapshot->enabled_processes.insert(p.copy.getBuffer()->pid); snapshot_handle_ignore(snapshot.get()); diff --git a/src/mc/mc_global.cpp b/src/mc/mc_global.cpp index 92d6e70087..d6d0b5102d 100644 --- a/src/mc/mc_global.cpp +++ b/src/mc/mc_global.cpp @@ -167,7 +167,7 @@ static void MC_dump_stacks(FILE* file) simgrid::mc::UnwindContext context; unw_context_t raw_context = - mc_model_checker->process().read( + (unw_context_t) mc_model_checker->process().read( simgrid::mc::remote((unw_context_t *)stack.context)); context.initialize(&mc_model_checker->process(), &raw_context); diff --git a/src/mc/mc_request.cpp b/src/mc/mc_request.cpp index 1d377795b1..962e954162 100644 --- a/src/mc/mc_request.cpp +++ b/src/mc/mc_request.cpp @@ -291,7 +291,7 @@ std::string simgrid::mc::request_to_string(smx_simcall_t req, int value, simgrid if (use_remote_comm) { mc_model_checker->process().read(temp_synchro, remote( static_cast(remote_act))); - act = static_cast(temp_synchro.data()); + act = temp_synchro.getBuffer(); } else act = remote_act; @@ -319,7 +319,7 @@ std::string simgrid::mc::request_to_string(smx_simcall_t req, int value, simgrid if (use_remote_comm) { mc_model_checker->process().read(temp_synchro, remote( static_cast(remote_act))); - act = static_cast(temp_synchro.data()); + act = temp_synchro.getBuffer(); } else act = remote_act; @@ -466,7 +466,7 @@ bool request_is_enabled_by_idx(smx_simcall_t req, unsigned int idx) simgrid::mc::Remote temp_comm; mc_model_checker->process().read(temp_comm, remote( static_cast(remote_act))); - simgrid::simix::Comm* comm = static_cast(temp_comm.data()); + simgrid::simix::Comm* comm = temp_comm.getBuffer(); return comm->src_proc && comm->dst_proc; } @@ -531,7 +531,7 @@ std::string request_get_dot_output(smx_simcall_t req, int value) simgrid::mc::Remote temp_comm; mc_model_checker->process().read(temp_comm, remote( static_cast(remote_act))); - simgrid::simix::Comm* comm = static_cast(temp_comm.data()); + simgrid::simix::Comm* comm = temp_comm.getBuffer(); smx_process_t src_proc = mc_model_checker->process().resolveProcess( simgrid::mc::remote(comm->src_proc)); @@ -557,7 +557,7 @@ std::string request_get_dot_output(smx_simcall_t req, int value) simgrid::mc::Remote temp_comm; mc_model_checker->process().read(temp_comm, remote( static_cast(remote_act))); - simgrid::simix::Comm* comm = static_cast(temp_comm.data()); + simgrid::simix::Comm* comm = temp_comm.getBuffer(); if (comm->src_proc == nullptr || comm->dst_proc == nullptr) { if (issuer->host) label = simgrid::xbt::string_printf("[(%lu)%s] Test FALSE", diff --git a/src/mc/mc_smx.cpp b/src/mc/mc_smx.cpp index 8f821f4c2f..8ed02492e8 100644 --- a/src/mc/mc_smx.cpp +++ b/src/mc/mc_smx.cpp @@ -20,7 +20,7 @@ using simgrid::mc::remote; -/** Statically "upcast" a s_smx_process_t into a SimixProcessInformation +/** HACK, Statically "upcast" a s_smx_process_t into a SimixProcessInformation * * This gets 'processInfo' from '&processInfo->copy'. It upcasts in the * sense that we could achieve the same thing by having SimixProcessInformation @@ -29,9 +29,11 @@ using simgrid::mc::remote; static inline simgrid::mc::SimixProcessInformation* process_info_cast(smx_process_t p) { + simgrid::mc::SimixProcessInformation temp; + std::size_t offset = (char*) temp.copy.getBuffer() - (char*)&temp; + simgrid::mc::SimixProcessInformation* process_info = - (simgrid::mc::SimixProcessInformation*) - ((char*) p - offsetof(simgrid::mc::SimixProcessInformation, copy)); + (simgrid::mc::SimixProcessInformation*) ((char*) p - offset); return process_info; } @@ -116,10 +118,10 @@ smx_process_t MC_smx_simcall_get_issuer(s_smx_simcall_t const* req) // Lookup by address: for (auto& p : mc_model_checker->process().simix_processes()) if (p.address == address) - return &p.copy; + return p.copy.getBuffer(); for (auto& p : mc_model_checker->process().old_simix_processes()) if (p.address == address) - return &p.copy; + return p.copy.getBuffer(); xbt_die("Issuer not found"); } @@ -151,10 +153,9 @@ const char* MC_smx_process_get_host_name(smx_process_t p) // Read the simgrid::xbt::string in the MCed process: simgrid::mc::SimixProcessInformation* info = process_info_cast(p); - simgrid::xbt::string_data remote_string; auto remote_string_address = remote( (simgrid::xbt::string_data*) ((char*) p->host + offset)); - process->read_bytes(&remote_string, sizeof(remote_string), remote_string_address); + simgrid::xbt::string_data remote_string = process->read(remote_string_address); char hostname[remote_string.len]; process->read_bytes(hostname, remote_string.len + 1, remote(remote_string.data)); info->hostname = mc_model_checker->get_host_name(hostname); @@ -165,13 +166,13 @@ const char* MC_smx_process_get_name(smx_process_t p) { simgrid::mc::Process* process = &mc_model_checker->process(); if (mc_model_checker == nullptr) - return p->name; - if (!p->name) - return nullptr; + return p->name.c_str(); simgrid::mc::SimixProcessInformation* info = process_info_cast(p); - if (info->name.empty()) - info->name = process->read_string(p->name); + if (info->name.empty()) { + simgrid::xbt::string_data string_data = (simgrid::xbt::string_data&)p->name; + info->name = process->read_string(remote(string_data.data), string_data.len); + } return info->name.c_str(); } diff --git a/src/mc/mc_state.cpp b/src/mc/mc_state.cpp index 771b348f20..9df85bf8c6 100644 --- a/src/mc/mc_state.cpp +++ b/src/mc/mc_state.cpp @@ -49,7 +49,7 @@ namespace mc { State::State() { - std::memset(this->internal_comm.data(), 0, this->internal_comm.size()); + this->internal_comm.clear(); std::memset(&this->internal_req, 0, sizeof(this->internal_req)); std::memset(&this->executed_req, 0, sizeof(this->executed_req)); } @@ -130,7 +130,7 @@ static inline smx_simcall_t MC_state_get_request_for_process( static_cast(simcall_comm_wait__get__comm(&process->simcall))); simgrid::mc::Remote temp_act; mc_model_checker->process().read(temp_act, remote_act); - simgrid::simix::Comm* act = static_cast(temp_act.data()); + simgrid::simix::Comm* act = temp_act.getBuffer(); if (act->src_proc && act->dst_proc) state->transition.argument = 0; else if (act->src_proc == nullptr && act->type == SIMIX_COMM_READY @@ -230,7 +230,7 @@ static inline smx_simcall_t MC_state_get_request_for_process( smx_simcall_t MC_state_get_request(simgrid::mc::State* state) { for (auto& p : mc_model_checker->process().simix_processes()) { - smx_simcall_t res = MC_state_get_request_for_process(state, &p.copy); + smx_simcall_t res = MC_state_get_request_for_process(state, p.copy.getBuffer()); if (res) return res; } diff --git a/src/simix/smx_process.cpp b/src/simix/smx_process.cpp index 84cee3478a..d4242959fa 100644 --- a/src/simix/smx_process.cpp +++ b/src/simix/smx_process.cpp @@ -252,7 +252,7 @@ smx_process_t SIMIX_process_create( xbt_assert(code && host != NULL, "Invalid parameters"); /* Process data */ process->pid = simix_process_maxpid++; - process->name = std::string(name); + process->name = simgrid::xbt::string(name); process->host = host; process->data = data; process->comms = xbt_fifo_new(); diff --git a/src/simix/smx_process_private.h b/src/simix/smx_process_private.h index 22af111eca..7f6172dd80 100644 --- a/src/simix/smx_process_private.h +++ b/src/simix/smx_process_private.h @@ -11,6 +11,7 @@ #include #include +#include #include #include "simgrid/simix.h" @@ -46,7 +47,7 @@ public: unsigned long pid = 0; unsigned long ppid = 0; - std::string name; + simgrid::xbt::string name; sg_host_t host = nullptr; /* the host on which the process is running */ smx_context_t context = nullptr; /* the context (uctx/raw/thread) that executes the user function */ xbt_running_ctx_t *running_ctx = nullptr; -- 2.20.1