[lldb/Plugins] Fix ScriptedThread IndexID reporting

When listing all the Scripted Threads of a ScriptedProcess, we can see that all
have the thread index set to 1. This is caused by the lldb_private::Thread
constructor, which sets the m_index_id member using the provided thread id `tid`.

Because the call to the super constructor is done before instantiating
the `ScriptedThreadInterface`, lldb can't fetch the thread id from the
script instance, so it uses `LLDB_INVALID_THREAD_ID` instead.

To mitigate this, this patch takes advantage of the `ScriptedThread::Create`
fallible constructor idiom to defer calling the `ScriptedThread` constructor
(and the `Thread` super constructor with it), until we can fetch a valid
thread id `tid` from the `ScriptedThreadInterface`.

rdar://87432065

Differential Revision: https://reviews.llvm.org/D117076

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>

lldb/include/lldb/Target/Thread.h

@@ -1244,7 +1244,7 @@ class Thread : public std::enable_shared_from_this<Thread>,
                                          // the stop info was checked against
                                          // the stop info override
   const uint32_t m_index_id; ///< A unique 1 based index assigned to each thread
-                             ///for easy UI/command line access.
+                             /// for easy UI/command line access.
   lldb::RegisterContextSP m_reg_context_sp; ///< The register context for this
                                             ///thread's current register state.
   lldb::StateType m_state;                  ///< The state of our process.

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

@@ -328,12 +328,14 @@ bool ScriptedProcess::DoUpdateThreadList(ThreadList &old_thread_list,
       return true;
     }
 
-    lldb::ThreadSP thread_sp =
-        std::make_shared<ScriptedThread>(*this, error, val->GetAsGeneric());
+    auto thread_or_error = ScriptedThread::Create(*this, val->GetAsGeneric());
 
-    if (!thread_sp || error.Fail())
-      return GetInterface().ErrorWithMessage<bool>(LLVM_PRETTY_FUNCTION,
-                                                   error.AsCString(), error);
+    if (!thread_or_error)
+      return GetInterface().ErrorWithMessage<bool>(
+          LLVM_PRETTY_FUNCTION, toString(thread_or_error.takeError()), error);
+
+    ThreadSP thread_sp = thread_or_error.get();
+    lldbassert(thread_sp && "Couldn't initialize scripted thread.");
 
     RegisterContextSP reg_ctx_sp = thread_sp->GetRegisterContext();
     if (!reg_ctx_sp)

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

@@ -28,52 +28,60 @@ void ScriptedThread::CheckInterpreterAndScriptObject() const {
   lldbassert(GetInterface() && "Invalid Scripted Thread Interface.");
 }
 
-ScriptedThread::ScriptedThread(ScriptedProcess &process, Status &error,
-                               StructuredData::Generic *script_object)
-    : Thread(process, LLDB_INVALID_THREAD_ID), m_scripted_process(process),
-      m_scripted_thread_interface_sp(
-          m_scripted_process.GetInterface().CreateScriptedThreadInterface()) {
-  if (!process.IsValid()) {
-    error.SetErrorString("Invalid scripted process");
-    return;
-  }
+llvm::Expected<std::shared_ptr<ScriptedThread>>
+ScriptedThread::Create(ScriptedProcess &process,
+                       StructuredData::Generic *script_object) {
+  if (!process.IsValid())
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "Invalid scripted process.");
 
   process.CheckInterpreterAndScriptObject();
 
-  auto scripted_thread_interface = GetInterface();
-  if (!scripted_thread_interface) {
-    error.SetErrorString("Failed to get scripted thread interface.");
-    return;
-  }
-
-  llvm::Optional<std::string> class_name =
-      process.GetInterface().GetScriptedThreadPluginName();
-  if (!class_name || class_name->empty()) {
-    error.SetErrorString("Failed to get scripted thread class name.");
-    return;
+  auto scripted_thread_interface =
+      process.GetInterface().CreateScriptedThreadInterface();
+  if (!scripted_thread_interface)
+    return llvm::createStringError(
+        llvm::inconvertibleErrorCode(),
+        "Failed to create scripted thread interface.");
+
+  llvm::StringRef thread_class_name;
+  if (!script_object) {
+    llvm::Optional<std::string> class_name =
+        process.GetInterface().GetScriptedThreadPluginName();
+    if (!class_name || class_name->empty())
+      return llvm::createStringError(
+          llvm::inconvertibleErrorCode(),
+          "Failed to get scripted thread class name.");
+    thread_class_name = *class_name;
   }
 
   ExecutionContext exe_ctx(process);
-
-  m_script_object_sp = scripted_thread_interface->CreatePluginObject(
-      class_name->c_str(), exe_ctx, process.m_scripted_process_info.GetArgsSP(),
-      script_object);
-
-  if (!m_script_object_sp) {
-    error.SetErrorString("Failed to create script object");
-    return;
-  }
-
-  if (!m_script_object_sp->IsValid()) {
-    m_script_object_sp = nullptr;
-    error.SetErrorString("Created script object is invalid");
-    return;
-  }
+  StructuredData::GenericSP owned_script_object_sp =
+      scripted_thread_interface->CreatePluginObject(
+          thread_class_name, exe_ctx,
+          process.m_scripted_process_info.GetArgsSP(), script_object);
+
+  if (!owned_script_object_sp)
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "Failed to create script object.");
+  if (!owned_script_object_sp->IsValid())
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "Created script object is invalid.");
 
   lldb::tid_t tid = scripted_thread_interface->GetThreadID();
