[Lldb-commits] [lldb] 78d124e - [lldb] Assert that CommandObject::DoExecute sets a return status (#196589)
via lldb-commits
lldb-commits at lists.llvm.org
Mon May 11 11:58:07 PDT 2026
Author: Jonas Devlieghere
Date: 2026-05-11T13:58:02-05:00
New Revision: 78d124eb16aa62e02a465b0fd6c3c2cab0a26dd8
URL: https://github.com/llvm/llvm-project/commit/78d124eb16aa62e02a465b0fd6c3c2cab0a26dd8
DIFF: https://github.com/llvm/llvm-project/commit/78d124eb16aa62e02a465b0fd6c3c2cab0a26dd8.diff
LOG: [lldb] Assert that CommandObject::DoExecute sets a return status (#196589)
Change the default value of CommandReturnObject::m_status from
eReturnStatusStarted to eReturnStatusInvalid, and add a debug-only RAII
check in CommandObjectParsed::Execute and CommandObjectRaw::Execute that
asserts the status is no longer Invalid after DoExecute returns.
This catches commands that forget to call SetStatus on a success or
failure path. Succeeded() still returns true when the status is Invalid
(0 sorts below eReturnStatusSuccessContinuingResult), so helpers that
read result.Succeeded() as a precondition before any explicit SetStatus
(e.g. StopProcessIfNecessary) continue to work.
rdar://176506732
Added:
lldb/unittests/Interpreter/TestCommandReturnObject.cpp
Modified:
lldb/include/lldb/Interpreter/CommandReturnObject.h
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Commands/CommandObjectProtocolServer.cpp
lldb/source/Interpreter/CommandObject.cpp
lldb/source/Interpreter/CommandReturnObject.cpp
lldb/test/API/commands/command/script/TestCommandScript.py
lldb/unittests/Interpreter/CMakeLists.txt
Removed:
################################################################################
diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h b/lldb/include/lldb/Interpreter/CommandReturnObject.h
index 743bd94f5d73e..dccb98cd2be90 100644
--- a/lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -189,7 +189,10 @@ class CommandReturnObject {
std::vector<DiagnosticDetail> m_diagnostics;
std::optional<uint16_t> m_diagnostic_indent;
- lldb::ReturnStatus m_status = lldb::eReturnStatusStarted;
+ /// The command's return status indicating success or failure. The default
+ /// value indicate no status has been set, which is enforced by an assert in
+ /// the CommandInterpreter.
+ 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/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp
index 551f98566a9a5..d7d6a9152e377 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -927,7 +927,9 @@ class CommandObjectProcessConnect : public CommandObjectParsed {
error);
if (error.Fail() || process_sp == nullptr) {
result.AppendError(error.AsCString("Error connecting to the process"));
+ return;
}
+ result.SetStatus(eReturnStatusSuccessFinishResult);
}
CommandOptions m_options;
diff --git a/lldb/source/Commands/CommandObjectProtocolServer.cpp b/lldb/source/Commands/CommandObjectProtocolServer.cpp
index 1a950899ea1c0..73f717660e47d 100644
--- a/lldb/source/Commands/CommandObjectProtocolServer.cpp
+++ b/lldb/source/Commands/CommandObjectProtocolServer.cpp
@@ -92,8 +92,8 @@ class CommandObjectProtocolServerStart : public CommandObjectParsed {
result.AppendMessageWithFormatv(
"{0} server started with connection listeners: {1}", protocol,
address);
- result.SetStatus(eReturnStatusSuccessFinishNoResult);
}
+ result.SetStatus(eReturnStatusSuccessFinishNoResult);
}
};
@@ -128,6 +128,7 @@ class CommandObjectProtocolServerStop : public CommandObjectParsed {
result.AppendErrorWithFormatv("{0}", llvm::fmt_consume(std::move(error)));
return;
}
+ result.SetStatus(eReturnStatusSuccessFinishNoResult);
}
};
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);
+}
More information about the lldb-commits
mailing list