[Lldb-commits] [lldb] Fix a crasher when using the public API. (PR #80508)

[Greg Clayton via lldb-commits]

https://github.com/clayborg updated https://github.com/llvm/llvm-project/pull/80508

>From c416b6f4c0a00684057947782413b66af4c197f3 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Fri, 2 Feb 2024 15:30:40 -0800
Subject: [PATCH 1/4] Fix a crasher when using the public API.

A user found a crash when they would do code like:
(lldb) script
>>> target = lldb.SBTarget()
>>> lldb.debugger.SetSelectedTarget(target)

We were not checking if the target was valid in SBDebugger::SetSelectedTarget(...).
---
 lldb/source/API/SBDebugger.cpp                   | 14 +++++++-------
 lldb/test/API/python_api/target/TestTargetAPI.py |  6 ++++++
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index fbcf30e67fc1cd..12cbe25a540eba 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1089,9 +1089,9 @@ void SBDebugger::SetSelectedTarget(SBTarget &sb_target) {
   Log *log = GetLog(LLDBLog::API);
 
   TargetSP target_sp(sb_target.GetSP());
-  if (m_opaque_sp) {
+  if (m_opaque_sp && target_sp)
     m_opaque_sp->GetTargetList().SetSelectedTarget(target_sp);
-  }
+
   if (log) {
     SBStream sstr;
     sb_target.GetDescription(sstr, eDescriptionLevelBrief);
@@ -1704,20 +1704,20 @@ SBDebugger::LoadTraceFromFile(SBError &error,
 
 void SBDebugger::RequestInterrupt() {
   LLDB_INSTRUMENT_VA(this);
-  
+
   if (m_opaque_sp)
-    m_opaque_sp->RequestInterrupt();  
+    m_opaque_sp->RequestInterrupt();
 }
 void SBDebugger::CancelInterruptRequest()  {
   LLDB_INSTRUMENT_VA(this);
-  
+
   if (m_opaque_sp)
-    m_opaque_sp->CancelInterruptRequest();  
+    m_opaque_sp->CancelInterruptRequest();
 }
 
 bool SBDebugger::InterruptRequested()   {
   LLDB_INSTRUMENT_VA(this);
-  
+
   if (m_opaque_sp)
     return m_opaque_sp->InterruptRequested();
   return false;
diff --git a/lldb/test/API/python_api/target/TestTargetAPI.py b/lldb/test/API/python_api/target/TestTargetAPI.py
index c8e1904428c8aa..63d34340a8836e 100644
--- a/lldb/test/API/python_api/target/TestTargetAPI.py
+++ b/lldb/test/API/python_api/target/TestTargetAPI.py
@@ -526,3 +526,9 @@ def test_is_loaded(self):
                 target.IsLoaded(module),
                 "Running the target should " "have loaded its modules.",
             )
+
+    @no_debug_info_test
+    def test_setting_selected_target_with_invalid_target(self):
+        """Make sure we don't crash when trying to select invalid target."""
+        target = lldb.SBTarget()
+        self.dbg.SetSelectedTarget(target)

>From 1473e88ff82ea45a76e1ab65ef471c78c0bc4f70 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Tue, 6 Feb 2024 12:27:58 -0800
Subject: [PATCH 2/4] Do the target check in TargetList::SetSelectedTarget()
 and also check if the target is valid.

---
 lldb/source/API/SBDebugger.cpp    |  2 +-
 lldb/source/Target/TargetList.cpp | 16 ++++++++++------
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 12cbe25a540eba..6a204228cb440e 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1089,7 +1089,7 @@ void SBDebugger::SetSelectedTarget(SBTarget &sb_target) {
   Log *log = GetLog(LLDBLog::API);
 
   TargetSP target_sp(sb_target.GetSP());
-  if (m_opaque_sp && target_sp)
+  if (m_opaque_sp)
     m_opaque_sp->GetTargetList().SetSelectedTarget(target_sp);
 
   if (log) {
diff --git a/lldb/source/Target/TargetList.cpp b/lldb/source/Target/TargetList.cpp
index 121b6253d2a59d..bfab46b7ea61fe 100644
--- a/lldb/source/Target/TargetList.cpp
+++ b/lldb/source/Target/TargetList.cpp
@@ -532,9 +532,13 @@ void TargetList::SetSelectedTarget(uint32_t index) {
 }
 
 void TargetList::SetSelectedTarget(const TargetSP &target_sp) {
-  std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
-  auto it = llvm::find(m_target_list, target_sp);
-  SetSelectedTargetInternal(std::distance(m_target_list.begin(), it));
+  // Don't allow an invalid target shared pointer or a target that has been
+  // destroyed to become the selected target.
+  if (target_sp && target_sp->IsValid()) {
+    std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
+    auto it = llvm::find(m_target_list, target_sp);
+    SetSelectedTargetInternal(std::distance(m_target_list.begin(), it));
+  }
 }
 
 lldb::TargetSP TargetList::GetSelectedTarget() {
@@ -564,15 +568,15 @@ bool TargetList::AnyTargetContainsModule(Module &module) {
         m_in_process_target_list.insert(target_sp);
     assert(was_added && "Target pointer was left in the in-process map");
   }
-  
+
   void TargetList::UnregisterInProcessTarget(TargetSP target_sp) {
     std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
     [[maybe_unused]] bool was_present =
         m_in_process_target_list.erase(target_sp);
     assert(was_present && "Target pointer being removed was not registered");
   }
-  
+
   bool TargetList::IsTargetInProcess(TargetSP target_sp) {
     std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
-    return m_in_process_target_list.count(target_sp) == 1; 
+    return m_in_process_target_list.count(target_sp) == 1;
   }

>From 3eef993dc14e5edcc7f61805204aa90cdc361548 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Tue, 6 Feb 2024 12:32:16 -0800
Subject: [PATCH 3/4] Undo changes to SBDebugger.cpp

---
 lldb/source/API/SBDebugger.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 6a204228cb440e..dec1b2c6bd1bfc 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1089,8 +1089,9 @@ void SBDebugger::SetSelectedTarget(SBTarget &sb_target) {
   Log *log = GetLog(LLDBLog::API);
 
   TargetSP target_sp(sb_target.GetSP());
-  if (m_opaque_sp)
+  if (m_opaque_sp) {
     m_opaque_sp->GetTargetList().SetSelectedTarget(target_sp);
+  }
 
   if (log) {
     SBStream sstr;

>From bd37d0242e65edf5c3e9af19d3a70ec78ebd6e2d Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Tue, 6 Feb 2024 12:32:50 -0800
Subject: [PATCH 4/4] Remove space.

---
 lldb/source/API/SBDebugger.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index dec1b2c6bd1bfc..36a763956011b0 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1092,7 +1092,6 @@ void SBDebugger::SetSelectedTarget(SBTarget &sb_target) {
   if (m_opaque_sp) {
     m_opaque_sp->GetTargetList().SetSelectedTarget(target_sp);
   }
-
   if (log) {
     SBStream sstr;
     sb_target.GetDescription(sstr, eDescriptionLevelBrief);