Author: labath Date: Wed Jan 25 05:19:45 2017 New Revision: 293046 URL: http://llvm.org/viewvc/llvm-project?rev=293046&view=rev Log: NPL: Compartmentalize arm64 single step workaround better
The main motivation for me doing this is being able to build an arm android lldb-server against api level 9 headers, but it seems like a good cleanup nonetheless. The entirety of the cpu_set_t dance now resides in SingleStepCheck.cpp, which is only built on arm64. Modified: lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.cpp lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.cpp lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.h Modified: lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.cpp?rev=293046&r1=293045&r2=293046&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.cpp (original) +++ lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.cpp Wed Jan 25 05:19:45 2017 @@ -220,63 +220,12 @@ Error NativeThreadLinux::Resume(uint32_t reinterpret_cast<void *>(data)); } -void NativeThreadLinux::MaybePrepareSingleStepWorkaround() { - if (!SingleStepWorkaroundNeeded()) - return; - - Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD)); - - if (sched_getaffinity(static_cast<::pid_t>(m_tid), sizeof m_original_cpu_set, - &m_original_cpu_set) != 0) { - // This should really not fail. But, just in case... - if (log) { - Error error(errno, eErrorTypePOSIX); - log->Printf( - "NativeThreadLinux::%s Unable to get cpu affinity for thread %" PRIx64 - ": %s", - __FUNCTION__, m_tid, error.AsCString()); - } - return; - } - - cpu_set_t set; - CPU_ZERO(&set); - CPU_SET(0, &set); - if (sched_setaffinity(static_cast<::pid_t>(m_tid), sizeof set, &set) != 0 && - log) { - // This may fail in very locked down systems, if the thread is not allowed - // to run on - // cpu 0. If that happens, only thing we can do is it log it and continue... - Error error(errno, eErrorTypePOSIX); - log->Printf( - "NativeThreadLinux::%s Unable to set cpu affinity for thread %" PRIx64 - ": %s", - __FUNCTION__, m_tid, error.AsCString()); - } -} - -void NativeThreadLinux::MaybeCleanupSingleStepWorkaround() { - if (!SingleStepWorkaroundNeeded()) - return; - - if (sched_setaffinity(static_cast<::pid_t>(m_tid), sizeof m_original_cpu_set, - &m_original_cpu_set) != 0) { - Error error(errno, eErrorTypePOSIX); - Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD)); - log->Printf( - "NativeThreadLinux::%s Unable to reset cpu affinity for thread %" PRIx64 - ": %s", - __FUNCTION__, m_tid, error.AsCString()); - } -} - Error NativeThreadLinux::SingleStep(uint32_t signo) { const StateType new_state = StateType::eStateStepping; MaybeLogStateChange(new_state); m_state = new_state; m_stop_info.reason = StopReason::eStopReasonNone; - - MaybePrepareSingleStepWorkaround(); + m_step_workaround = SingleStepWorkaround::Get(m_tid); intptr_t data = 0; if (signo != LLDB_INVALID_SIGNAL_NUMBER) @@ -338,7 +287,7 @@ bool NativeThreadLinux::IsStopped(int *s void NativeThreadLinux::SetStopped() { if (m_state == StateType::eStateStepping) - MaybeCleanupSingleStepWorkaround(); + m_step_workaround.reset(); const StateType new_state = StateType::eStateStopped; MaybeLogStateChange(new_state); Modified: lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h?rev=293046&r1=293045&r2=293046&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h (original) +++ lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h Wed Jan 25 05:19:45 2017 @@ -10,11 +10,10 @@ #ifndef liblldb_NativeThreadLinux_H_ #define liblldb_NativeThreadLinux_H_ +#include "SingleStepCheck.h" #include "lldb/Host/common/NativeThreadProtocol.h" #include "lldb/lldb-private-forward.h" -#include <sched.h> - #include <map> #include <memory> #include <string> @@ -94,10 +93,6 @@ private: void SetStopped(); - inline void MaybePrepareSingleStepWorkaround(); - - inline void MaybeCleanupSingleStepWorkaround(); - // --------------------------------------------------------------------- // Member Variables // --------------------------------------------------------------------- @@ -107,7 +102,7 @@ private: std::string m_stop_description; using WatchpointIndexMap = std::map<lldb::addr_t, uint32_t>; WatchpointIndexMap m_watchpoint_index_map; - cpu_set_t m_original_cpu_set; // For single-step workaround. + llvm::Optional<SingleStepWorkaround> m_step_workaround; }; typedef std::shared_ptr<NativeThreadLinux> NativeThreadLinuxSP; Modified: lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.cpp?rev=293046&r1=293045&r2=293046&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.cpp (original) +++ lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.cpp Wed Jan 25 05:19:45 2017 @@ -18,10 +18,12 @@ #include "llvm/Support/Compiler.h" +#include "Plugins/Process/POSIX/ProcessPOSIXLog.h" #include "lldb/Core/Error.h" -#include "lldb/Core/Log.h" #include "lldb/Host/linux/Ptrace.h" +using namespace lldb; +using namespace lldb_private; using namespace lldb_private::process_linux; #if defined(__arm64__) || defined(__aarch64__) @@ -32,16 +34,14 @@ void LLVM_ATTRIBUTE_NORETURN Child() { _exit(1); // We just do an endless loop SIGSTOPPING ourselves until killed. The tracer - // will fiddle with our cpu - // affinities and monitor the behaviour. + // will fiddle with our cpu affinities and monitor the behaviour. for (;;) { raise(SIGSTOP); // Generate a bunch of instructions here, so that a single-step does not - // land in the - // raise() accidentally. If single-stepping works, we will be spinning in - // this loop. If - // it doesn't, we'll land in the raise() call above. + // land in the raise() accidentally. If single-stepping works, we will be + // spinning in this loop. If it doesn't, we'll land in the raise() call + // above. for (volatile unsigned i = 0; i < CPU_SETSIZE; ++i) ; } @@ -57,25 +57,16 @@ struct ChildDeleter { } }; -} // end anonymous namespace - -bool impl::SingleStepWorkaroundNeeded() { +bool WorkaroundNeeded() { // We shall spawn a child, and use it to verify the debug capabilities of the - // cpu. We shall - // iterate through the cpus, bind the child to each one in turn, and verify - // that - // single-stepping works on that cpu. A workaround is needed if we find at - // least one broken - // cpu. + // cpu. We shall iterate through the cpus, bind the child to each one in turn, + // and verify that single-stepping works on that cpu. A workaround is needed + // if we find at least one broken cpu. - Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD)); - Error error; + Log *log = ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD); ::pid_t child_pid = fork(); if (child_pid == -1) { - if (log) { - error.SetErrorToErrno(); - log->Printf("%s failed to fork(): %s", __FUNCTION__, error.AsCString()); - } + LLDB_LOG(log, "failed to fork(): {0}", Error(errno, eErrorTypePOSIX)); return false; } if (child_pid == 0) @@ -85,22 +76,16 @@ bool impl::SingleStepWorkaroundNeeded() cpu_set_t available_cpus; if (sched_getaffinity(child_pid, sizeof available_cpus, &available_cpus) == -1) { - if (log) { - error.SetErrorToErrno(); - log->Printf("%s failed to get available cpus: %s", __FUNCTION__, - error.AsCString()); - } + LLDB_LOG(log, "failed to get available cpus: {0}", + Error(errno, eErrorTypePOSIX)); return false; } int status; ::pid_t wpid = waitpid(child_pid, &status, __WALL); if (wpid != child_pid || !WIFSTOPPED(status)) { - if (log) { - error.SetErrorToErrno(); - log->Printf("%s waitpid() failed (status = %x): %s", __FUNCTION__, status, - error.AsCString()); - } + LLDB_LOG(log, "waitpid() failed (status = {0:x}): {1}", status, + Error(errno, eErrorTypePOSIX)); return false; } @@ -113,46 +98,37 @@ bool impl::SingleStepWorkaroundNeeded() CPU_ZERO(&cpus); CPU_SET(cpu, &cpus); if (sched_setaffinity(child_pid, sizeof cpus, &cpus) == -1) { - if (log) { - error.SetErrorToErrno(); - log->Printf("%s failed to switch to cpu %u: %s", __FUNCTION__, cpu, - error.AsCString()); - } + LLDB_LOG(log, "failed to switch to cpu {0}: {1}", cpu, + Error(errno, eErrorTypePOSIX)); continue; } int status; - error = NativeProcessLinux::PtraceWrapper(PTRACE_SINGLESTEP, child_pid); + Error error = + NativeProcessLinux::PtraceWrapper(PTRACE_SINGLESTEP, child_pid); if (error.Fail()) { - if (log) - log->Printf("%s single step failed: %s", __FUNCTION__, - error.AsCString()); + LLDB_LOG(log, "single step failed: {0}", error); break; } wpid = waitpid(child_pid, &status, __WALL); if (wpid != child_pid || !WIFSTOPPED(status)) { - if (log) { - error.SetErrorToErrno(); - log->Printf("%s waitpid() failed (status = %x): %s", __FUNCTION__, - status, error.AsCString()); - } + LLDB_LOG(log, "waitpid() failed (status = {0:x}): {1}", status, + Error(errno, eErrorTypePOSIX)); break; } if (WSTOPSIG(status) != SIGTRAP) { - if (log) - log->Printf("%s single stepping on cpu %d failed with status %x", - __FUNCTION__, cpu, status); + LLDB_LOG(log, "single stepping on cpu {0} failed with status {1:x}", cpu, + status); break; } } // cpu is either the index of the first broken cpu, or CPU_SETSIZE. if (cpu == 0) { - if (log) - log->Printf("%s SINGLE STEPPING ON FIRST CPU IS NOT WORKING. DEBUGGING " - "LIKELY TO BE UNRELIABLE.", - __FUNCTION__); + LLDB_LOG(log, + "SINGLE STEPPING ON FIRST CPU IS NOT WORKING. DEBUGGING " + "LIKELY TO BE UNRELIABLE."); // No point in trying to fiddle with the affinities, just give it our best // shot and see how it goes. return false; @@ -161,6 +137,45 @@ bool impl::SingleStepWorkaroundNeeded() return cpu != CPU_SETSIZE; } -#else // !arm64 -bool impl::SingleStepWorkaroundNeeded() { return false; } +} // end anonymous namespace + +llvm::Optional<SingleStepWorkaround> SingleStepWorkaround::Get(::pid_t tid) { + Log *log = ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD); + + static bool workaround_needed = WorkaroundNeeded(); + if (!workaround_needed) { + LLDB_LOG(log, "workaround for thread {0} not needed", tid); + return llvm::None; + } + + cpu_set_t original_set; + if (sched_getaffinity(tid, sizeof original_set, &original_set) != 0) { + // This should really not fail. But, just in case... + LLDB_LOG(log, "Unable to get cpu affinity for thread {0}: {1}", tid, + Error(errno, eErrorTypePOSIX)); + return llvm::None; + } + + cpu_set_t set; + CPU_ZERO(&set); + CPU_SET(0, &set); + if (sched_setaffinity(tid, sizeof set, &set) != 0) { + // This may fail in very locked down systems, if the thread is not allowed + // to run on cpu 0. If that happens, only thing we can do is it log it and + // continue... + LLDB_LOG(log, "Unable to set cpu affinity for thread {0}: {1}", tid, + Error(errno, eErrorTypePOSIX)); + } + + LLDB_LOG(log, "workaround for thread {0} prepared", tid); + return SingleStepWorkaround(tid, original_set); +} + +SingleStepWorkaround::~SingleStepWorkaround() { + if (sched_setaffinity(m_tid, sizeof m_original_set, &m_original_set) != 0) { + Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD)); + LLDB_LOG(log, "Unable to reset cpu affinity for thread {0}: {1}", m_tid, + Error(errno, eErrorTypePOSIX)); + } +} #endif Modified: lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.h?rev=293046&r1=293045&r2=293046&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.h (original) +++ lldb/trunk/source/Plugins/Process/Linux/SingleStepCheck.h Wed Jan 25 05:19:45 2017 @@ -10,30 +10,44 @@ #ifndef liblldb_SingleStepCheck_H_ #define liblldb_SingleStepCheck_H_ +#include "sched.h" +#include "llvm/ADT/Optional.h" +#include <sys/types.h> + namespace lldb_private { namespace process_linux { -namespace impl { -extern bool SingleStepWorkaroundNeeded(); -} - // arm64 linux had a bug which prevented single-stepping and watchpoints from -// working on non-boot -// cpus, due to them being incorrectly initialized after coming out of suspend. -// This issue is -// particularly affecting android M, which uses suspend ("doze mode") quite -// aggressively. This -// code detects that situation and makes single-stepping work by doing all the -// step operations on +// working on non-boot cpus, due to them being incorrectly initialized after +// coming out of suspend. This issue is particularly affecting android M, which +// uses suspend ("doze mode") quite aggressively. This code detects that +// situation and makes single-stepping work by doing all the step operations on // the boot cpu. // // The underlying issue has been fixed in android N and linux 4.4. This code can -// be removed once -// these systems become obsolete. -inline bool SingleStepWorkaroundNeeded() { - static bool value = impl::SingleStepWorkaroundNeeded(); - return value; -} +// be removed once these systems become obsolete. + +#if defined(__arm64__) || defined(__aarch64__) +class SingleStepWorkaround { + ::pid_t m_tid; + cpu_set_t m_original_set; + +public: + SingleStepWorkaround(::pid_t tid, cpu_set_t original_set) + : m_tid(tid), m_original_set(original_set) {} + ~SingleStepWorkaround(); + + static llvm::Optional<SingleStepWorkaround> Get(::pid_t tid); +}; +#else +class SingleStepWorkaround { +public: + static llvm::Optional<SingleStepWorkaround> Get(::pid_t tid) { + return llvm::None; + } +}; +#endif + } // end namespace process_linux } // end namespace lldb_private _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits