From 14dfe30555fbd9ca939cd9b8e256e2a25dc54c3d Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Sun, 31 Jul 2022 00:05:25 +0200 Subject: [PATCH] And now cleanup the App-side of cruft that was needed for Checker to read actor informations --- src/kernel/EngineImpl.cpp | 6 +----- src/kernel/EngineImpl.hpp | 15 --------------- src/mc/ModelChecker.cpp | 2 +- src/mc/mc_base.cpp | 9 --------- src/mc/remote/AppSide.cpp | 4 ++-- src/mc/remote/AppSide.hpp | 2 +- src/mc/remote/RemoteProcess.cpp | 3 +-- src/mc/remote/RemoteProcess.hpp | 9 ++------- src/mc/remote/mc_protocol.h | 1 - src/mc/sosp/Snapshot_test.cpp | 2 +- .../mc/dwarf-expression/dwarf-expression.cpp | 2 +- teshsuite/mc/dwarf/dwarf.cpp | 2 +- 12 files changed, 11 insertions(+), 46 deletions(-) diff --git a/src/kernel/EngineImpl.cpp b/src/kernel/EngineImpl.cpp index 00d0f16557..717e7710d3 100644 --- a/src/kernel/EngineImpl.cpp +++ b/src/kernel/EngineImpl.cpp @@ -178,10 +178,6 @@ EngineImpl::~EngineImpl() delete maestro_; delete context_factory_; - /* Free the remaining data structures */ -#if SIMGRID_HAVE_MC - xbt_dynar_free(&actors_vector_); -#endif /* clear models before freeing handle, network models can use external callback defined in the handle */ models_prio_.clear(); } @@ -195,7 +191,7 @@ void EngineImpl::initialize(int* argc, char** argv) // The communication initialization is done ASAP, as we need to get some init parameters from the MC for different // layers. But instance_ needs to be created, as we send the address of some of its fields to the MC that wants to // read them directly. - simgrid::mc::AppSide::initialize(actors_vector_); + simgrid::mc::AppSide::initialize(); #endif if (xbt_initialized == 0) { diff --git a/src/kernel/EngineImpl.hpp b/src/kernel/EngineImpl.hpp index 6ea9106128..d2d410897b 100644 --- a/src/kernel/EngineImpl.hpp +++ b/src/kernel/EngineImpl.hpp @@ -48,16 +48,6 @@ class EngineImpl { boost::intrusive::member_hook, &actor::ActorImpl::kernel_destroy_list_hook>> actors_to_destroy_; -#if SIMGRID_HAVE_MC - /* MCer cannot read members actor_list_ above in the remote process, so we copy the info it needs in a dynar. - * FIXME: This is supposed to be a temporary hack. - * A better solution would be to change the split between MCer and MCed, where the responsibility - * to compute the list of the enabled transitions goes to the MCed. - * That way, the MCer would not need to have the list of actors on its side. - * These info could be published by the MCed to the MCer in a way inspired of vd.so - */ - xbt_dynar_t actors_vector_ = xbt_dynar_new(sizeof(actor::ActorImpl*), nullptr); -#endif static double now_; static EngineImpl* instance_; @@ -136,11 +126,6 @@ public: void add_actor(aid_t pid, actor::ActorImpl* actor) { actor_list_[pid] = actor; } void remove_actor(aid_t pid) { actor_list_.erase(pid); } -#if SIMGRID_HAVE_MC - void reset_actor_dynar() { xbt_dynar_reset(actors_vector_); } - void add_actor_to_dynar(actor::ActorImpl* actor) { xbt_dynar_push_as(actors_vector_, actor::ActorImpl*, actor); } -#endif - const std::map& get_actor_list() const { return actor_list_; } const std::vector& get_actors_to_run() const { return actors_to_run_; } const std::vector& get_actors_that_ran() const { return actors_that_ran_; } diff --git a/src/mc/ModelChecker.cpp b/src/mc/ModelChecker.cpp index 1e3c2be931..4c1415a35a 100644 --- a/src/mc/ModelChecker.cpp +++ b/src/mc/ModelChecker.cpp @@ -163,7 +163,7 @@ bool ModelChecker::handle_message(const char* buffer, ssize_t size) 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_process().init(message.mmalloc_default_mdp, message.maxpid, message.actors); + get_remote_process().init(message.mmalloc_default_mdp, message.maxpid); break; } diff --git a/src/mc/mc_base.cpp b/src/mc/mc_base.cpp index fe612c92ad..93eef1ae19 100644 --- a/src/mc/mc_base.cpp +++ b/src/mc/mc_base.cpp @@ -51,15 +51,6 @@ void execute_actors() actor->simcall_handle(0); } } -#if SIMGRID_HAVE_MC - engine->reset_actor_dynar(); - for (auto const& [_, actor] : engine->get_actor_list()) { - // Only visible requests remain at this point, and they all have an observer - actor->simcall_.mc_max_consider_ = actor->simcall_.observer_->get_max_consider(); - - engine->add_actor_to_dynar(actor); - } -#endif } /** @brief returns if there this transition can proceed in a finite amount of time diff --git a/src/mc/remote/AppSide.cpp b/src/mc/remote/AppSide.cpp index 8ed73b4d9a..c361f58603 100644 --- a/src/mc/remote/AppSide.cpp +++ b/src/mc/remote/AppSide.cpp @@ -34,7 +34,7 @@ namespace simgrid::mc { std::unique_ptr AppSide::instance_; -AppSide* AppSide::initialize(xbt_dynar_t actors_addr) +AppSide* AppSide::initialize() { if (not std::getenv(MC_ENV_SOCKET_FD)) // We are not in MC mode: don't initialize the MC world return nullptr; @@ -74,7 +74,7 @@ AppSide* AppSide::initialize(xbt_dynar_t actors_addr) strerror(errno)); s_mc_message_initial_addresses_t message{MessageType::INITIAL_ADDRESSES, mmalloc_get_current_heap(), - kernel::actor::ActorImpl::get_maxpid_addr(), actors_addr}; + kernel::actor::ActorImpl::get_maxpid_addr()}; xbt_assert(instance_->channel_.send(message) == 0, "Could not send the initial message with addresses."); instance_->handle_messages(); diff --git a/src/mc/remote/AppSide.hpp b/src/mc/remote/AppSide.hpp index 6fe23a33ec..b63a836b1b 100644 --- a/src/mc/remote/AppSide.hpp +++ b/src/mc/remote/AppSide.hpp @@ -49,7 +49,7 @@ public: // Singleton :/ // TODO, remove the singleton antipattern. - static AppSide* initialize(xbt_dynar_t actors_addr); + static AppSide* initialize(); static AppSide* get() { return instance_.get(); } }; } // namespace simgrid::mc diff --git a/src/mc/remote/RemoteProcess.cpp b/src/mc/remote/RemoteProcess.cpp index afda96232e..2256c47888 100644 --- a/src/mc/remote/RemoteProcess.cpp +++ b/src/mc/remote/RemoteProcess.cpp @@ -105,11 +105,10 @@ int open_vm(pid_t pid, int flags) RemoteProcess::RemoteProcess(pid_t pid) : AddressSpace(this), pid_(pid), running_(true) {} -void RemoteProcess::init(xbt_mheap_t mmalloc_default_mdp, unsigned long* maxpid, xbt_dynar_t actors) +void RemoteProcess::init(xbt_mheap_t mmalloc_default_mdp, unsigned long* maxpid) { this->heap_address = remote(mmalloc_default_mdp); this->maxpid_addr_ = remote(maxpid); - this->actors_addr_ = remote(actors); this->memory_map_ = simgrid::xbt::get_memory_map(this->pid_); this->init_memory_map_info(); diff --git a/src/mc/remote/RemoteProcess.hpp b/src/mc/remote/RemoteProcess.hpp index 64b2ff7b3a..924aa567dd 100644 --- a/src/mc/remote/RemoteProcess.hpp +++ b/src/mc/remote/RemoteProcess.hpp @@ -50,16 +50,12 @@ struct IgnoredHeapRegion { std::size_t size; }; -/** The Application's process memory, seen from the MCer perspective - * - * This class is mixing a lot of different responsibilities and is tied - * to SIMIX. It should probably be split into different classes. +/** The Application's process memory, seen from the Checker perspective * * Responsibilities: * * - reading from the process memory (`AddressSpace`); * - accessing the system state of the process (heap, …); - * - storing the SIMIX state of the process; * - privatization; * - stack unwinding; * - etc. @@ -75,7 +71,7 @@ private: public: explicit RemoteProcess(pid_t pid); ~RemoteProcess() override; - void init(xbt_mheap_t mmalloc_default_mdp, unsigned long* maxpid, xbt_dynar_t actors); + void init(xbt_mheap_t mmalloc_default_mdp, unsigned long* maxpid); RemoteProcess(RemoteProcess const&) = delete; RemoteProcess(RemoteProcess&&) = delete; @@ -167,7 +163,6 @@ public: private: // Cache the address of the variables we read directly in the memory of remote RemotePtr maxpid_addr_; - RemotePtr actors_addr_; public: unsigned long get_maxpid() const { return this->read(maxpid_addr_); } diff --git a/src/mc/remote/mc_protocol.h b/src/mc/remote/mc_protocol.h index 6a8b5adeeb..6895fa0e54 100644 --- a/src/mc/remote/mc_protocol.h +++ b/src/mc/remote/mc_protocol.h @@ -56,7 +56,6 @@ struct s_mc_message_initial_addresses_t { simgrid::mc::MessageType type; xbt_mheap_t mmalloc_default_mdp; unsigned long* maxpid; - xbt_dynar_t actors; }; struct s_mc_message_ignore_heap_t { diff --git a/src/mc/sosp/Snapshot_test.cpp b/src/mc/sosp/Snapshot_test.cpp index 3d9bb72480..01081e3ab7 100644 --- a/src/mc/sosp/Snapshot_test.cpp +++ b/src/mc/sosp/Snapshot_test.cpp @@ -58,7 +58,7 @@ void snap_test_helper::Init() REQUIRE(1 << xbt_pagebits == xbt_pagesize); process = std::make_unique(getpid()); - process->init(nullptr, nullptr, nullptr); + process->init(nullptr, nullptr); mc_model_checker = new ::simgrid::mc::ModelChecker(std::move(process), -1); } diff --git a/teshsuite/mc/dwarf-expression/dwarf-expression.cpp b/teshsuite/mc/dwarf-expression/dwarf-expression.cpp index 8ed3273ed2..fee5b7b47e 100644 --- a/teshsuite/mc/dwarf-expression/dwarf-expression.cpp +++ b/teshsuite/mc/dwarf-expression/dwarf-expression.cpp @@ -149,7 +149,7 @@ static void test_deref(simgrid::dwarf::ExpressionContext const& state) int main() { auto* process = new simgrid::mc::RemoteProcess(getpid()); - process->init(nullptr, nullptr, nullptr); + process->init(nullptr, nullptr); simgrid::dwarf::ExpressionContext state; state.address_space = (simgrid::mc::AddressSpace*) process; diff --git a/teshsuite/mc/dwarf/dwarf.cpp b/teshsuite/mc/dwarf/dwarf.cpp index 0a1e41e0f0..352a73b00d 100644 --- a/teshsuite/mc/dwarf/dwarf.cpp +++ b/teshsuite/mc/dwarf/dwarf.cpp @@ -123,7 +123,7 @@ int main(int argc, char** argv) simgrid::mc::Type* type; simgrid::mc::RemoteProcess process(getpid()); - process.init(nullptr, nullptr, nullptr); + process.init(nullptr, nullptr); test_global_variable(process, process.binary_info.get(), "some_local_variable", &some_local_variable, sizeof(int)); -- 2.20.1