[windows] LLDB shows the wrong values when register read is executed at a frame other than zero

[stella.stamenova]

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.

[aleksandr.urakov]

Looks good to me, except formatting.

source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp

74–81 ↗

(On Diff #154739)

It seems that lines are too long, you can easily reformat this with clang-format.

[tatyana-krasnukha]

Some code style notes

source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp

52 ↗

(On Diff #154739)

Smart pointer's operator bool is intended just for this case, no need in this change.

53 ↗

(On Diff #154739)

Please, use nullptr instead of NULL.

68 ↗

(On Diff #154739)

Revert to using operator bool also.

92 ↗

(On Diff #154739)

Here you check plain pointer, don't cast it to bool - compare with nullptr.

source/Plugins/Process/elf-core/ThreadElfCore.cpp

232 ↗

(On Diff #154739)

ditto

[aleksandr.urakov]

source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp

92 ↗

(On Diff #154739)

What are the disadvantages of casting to bool in comparison with nullptr checking?

[tatyana-krasnukha]

source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp

92 ↗

(On Diff #154739)

That's a matter of opinion, but most coding styles consider implicit casts as less readable. E.g. if you run clang-tidy with enabled readability checks, you get readability-implicit-bool-conversion warning on this code.

[stella.stamenova]

I made the requested changes and I'll update the revision once all the tests pass.

source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp

92 ↗

(On Diff #154739)

I actually intentionally copied this line and several of the others verbatim from the other targets (so that they are the same) but I can fix both places.

[zturner]

I think this is a matter of opinion and the code is fine as is personally.
If you prefer the implicit cast, feel free to leave it. This pattern is
already pervasive all over LLVM and clang, so you can treat it as a nit
(fix it if you agree, leave it if you prefer).

[stella.stamenova]

Update to address style comments

[stella.stamenova]

In D49111#1157633, @zturner wrote:

I think this is a matter of opinion and the code is fine as is personally.
If you prefer the implicit cast, feel free to leave it. This pattern is
already pervasive all over LLVM and clang, so you can treat it as a nit
(fix it if you agree, leave it if you prefer).

It's a small change and it makes things a little more consistent, so I might as well fix it :). Thanks!

[aleksandr.urakov]

LGTM - thank you! But let's wait also for Zachary or Aaron.

[tatyana-krasnukha]

Oops! I meant only your changes when asking to use nullptr instead of NULL;)

Such modernizing is pleasing to the eye, but is worth separate commit, possibly.

lldb/trunk/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp

 #include "lldb/Target/ThreadSpec.h"
 #include "llvm/ADT/SmallString.h"
 
 using namespace lldb;
 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);
 
   // Set the current watchpoints for this thread.
   Target &target = GetProcess()->GetTarget();
   const WatchpointList &wp_list = target.GetWatchpointList();
   size_t wp_size = wp_list.GetSize();
 
void FreeBSDThread::RefreshStateAfterStop() {
   // register supply functions where they check the process stop ID and do the
   // right thing. if (StateIsStoppedState(GetState())
   {
     const bool force = false;
     GetRegisterContext()->InvalidateIfNeeded(force);
   }
 }
 
-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]);
   if (m_thread_name_valid)
     m_thread_name.assign(name);
   else
     m_thread_name.clear();
 }
for (size_t i = 0; i < len / sizeof(*kp); i++) {
         break;
       }
     }
     free(kp);
     m_thread_name_valid = true;
   }
 
   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);
     switch (target_arch.GetMachine()) {
     case llvm::Triple::aarch64:
       reg_interface = new RegisterInfoPOSIX_arm64(target_arch);
       break;
     case llvm::Triple::arm:

 }
 
 bool FreeBSDThread::CalculateStopInfo() {
   SetStopInfo(m_stop_info_sp);
   return true;
 }
 
 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();
 }
 
 void FreeBSDThread::DidStop() {
   // Don't set the thread state to stopped unless we really stopped.
 }
void FreeBSDThread::BreakNotify(const ProcessMessage &message) {
   // breakpoint under the pc.
   if (bp_site) {
     lldb::break_id_t bp_id = bp_site->GetID();
     // If we have an operating system plug-in, we might have set a thread
     // specific breakpoint using the operating system thread ID, so we can't
     // 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;
       SetStopInfo(StopInfo::CreateStopReasonWithBreakpointSiteID(*this, bp_id,
                                                                  should_stop));
     }
   } else
     SetStopInfo(StopInfoSP());