-  SetID(tid);
+
+  return std::make_shared<ScriptedThread>(process, scripted_thread_interface,
+                                          tid, owned_script_object_sp);
 }
 
+ScriptedThread::ScriptedThread(ScriptedProcess &process,
+                               ScriptedThreadInterfaceSP interface_sp,
+                               lldb::tid_t tid,
+                               StructuredData::GenericSP script_object_sp)
+    : Thread(process, tid), m_scripted_process(process),
+      m_scripted_thread_interface_sp(interface_sp),
+      m_script_object_sp(script_object_sp) {}
+
 ScriptedThread::~ScriptedThread() { DestroyThread(); }
 
 const char *ScriptedThread::GetName() {

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

@@ -25,12 +25,18 @@ class ScriptedProcess;
 namespace lldb_private {
 
 class ScriptedThread : public lldb_private::Thread {
+
 public:
-  ScriptedThread(ScriptedProcess &process, Status &error,
-                 StructuredData::Generic *script_object = nullptr);
+  ScriptedThread(ScriptedProcess &process,
+                 lldb::ScriptedThreadInterfaceSP interface_sp, lldb::tid_t tid,
+                 StructuredData::GenericSP script_object_sp = nullptr);
 
   ~ScriptedThread() override;
 
+  static llvm::Expected<std::shared_ptr<ScriptedThread>>
+  Create(ScriptedProcess &process,
+         StructuredData::Generic *script_object = nullptr);
+
   lldb::RegisterContextSP GetRegisterContext() override;
 
   lldb::RegisterContextSP
@@ -61,8 +67,8 @@ class ScriptedThread : public lldb_private::Thread {
 
   const ScriptedProcess &m_scripted_process;
   lldb::ScriptedThreadInterfaceSP m_scripted_thread_interface_sp = nullptr;
+  lldb_private::StructuredData::GenericSP m_script_object_sp = nullptr;
   std::shared_ptr<DynamicRegisterInfo> m_register_info_sp = nullptr;
-  lldb_private::StructuredData::ObjectSP m_script_object_sp = nullptr;
 };
 
 } // namespace lldb_private

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp

@@ -32,7 +32,7 @@ ScriptedThreadPythonInterface::ScriptedThreadPythonInterface(
 StructuredData::GenericSP ScriptedThreadPythonInterface::CreatePluginObject(
     const llvm::StringRef class_name, ExecutionContext &exe_ctx,
     StructuredData::DictionarySP args_sp, StructuredData::Generic *script_obj) {
-  if (class_name.empty())
+  if (class_name.empty() && !script_obj)
     return {};
 
   ProcessSP process_sp = exe_ctx.GetProcessSP();