Fix a crasher when using the public API.

[clayborg]

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(...).

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Greg Clayton (clayborg)

Changes

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(...).

---

Full diff: https://github.com/llvm/llvm-project/pull/80508.diff

2 Files Affected:

• (modified) lldb/source/API/SBDebugger.cpp (+7-7)
• (modified) lldb/test/API/python_api/target/TestTargetAPI.py (+6)

diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index fbcf30e67fc1c..12cbe25a540eb 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 c8e1904428c8a..63d34340a8836 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)

[jimingham]

Do we also want to check target_sp->IsValid() here?

[jimingham]

Actually, is there any circumstance in which we'd want to set the selected target to an empty or invalid target? I don't think so, in which case we should probably do that checking in TargetList::SetSelectedTarget.

[kusmour]

Had a discussion with Greg earlier:

A default constructed SBTarget should have an invalid shared pointer, so this shouldn't ever try to be added to the list...

So I think the change here is good

[kusmour]

probs should remove the irrelevant whitespace changes
Otherwise looking good!

[kusmour]

Had a discussion with Greg earlier:

A default constructed SBTarget should have an invalid shared pointer, so this shouldn't ever try to be added to the list...

So I think the change here is good

[kusmour]

revert whitespace changes and we're good to go!