From: Arnaud Giersch Date: Tue, 12 Mar 2019 13:31:16 +0000 (+0100) Subject: Save valgrind_stack_id in a private field. X-Git-Tag: v3_22~112 X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/0d8e66239690b90dd2ced42f6bacab683661b20f Save valgrind_stack_id in a private field. Only register stacks when running on Valgrind to avoid false alarms from Asan. --- diff --git a/src/kernel/context/ContextBoost.cpp b/src/kernel/context/ContextBoost.cpp index ee99def4c2..9717474bc6 100644 --- a/src/kernel/context/ContextBoost.cpp +++ b/src/kernel/context/ContextBoost.cpp @@ -31,14 +31,14 @@ BoostContext::BoostContext(std::function&& code, smx_actor_t actor, Swap /* We need to pass the bottom of the stack to make_fcontext, depending on the stack direction it may be the lower or higher address: */ #if PTH_STACKGROWTH == -1 - unsigned char* stack = get_stack() + smx_context_usable_stack_size; + unsigned char* stack = get_stack() + smx_context_stack_size; #else unsigned char* stack = get_stack(); #endif #if BOOST_VERSION < 106100 - this->fc_ = boost::context::make_fcontext(stack, smx_context_usable_stack_size, BoostContext::wrapper); + this->fc_ = boost::context::make_fcontext(stack, smx_context_stack_size, BoostContext::wrapper); #else - this->fc_ = boost::context::detail::make_fcontext(stack, smx_context_usable_stack_size, BoostContext::wrapper); + this->fc_ = boost::context::detail::make_fcontext(stack, smx_context_stack_size, BoostContext::wrapper); #endif } else { diff --git a/src/kernel/context/ContextRaw.cpp b/src/kernel/context/ContextRaw.cpp index 520d8343a1..328e8f595b 100644 --- a/src/kernel/context/ContextRaw.cpp +++ b/src/kernel/context/ContextRaw.cpp @@ -199,7 +199,7 @@ RawContext::RawContext(std::function&& code, smx_actor_t actor, SwappedC : SwappedContext(std::move(code), actor, factory) { if (has_code()) { - this->stack_top_ = raw_makecontext(get_stack(), smx_context_usable_stack_size, RawContext::wrapper, this); + this->stack_top_ = raw_makecontext(get_stack(), smx_context_stack_size, RawContext::wrapper, this); } else { if (MC_is_active()) MC_ignore_heap(&stack_top_, sizeof(stack_top_)); diff --git a/src/kernel/context/ContextSwapped.cpp b/src/kernel/context/ContextSwapped.cpp index 822681552c..3d7740ff0c 100644 --- a/src/kernel/context/ContextSwapped.cpp +++ b/src/kernel/context/ContextSwapped.cpp @@ -52,6 +52,7 @@ SwappedContext::SwappedContext(std::function&& code, smx_actor_t actor, factory_->workers_context_[0] = this; if (has_code()) { + xbt_assert((smx_context_stack_size & 0xf) == 0, "smx_context_stack_size should be multiple of 16"); if (smx_context_guard_size > 0 && not MC_is_active()) { #if !defined(PTH_STACKGROWTH) || (PTH_STACKGROWTH != -1) @@ -97,13 +98,13 @@ SwappedContext::SwappedContext(std::function&& code, smx_actor_t actor, } #if PTH_STACKGROWTH == -1 - ASAN_ONLY(this->asan_stack_ = this->stack_ + smx_context_usable_stack_size); + ASAN_ONLY(this->asan_stack_ = this->stack_ + smx_context_stack_size); #else ASAN_ONLY(this->asan_stack_ = this->stack_); #endif #if HAVE_VALGRIND_H - unsigned int valgrind_stack_id = VALGRIND_STACK_REGISTER(this->stack_, this->stack_ + smx_context_stack_size); - memcpy(this->stack_ + smx_context_usable_stack_size, &valgrind_stack_id, sizeof valgrind_stack_id); + if (RUNNING_ON_VALGRIND) + this->valgrind_stack_id_ = VALGRIND_STACK_REGISTER(this->stack_, this->stack_ + smx_context_stack_size); #endif } } @@ -114,9 +115,8 @@ SwappedContext::~SwappedContext() return; #if HAVE_VALGRIND_H - unsigned int valgrind_stack_id; - memcpy(&valgrind_stack_id, stack_ + smx_context_usable_stack_size, sizeof valgrind_stack_id); - VALGRIND_STACK_DEREGISTER(valgrind_stack_id); + if (RUNNING_ON_VALGRIND) + VALGRIND_STACK_DEREGISTER(valgrind_stack_id_); #endif #ifndef _WIN32 diff --git a/src/kernel/context/ContextSwapped.hpp b/src/kernel/context/ContextSwapped.hpp index ad888cd9d5..c132b63f81 100644 --- a/src/kernel/context/ContextSwapped.hpp +++ b/src/kernel/context/ContextSwapped.hpp @@ -62,6 +62,10 @@ public: private: unsigned char* stack_ = nullptr; /* the thread stack */ SwappedContextFactory* const factory_; // for sequential and parallel run_all() + +#if HAVE_VALGRIND_H + unsigned int valgrind_stack_id_; +#endif }; } // namespace context diff --git a/src/kernel/context/ContextUnix.cpp b/src/kernel/context/ContextUnix.cpp index 70a87d20ca..77b952e594 100644 --- a/src/kernel/context/ContextUnix.cpp +++ b/src/kernel/context/ContextUnix.cpp @@ -72,7 +72,7 @@ UContext::UContext(std::function&& code, smx_actor_t actor, SwappedConte getcontext(&this->uc_); this->uc_.uc_link = nullptr; this->uc_.uc_stack.ss_sp = sg_makecontext_stack_addr(get_stack()); - this->uc_.uc_stack.ss_size = sg_makecontext_stack_size(smx_context_usable_stack_size); + this->uc_.uc_stack.ss_size = sg_makecontext_stack_size(smx_context_stack_size); // 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. @@ -83,7 +83,7 @@ UContext::UContext(std::function&& code, smx_actor_t actor, SwappedConte #if SIMGRID_HAVE_MC if (MC_is_active()) { - MC_register_stack_area(get_stack(), actor, &(this->uc_), smx_context_usable_stack_size); + MC_register_stack_area(get_stack(), actor, &(this->uc_), smx_context_stack_size); } #endif } diff --git a/src/kernel/context/context_private.hpp b/src/kernel/context/context_private.hpp index 0bf2a78b0f..8a47f56e00 100644 --- a/src/kernel/context/context_private.hpp +++ b/src/kernel/context/context_private.hpp @@ -20,14 +20,4 @@ #define ASAN_FINISH_SWITCH(fake_stack_save, bottom_old, size_old) (void)0 #endif -/* We are using the bottom of the stack to save some information, like the - * valgrind_stack_id. Define smx_context_usable_stack_size to give the remaining - * size for the stack. Round its value to a multiple of 16 (asan wants the stacks to be aligned this way). */ -#if HAVE_VALGRIND_H -#define smx_context_usable_stack_size \ - ((smx_context_stack_size - sizeof(unsigned int)) & ~0xf) /* for valgrind_stack_id */ -#else -#define smx_context_usable_stack_size (smx_context_stack_size & ~0xf) -#endif - #endif