[Lldb-commits] [lldb] [lldb] Fix CommandObjects that don't set a return status (PR #196588)

[via lldb-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

<details>
<summary>Changes</summary>

Several CommandObject subclasses had DoExecute paths that returned without ever calling SetStatus on the CommandReturnObject. The status was silently left at its initial eReturnStatusStarted value, which made Succeeded() report false for what were really successful commands and left CommandReturnObject in an undefined state.

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


10 Files Affected:

- (modified) lldb/source/Commands/CommandObjectCommands.cpp (+2) 
- (modified) lldb/source/Commands/CommandObjectExpression.cpp (+4) 
- (modified) lldb/source/Commands/CommandObjectLog.cpp (+2-1) 
- (modified) lldb/source/Commands/CommandObjectPlatform.cpp (+1) 
- (modified) lldb/source/Commands/CommandObjectRegister.cpp (+2) 
- (modified) lldb/source/Commands/CommandObjectScripting.cpp (+2) 
- (modified) lldb/source/Commands/CommandObjectSettings.cpp (+3) 
- (modified) lldb/source/Commands/CommandObjectSource.cpp (+2) 
- (modified) lldb/source/Commands/CommandObjectTarget.cpp (+7) 
- (modified) lldb/source/Commands/CommandObjectType.cpp (+4-2) 


``````````diff
diff --git a/lldb/source/Commands/CommandObjectCommands.cpp b/lldb/source/Commands/CommandObjectCommands.cpp
index 1f4783c26be8e..8db2f985984ba 100644
--- a/lldb/source/Commands/CommandObjectCommands.cpp
+++ b/lldb/source/Commands/CommandObjectCommands.cpp
@@ -2498,6 +2498,8 @@ class CommandObjectCommandsScriptAdd : public CommandObjectParsed,
     if (m_options.m_class_name.empty() && m_options.m_funct_name.empty()) {
       m_interpreter.GetPythonCommandsFromIOHandler("     ", // Prompt
                                                    *this);  // IOHandlerDelegate
+      // Still gathering input; the IOHandler will set the final status.
+      result.SetStatus(eReturnStatusStarted);
       return;
     }
 
diff --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp
index 0b86c329572ee..76cb60c7e4804 100644
--- a/lldb/source/Commands/CommandObjectExpression.cpp
+++ b/lldb/source/Commands/CommandObjectExpression.cpp
@@ -593,6 +593,8 @@ void CommandObjectExpression::DoExecute(llvm::StringRef command,
 
   if (command.empty()) {
     GetMultilineExpression();
+    // Still gathering input; the IOHandler will set the final status.
+    result.SetStatus(eReturnStatusStarted);
     return;
   }
 
@@ -660,6 +662,8 @@ void CommandObjectExpression::DoExecute(llvm::StringRef command,
     // No expression following options
     else if (expr.empty()) {
       GetMultilineExpression();
+      // Still gathering input; the IOHandler will set the final status.
+      result.SetStatus(eReturnStatusStarted);
       return;
     }
   }
diff --git a/lldb/source/Commands/CommandObjectLog.cpp b/lldb/source/Commands/CommandObjectLog.cpp
index b61627f4d4d55..e51a85e9f0308 100644
--- a/lldb/source/Commands/CommandObjectLog.cpp
+++ b/lldb/source/Commands/CommandObjectLog.cpp
@@ -273,7 +273,8 @@ class CommandObjectLogDisable : public CommandObjectParsed {
       if (Log::DisableLogChannel(channel, args.GetArgumentArrayRef(),
                                  error_stream))
         result.SetStatus(eReturnStatusSuccessFinishNoResult);
-      result.GetErrorStream() << error;
+      else
+        result.AppendError(error);
     }
   }
 };
diff --git a/lldb/source/Commands/CommandObjectPlatform.cpp b/lldb/source/Commands/CommandObjectPlatform.cpp
index 37ae2899f79b3..52b571acd13ee 100644
--- a/lldb/source/Commands/CommandObjectPlatform.cpp
+++ b/lldb/source/Commands/CommandObjectPlatform.cpp
@@ -1300,6 +1300,7 @@ class CommandObjectPlatformProcessList : public CommandObjectParsed {
                 ostrm, platform_sp->GetUserIDResolver(), m_options.show_args,
                 m_options.verbose);
           }
+          result.SetStatus(eReturnStatusSuccessFinishResult);
         }
       }
     } else {
diff --git a/lldb/source/Commands/CommandObjectRegister.cpp b/lldb/source/Commands/CommandObjectRegister.cpp
index 29d1cd6dc13e4..c86fd11d4d9e3 100644
--- a/lldb/source/Commands/CommandObjectRegister.cpp
+++ b/lldb/source/Commands/CommandObjectRegister.cpp
@@ -220,6 +220,8 @@ class CommandObjectRegisterRead : public CommandObjectParsed {
         }
       }
     }
