Author: stella.stamenova Date: Tue Jul 10 15:05:33 2018 New Revision: 336732
URL: http://llvm.org/viewvc/llvm-project?rev=336732&view=rev Log: [windows] LLDB shows the wrong values when register read is executed at a frame other than zero Summary: This is a clean version of the change suggested here: https://bugs.llvm.org/show_bug.cgi?id=37495 The main change is to follow the same pattern as non-windows targets and use an unwinder object to retrieve the register context. I also changed a couple of the comments to actually log, so that issues with unsupported scenarios can be tracked down more easily. Lastly, ClearStackFrames is implemented in the base class, so individual thread implementations don't have to override it. Reviewers: asmith, zturner, aleksandr.urakov Reviewed By: aleksandr.urakov Subscribers: emaste, stella.stamenova, tatyana-krasnukha, llvm-commits Differential Revision: https://reviews.llvm.org/D49111 Modified: lldb/trunk/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ThreadKDP.cpp lldb/trunk/source/Plugins/Process/Utility/ThreadMemory.cpp lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.h lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.cpp lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.h lldb/trunk/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp lldb/trunk/source/Plugins/Process/mach-core/ThreadMachCore.cpp Modified: lldb/trunk/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp?rev=336732&r1=336731&r2=336732&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp (original) +++ lldb/trunk/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp Tue Jul 10 15:05:33 2018 @@ -57,7 +57,7 @@ using namespace lldb_private; FreeBSDThread::FreeBSDThread(Process &process, lldb::tid_t tid) : Thread(process, tid), m_frame_ap(), m_breakpoint(), - m_thread_name_valid(false), m_thread_name(), m_posix_thread(NULL) { + m_thread_name_valid(false), m_thread_name(), m_posix_thread(nullptr) { Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD)); LLDB_LOGV(log, "tid = {0}", tid); @@ -106,7 +106,7 @@ void FreeBSDThread::RefreshStateAfterSto } } -const char *FreeBSDThread::GetInfo() { return NULL; } +const char *FreeBSDThread::GetInfo() { return nullptr; } void FreeBSDThread::SetName(const char *name) { m_thread_name_valid = (name && name[0]); @@ -157,15 +157,15 @@ const char *FreeBSDThread::GetName() { } if (m_thread_name.empty()) - return NULL; + return nullptr; return m_thread_name.c_str(); } lldb::RegisterContextSP FreeBSDThread::GetRegisterContext() { if (!m_reg_context_sp) { - m_posix_thread = NULL; + m_posix_thread = nullptr; - RegisterInfoInterface *reg_interface = NULL; + RegisterInfoInterface *reg_interface = nullptr; const ArchSpec &target_arch = GetProcess()->GetTarget().GetArchitecture(); assert(target_arch.GetTriple().getOS() == llvm::Triple::FreeBSD); @@ -281,7 +281,7 @@ bool FreeBSDThread::CalculateStopInfo() } Unwind *FreeBSDThread::GetUnwinder() { - if (m_unwinder_ap.get() == NULL) + if (!m_unwinder_ap) m_unwinder_ap.reset(new UnwindLLDB(*this)); return m_unwinder_ap.get(); @@ -480,7 +480,7 @@ void FreeBSDThread::BreakNotify(const Pr // make any assumptions about the thread ID so we must always report the // breakpoint regardless of the thread. if (bp_site->ValidForThisThread(this) || - GetProcess()->GetOperatingSystem() != NULL) + GetProcess()->GetOperatingSystem() != nullptr) SetStopInfo(StopInfo::CreateStopReasonWithBreakpointSiteID(*this, bp_id)); else { const bool should_stop = false; @@ -544,7 +544,7 @@ void FreeBSDThread::TraceNotify(const Pr // assumptions about the thread ID so we must always report the breakpoint // regardless of the thread. if (bp_site && (bp_site->ValidForThisThread(this) || - GetProcess()->GetOperatingSystem() != NULL)) + GetProcess()->GetOperatingSystem() != nullptr)) SetStopInfo(StopInfo::CreateStopReasonWithBreakpointSiteID( *this, bp_site->GetID())); else { Modified: lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ThreadKDP.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ThreadKDP.cpp?rev=336732&r1=336731&r2=336732&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ThreadKDP.cpp (original) +++ lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ThreadKDP.cpp Tue Jul 10 15:05:33 2018 @@ -52,11 +52,11 @@ ThreadKDP::~ThreadKDP() { const char *ThreadKDP::GetName() { if (m_thread_name.empty()) - return NULL; + return nullptr; return m_thread_name.c_str(); } -const char *ThreadKDP::GetQueueName() { return NULL; } +const char *ThreadKDP::GetQueueName() { return nullptr; } void ThreadKDP::RefreshStateAfterStop() { // Invalidate all registers in our register context. We don't set "force" to @@ -79,8 +79,8 @@ void ThreadKDP::Dump(Log *log, uint32_t bool ThreadKDP::ShouldStop(bool &step_more) { return true; } lldb::RegisterContextSP ThreadKDP::GetRegisterContext() { - if (m_reg_context_sp.get() == NULL) - m_reg_context_sp = CreateRegisterContextForFrame(NULL); + if (!m_reg_context_sp) + m_reg_context_sp = CreateRegisterContextForFrame(nullptr); return m_reg_context_sp; } @@ -119,7 +119,7 @@ ThreadKDP::CreateRegisterContextForFrame } } else { Unwind *unwinder = GetUnwinder(); - if (unwinder) + if (unwinder != nullptr) reg_ctx_sp = unwinder->CreateRegisterContextForFrame(frame); } return reg_ctx_sp; Modified: lldb/trunk/source/Plugins/Process/Utility/ThreadMemory.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Utility/ThreadMemory.cpp?rev=336732&r1=336731&r2=336732&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/Utility/ThreadMemory.cpp (original) +++ lldb/trunk/source/Plugins/Process/Utility/ThreadMemory.cpp Tue Jul 10 15:05:33 2018 @@ -62,7 +62,7 @@ ThreadMemory::CreateRegisterContextForFr reg_ctx_sp = GetRegisterContext(); } else { Unwind *unwinder = GetUnwinder(); - if (unwinder) + if (unwinder != nullptr) reg_ctx_sp = unwinder->CreateRegisterContextForFrame(frame); } return reg_ctx_sp; Modified: lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp?rev=336732&r1=336731&r2=336732&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp (original) +++ lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp Tue Jul 10 15:05:33 2018 @@ -16,10 +16,10 @@ #include "lldb/Utility/Log.h" #include "lldb/Utility/Logging.h" +#include "Plugins/Process/Utility/UnwindLLDB.h" #include "ProcessWindows.h" #include "ProcessWindowsLog.h" #include "TargetThreadWindows.h" -#include "Plugins/Process/Utility/UnwindLLDB.h" #if defined(_WIN64) #include "x64/RegisterContextWindows_x64.h" @@ -33,7 +33,7 @@ using namespace lldb_private; TargetThreadWindows::TargetThreadWindows(ProcessWindows &process, const HostThread &thread) : Thread(process, thread.GetNativeThread().GetThreadId()), - m_host_thread(thread) {} + m_thread_reg_ctx_sp(), m_host_thread(thread) {} TargetThreadWindows::~TargetThreadWindows() { DestroyThread(); } @@ -49,40 +49,53 @@ void TargetThreadWindows::DidStop() {} RegisterContextSP TargetThreadWindows::GetRegisterContext() { if (!m_reg_context_sp) - m_reg_context_sp = CreateRegisterContextForFrameIndex(0); + m_reg_context_sp = CreateRegisterContextForFrame(nullptr); return m_reg_context_sp; } RegisterContextSP TargetThreadWindows::CreateRegisterContextForFrame(StackFrame *frame) { - return CreateRegisterContextForFrameIndex(frame->GetConcreteFrameIndex()); -} - -RegisterContextSP -TargetThreadWindows::CreateRegisterContextForFrameIndex(uint32_t idx) { - if (!m_reg_context_sp) { - ArchSpec arch = HostInfo::GetArchitecture(); - switch (arch.GetMachine()) { - case llvm::Triple::x86: + RegisterContextSP reg_ctx_sp; + uint32_t concrete_frame_idx = 0; + Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD)); + + if (frame) + concrete_frame_idx = frame->GetConcreteFrameIndex(); + + if (concrete_frame_idx == 0) { + if (!m_thread_reg_ctx_sp) { + ArchSpec arch = HostInfo::GetArchitecture(); + switch (arch.GetMachine()) { + case llvm::Triple::x86: #if defined(_WIN64) -// FIXME: This is a Wow64 process, create a RegisterContextWindows_Wow64 + // FIXME: This is a Wow64 process, create a RegisterContextWindows_Wow64 + LLDB_LOG(log, "This is a Wow64 process, we should create a " + "RegisterContextWindows_Wow64, but we don't."); #else - m_reg_context_sp.reset(new RegisterContextWindows_x86(*this, idx)); + m_thread_reg_ctx_sp.reset( + new RegisterContextWindows_x86(*this, concrete_frame_idx)); #endif - break; - case llvm::Triple::x86_64: + break; + case llvm::Triple::x86_64: #if defined(_WIN64) - m_reg_context_sp.reset(new RegisterContextWindows_x64(*this, idx)); + m_thread_reg_ctx_sp.reset( + new RegisterContextWindows_x64(*this, concrete_frame_idx)); #else -// LLDB is 32-bit, but the target process is 64-bit. We probably can't debug -// this. + LLDB_LOG(log, "LLDB is 32-bit, but the target process is 64-bit."); #endif - default: - break; + default: + break; + } } + reg_ctx_sp = m_thread_reg_ctx_sp; + } else { + Unwind *unwinder = GetUnwinder(); + if (unwinder != nullptr) + reg_ctx_sp = unwinder->CreateRegisterContextForFrame(frame); } - return m_reg_context_sp; + + return reg_ctx_sp; } bool TargetThreadWindows::CalculateStopInfo() { @@ -93,7 +106,7 @@ bool TargetThreadWindows::CalculateStopI Unwind *TargetThreadWindows::GetUnwinder() { // FIXME: Implement an unwinder based on the Windows unwinder exposed through // DIA SDK. - if (m_unwinder_ap.get() == NULL) + if (!m_unwinder_ap) m_unwinder_ap.reset(new UnwindLLDB(*this)); return m_unwinder_ap.get(); } @@ -118,9 +131,10 @@ Status TargetThreadWindows::DoResume() { DWORD previous_suspend_count = 0; HANDLE thread_handle = m_host_thread.GetNativeThread().GetSystemHandle(); do { - // ResumeThread returns -1 on error, or the thread's *previous* suspend count on success. - // This means that the return value is 1 when the thread was restarted. - // Note that DWORD is an unsigned int, so we need to explicitly compare with -1. + // ResumeThread returns -1 on error, or the thread's *previous* suspend + // count on success. This means that the return value is 1 when the thread + // was restarted. Note that DWORD is an unsigned int, so we need to + // explicitly compare with -1. previous_suspend_count = ::ResumeThread(thread_handle); if (previous_suspend_count == (DWORD)-1) Modified: lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.h?rev=336732&r1=336731&r2=336732&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.h (original) +++ lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.h Tue Jul 10 15:05:33 2018 @@ -42,10 +42,9 @@ public: HostThread GetHostThread() const { return m_host_thread; } private: - lldb::RegisterContextSP CreateRegisterContextForFrameIndex(uint32_t idx); - + lldb::RegisterContextSP m_thread_reg_ctx_sp; HostThread m_host_thread; }; -} +} // namespace lldb_private #endif Modified: lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.cpp?rev=336732&r1=336731&r2=336732&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.cpp (original) +++ lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.cpp Tue Jul 10 15:05:33 2018 @@ -55,16 +55,9 @@ void ThreadElfCore::RefreshStateAfterSto GetRegisterContext()->InvalidateIfNeeded(false); } -void ThreadElfCore::ClearStackFrames() { - Unwind *unwinder = GetUnwinder(); - if (unwinder) - unwinder->Clear(); - Thread::ClearStackFrames(); -} - RegisterContextSP ThreadElfCore::GetRegisterContext() { - if (m_reg_context_sp.get() == NULL) { - m_reg_context_sp = CreateRegisterContextForFrame(NULL); + if (!m_reg_context_sp) { + m_reg_context_sp = CreateRegisterContextForFrame(nullptr); } return m_reg_context_sp; } @@ -84,7 +77,7 @@ ThreadElfCore::CreateRegisterContextForF ProcessElfCore *process = static_cast<ProcessElfCore *>(GetProcess().get()); ArchSpec arch = process->GetArchitecture(); - RegisterInfoInterface *reg_interface = NULL; + RegisterInfoInterface *reg_interface = nullptr; switch (arch.GetTriple().getOS()) { case llvm::Triple::FreeBSD: { @@ -234,8 +227,10 @@ ThreadElfCore::CreateRegisterContextForF } reg_ctx_sp = m_thread_reg_ctx_sp; - } else if (m_unwinder_ap.get()) { - reg_ctx_sp = m_unwinder_ap->CreateRegisterContextForFrame(frame); + } else { + Unwind *unwinder = GetUnwinder(); + if (unwinder != nullptr) + reg_ctx_sp = unwinder->CreateRegisterContextForFrame(frame); } return reg_ctx_sp; } @@ -340,7 +335,7 @@ size_t ELFLinuxPrPsInfo::GetSize(const l return sizeof(ELFLinuxPrPsInfo); return mips_linux_pr_psinfo_size_o32_n32; } - + switch (arch.GetCore()) { case lldb_private::ArchSpec::eCore_s390x_generic: case lldb_private::ArchSpec::eCore_x86_64_x86_64: @@ -382,9 +377,9 @@ Status ELFLinuxPrPsInfo::Parse(const Dat pr_uid = data.GetU32(&offset); pr_gid = data.GetU32(&offset); } else { - // 16 bit on 32 bit platforms, 32 bit on 64 bit platforms - pr_uid = data.GetMaxU64(&offset, data.GetAddressByteSize() >> 1); - pr_gid = data.GetMaxU64(&offset, data.GetAddressByteSize() >> 1); + // 16 bit on 32 bit platforms, 32 bit on 64 bit platforms + pr_uid = data.GetMaxU64(&offset, data.GetAddressByteSize() >> 1); + pr_gid = data.GetMaxU64(&offset, data.GetAddressByteSize() >> 1); } pr_pid = data.GetU32(&offset); Modified: lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.h?rev=336732&r1=336731&r2=336732&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.h (original) +++ lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.h Tue Jul 10 15:05:33 2018 @@ -146,8 +146,6 @@ public: lldb::RegisterContextSP CreateRegisterContextForFrame(lldb_private::StackFrame *frame) override; - void ClearStackFrames() override; - static bool ThreadIDIsValid(lldb::tid_t thread) { return thread != 0; } const char *GetName() override { Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp?rev=336732&r1=336731&r2=336732&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp Tue Jul 10 15:05:33 2018 @@ -55,7 +55,7 @@ ThreadGDBRemote::~ThreadGDBRemote() { const char *ThreadGDBRemote::GetName() { if (m_thread_name.empty()) - return NULL; + return nullptr; return m_thread_name.c_str(); } @@ -109,7 +109,7 @@ const char *ThreadGDBRemote::GetQueueNam return m_dispatch_queue_name.c_str(); } } - return NULL; + return nullptr; } QueueKind ThreadGDBRemote::GetQueueKind() { @@ -289,8 +289,8 @@ void ThreadGDBRemote::Dump(Log *log, uin bool ThreadGDBRemote::ShouldStop(bool &step_more) { return true; } lldb::RegisterContextSP ThreadGDBRemote::GetRegisterContext() { - if (m_reg_context_sp.get() == NULL) - m_reg_context_sp = CreateRegisterContextForFrame(NULL); + if (!m_reg_context_sp) + m_reg_context_sp = CreateRegisterContextForFrame(nullptr); return m_reg_context_sp; } @@ -317,7 +317,7 @@ ThreadGDBRemote::CreateRegisterContextFo } } else { Unwind *unwinder = GetUnwinder(); - if (unwinder) + if (unwinder != nullptr) reg_ctx_sp = unwinder->CreateRegisterContextForFrame(frame); } return reg_ctx_sp; Modified: lldb/trunk/source/Plugins/Process/mach-core/ThreadMachCore.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/mach-core/ThreadMachCore.cpp?rev=336732&r1=336731&r2=336732&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/mach-core/ThreadMachCore.cpp (original) +++ lldb/trunk/source/Plugins/Process/mach-core/ThreadMachCore.cpp Tue Jul 10 15:05:33 2018 @@ -43,7 +43,7 @@ ThreadMachCore::~ThreadMachCore() { Dest const char *ThreadMachCore::GetName() { if (m_thread_name.empty()) - return NULL; + return nullptr; return m_thread_name.c_str(); } @@ -63,8 +63,8 @@ void ThreadMachCore::RefreshStateAfterSt bool ThreadMachCore::ThreadIDIsValid(lldb::tid_t thread) { return thread != 0; } lldb::RegisterContextSP ThreadMachCore::GetRegisterContext() { - if (m_reg_context_sp.get() == NULL) - m_reg_context_sp = CreateRegisterContextForFrame(NULL); + if (!m_reg_context_sp) + m_reg_context_sp = CreateRegisterContextForFrame(nullptr); return m_reg_context_sp; } @@ -89,7 +89,7 @@ ThreadMachCore::CreateRegisterContextFor reg_ctx_sp = m_thread_reg_ctx_sp; } else { Unwind *unwinder = GetUnwinder(); - if (unwinder) + if (unwinder != nullptr) reg_ctx_sp = unwinder->CreateRegisterContextForFrame(frame); } return reg_ctx_sp; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
