[lldb/Plugins] Refactor ScriptedThread register context creation

[mib]

Summary

This patch changes the ScriptedThread class to create the register
context when Process::RefreshStateAfterStop is called rather than
doing it in the thread constructor.

This is required to update the thread state for execution control
support.

Signed-off-by: Med Ismail Bennani <[e-mail]>

[shafik]

lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

186

It should be /*force=*/false

See clang-tidy - bugprone-argument-comment

[shafik]

lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

99

const

[mib]

Address @shafik comment.

[JDevlieghere]

lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

105

It looks like this error is set but never used/returned? Should this return an Expected<RegisterContextSP> with the caller dealing with the error?

[mib]

lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

105

This follows the Thread::CreateRegisterContextForFrame base definition, returning a RegisterContextSP and not taking a Status& as an argument. I agree this should be refactored but that should be done on a separate patch.

For now, I refactored the code to log the error on the thread channel.

[mib]

Addressed @JDevlieghere & @shafik comments.

[JDevlieghere]

LGTM

lldb/source/Plugins/Process/scripted/ScriptedProcess.h

public:
   Status DoLaunch(Module *exe_module, ProcessLaunchInfo &launch_info) override;
 
   void DidLaunch() override;
 
   Status DoResume() override;
 
   Status DoDestroy() override;
 
-  void RefreshStateAfterStop() override{};
+  void RefreshStateAfterStop() override;
 
   bool IsAlive() override;
 
   size_t DoReadMemory(lldb::addr_t addr, void *buf, size_t size,
                       Status &error) override;
 
   ArchSpec GetArchitecture();
 

lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp

 #include "lldb/Host/OptionParser.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/OptionArgParser.h"
 #include "lldb/Interpreter/OptionGroupBoolean.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/RegisterContext.h"
