I figured something would fail. I've reverted and will reland with fixed tests.
On Tue, 8 Jun 2021 at 22:17, Jim Ingham <jing...@apple.com> wrote: > > Hey, David, > > This commit seems to have caused a new failure in TestRegisters.py, e.g.: > > http://green.lab.llvm.org/green/blue/organizations/jenkins/lldb-cmake/detail/lldb-cmake/32693/pipeline/ > > Do you have time to take a look? > > Jim > > > > > > On Jun 8, 2021, at 1:41 AM, David Spickett via lldb-commits > > <lldb-commits@lists.llvm.org> wrote: > > > > > > 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 > > > > > > > > _______________________________________________ > > lldb-commits mailing list > > lldb-commits@lists.llvm.org > > https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits > _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