void FreeBSDThread::TraceNotify(const ProcessMessage &message) {
 
   // If the current pc is a breakpoint site then set the StopInfo to
   // Breakpoint. Otherwise, set the StopInfo to Watchpoint or Trace. If we have
   // an operating system plug-in, we might have set a thread specific
   // breakpoint using the operating system thread ID, so we can't make any
   // 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 {
     POSIXBreakpointProtocol *reg_ctx = GetPOSIXBreakpointProtocol();
     if (reg_ctx) {
       uint32_t num_hw_wps = reg_ctx->NumSupportedHardwareWatchpoints();
       uint32_t wp_idx;
       for (wp_idx = 0; wp_idx < num_hw_wps; wp_idx++) {

lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ThreadKDP.cpp

 ThreadKDP::~ThreadKDP() {
   Log *log = ProcessKDPLog::GetLogIfAllCategoriesSet(KDP_LOG_THREAD);
   LLDB_LOG(log, "this = {0}, tid = {1:x}", this, GetID());
   DestroyThread();
 }
 
 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
   // true because the stop reply packet might have had some register values
   // that were expedited and these will already be copied into the register
   // context by the time this function gets called. The KDPRegisterContext
   // class has been made smart enough to detect when it needs to invalidate
   // which registers are valid by putting hooks in the register read and
   // register supply functions where they check the process stop ID and do the
   // right thing.
   const bool force = false;
   lldb::RegisterContextSP reg_ctx_sp(GetRegisterContext());
   if (reg_ctx_sp)
     reg_ctx_sp->InvalidateIfNeeded(force);
 }
 
 bool ThreadKDP::ThreadIDIsValid(lldb::tid_t thread) { return thread != 0; }
 
 void ThreadKDP::Dump(Log *log, uint32_t index) {}
 
 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;
 }
 
 lldb::RegisterContextSP
 ThreadKDP::CreateRegisterContextForFrame(StackFrame *frame) {
   lldb::RegisterContextSP reg_ctx_sp;
   uint32_t concrete_frame_idx = 0;
 
if (process_sp) {
             new RegisterContextKDP_x86_64(*this, concrete_frame_idx));
         break;
       default:
         llvm_unreachable("Add CPU type support in KDP");
       }
     }
   } else {
     Unwind *unwinder = GetUnwinder();
-    if (unwinder)
+    if (unwinder != nullptr)
       reg_ctx_sp = unwinder->CreateRegisterContextForFrame(frame);
   }
   return reg_ctx_sp;
 }
 
 bool ThreadKDP::CalculateStopInfo() {
   ProcessSP process_sp(GetProcess());
   if (process_sp) {

lldb/trunk/source/Plugins/Process/Utility/ThreadMemory.cpp

ThreadMemory::CreateRegisterContextForFrame(StackFrame *frame) {
 
   if (frame)
     concrete_frame_idx = frame->GetConcreteFrameIndex();
 
   if (concrete_frame_idx == 0) {
     reg_ctx_sp = GetRegisterContext();
   } else {
     Unwind *unwinder = GetUnwinder();
-    if (unwinder)
+    if (unwinder != nullptr)
       reg_ctx_sp = unwinder->CreateRegisterContextForFrame(frame);
   }
   return reg_ctx_sp;
 }
 
 bool ThreadMemory::CalculateStopInfo() {
   if (m_backing_thread_sp) {
     lldb::StopInfoSP backing_stop_info_sp(

lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.h

public:
   bool CalculateStopInfo() override;
   Unwind *GetUnwinder() override;
 
   Status DoResume();
 
   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

lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp

 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/HostNativeThreadBase.h"
 #include "lldb/Host/windows/HostThreadWindows.h"
 #include "lldb/Host/windows/windows.h"
 #include "lldb/Target/RegisterContext.h"
 #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"
 #else
 #include "x86/RegisterContextWindows_x86.h"
 #endif
 
 using namespace lldb;
 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(); }
 
 void TargetThreadWindows::RefreshStateAfterStop() {
   ::SuspendThread(m_host_thread.GetNativeThread().GetSystemHandle());
   SetState(eStateStopped);
   GetRegisterContext()->InvalidateIfNeeded(false);
 }
 
 void TargetThreadWindows::WillResume(lldb::StateType resume_state) {}
 
 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 reg_ctx_sp;
+  uint32_t concrete_frame_idx = 0;
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD));
 
-RegisterContextSP
-TargetThreadWindows::CreateRegisterContextForFrameIndex(uint32_t idx) {
-  if (!m_reg_context_sp) {
+  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
+        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:
 #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;
       }
     }
-  return m_reg_context_sp;
+    reg_ctx_sp = m_thread_reg_ctx_sp;
+  } else {
+    Unwind *unwinder = GetUnwinder();
+    if (unwinder != nullptr)
+      reg_ctx_sp = unwinder->CreateRegisterContextForFrame(frame);
+  }
+
+  return reg_ctx_sp;
 }
 
 bool TargetThreadWindows::CalculateStopInfo() {
   SetStopInfo(m_stop_info_sp);
   return true;
 }
 
 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();
 }
 
 Status TargetThreadWindows::DoResume() {
   StateType resume_state = GetTemporaryResumeState();
   StateType current_state = GetState();
   if (resume_state == current_state)
     return Status();
 
   if (resume_state == eStateStepping) {
     uint32_t flags_index =
         GetRegisterContext()->ConvertRegisterKindToRegisterNumber(
             eRegisterKindGeneric, LLDB_REGNUM_GENERIC_FLAGS);
     uint64_t flags_value =
         GetRegisterContext()->ReadRegisterAsUnsigned(flags_index, 0);
     flags_value |= 0x100; // Set the trap flag on the CPU
     GetRegisterContext()->WriteRegisterFromUnsigned(flags_index, flags_value);
   }
 
   if (resume_state == eStateStepping || resume_state == eStateRunning) {
     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)
         return Status(::GetLastError(), eErrorTypeWin32);
 
     } while (previous_suspend_count > 1);
   }
 
   return Status();
 }

lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.h

public:
 
   void RefreshStateAfterStop() override;
 
   lldb::RegisterContextSP GetRegisterContext() override;
 
   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 {
     if (m_thread_name.empty())
       return NULL;
     return m_thread_name.c_str();
   }
 

lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.cpp

: Thread(process, td.tid), m_thread_name(td.name), m_thread_reg_ctx_sp(),
       m_signo(td.signo), m_gpregset_data(td.gpregset), m_notes(td.notes) {}
 
 ThreadElfCore::~ThreadElfCore() { DestroyThread(); }
 
 void ThreadElfCore::RefreshStateAfterStop() {
   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;
 }
 
 RegisterContextSP
 ThreadElfCore::CreateRegisterContextForFrame(StackFrame *frame) {
   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)
       return m_thread_reg_ctx_sp;
 
     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: {
       switch (arch.GetMachine()) {
       case llvm::Triple::aarch64:
         reg_interface = new RegisterInfoPOSIX_arm64(arch);
         break;
       case llvm::Triple::arm:
case llvm::Triple::x86_64:
       m_thread_reg_ctx_sp.reset(new RegisterContextCorePOSIX_x86_64(
           *this, reg_interface, m_gpregset_data, m_notes));
       break;
     default:
       break;
     }
 
     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;
 }
 
 bool ThreadElfCore::CalculateStopInfo() {
   ProcessSP process_sp(GetProcess());
   if (process_sp) {
     SetStopInfo(StopInfo::CreateStopReasonWithSignal(*this, m_signo));

 size_t ELFLinuxPrPsInfo::GetSize(const lldb_private::ArchSpec &arch) {
   constexpr size_t mips_linux_pr_psinfo_size_o32_n32 = 128;
   if (arch.IsMIPS()) {
     uint8_t address_byte_size = arch.GetAddressByteSize();
     if (address_byte_size == 8)
       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:
     return sizeof(ELFLinuxPrPsInfo);
   case lldb_private::ArchSpec::eCore_x86_32_i386:
   case lldb_private::ArchSpec::eCore_x86_32_i486:
     return 124;
   default:
Status ELFLinuxPrPsInfo::Parse(const DataExtractor &data,
 
   pr_flag = data.GetPointer(&offset);
 
   if (arch.IsMIPS()) {
     // The pr_uid and pr_gid is always 32 bit irrespective of platforms
     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);
   }
 
   pr_pid = data.GetU32(&offset);
   pr_ppid = data.GetU32(&offset);
   pr_pgrp = data.GetU32(&offset);
   pr_sid = data.GetU32(&offset);
 
   size = 16;

lldb/trunk/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp

ThreadGDBRemote::~ThreadGDBRemote() {
   Log *log(GetLogIfAnyCategoriesSet(GDBR_LOG_THREAD));
   LLDB_LOG(log, "this = {0}, pid = {1}, tid = {2}", this,
            process_sp ? process_sp->GetID() : LLDB_INVALID_PROCESS_ID, GetID());
   DestroyThread();
 }
 
 const char *ThreadGDBRemote::GetName() {
   if (m_thread_name.empty())
-    return NULL;
+    return nullptr;
   return m_thread_name.c_str();
 }
 
 void ThreadGDBRemote::ClearQueueInfo() {
   m_dispatch_queue_name.clear();
   m_queue_kind = eQueueKindUnknown;
   m_queue_serial_number = 0;
   m_dispatch_queue_t = LLDB_INVALID_ADDRESS;
if (process_sp) {
             runtime->GetQueueNameFromThreadQAddress(m_thread_dispatch_qaddr);
       else
         m_dispatch_queue_name.clear();
 
       if (!m_dispatch_queue_name.empty())
         return m_dispatch_queue_name.c_str();
     }
   }
-  return NULL;
+  return nullptr;
 }
 
 QueueKind ThreadGDBRemote::GetQueueKind() {
   // If our cached queue info is valid, then someone called
   // ThreadGDBRemote::SetQueueInfo(...) with valid information that was gleaned
   // from the stop reply packet. In this case we trust that the info is valid
   // in m_dispatch_queue_name without refetching it
   if (CachedQueueInfoIsValid()) {

 bool ThreadGDBRemote::ThreadIDIsValid(lldb::tid_t thread) {
   return thread != 0;
 }
 
 void ThreadGDBRemote::Dump(Log *log, uint32_t index) {}
 
 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;
 }
 
 lldb::RegisterContextSP
 ThreadGDBRemote::CreateRegisterContextForFrame(StackFrame *frame) {
   lldb::RegisterContextSP reg_ctx_sp;
   uint32_t concrete_frame_idx = 0;
 
if (process_sp) {
       bool read_all_registers_at_once =
           !gdb_process->GetGDBRemote().GetpPacketSupported(GetID());
       reg_ctx_sp.reset(new GDBRemoteRegisterContext(
           *this, concrete_frame_idx, gdb_process->m_register_info,
           read_all_registers_at_once));
     }
   } else {
     Unwind *unwinder = GetUnwinder();
-    if (unwinder)
+    if (unwinder != nullptr)
       reg_ctx_sp = unwinder->CreateRegisterContextForFrame(frame);
   }
   return reg_ctx_sp;
 }
 
 bool ThreadGDBRemote::PrivateSetRegisterValue(uint32_t reg,
                                               llvm::ArrayRef<uint8_t> data) {
   GDBRemoteRegisterContext *gdb_reg_ctx =

lldb/trunk/source/Plugins/Process/mach-core/ThreadMachCore.cpp

 ThreadMachCore::ThreadMachCore(Process &process, lldb::tid_t tid)
     : Thread(process, tid), m_thread_name(), m_dispatch_queue_name(),
       m_thread_dispatch_qaddr(LLDB_INVALID_ADDRESS), m_thread_reg_ctx_sp() {}
 
 ThreadMachCore::~ThreadMachCore() { DestroyThread(); }
 
 const char *ThreadMachCore::GetName() {
   if (m_thread_name.empty())
-    return NULL;
+    return nullptr;
   return m_thread_name.c_str();
 }
 
 void ThreadMachCore::RefreshStateAfterStop() {
   // Invalidate all registers in our register context. We don't set "force" to
   // true because the stop reply packet might have had some register values
   // that were expedited and these will already be copied into the register
   // context by the time this function gets called. The KDPRegisterContext
   // class has been made smart enough to detect when it needs to invalidate
   // which registers are valid by putting hooks in the register read and
   // register supply functions where they check the process stop ID and do the
   // right thing.
   const bool force = false;
   GetRegisterContext()->InvalidateIfNeeded(force);
 }
 
 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;
 }
 
 lldb::RegisterContextSP
 ThreadMachCore::CreateRegisterContextForFrame(StackFrame *frame) {
   lldb::RegisterContextSP reg_ctx_sp;
   uint32_t concrete_frame_idx = 0;
 
   if (frame)
     concrete_frame_idx = frame->GetConcreteFrameIndex();
 
   if (concrete_frame_idx == 0) {
     if (!m_thread_reg_ctx_sp) {
       ProcessSP process_sp(GetProcess());
 
       ObjectFile *core_objfile =
           static_cast<ProcessMachCore *>(process_sp.get())->GetCoreObjectFile();
       if (core_objfile)
         m_thread_reg_ctx_sp =
             core_objfile->GetThreadContextAtIndex(GetID(), *this);
     }
     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;
 }
 
 bool ThreadMachCore::CalculateStopInfo() {
   ProcessSP process_sp(GetProcess());
   if (process_sp) {
     SetStopInfo(StopInfo::CreateStopReasonWithSignal(*this, SIGSTOP));
     return true;
   }
   return false;
 }