https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/196589
>From d550c4353f8a9bcd854c90907c1ac6e96fdc8125 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <[email protected]> Date: Fri, 8 May 2026 15:18:22 -0700 Subject: [PATCH 1/2] [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); +} >From 941fe7bf89c5e412f9c0e917293bd3def15d8b31 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <[email protected]> Date: Fri, 8 May 2026 20:49:24 -0700 Subject: [PATCH 2/2] Fix two more tests --- lldb/source/Commands/CommandObjectProcess.cpp | 2 ++ lldb/source/Commands/CommandObjectProtocolServer.cpp | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) 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); } }; _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