-
 #include "lldb/Utility/State.h"
 
 #include <mutex>
 
 LLDB_PLUGIN_DEFINE(ScriptedProcess)
 
 using namespace lldb;
 using namespace lldb_private;

 bool ScriptedProcess::DoUpdateThreadList(ThreadList &old_thread_list,
                                          ThreadList &new_thread_list) {
   // TODO: Implement
   // This is supposed to get the current set of threads, if any of them are in
   // old_thread_list then they get copied to new_thread_list, and then any
   // actually new threads will get added to new_thread_list.
 
   CheckInterpreterAndScriptObject();
+  m_thread_plans.ClearThreadCache();
 
   Status error;
   ScriptLanguage language = m_interpreter->GetLanguage();
 
   if (language != eScriptLanguagePython)
     return GetInterface().ErrorWithMessage<bool>(
         LLVM_PRETTY_FUNCTION,
         llvm::Twine("ScriptInterpreter language (" +
if (!thread_sp || error.Fail())
     return GetInterface().ErrorWithMessage<bool>(LLVM_PRETTY_FUNCTION,
                                                  error.AsCString(), error);
 
   new_thread_list.AddThread(thread_sp);
 
   return new_thread_list.GetSize(false) > 0;
 }
 
+void ScriptedProcess::RefreshStateAfterStop() {
+  // Let all threads recover from stopping and do any clean up based on the
+  // previous thread state (if any).
+  m_thread_list.RefreshStateAfterStop();
+}
+
 bool ScriptedProcess::GetProcessInfo(ProcessInstanceInfo &info) {
   info.Clear();
   info.SetProcessID(GetID());
   info.SetArchitecture(GetArchitecture());
   lldb::ModuleSP module_sp = GetTarget().GetExecutableModule();
   if (module_sp) {
     const bool add_exe_file_as_first_arg = false;
     info.SetExecutableFile(GetTarget().GetExecutableModule()->GetFileSpec(),
                            add_exe_file_as_first_arg);
   }
   return true;
 }
 
 ScriptedProcessInterface &ScriptedProcess::GetInterface() const {
   return m_interpreter->GetScriptedProcessInterface();
 }

lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

ScriptedThread::ScriptedThread(ScriptedProcess &process, Status &error)
   if (!object_sp || !object_sp->IsValid()) {
     error.SetErrorString("Failed to create valid script object");
     return;
   }
 
   m_script_object_sp = object_sp;
 
   SetID(scripted_thread_interface->GetThreadID());
-
-  llvm::Optional<std::string> reg_data =
-      scripted_thread_interface->GetRegisterContext();
-  if (!reg_data) {
-    error.SetErrorString("Failed to get scripted thread registers data.");
-    return;
-  }
-
-  DataBufferSP data_sp(
-      std::make_shared<DataBufferHeap>(reg_data->c_str(), reg_data->size()));
-
-  if (!data_sp->GetByteSize()) {
-    error.SetErrorString("Failed to copy raw registers data.");
-    return;
-  }
-
-  std::shared_ptr<RegisterContextMemory> reg_ctx_memory =
-      std::make_shared<RegisterContextMemory>(
-          *this, 0, *GetDynamicRegisterInfo(), LLDB_INVALID_ADDRESS);
-  if (!reg_ctx_memory) {
-    error.SetErrorString("Failed to create a register context.");
-    return;
-  }
-
-  reg_ctx_memory->SetAllRegisterData(data_sp);
-  m_reg_context_sp = reg_ctx_memory;
 }
 
 ScriptedThread::~ScriptedThread() { DestroyThread(); }
 
 const char *ScriptedThread::GetName() {
   CheckInterpreterAndScriptObject();
   llvm::Optional<std::string> thread_name = GetInterface()->GetName();
   if (!thread_name)
const char *ScriptedThread::GetQueueName() {
   return ConstString(queue_name->c_str()).AsCString();
 }
 
 void ScriptedThread::WillResume(StateType resume_state) {}
 
 void ScriptedThread::ClearStackFrames() { Thread::ClearStackFrames(); }
 
 RegisterContextSP ScriptedThread::GetRegisterContext() {
-  if (!m_reg_context_sp) {
-    m_reg_context_sp = std::make_shared<RegisterContextThreadMemory>(
-        *this, LLDB_INVALID_ADDRESS);
-    GetInterface()->GetRegisterContext();
-  }
+  if (!m_reg_context_sp)
+    m_reg_context_sp = CreateRegisterContextForFrame(nullptr);
   return m_reg_context_sp;
 }
 
 RegisterContextSP
 ScriptedThread::CreateRegisterContextForFrame(StackFrame *frame) {
   uint32_t concrete_frame_idx = frame ? frame->GetConcreteFrameIndex() : 0;
 
-  if (concrete_frame_idx == 0)
-    return GetRegisterContext();
+  if (concrete_frame_idx)
     return GetUnwinder().CreateRegisterContextForFrame(frame);
+
+  lldb::RegisterContextSP reg_ctx_sp;
+  Status error;
+
+  llvm::Optional<std::string> reg_data = GetInterface()->GetRegisterContext();
+  if (!reg_data) {
+    error.SetErrorString("Failed to get scripted thread registers data.");
+    return nullptr;
+  }
+
+  DataBufferSP data_sp(
+      std::make_shared<DataBufferHeap>(reg_data->c_str(), reg_data->size()));
+
+  if (!data_sp->GetByteSize()) {
+    error.SetErrorString("Failed to copy raw registers data.");
+    return nullptr;
+  }
+
+  std::shared_ptr<RegisterContextMemory> reg_ctx_memory =
+      std::make_shared<RegisterContextMemory>(
+          *this, 0, *GetDynamicRegisterInfo(), LLDB_INVALID_ADDRESS);
+  if (!reg_ctx_memory) {
+    error.SetErrorString("Failed to create a register context.");
+    return nullptr;
+  }
+
+  reg_ctx_memory->SetAllRegisterData(data_sp);
+  m_reg_context_sp = reg_ctx_memory;
+
+  return m_reg_context_sp;
 }
 
 bool ScriptedThread::CalculateStopInfo() {
   StructuredData::DictionarySP dict_sp = GetInterface()->GetStopReason();
 
   Status error;
   lldb::StopInfoSP stop_info_sp;
   lldb::StopReason stop_reason_type;
return GetInterface()->ErrorWithMessage<bool>(
         error);
   }
 
   SetStopInfo(stop_info_sp);
   return true;
 }
 
 void ScriptedThread::RefreshStateAfterStop() {
-  // TODO: Implement
-  if (m_reg_context_sp)
-    m_reg_context_sp->InvalidateAllRegisters();
+  GetRegisterContext()->InvalidateIfNeeded(false /*=force*/);
 }
 
 lldb::ScriptedThreadInterfaceSP ScriptedThread::GetInterface() const {
   return m_scripted_process.GetInterface().GetScriptedThreadInterface();
 }
 
 std::shared_ptr<DynamicRegisterInfo> ScriptedThread::GetDynamicRegisterInfo() {
   CheckInterpreterAndScriptObject();