From a4f160dfc1aa508a88a83a9cb1420d762f863408 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani <medismail.bennani@gmail.com>
Date: Wed, 19 Apr 2023 16:53:19 -0700
Subject: [PATCH] [lldb] Fix bug to update process public run lock with process
 state

This patch should address an issue that caused the process public run
lock to not be updated during a process launch/attach when the process
stops.

That caused the public run lock to report its state as running while the
process state is stopped. This prevents the users to interact with the
process (through the command line or via the SBAPI) since it's
considered still running.

To address that, this patch refactors the name of the internal hijack
listeners to a specific pattern `lldb.internal.<action>.hijack` that
are used to ensure that we've attached to or launched a process successfully.

Then, when updating the process public state, after updating the state
value, if the process is not hijacked externally, meaning if the process
doens't have a hijack listener that matches the internal hijack
listeners pattern, we can update the public run lock accordingly.

rdar://108283017

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

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
---
 lldb/include/lldb/Target/Process.h |  7 +++++++
 lldb/source/Target/Process.cpp     | 14 +++++---------
 lldb/source/Target/Target.cpp      |  8 ++++----
 3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index 255ba8c..0c90078 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -386,6 +386,13 @@ public:
 
   static ConstString &GetStaticBroadcasterClass();
 
+  static constexpr llvm::StringRef AttachSynchronousHijackListenerName =
+      "lldb.internal.Process.AttachSynchronous.hijack";
+  static constexpr llvm::StringRef LaunchSynchronousHijackListenerName =
+      "lldb.internal.Process.LaunchSynchronous.hijack";
+  static constexpr llvm::StringRef ResumeSynchronousHijackListenerName =
+      "lldb.internal.Process.ResumeSynchronous.hijack";
+
   ConstString &GetBroadcasterClass() const override {
     return GetStaticBroadcasterClass();
   }
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index ad07647..53b7f98 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -1355,8 +1355,6 @@ Status Process::Resume() {
   return error;
 }
 
-static const char *g_resume_sync_name = "lldb.Process.ResumeSynchronous.hijack";
-
 Status Process::ResumeSynchronous(Stream *stream) {
   Log *log(GetLog(LLDBLog::State | LLDBLog::Process));
   LLDB_LOGF(log, "Process::ResumeSynchronous -- locking run lock");
@@ -1367,7 +1365,7 @@ Status Process::ResumeSynchronous(Stream *stream) {
   }
 
   ListenerSP listener_sp(
-      Listener::MakeListener(g_resume_sync_name));
+      Listener::MakeListener(ResumeSynchronousHijackListenerName.data()));
   HijackProcessEvents(listener_sp);
 
   Status error = PrivateResume();
@@ -1393,9 +1391,8 @@ Status Process::ResumeSynchronous(Stream *stream) {
 
 bool Process::StateChangedIsExternallyHijacked() {
   if (IsHijackedForEvent(eBroadcastBitStateChanged)) {
-    const char *hijacking_name = GetHijackingListenerName();
-    if (hijacking_name &&
-        strcmp(hijacking_name, g_resume_sync_name))
+    llvm::StringRef hijacking_name = GetHijackingListenerName();
+    if (!hijacking_name.starts_with("lldb.internal"))
       return true;
   }
   return false;
@@ -1403,9 +1400,8 @@ bool Process::StateChangedIsExternallyHijacked() {
 
 bool Process::StateChangedIsHijackedForSynchronousResume() {
   if (IsHijackedForEvent(eBroadcastBitStateChanged)) {
-    const char *hijacking_name = GetHijackingListenerName();
-    if (hijacking_name &&
-        strcmp(hijacking_name, g_resume_sync_name) == 0)
+    llvm::StringRef hijacking_name = GetHijackingListenerName();
+    if (hijacking_name == ResumeSynchronousHijackListenerName)
       return true;
   }
   return false;
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 473b23e..c2f8209 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3157,8 +3157,8 @@ Status Target::Launch(ProcessLaunchInfo &launch_info, Stream *stream) {
   // its own hijacking listener or if the process is created by the target
   // manually, without the platform).
   if (!launch_info.GetHijackListener())
-    launch_info.SetHijackListener(
-        Listener::MakeListener("lldb.Target.Launch.hijack"));
+    launch_info.SetHijackListener(Listener::MakeListener(
+        Process::LaunchSynchronousHijackListenerName.data()));
 
   // If we're not already connected to the process, and if we have a platform
   // that can launch a process for debugging, go ahead and do that here.
@@ -3334,8 +3334,8 @@ Status Target::Attach(ProcessAttachInfo &attach_info, Stream *stream) {
   ListenerSP hijack_listener_sp;
   const bool async = attach_info.GetAsync();
   if (!async) {
-    hijack_listener_sp =
-        Listener::MakeListener("lldb.Target.Attach.attach.hijack");
+    hijack_listener_sp = Listener::MakeListener(
+        Process::AttachSynchronousHijackListenerName.data());
     attach_info.SetHijackListener(hijack_listener_sp);
   }
 
-- 
2.7.4