[lldb] Fix a potential bug that may cause assert failure in CommandObject::CheckRequirements

[MrHate]

Summary

CommandObject::CheckRequirements requires cleaning up m_exe_ctx between commands. Function HandleOptionCompletion returns without cleaning up m_exe_ctx could cause assert failure in later CheckRequirements.

[JDevlieghere]

lldb/source/Interpreter/CommandObject.cpp

281

Should we reset the context here as well? Maybe it's wroth wrapping this in a simple RAII class that unsets the context on destruction.

[MrHate]

Thanks very much Jonas for pointing out my carelessness! And with your RAII idea, I think we can use the smart pointer to simulate the defer thing.

[teemperor]

LLVM actually has it's own cod for doing this (IIUC, it has the benefit of being easier to migrate to an upcoming standard library utility class with the same purpose).

Beside that this LGTM.

lldb/source/Interpreter/CommandObject.cpp

273

The LLVM way of doing this is auto reset_ctx = llvm::make_scope_exit([this]() { Cleanup(); }); (from the #include "llvm/ADT/ScopeExit.h" header).

[teemperor]

LGTM, thanks!

lldb/source/Interpreter/CommandObject.cpp

 #include <string>
 
 #include <ctype.h>
 #include <stdlib.h>
 
 #include "lldb/Core/Address.h"
 #include "lldb/Interpreter/Options.h"
 #include "lldb/Utility/ArchSpec.h"
+#include "llvm/ADT/ScopeExit.h"
 
 // These are for the Sourcename completers.
 // FIXME: Make a separate file for the completers.
 #include "lldb/Core/FileSpecList.h"
 #include "lldb/DataFormatters/FormatManager.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/FileSpec.h"
void CommandObject::Cleanup() {
   m_exe_ctx.Clear();
   if (m_api_locker.owns_lock())
     m_api_locker.unlock();
 }
 
 void CommandObject::HandleCompletion(CompletionRequest &request) {
 
   m_exe_ctx = m_interpreter.GetExecutionContext();
+  auto reset_ctx = llvm::make_scope_exit([this]() { Cleanup(); });
 
   // Default implementation of WantsCompletion() is !WantsRawCommandString().
   // Subclasses who want raw command string but desire, for example, argument
   // completion should override WantsCompletion() to return true, instead.
   if (WantsRawCommandString() && !WantsCompletion()) {
     // FIXME: Abstract telling the completion to insert the completion
     // character.
     return;
   } else {
     // Can we do anything generic with the options?
     Options *cur_options = GetOptions();
     CommandReturnObject result;
     OptionElementVector opt_element_vector;
 
     if (cur_options != nullptr) {
       opt_element_vector = cur_options->ParseForCompletion(
           request.GetParsedLine(), request.GetCursorIndex());
 
       bool handled_by_options = cur_options->HandleOptionCompletion(
           request, opt_element_vector, GetCommandInterpreter());
       if (handled_by_options)
         return;
     }
 
     // If we got here, the last word is not an option or an option argument.
     HandleArgumentCompletion(request, opt_element_vector);
   }
-
-  m_exe_ctx.Clear();
 }
 
 bool CommandObject::HelpTextContainsWord(llvm::StringRef search_word,
                                          bool search_short_help,
                                          bool search_long_help,
                                          bool search_syntax,
                                          bool search_options) {
   std::string options_usage_help;