From b741bb048b44a04d99689f6b64cf6628f15e550a Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Sun, 20 Jan 2019 22:25:16 +0100 Subject: [PATCH] cosmetics in UCtx: hide one internal function and inline another --- src/kernel/context/ContextUnix.cpp | 73 +++++++++++++----------------- src/kernel/context/ContextUnix.hpp | 5 +- src/mc/sosp/mc_checkpoint.cpp | 2 +- 3 files changed, 34 insertions(+), 46 deletions(-) diff --git a/src/kernel/context/ContextUnix.cpp b/src/kernel/context/ContextUnix.cpp index de67f76e93..ba5ed9a8a3 100644 --- a/src/kernel/context/ContextUnix.cpp +++ b/src/kernel/context/ContextUnix.cpp @@ -17,17 +17,37 @@ XBT_LOG_EXTERNAL_DEFAULT_CATEGORY(simix_context); -/** Many integers are needed to store a pointer - * - * Support up to two ints. */ +/** Up to two integers may be needed to store a pointer on the system we target */ constexpr int CTX_ADDR_LEN = 2; static_assert(sizeof(simgrid::kernel::context::UContext*) <= CTX_ADDR_LEN * sizeof(int), - "Ucontexts are not supported on this arch yet"); + "Ucontexts are not supported on this arch yet. Please increase CTX_ADDR_LEN."); namespace simgrid { namespace kernel { namespace context { +// The name of this function is currently hardcoded in MC (as string). +// Do not change it without fixing those references as well. +static void smx_ctx_wrapper(int i1, int i2) +{ + // Rebuild the Context* pointer from the integers: + int ctx_addr[CTX_ADDR_LEN] = {i1, i2}; + simgrid::kernel::context::UContext* context; + memcpy(&context, ctx_addr, sizeof context); + + ASAN_FINISH_SWITCH(nullptr, &context->asan_ctx_->asan_stack_, &context->asan_ctx_->asan_stack_size_); + try { + (*context)(); + } catch (simgrid::kernel::context::Context::StopRequest const&) { + XBT_DEBUG("Caught a StopRequest"); + } catch (simgrid::Exception const& e) { + XBT_INFO("Actor killed by an uncatched exception %s", simgrid::xbt::demangle(typeid(e).name()).get()); + throw; + } + context->Context::stop(); + ASAN_ONLY(context->asan_stop_ = true); + context->suspend(); +} // UContextFactory Context* UContextFactory::create_context(std::function code, void_pfn_smxprocess_t cleanup, smx_actor_t actor) @@ -42,7 +62,7 @@ UContext::UContext(std::function code, void_pfn_smxprocess_t cleanup_fun SwappedContextFactory* factory) : SwappedContext(std::move(code), cleanup_func, actor, factory) { - /* if the user provided a function for the process then use it, otherwise it is the context for maestro */ + /* if the user provided a function for the actor then use it. If not, nothing to do for maestro. */ if (has_code()) { getcontext(&this->uc_); this->uc_.uc_link = nullptr; @@ -53,7 +73,13 @@ UContext::UContext(std::function code, void_pfn_smxprocess_t cleanup_fun #else ASAN_ONLY(this->asan_stack_ = get_stack()); #endif - UContext::make_ctx(&this->uc_, UContext::smx_ctx_sysv_wrapper, this); + // Makecontext expects integer arguments; we want to pass a pointer. + // This context address is decomposed into a serie of integers, which are passed as arguments to makecontext. + + int ctx_addr[CTX_ADDR_LEN]{}; + UContext* arg = this; + memcpy(ctx_addr, &arg, sizeof this); + makecontext(&this->uc_, (void (*)())smx_ctx_wrapper, 2, ctx_addr[0], ctx_addr[1]); } #if SIMGRID_HAVE_MC @@ -63,41 +89,6 @@ UContext::UContext(std::function code, void_pfn_smxprocess_t cleanup_fun #endif } -// The name of this function is currently hardcoded in the code (as string). -// Do not change it without fixing those references as well. -void UContext::smx_ctx_sysv_wrapper(int i1, int i2) -{ - // Rebuild the Context* pointer from the integers: - int ctx_addr[CTX_ADDR_LEN] = {i1, i2}; - simgrid::kernel::context::UContext* context; - memcpy(&context, ctx_addr, sizeof context); - - ASAN_FINISH_SWITCH(nullptr, &context->asan_ctx_->asan_stack_, &context->asan_ctx_->asan_stack_size_); - try { - (*context)(); - } catch (simgrid::kernel::context::Context::StopRequest const&) { - XBT_DEBUG("Caught a StopRequest"); - } catch (simgrid::Exception const& e) { - XBT_INFO("Actor killed by an uncatched exception %s", simgrid::xbt::demangle(typeid(e).name()).get()); - throw; - } - context->Context::stop(); - ASAN_ONLY(context->asan_stop_ = true); - context->suspend(); -} - -/** A better makecontext - * - * Makecontext expects integer arguments, we the context variable is decomposed into a serie of integers and each - * integer is passed as argument to makecontext. - */ -void UContext::make_ctx(ucontext_t* ucp, void (*func)(int, int), UContext* arg) -{ - int ctx_addr[CTX_ADDR_LEN]{}; - memcpy(ctx_addr, &arg, sizeof arg); - makecontext(ucp, (void (*)())func, 2, ctx_addr[0], ctx_addr[1]); -} - void UContext::swap_into(SwappedContext* to_) { UContext* to = static_cast(to_); diff --git a/src/kernel/context/ContextUnix.hpp b/src/kernel/context/ContextUnix.hpp index 1c8d7da5fa..4df2375bf0 100644 --- a/src/kernel/context/ContextUnix.hpp +++ b/src/kernel/context/ContextUnix.hpp @@ -39,14 +39,11 @@ private: UContext* asan_ctx_ = nullptr; bool asan_stop_ = false; #endif - - static void smx_ctx_sysv_wrapper(int, int); - static void make_ctx(ucontext_t* ucp, void (*func)(int, int), UContext* arg); }; class UContextFactory : public SwappedContextFactory { public: - Context* create_context(std::function code, void_pfn_smxprocess_t cleanup, smx_actor_t process) override; + Context* create_context(std::function code, void_pfn_smxprocess_t cleanup, smx_actor_t actor) override; }; }}} // namespace diff --git a/src/mc/sosp/mc_checkpoint.cpp b/src/mc/sosp/mc_checkpoint.cpp index 0d172a822c..4f8585d181 100644 --- a/src/mc/sosp/mc_checkpoint.cpp +++ b/src/mc/sosp/mc_checkpoint.cpp @@ -343,7 +343,7 @@ static std::vector unwind_stack_frames(simgrid::mc::UnwindCo result.push_back(std::move(stack_frame)); /* Stop before context switch with maestro */ - if (frame != nullptr && frame->name == "smx_ctx_sysv_wrapper") + if (frame != nullptr && frame->name == "smx_ctx_wrapper") break; int ret = unw_step(&c); -- 2.20.1