[LLDB] Clean up after command fails

[zequanwu]

Summary

CommandObject::CheckRequirements() requires m_exe_ctx being cleaned up.

[labath]

I'm not really familiar this code, but could something like llvm::make_scope_exit be useful here?
Also, what is the effect of not calling Cleanup? That subsequent commands fail in some way? Could we make a test case for that?

[jingham]

Each new command invocation is expected to prime the ExecutionContext before invoking the command. I don't think there's any situation where one command should use a previous command's execution context on purpose. So if we're getting to Execute without the CommandInterpreter having proactively set the ExecutionContext for that invocation, that is a bug that should be fixed. This change will make sure that the incorrectly inherited ExecutionContext is empty, but that still means the command is somehow getting invoked with the wrong execution context.

[jingham]

Actually, I think this is 100% correct, and corrects an error I made when I added argument number checking a little while back.

The comments on CheckRequirements say:

Every command should call

// CommandObject::Cleanup() after it has completed.

The early exit in the error case bypassed the call to cleanup that the command was required to do. The m_exe_ctx is probably the less important part of the cleanup that gets missed. If you called a command that has eCommandTryTargetAPILock set in its requirements, but passed it the wrong number of arguments, then we would error out here w/o releasing the m_api_lock, and then if the next command was also a CommandObjectParsed command and required the API lock, it would deadlock. You could probably even write a test that would show this deadlock.

[zequanwu]

Add a test case.

lldb/source/Interpreter/CommandObject.cpp

if (!handled) {
 
     if (CheckRequirements(result)) {
       if (ParseOptions(cmd_args, result)) {
         // Call the command-specific version of 'Execute', passing it the
         // already processed arguments.
         if (cmd_args.GetArgumentCount() != 0 && m_arguments.empty()) {
           result.AppendErrorWithFormatv("'{0}' doesn't take any arguments.",
                                         GetCommandName());
+          Cleanup();
           return false;
         }
         handled = DoExecute(cmd_args, result);
       }
     }
 
     Cleanup();
   }

lldb/test/Shell/Error/Inputs/cleanup.lldbinit

+b main
+run
+dis a
+dis -a $pc
+exit

lldb/test/Shell/Error/cleanup.cpp

+// Test CommandObject is cleaned up even after commands fail due to not taking any argument.
+// RUN: %clang_host -g %s -o %t
+// RUN: %lldb -f %t -o "settings set interpreter.stop-command-source-on-error false" -s \
+// RUN:   %S/Inputs/cleanup.lldbinit
+int main() { return 0; }