+    if (result.GetStatus() != eReturnStatusFailed)
+      result.SetStatus(eReturnStatusSuccessFinishResult);
   }
 
   class CommandOptions : public OptionGroup {
diff --git a/lldb/source/Commands/CommandObjectScripting.cpp b/lldb/source/Commands/CommandObjectScripting.cpp
index 1f8ee0a9554ec..21400a62d697f 100644
--- a/lldb/source/Commands/CommandObjectScripting.cpp
+++ b/lldb/source/Commands/CommandObjectScripting.cpp
@@ -229,6 +229,8 @@ class CommandObjectScriptingExtensionList : public CommandObjectParsed {
 
     if (!num_listed_interface)
       s << " None\n";
+
+    result.SetStatus(eReturnStatusSuccessFinishResult);
   }
 
 private:
diff --git a/lldb/source/Commands/CommandObjectSettings.cpp b/lldb/source/Commands/CommandObjectSettings.cpp
index 126f57c738115..26a58a7a7d0a5 100644
--- a/lldb/source/Commands/CommandObjectSettings.cpp
+++ b/lldb/source/Commands/CommandObjectSettings.cpp
@@ -393,6 +393,7 @@ class CommandObjectSettingsWrite : public CommandObjectParsed {
     if (args.empty()) {
       GetDebugger().DumpAllPropertyValues(&clean_ctx, out_file,
                                           OptionValue::eDumpGroupExport);
+      result.SetStatus(eReturnStatusSuccessFinishNoResult);
       return;
     }
 
@@ -403,6 +404,8 @@ class CommandObjectSettingsWrite : public CommandObjectParsed {
         result.AppendError(error.AsCString());
       }
     }
+    if (result.GetStatus() != eReturnStatusFailed)
+      result.SetStatus(eReturnStatusSuccessFinishNoResult);
   }
 
 private:
diff --git a/lldb/source/Commands/CommandObjectSource.cpp b/lldb/source/Commands/CommandObjectSource.cpp
index b2896e02264f7..0d131bcd626aa 100644
--- a/lldb/source/Commands/CommandObjectSource.cpp
+++ b/lldb/source/Commands/CommandObjectSource.cpp
@@ -1199,6 +1199,8 @@ class CommandObjectSourceList : public CommandObjectParsed {
         }
       }
     }
+    if (result.GetStatus() != eReturnStatusFailed)
+      result.SetStatus(eReturnStatusSuccessFinishResult);
   }
 
   const SymbolContextList *GetBreakpointLocations() {
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index eb71e77447281..40da1418e8d7b 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -305,6 +305,8 @@ class CommandObjectTargetCreate : public CommandObjectParsed {
       if (!label.empty()) {
         if (auto E = target_sp->SetLabel(label))
           result.SetError(std::move(E));
+        else
+          result.SetStatus(eReturnStatusSuccessFinishNoResult);
         return;
       }
 
@@ -989,6 +991,8 @@ class CommandObjectTargetVariable : public CommandObjectParsed {
 
     m_interpreter.PrintWarningsIfNecessary(result.GetOutputStream(),
                                            m_cmd_name);
+    if (result.GetStatus() != eReturnStatusFailed)
+      result.SetStatus(eReturnStatusSuccessFinishResult);
   }
 
   OptionGroupOptions m_option_group;
@@ -3104,6 +3108,8 @@ class CommandObjectTargetModulesLoad
       result.AppendError("either the \"--file <module>\" or the \"--uuid "
                          "<uuid>\" option must be specified.\n");
     }
+    if (result.GetStatus() != eReturnStatusFailed)
+      result.SetStatus(eReturnStatusSuccessFinishResult);
   }
 
   OptionGroupOptions m_option_group;
@@ -3772,6 +3778,7 @@ class CommandObjectTargetModulesShowUnwind : public CommandObjectParsed {
 
       result.GetOutputStream().Printf("\n");
     }
+    result.SetStatus(eReturnStatusSuccessFinishResult);
   }
 
   CommandOptions m_options;
diff --git a/lldb/source/Commands/CommandObjectType.cpp b/lldb/source/Commands/CommandObjectType.cpp
index a1a593ddfac63..9b3d4428272cd 100644
--- a/lldb/source/Commands/CommandObjectType.cpp
+++ b/lldb/source/Commands/CommandObjectType.cpp
@@ -1559,10 +1559,12 @@ void CommandObjectTypeSummaryAdd::DoExecute(Args &command,
 #else
     result.AppendError("python is disabled");
 #endif
-    return;
+  } else {
+    Execute_StringSummary(command, result);
   }
 
-  Execute_StringSummary(command, result);
+  if (result.GetStatus() != eReturnStatusFailed)
+    result.SetStatus(eReturnStatusSuccessFinishResult);
 }
 
 static bool FixArrayTypeNameWithRegex(ConstString &type_name) {

``````````

</details>


https://github.com/llvm/llvm-project/pull/196588