Set "success" status correctly

[jafl]

Summary

In CommandObjectRegisterRead.DoExecute(), set the status to "success" when appropriate.

[labath]

Thanks for the patch. Could you please also add an appropriate test for it? Doing something similar to what packages/Python/lldbsuite/test/functionalities/frame_var/TestFrameVar.py does should be the easiest way to test this.

[tatyana-krasnukha]

It can make sense to add

assert(eReturnStatusStarted != result);

after cmd_obj->Execute() invocation at CommandInterpreter::HandleCommand, do you think?

[labath]

that sounds like an excellent idea, as it will check all executed commands, and not the ones we've remembered checking. It should probably be an lldbassert though. (And we'd need to check that the existing tests still pass after that.)

[jafl]

Thanks for the suggestions. I will get around to this - just swamped right now!

[clayborg]

Yes: lldbassert would be fine for that since those get compiled out during release. Patch looks fine. If we already have a test that would trigger the new "lldbassert" you will add, then no need for a special test for this, else we need a test that triggers this.

[jafl]

I will try to ensure that the lldbassert gets tested, but right now, none of the tests work on my machine. I emailed lldb-dev for help.

[jafl]

Added lldbassert

[clayborg]

This is sure to trigger things in the test suite. We will need to ensure a few things:

• test suite runs cleanly in debug mode after the lldbassert is added
• without changes to CommandObjectRegister.cpp that the lldbassert is triggered, and if not, add a test

[jafl]

Absolutely!

source/Commands/CommandObjectRegister.cpp

           set_idx = m_command_options.set_indexes[i]->GetUInt64Value(UINT32_MAX,
                                                                      nullptr);
           if (set_idx < reg_ctx->GetRegisterSetCount()) {
-            if (!DumpRegisterSet(m_exe_ctx, strm, reg_ctx, set_idx)) {
+            if (DumpRegisterSet(m_exe_ctx, strm, reg_ctx, set_idx)) {
+              result.SetStatus(eReturnStatusSuccessFinishResult);
+            } else {
               if (errno)
                 result.AppendErrorWithFormatv("register read failed: {0}\n",
                                               llvm::sys::StrError());

           // registers.
           DumpRegisterSet(m_exe_ctx, strm, reg_ctx, set_idx,
                           !m_command_options.dump_all_sets.GetCurrentValue());
+          result.SetStatus(eReturnStatusSuccessFinishResult);
         }
       }
     } else {

           reg_info = reg_ctx->GetRegisterInfoByName(arg_str);
 
           if (reg_info) {
-            if (!DumpRegister(m_exe_ctx, strm, reg_ctx, reg_info))
+            if (DumpRegister(m_exe_ctx, strm, reg_ctx, reg_info))
+              result.SetStatus(eReturnStatusSuccessFinishResult);
+            else
               strm.Printf("%-12s = error: unavailable\n", reg_info->name);
           } else {
             result.AppendErrorWithFormat("Invalid register name '%s'.\n",

source/Interpreter/CommandInterpreter.cpp

 #include "lldb/Target/Thread.h"
 
 #include "lldb/Utility/CleanUp.h"
+#include "lldb/Utility/LLDBAssert.h"
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"

           remainder.c_str());
 
     cmd_obj->Execute(remainder.c_str(), result);
+    lldbassert(eReturnStatusStarted != result.GetStatus());
   } else {
     // We didn't find the first command object, so complete the first argument.
     Args command_args(command_string);