[Lldb-commits] [lldb] [lldb] Assert that CommandObject::DoExecute sets a return status (PR #196589)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/196589

>From 95fa0ac40d3ddf9e6a648a8faa0280c824492e81 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Fri, 8 May 2026 15:18:22 -0700
Subject: [PATCH] [lldb] Assert that CommandObject::DoExecute sets a return
 status

---
 .../lldb/Interpreter/CommandReturnObject.h    |  2 +-
 lldb/source/Interpreter/CommandObject.cpp     | 25 +++++++++++-
 .../Interpreter/CommandReturnObject.cpp       |  2 +-
 .../command/script/TestCommandScript.py       |  4 +-
 lldb/unittests/Interpreter/CMakeLists.txt     |  1 +
 .../Interpreter/TestCommandReturnObject.cpp   | 38 +++++++++++++++++++
 6 files changed, 67 insertions(+), 5 deletions(-)
 create mode 100644 lldb/unittests/Interpreter/TestCommandReturnObject.cpp

diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h b/lldb/include/lldb/Interpreter/CommandReturnObject.h
index 743bd94f5d73e..4e4b565b88024 100644
--- a/lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -189,7 +189,7 @@ class CommandReturnObject {
   std::vector<DiagnosticDetail> m_diagnostics;
   std::optional<uint16_t> m_diagnostic_indent;
 
-  lldb::ReturnStatus m_status = lldb::eReturnStatusStarted;
+  lldb::ReturnStatus m_status = lldb::eReturnStatusInvalid;
 
   /// An optionally empty list of values produced by this command.
   ValueObjectList m_value_objects;
diff --git a/lldb/source/Interpreter/CommandObject.cpp b/lldb/source/Interpreter/CommandObject.cpp
index 3b3b2d7a302d9..dbdfc66204660 100644
--- a/lldb/source/Interpreter/CommandObject.cpp
+++ b/lldb/source/Interpreter/CommandObject.cpp
@@ -37,6 +37,26 @@
 using namespace lldb;
 using namespace lldb_private;
 
+namespace {
+/// RAII scope that resets the result's status to eReturnStatusInvalid on entry
+/// and asserts on exit that DoExecute changed it (directly via SetStatus, or
+/// indirectly via AppendError/SetError, which call SetStatus internally).
+class DoExecuteStatusCheck {
+public:
+  explicit DoExecuteStatusCheck(CommandReturnObject &result)
+      : m_result(result) {
+    m_result.SetStatus(eReturnStatusInvalid);
+  }
+  ~DoExecuteStatusCheck() {
+    assert(m_result.GetStatus() != eReturnStatusInvalid &&
+           "DoExecute did not set a status on the CommandReturnObject");
+  }
+
+private:
+  CommandReturnObject &m_result;
+};
+} // namespace
+
 // CommandObject
 
 CommandObject::CommandObject(CommandInterpreter &interpreter,
@@ -825,6 +845,7 @@ void CommandObjectParsed::Execute(const char *args_string,
           return;
         }
         m_interpreter.IncreaseCommandUsage(*this);
+        DoExecuteStatusCheck check(result);
         DoExecute(cmd_args, result);
       }
     }
@@ -845,8 +866,10 @@ void CommandObjectRaw::Execute(const char *args_string,
     handled = InvokeOverrideCallback(argv, result);
   }
   if (!handled) {
-    if (CheckRequirements(result))
+    if (CheckRequirements(result)) {
+      DoExecuteStatusCheck check(result);
       DoExecute(args_string, result);
+    }
 
     Cleanup();
   }
diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp b/lldb/source/Interpreter/CommandReturnObject.cpp
index a6b0d56c66cca..c21b9b3a92c05 100644
--- a/lldb/source/Interpreter/CommandReturnObject.cpp
+++ b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -181,7 +181,7 @@ void CommandReturnObject::Clear() {
   if (stream_sp)
     static_cast<StreamString *>(stream_sp.get())->Clear();
   m_diagnostics.clear();
-  m_status = eReturnStatusStarted;
+  m_status = eReturnStatusInvalid;
   m_did_change_process_state = false;
   m_suppress_immediate_output = false;
   m_interactive = true;
diff --git a/lldb/test/API/commands/command/script/TestCommandScript.py b/lldb/test/API/commands/command/script/TestCommandScript.py
index fdd5216a1c6cc..eb1584c64c90d 100644
--- a/lldb/test/API/commands/command/script/TestCommandScript.py
+++ b/lldb/test/API/commands/command/script/TestCommandScript.py
@@ -214,8 +214,8 @@ def test_persistence(self):
         # valid.
         self.expect("script str(persistence.debugger_copy)", substrs=[str(self.dbg)])
         # The result object will be replaced by an empty result object (in the
-        # "Started" state).
-        self.expect("script str(persistence.result_copy)", substrs=["Started"])
+        # default "Invalid" state).
+        self.expect("script str(persistence.result_copy)", substrs=["Invalid"])
 
     def test_interactive(self):
         """
diff --git a/lldb/unittests/Interpreter/CMakeLists.txt b/lldb/unittests/Interpreter/CMakeLists.txt
index d4ba5b3d58334..7eec76105aad2 100644
--- a/lldb/unittests/Interpreter/CMakeLists.txt
+++ b/lldb/unittests/Interpreter/CMakeLists.txt
@@ -1,5 +1,6 @@
 add_lldb_unittest(InterpreterTests
   TestCommandPaths.cpp
+  TestCommandReturnObject.cpp
   TestCompletion.cpp
   TestOptionArgParser.cpp
   TestOptions.cpp
diff --git a/lldb/unittests/Interpreter/TestCommandReturnObject.cpp b/lldb/unittests/Interpreter/TestCommandReturnObject.cpp
new file mode 100644
index 0000000000000..a8c66958e5d06
--- /dev/null
+++ b/lldb/unittests/Interpreter/TestCommandReturnObject.cpp
@@ -0,0 +1,38 @@
+//===-- TestCommandReturnObject.cpp ---------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Interpreter/CommandReturnObject.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+TEST(CommandReturnObjectTest, DefaultStatusIsInvalid) {
+  CommandReturnObject result(/*colors=*/false);
+  EXPECT_EQ(result.GetStatus(), eReturnStatusInvalid);
+}
+
+TEST(CommandReturnObjectTest, SetStatusUpdatesStatus) {
+  CommandReturnObject result(false);
+  result.SetStatus(eReturnStatusSuccessFinishResult);
+  EXPECT_EQ(result.GetStatus(), eReturnStatusSuccessFinishResult);
+}
+
+TEST(CommandReturnObjectTest, AppendErrorSetsFailed) {
+  CommandReturnObject result(false);
+  result.AppendError("boom");
+  EXPECT_EQ(result.GetStatus(), eReturnStatusFailed);
+}
+
+TEST(CommandReturnObjectTest, ClearResetsToInvalid) {
+  CommandReturnObject result(false);
+  result.SetStatus(eReturnStatusSuccessFinishResult);
+  ASSERT_EQ(result.GetStatus(), eReturnStatusSuccessFinishResult);
+  result.Clear();
+  EXPECT_EQ(result.GetStatus(), eReturnStatusInvalid);
+}