[Lldb-commits] [lldb] e05b03c - [lldb] Set return status to failed when adding a command error
David Spickett via lldb-commits
lldb-commits at lists.llvm.org
Tue Jun 8 01:41:15 PDT 2021
Author: David Spickett
Date: 2021-06-08T09:41:07+01:00
New Revision: e05b03cf4f45ac5ee63c59a3464e7d484884645c
URL: https://github.com/llvm/llvm-project/commit/e05b03cf4f45ac5ee63c59a3464e7d484884645c
DIFF: https://github.com/llvm/llvm-project/commit/e05b03cf4f45ac5ee63c59a3464e7d484884645c.diff
LOG: [lldb] Set return status to failed when adding a command error
There is a common pattern:
result.AppendError(...);
result.SetStatus(eReturnStatusFailed);
I found that some commands don't actually "fail" but only
print "error: ..." because the second line got missed.
This can cause you to miss a failed command when you're
using the Python interface during testing.
(and produce some confusing script results)
I did not find any place where you would want to add
an error without setting the return status, so just
set eReturnStatusFailed whenever you add an error to
a command result.
This change does not remove any of the now redundant
SetStatus. This should allow us to see if there are any
tests that have commands unexpectedly fail with this change.
(the test suite passes for me but I don't have access to all
the systems we cover so there could be some corner cases)
Some tests that failed on x86 and AArch64 have been modified
to work with the new behaviour.
Differential Revision: https://reviews.llvm.org/D103701
Added:
lldb/test/Shell/Commands/command-backtrace-parser-1.test
lldb/test/Shell/Commands/command-backtrace-parser-2.test
Modified:
lldb/source/Interpreter/CommandReturnObject.cpp
lldb/test/API/commands/register/register/register_command/TestRegisters.py
Removed:
lldb/test/Shell/Commands/command-backtrace.test
################################################################################
diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp b/lldb/source/Interpreter/CommandReturnObject.cpp
index 77d94bd9389c3..980d39bfb1681 100644
--- a/lldb/source/Interpreter/CommandReturnObject.cpp
+++ b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -46,6 +46,8 @@ CommandReturnObject::CommandReturnObject(bool colors)
m_interactive(true) {}
void CommandReturnObject::AppendErrorWithFormat(const char *format, ...) {
+ SetStatus(eReturnStatusFailed);
+
if (!format)
return;
va_list args;
@@ -100,6 +102,7 @@ void CommandReturnObject::AppendWarning(llvm::StringRef in_string) {
void CommandReturnObject::AppendError(llvm::StringRef in_string) {
if (in_string.empty())
return;
+ SetStatus(eReturnStatusFailed);
error(GetErrorStream()) << in_string.rtrim() << '\n';
}
@@ -116,7 +119,6 @@ void CommandReturnObject::SetError(llvm::StringRef error_str) {
return;
AppendError(error_str);
- SetStatus(eReturnStatusFailed);
}
// Similar to AppendError, but do not prepend 'Status: ' to message, and don't
@@ -126,6 +128,7 @@ void CommandReturnObject::AppendRawError(llvm::StringRef in_string) {
if (in_string.empty())
return;
GetErrorStream() << in_string;
+ SetStatus(eReturnStatusFailed);
}
void CommandReturnObject::SetStatus(ReturnStatus status) { m_status = status; }
diff --git a/lldb/test/API/commands/register/register/register_command/TestRegisters.py b/lldb/test/API/commands/register/register/register_command/TestRegisters.py
index 5ec46c175e621..7acf3a4098756 100644
--- a/lldb/test/API/commands/register/register/register_command/TestRegisters.py
+++ b/lldb/test/API/commands/register/register/register_command/TestRegisters.py
@@ -41,13 +41,18 @@ def test_register_commands(self):
self.expect("register read -a", MISSING_EXPECTED_REGISTERS,
substrs=['registers were unavailable'], matching=False)
+ all_registers = self.res.GetOutput()
+
if self.getArchitecture() in ['amd64', 'i386', 'x86_64']:
self.runCmd("register read xmm0")
- self.runCmd("register read ymm15") # may be available
- self.runCmd("register read bnd0") # may be available
+ if "ymm15 = " in all_registers:
+ self.runCmd("register read ymm15") # may be available
+ if "bnd0 = " in all_registers:
+ self.runCmd("register read bnd0") # may be available
elif self.getArchitecture() in ['arm', 'armv7', 'armv7k', 'arm64', 'arm64e', 'arm64_32']:
self.runCmd("register read s0")
- self.runCmd("register read q15") # may be available
+ if "q15 = " in all_registers:
+ self.runCmd("register read q15") # may be available
self.expect(
"register read -s 4",
@@ -413,7 +418,8 @@ def fp_register_write(self):
self.write_and_read(currentFrame, "ymm7", new_value)
self.expect("expr $ymm0", substrs=['vector_type'])
else:
- self.runCmd("register read ymm0")
+ self.expect("register read ymm0", substrs=["Invalid register name 'ymm0'"],
+ error=True)
if has_mpx:
# Test write and read for bnd0.
@@ -428,7 +434,8 @@ def fp_register_write(self):
self.write_and_read(currentFrame, "bndstatus", new_value)
self.expect("expr $bndstatus", substrs = ['vector_type'])
else:
- self.runCmd("register read bnd0")
+ self.expect("register read bnd0", substrs=["Invalid register name 'bnd0'"],
+ error=True)
def convenience_registers(self):
"""Test convenience registers."""
@@ -450,7 +457,7 @@ def convenience_registers(self):
# Now write rax with a unique bit pattern and test that eax indeed
# represents the lower half of rax.
self.runCmd("register write rax 0x1234567887654321")
- self.expect("register read rax 0x1234567887654321",
+ self.expect("register read rax",
substrs=['0x1234567887654321'])
def convenience_registers_with_process_attach(self, test_16bit_regs):
diff --git a/lldb/test/Shell/Commands/command-backtrace-parser-1.test b/lldb/test/Shell/Commands/command-backtrace-parser-1.test
new file mode 100644
index 0000000000000..339c6664b3726
--- /dev/null
+++ b/lldb/test/Shell/Commands/command-backtrace-parser-1.test
@@ -0,0 +1,6 @@
+# RUN: %lldb -s %s 2>&1 | FileCheck %s
+
+# Make sure this is not rejected by the parser as invalid syntax.
+# Blank characters after the '1' are important, as we're testing the parser.
+bt 1
+# CHECK: error: invalid target
diff --git a/lldb/test/Shell/Commands/command-backtrace.test b/lldb/test/Shell/Commands/command-backtrace-parser-2.test
similarity index 50%
rename from lldb/test/Shell/Commands/command-backtrace.test
rename to lldb/test/Shell/Commands/command-backtrace-parser-2.test
index 2816f5f2e33ce..5f91cf30ac719 100644
--- a/lldb/test/Shell/Commands/command-backtrace.test
+++ b/lldb/test/Shell/Commands/command-backtrace-parser-2.test
@@ -1,11 +1,5 @@
-# Check basic functionality of command bt.
# RUN: %lldb -s %s 2>&1 | FileCheck %s
-# Make sure this is not rejected by the parser as invalid syntax.
-# Blank characters after the '1' are important, as we're testing the parser.
-bt 1
-# CHECK: error: invalid target
-
# Make sure this is not rejected by the parser as invalid syntax.
# Blank characters after the 'all' are important, as we're testing the parser.
bt all
More information about the lldb-commits
mailing list