[Lldb-commits] [lldb] [lldb] Assert that CommandObject::DoExecute sets a return status (PR #196589)
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Fri May 8 10:34:13 PDT 2026
https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/196589
Add a debug-mode assert in the leaf CommandObjectParsed and CommandObjectRaw Execute paths that fires if DoExecute returns without calling SetStatus on the CommandReturnObject. Forgetting to set a status leaves the object in an undefined state and silently misreports success/failure to callers.
rdar://176506732
>From 276aa8baf3d3fc6c2a06cf973650d4ece0a143de Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Fri, 8 May 2026 10:32:59 -0700
Subject: [PATCH] [lldb] Assert that CommandObject::DoExecute sets a return
status
Add a debug-mode assert in the leaf CommandObjectParsed and
CommandObjectRaw Execute paths that fires if DoExecute returns without
calling SetStatus on the CommandReturnObject. Forgetting to set a
status leaves the object in an undefined state and silently misreports
success/failure to callers.
rdar://176506732
---
.../lldb/Interpreter/CommandReturnObject.h | 12 ++++
lldb/source/Interpreter/CommandObject.cpp | 25 ++++++++-
.../Interpreter/CommandReturnObject.cpp | 6 +-
lldb/unittests/Interpreter/CMakeLists.txt | 1 +
.../Interpreter/TestCommandReturnObject.cpp | 55 +++++++++++++++++++
5 files changed, 97 insertions(+), 2 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..55fabe508e968 100644
--- a/lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -163,6 +163,17 @@ class CommandReturnObject {
void SetStatus(lldb::ReturnStatus status);
+ /// Returns true if SetStatus has been called since construction or the last
+ /// Clear(). Used to assert that commands explicitly report a status rather
+ /// than silently leaving the initial value in place.
+ bool WasStatusSet() const { return m_status_set; }
+
+ /// Clears the "was set" flag without modifying the status value. Used by
+ /// the Execute machinery to scope the WasStatusSet() check to a single
+ /// DoExecute call without disturbing the status value seen by helpers
+ /// running inside DoExecute.
+ void ResetStatusSetFlag() { m_status_set = false; }
+
bool Succeeded() const;
bool HasResult() const;
@@ -190,6 +201,7 @@ class CommandReturnObject {
std::optional<uint16_t> m_diagnostic_indent;
lldb::ReturnStatus m_status = lldb::eReturnStatusStarted;
+ bool m_status_set = false;
/// 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..dec899a94970e 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 was set" flag on entry and
+/// asserts on exit that DoExecute called SetStatus (directly or via
+/// AppendError/SetError, which call SetStatus internally).
+class DoExecuteStatusCheck {
+public:
+ explicit DoExecuteStatusCheck(CommandReturnObject &result)
+ : m_result(result) {
+ m_result.ResetStatusSetFlag();
+ }
+ ~DoExecuteStatusCheck() {
+ assert(m_result.WasStatusSet() &&
+ "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..665153ed5972b 100644
--- a/lldb/source/Interpreter/CommandReturnObject.cpp
+++ b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -159,7 +159,10 @@ StructuredData::ObjectSP CommandReturnObject::GetErrorData() {
return Serialize(m_diagnostics);
}
-void CommandReturnObject::SetStatus(ReturnStatus status) { m_status = status; }
+void CommandReturnObject::SetStatus(ReturnStatus status) {
+ m_status = status;
+ m_status_set = true;
+}
ReturnStatus CommandReturnObject::GetStatus() const { return m_status; }
@@ -182,6 +185,7 @@ void CommandReturnObject::Clear() {
static_cast<StreamString *>(stream_sp.get())->Clear();
m_diagnostics.clear();
m_status = eReturnStatusStarted;
+ m_status_set = false;
m_did_change_process_state = false;
m_suppress_immediate_output = false;
m_interactive = true;
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..c4e23e10a45a6
--- /dev/null
+++ b/lldb/unittests/Interpreter/TestCommandReturnObject.cpp
@@ -0,0 +1,55 @@
+//===-- 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, WasStatusSetInitiallyFalse) {
+ CommandReturnObject result(/*colors=*/false);
+ EXPECT_FALSE(result.WasStatusSet());
+}
+
+TEST(CommandReturnObjectTest, SetStatusSetsFlag) {
+ CommandReturnObject result(false);
+ result.SetStatus(eReturnStatusSuccessFinishResult);
+ EXPECT_TRUE(result.WasStatusSet());
+ EXPECT_EQ(result.GetStatus(), eReturnStatusSuccessFinishResult);
+}
+
+TEST(CommandReturnObjectTest, AppendErrorSetsFlag) {
+ CommandReturnObject result(false);
+ result.AppendError("boom");
+ EXPECT_TRUE(result.WasStatusSet());
+ EXPECT_EQ(result.GetStatus(), eReturnStatusFailed);
+}
+
+TEST(CommandReturnObjectTest, ClearResetsFlag) {
+ CommandReturnObject result(false);
+ result.SetStatus(eReturnStatusSuccessFinishResult);
+ ASSERT_TRUE(result.WasStatusSet());
+ result.Clear();
+ EXPECT_FALSE(result.WasStatusSet());
+ EXPECT_EQ(result.GetStatus(), eReturnStatusStarted);
+}
+
+TEST(CommandReturnObjectTest, ResetStatusSetFlagPreservesStatus) {
+ CommandReturnObject result(false);
+ result.SetStatus(eReturnStatusSuccessFinishResult);
+ ASSERT_TRUE(result.WasStatusSet());
+ ASSERT_EQ(result.GetStatus(), eReturnStatusSuccessFinishResult);
+
+ result.ResetStatusSetFlag();
+ EXPECT_FALSE(result.WasStatusSet());
+ // Status itself must be untouched so helpers reading Succeeded()/GetStatus()
+ // inside DoExecute still see what was pre-seeded by ParseOptions.
+ EXPECT_EQ(result.GetStatus(), eReturnStatusSuccessFinishResult);
+ EXPECT_TRUE(result.Succeeded());
+}
More information about the lldb-commits
mailing list