Further refinements on reporting interruption in lldb

[jingham]

Summary

This patch enhances the interruption features I added a little while back. There are two main changes. The main one here is that I made a nicer interface to reporting interruption events. I also added a way to ask a debugger if it was using a Module, so that we can query for interruption in Module code where we don't have access to a debugger. I used that to support interrupting debug info reading on a per-module basis - since that's a long-running but - at least on a module boundary - resumable operation.

The current method of checking "Does this module one belong to a debugger requesting interruption" is quick and dirty here. IMO, a better solution would be to have Modules keep a list of the Debuggers (or maybe Targets) using them. But we should figure out all the ways we want to use that and design something nice for the purpose.

[mib]

This looks very cool! I left "a few" comments on how I think we can simplify this patch, and also had some remarks regarding the in_process_target sanity checks.

lldb/include/lldb/Core/Debugger.h

435–436

If you use LLVM_PRETTY_FUNCTION you won't need this.

446

You could use LLVM_PRETTY_FUNCTION which will give you both the function signature and the argument well formatted.

454

460–468

Using LLVM_PRETTY_FUNCTION we can simplify the class by removing the variadic constructors and replace to std::string with a llvm::StringLiteral

lldb/include/lldb/Target/TargetList.h

192

Can the module be null ? If not, can we pass a reference instead of a raw pointer here ?

200

Same question as above, can any of the in_process_target be null ? If not, lets make it a Target&

216

ditto

218

ditto

220

ditto

lldb/source/API/SBFrame.cpp

57–58

Is this still necessary ?

lldb/source/Commands/CommandObjectTarget.cpp

67–68

Is this still necessary ?

lldb/source/Core/Debugger.cpp

1278–1286

You can get rid of this is you use LLVM_PRETTY_FUNCTION.

1288–1292

It would be nice if InterruptionReport had a Dump method.

lldb/source/Core/Module.cpp

1072–1073

You can get the Module reference by dereferencing this.

[bulbazord]

I really like the change to the interface, I especially like that it can report what was interrupted and how much work actually done. I had a lot of the same feedback as Ismail but also had some questions to help me understand all the details.

lldb/include/lldb/Core/Debugger.h

430–431

I think it would make more sense to have cur_func and formatv be of type llvm::StringRef. Concretely, if you construct a std::string from nullptr (like you're doing below when you make an InterruptionReport object), you will crash.

I know the recommendation is to use INTERRUPT_REQUESTED instead of filling this manually, but inevitably somebody will go against the advice and make a mistake.

446

+1

455–456

To avoid some unnecessary copies

Could also do what Ismail is suggesting.

465

since we're passing format to formatv, I think it would make sense for its type to be llvm::StringRef up-front here.

lldb/include/lldb/Target/TargetList.h

191

192

+1

Alternatively, if these are being pulled out of shared pointers, pass the shared pointer directly so we can avoid the issue of dangling raw pointers if the module ever is freed by the shared pointer.

200

+1

Same as above, let's not circumvent shared pointer semantics since we're storing references/pointers to these objects.

216–221

Same as my comment above.

lldb/source/API/SBFrame.cpp

57

What is this used for?

lldb/source/Commands/CommandObjectTarget.cpp

67–68

Where is this used?

lldb/source/Core/Debugger.cpp

1266–1267

Did clang-format give this to you?

1290–1291

lldb/source/Core/Module.cpp

1076

1078

I don't think you need to specify this->?

lldb/source/Target/StackFrameList.cpp

31

Where is this used?

lldb/source/Target/TargetList.cpp

518

I'm somewhat puzzled by this. Why are we unregistering the target in AddTargetInternal when we're registering it in CreateTargetInternal? Maybe I don't understand the intent behind {Register,Unregister}InProcessTarget.

[mib]

I had an offline chat with Jim and I misunderstood originally, what he was trying to achieve with the InterruptionReport:: m_function_name so my suggestion of using LLVM_PRETTY_FUNCTION doesn't actually work here.

[jingham]

lldb/include/lldb/Core/Debugger.h

435–436

I'm not sure I understand how you want to use LLVM_PRETTY_FUNCTION. From what I can tell, LLVM_PRETTY_FUNCTION prints something that looks like the function invocation as written. But ReportInterruption and the associated macros provide a formatv API.

lldb/source/Target/TargetList.cpp

518

The point behind this is someone says "make me a target" and then in the process of making the target, the Target code does something slow (e.g. look for external debug files) that we want to interrupt. So we need a way for the debugger to find the "in the process of being made" Targets.

"AddTargetInternal" is the API where the target gets added to the Debugger's TargetList, so after that method has run we will find the target in the regular TargetList. But between CreateTargetInternal and AddTargetInternal, that target isn't stored anywhere that the Debugger knows how to query. I added this list to hold the targets that are in the process of getting made, before they get added to the Debugger's TargetList.

[jingham]

Address review comments:

Made the in_process target list shared pointers.

Removed sstream includes.

[jingham]

lldb/include/lldb/Core/Debugger.h

430–431

I think it would make more sense to have cur_func and formatv be of type llvm::StringRef. Concretely, if you construct a std::string from nullptr (like you're doing below when you make an InterruptionReport object), you will crash.

I know the recommendation is to use INTERRUPT_REQUESTED instead of filling this manually, but inevitably somebody will go against the advice and make a mistake.

I don't see how I can make the formatv option a StringRef. The llvm::formatv API only offers a version that takes a const char *. Anyway, these are formatv strings, they are almost universally going to be const strings. Turning them into llvm::StringRef's and back out again to use in llvm::formatv seems odd.

But you are right, we should protect against someone passing in a null pointer for the function or format to InterruptRequested. Since this is just logging, an assert seems overkill, I'll just add null pointer checks here and turn them into "UNKNOWN FUNCTION" and "Unknown message".

lldb/source/API/SBFrame.cpp

57–58

Not sure what you are asking here. We use StackFrameSP w/o saying lldb::StackFrameSP and we use VariableList for instance rather than lldb_private::VariableList. We could remove the using statements here but that's not how we do it in general. In some cases we don't do using namespace lldb but that's mostly in files that use clang API's heavily since those conflict with some clang type names and it was good to be explicit there. But I don't think we want to be verbose like that in the SB files.

lldb/source/Target/StackFrameList.cpp

31

Dunno, but this seems more like an orthogonal cleanup unrelated to the current patch.

[jingham]

Protect InterruptRequested from null function & format strings.

lldb/include/lldb/Core/Debugger.h

455–456

This is a local that is copied to an ivar and never used again. Do I really have to put move in there explicitly for the optimizer to know it can reuse the value?

[bulbazord]

lldb/include/lldb/Core/Debugger.h

430–431

Oh... so llvm::formatv does take a const char *... My bad there, that makes sense, no need to change the type of format then.

Otherwise, I'm satisfied with the safety checks.

455–456

Yes. I created a simple example on Godbolt: https://godbolt.org/z/G4ej76Enn

Observe that the constructor in the example invokes the std::string copy constructor. Add a std::move and it then invokes the move constructor.

465

As you've pointed out above, formatv takes a const char * so ignore this.

lldb/source/Core/Debugger.cpp

1280

You'll also want to std::move(function_name) here too, I think.

lldb/source/Target/TargetList.cpp

518

Gotcha, that makes sense. Thanks for explaining!

[jingham]

lldb/include/lldb/Core/Debugger.h

455–456

Thanks for the example, that was interesting.

Note however that in your example, you the flags you specified were -std=c++17 so that was an unoptimized build. That's expected, unoptimized builds are supposed to be as literal as they can be. If you instead use the same example with -std=c++17 -O3 you won't see any copy constructors. I don't 100% mind putting this std::move in, but it seems noisy to decorate up your code with hints that the optimizer can figure out on its own.

[bulbazord]

lldb/include/lldb/Core/Debugger.h

455–456

You're right that I did not consider higher optimization levels there. That being said, I believe this is a matter of language semantics and not optimization level. I've created a slightly more complex example to illustrate: https://godbolt.org/z/naMTcd7Ev

In this other example, I have passed -O3. You'll notice that in the emitted constructor code there are 2 calls to operator new. If you add std::move for both of the parameters of the constructor, both of these allocations go away and the generated code becomes ~60% smaller. Even with optimization involved, the compiler can only optimize so much without breaking semantics.

[jingham]

Add some std::move's, Jonas agrees all the cool kids are doing that these days.

[bulbazord]

LGTM!

[mib]

LGTM! Thanks Jim!

[MaskRay]

Hi! The added lines have some spaces at line ends. I think using clang-format-diff.py would remove them.

lldb/include/lldb/Core/Debugger.h

 #include "lldb/lldb-private-enumerations.h"
 #include "lldb/lldb-private-types.h"
 #include "lldb/lldb-types.h"
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/DynamicLibrary.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Threading.h"
 
 #include <cassert>
 #include <cstddef>
 #include <cstdio>
 
 namespace llvm {
 class raw_ostream;
public:
   /// Broadcaster event bits definitions.
   enum {
     eBroadcastBitProgress = (1 << 0),
     eBroadcastBitWarning = (1 << 1),
     eBroadcastBitError = (1 << 2),
     eBroadcastSymbolChange = (1 << 3),
   };
 
+  using DebuggerList = std::vector<lldb::DebuggerSP>;
+
   static ConstString GetStaticBroadcasterClass();
 
   /// Get the public broadcaster for this debugger.
   Broadcaster &GetBroadcaster() { return m_broadcaster; }
   const Broadcaster &GetBroadcaster() const { return m_broadcaster; }
 
   ~Debugger() override;
 
public:
 
   /// Decrement the "interrupt requested" counter.
   void CancelInterruptRequest();
 
   /// This is the correct way to query the state of Interruption.
   /// If you are on the RunCommandInterpreter thread, it will check the
   /// command interpreter state, and if it is on another thread it will
   /// check the debugger Interrupt Request state.
-  ///
+  /// \param[in] cur_func
+  /// For reporting if the interruption was requested.  Don't provide this by
+  /// hand, use INTERRUPT_REQUESTED so this gets done consistently.
+  ///
+  /// \param[in] formatv
+  /// A formatv string for the interrupt message.  If the elements of the 
+  /// message are expensive to compute, you can use the no-argument form of
+  /// InterruptRequested, then make up the report using REPORT_INTERRUPTION. 
+  /// 
   /// \return
   ///  A boolean value, if \b true an interruptible operation should interrupt
   ///  itself.
+  template <typename... Args>
+  bool InterruptRequested(const char *cur_func, 
+                          const char *formatv, Args &&... args) {
+    bool ret_val = InterruptRequested();
+    if (ret_val) {
+      ReportInterruption(InterruptionReport(cur_func, 
+                                            llvm::formatv(formatv, 
+                                            std::forward<Args>(args)...)));
+    }
+    return ret_val;
+  }
+  
+  
+  /// This handy define will keep you from having to generate a report for the
+  /// interruption by hand.  Use this except in the case where the arguments to
+  /// the message description are expensive to compute.
+#define INTERRUPT_REQUESTED(debugger, ...) \
+    (debugger).InterruptRequested(__func__, __VA_ARGS__)
+
+  // This form just queries for whether to interrupt, and does no reporting:
   bool InterruptRequested();
   
+  // FIXME: Do we want to capture a backtrace at the interruption point?
+  class InterruptionReport {
+  public:
+    InterruptionReport(std::string function_name, std::string description) :
+        m_function_name(function_name), 
+        m_description(description),
+        m_interrupt_time(std::chrono::system_clock::now()),
+        m_thread_id(llvm::get_threadid()) {}
+        
+    InterruptionReport(std::string function_name, 
+        const llvm::formatv_object_base &payload);
+
+  template <typename... Args>
+  InterruptionReport(std::string function_name,
+              const char *format, Args &&... args) :
+    InterruptionReport(function_name, llvm::formatv(format, std::forward<Args>(args)...)) {}
+
+    std::string m_function_name;
+    std::string m_description;
+    const std::chrono::time_point<std::chrono::system_clock> m_interrupt_time;
+    const uint64_t m_thread_id;
+  };
+  void ReportInterruption(const InterruptionReport &report);
+#define REPORT_INTERRUPTION(debugger, ...) \
+    (debugger).ReportInterruption(Debugger::InterruptionReport(__func__, \
+                                                        __VA_ARGS__))
+
+  static DebuggerList DebuggersRequestingInterruption();
+
+public:
+  
   // This is for use in the command interpreter, when you either want the
   // selected target, or if no target is present you want to prime the dummy
   // target with entities that will be copied over to new targets.
   Target &GetSelectedOrDummyTarget(bool prefer_dummy = false);
   Target &GetDummyTarget() { return *m_dummy_target_sp; }
 
   lldb::BroadcasterManagerSP GetBroadcasterManager() {
     return m_broadcaster_manager_sp;

lldb/include/lldb/Target/TargetList.h

public:
 
   uint32_t SignalIfRunning(lldb::pid_t pid, int signo);
 
   void SetSelectedTarget(uint32_t index);
 
   void SetSelectedTarget(const lldb::TargetSP &target);
 
   lldb::TargetSP GetSelectedTarget();
   
+  ///  Returns whether any module, including ones in the process of being
+  ///  added, contains this module.  I don't want to give direct access to
+  ///  these not yet added target, but for interruption purposes, we might
+  ///  need to ask whetehr this target contains this module. 
+  bool AnyTargetContainsModule(Module &module);
+
   TargetIterable Targets() {
     return TargetIterable(m_target_list, m_target_list_mutex);
   }
 
 private:
   collection m_target_list;
+  std::unordered_set<lldb::TargetSP> m_in_process_target_list;
   mutable std::recursive_mutex m_target_list_mutex;
   uint32_t m_selected_target_idx;
 
   static Status CreateTargetInternal(
       Debugger &debugger, llvm::StringRef user_exe_path,
       llvm::StringRef triple_str, LoadDependentFiles load_dependent_files,
       const OptionGroupPlatform *platform_options, lldb::TargetSP &target_sp);
 
   static Status CreateTargetInternal(Debugger &debugger,
                                      llvm::StringRef user_exe_path,
                                      const ArchSpec &arch,
                                      LoadDependentFiles get_dependent_modules,
                                      lldb::PlatformSP &platform_sp,
                                      lldb::TargetSP &target_sp);
 
+  void RegisterInProcessTarget(lldb::TargetSP target_sp);
+  
+  void UnregisterInProcessTarget(lldb::TargetSP target_sp);
+  
+  bool IsTargetInProcess(lldb::TargetSP target_sp);
+  
   void AddTargetInternal(lldb::TargetSP target_sp, bool do_select);
 
   void SetSelectedTargetInternal(uint32_t index);
 
   TargetList(const TargetList &) = delete;
   const TargetList &operator=(const TargetList &) = delete;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_TARGET_TARGETLIST_H

lldb/source/API/SBFrame.cpp

 #include "lldb/API/SBStream.h"
 #include "lldb/API/SBSymbolContext.h"
 #include "lldb/API/SBThread.h"
 #include "lldb/API/SBValue.h"
 #include "lldb/API/SBVariablesOptions.h"
 
 #include "llvm/Support/PrettyStackTrace.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 SBFrame::SBFrame() : m_opaque_sp(new ExecutionContextRef()) {
   LLDB_INSTRUMENT_VA(this);
 }
 
 SBFrame::SBFrame(const StackFrameSP &lldb_object_sp)
     : m_opaque_sp(new ExecutionContextRef(lldb_object_sp)) {
   LLDB_INSTRUMENT_VA(this, lldb_object_sp);
if (stop_locker.TryLock(&process->GetRunLock())) {
         VariableList *variable_list = nullptr;
         Status var_error;
         variable_list = frame->GetVariableList(true, &var_error);
         if (var_error.Fail())
           value_list.SetError(var_error);
         if (variable_list) {
           const size_t num_variables = variable_list->GetSize();
           if (num_variables) {
+            size_t num_produced = 0;
             for (const VariableSP &variable_sp : *variable_list) {
-              if (dbg.InterruptRequested()) {
-                Log *log = GetLog(LLDBLog::Host);
-                LLDB_LOG(log, "Interrupted SBFrame::GetVariables");
+              if (INTERRUPT_REQUESTED(dbg, 
+                    "Interrupted getting frame variables with {0} of {1} "
+                    "produced.", num_produced, num_variables))
                 return {};
-              }
+
               if (variable_sp) {
                 bool add_variable = false;
                 switch (variable_sp->GetScope()) {
                 case eValueTypeVariableGlobal:
                 case eValueTypeVariableStatic:
                 case eValueTypeVariableThreadLocal:
                   add_variable = statics;
                   break;
if (stop_locker.TryLock(&process->GetRunLock())) {
                     continue;
 
                   SBValue value_sb;
                   value_sb.SetSP(valobj_sp, use_dynamic);
                   value_list.Append(value_sb);
                 }
               }
             }
+            num_produced++;
           }
         }
         if (recognized_arguments) {
           auto recognized_frame = frame->GetRecognizedFrame();
           if (recognized_frame) {
             ValueObjectListSP recognized_arg_list =
                 recognized_frame->GetRecognizedArguments();
             if (recognized_arg_list) {

lldb/source/Commands/CommandCompletions.cpp

void CommandCompletions::FrameIndexes(CommandInterpreter &interpreter,
 
   lldb::ThreadSP thread_sp = exe_ctx.GetThreadSP();
   Debugger &dbg = interpreter.GetDebugger();
   const uint32_t frame_num = thread_sp->GetStackFrameCount();
   for (uint32_t i = 0; i < frame_num; ++i) {
     lldb::StackFrameSP frame_sp = thread_sp->GetStackFrameAtIndex(i);
     StreamString strm;
     // Dumping frames can be slow, allow interruption.
-    if (dbg.InterruptRequested())
+    if (INTERRUPT_REQUESTED(dbg, "Interrupted in frame completion"))
       break;
     frame_sp->Dump(&strm, false, true);
     request.TryCompleteCurrentArg(std::to_string(i), strm.GetString());
   }
 }
 
 void CommandCompletions::StopHookIDs(CommandInterpreter &interpreter,
                                      CompletionRequest &request,

lldb/source/Commands/CommandObjectFrame.cpp

if (m_options.relative_frame_offset) {
             // and don't reset the frame.
             result.AppendError("Already at the bottom of the stack.");
             return false;
           } else
             frame_idx = 0;
         }
       } else if (*m_options.relative_frame_offset > 0) {
         // I don't want "up 20" where "20" takes you past the top of the stack
-        // to produce
-        // an error, but rather to just go to the top.  So I have to count the
-        // stack here...
+        // to produce an error, but rather to just go to the top.  OTOH, start
+        // by seeing if the requested frame exists, in which case we can avoid 
+        // counting the stack here...
+        const uint32_t frame_requested = frame_idx 
+            + *m_options.relative_frame_offset;
+        StackFrameSP frame_sp = thread->GetStackFrameAtIndex(frame_requested);
+        if (frame_sp)
+          frame_idx = frame_requested;
+        else {
+          // The request went past the stack, so handle that case:
           const uint32_t num_frames = thread->GetStackFrameCount();
           if (static_cast<int32_t>(num_frames - frame_idx) >
               *m_options.relative_frame_offset)
           frame_idx += *m_options.relative_frame_offset;
           else {
             if (frame_idx == num_frames - 1) {
               // If we are already at the top of the stack, just warn and don't
               // reset the frame.
               result.AppendError("Already at the top of the stack.");
               return false;
             } else
               frame_idx = num_frames - 1;
           }
         }
+      }
     } else {
       if (command.GetArgumentCount() > 1) {
         result.AppendErrorWithFormat(
             "too many arguments; expected frame-index, saw '%s'.\n",
             command[0].c_str());
         m_options.GenerateOptionUsage(
             result.GetErrorStream(), *this,
             GetCommandInterpreter().GetDebugger().GetTerminalWidth());

lldb/source/Commands/CommandObjectTarget.cpp

 #include "clang/CodeGen/ObjectFilePCHContainerOperations.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FormatAdapters.h"
 
 
 using namespace lldb;
 using namespace lldb_private;
 
 static void DumpTargetInfo(uint32_t target_idx, Target *target,
                            const char *prefix_cstr,
                            bool show_stopped_process_status, Stream &strm) {
   const ArchSpec &target_arch = target->GetArchitecture();
 
   Module *exe_module = target->GetExecutableModulePointer();
if (command.GetArgumentCount() == 0) {
       if (num_modules > 0) {
         result.GetOutputStream().Format(
             "Dumping symbol table for {0} modules.\n", num_modules);
         for (ModuleSP module_sp : module_list.ModulesNoLocking()) {
           if (num_dumped > 0) {
             result.GetOutputStream().EOL();
             result.GetOutputStream().EOL();
           }
-          if (GetDebugger().InterruptRequested())
+          if (INTERRUPT_REQUESTED(GetDebugger(), 
+                                  "Interrupted in dump all symtabs with {0} "
+                                  "of {1} dumped.", num_dumped, num_modules))
             break;
+
           num_dumped++;
           DumpModuleSymtab(m_interpreter, result.GetOutputStream(),
                            module_sp.get(), m_options.m_sort_order,
                            name_preference);
         }
       } else {
         result.AppendError("the target has no associated executable images");
         return false;
if (command.GetArgumentCount() == 0) {
             FindModulesByName(target, arg_cstr, module_list, true);
         if (num_matches > 0) {
           for (ModuleSP module_sp : module_list.Modules()) {
             if (module_sp) {
               if (num_dumped > 0) {
                 result.GetOutputStream().EOL();
                 result.GetOutputStream().EOL();
               }
-              if (GetDebugger().InterruptRequested())
+              if (INTERRUPT_REQUESTED(GetDebugger(), 
+                    "Interrupted in dump symtab list with {0} of {1} dumped.", 
+                    num_dumped, num_matches))
                 break;
+
               num_dumped++;
               DumpModuleSymtab(m_interpreter, result.GetOutputStream(),
                                module_sp.get(), m_options.m_sort_order,
                                name_preference);
             }
           }
         } else
           result.AppendWarningWithFormat(
if (command.GetArgumentCount() == 0) {
       if (num_modules == 0) {
         result.AppendError("the target has no associated executable images");
         return false;
       }
 
       result.GetOutputStream().Format("Dumping sections for {0} modules.\n",
                                       num_modules);
       for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
-        if (GetDebugger().InterruptRequested())
+        if (INTERRUPT_REQUESTED(GetDebugger(), 
+              "Interrupted in dump all sections with {0} of {1} dumped",
+              image_idx, num_modules))
           break;
+
         num_dumped++;
         DumpModuleSections(
             m_interpreter, result.GetOutputStream(),
             target->GetImages().GetModulePointerAtIndex(image_idx));
       }
     } else {
       // Dump specified images (by basename or fullpath)
       const char *arg_cstr;
       for (int arg_idx = 0;
            (arg_cstr = command.GetArgumentAtIndex(arg_idx)) != nullptr;
            ++arg_idx) {
         ModuleList module_list;
         const size_t num_matches =
             FindModulesByName(target, arg_cstr, module_list, true);
         if (num_matches > 0) {
           for (size_t i = 0; i < num_matches; ++i) {
-            if (GetDebugger().InterruptRequested())
+            if (INTERRUPT_REQUESTED(GetDebugger(), 
+                  "Interrupted in dump section list with {0} of {1} dumped.",
+                  i, num_matches))
               break;
+
             Module *module = module_list.GetModulePointerAtIndex(i);
             if (module) {
               num_dumped++;
               DumpModuleSections(m_interpreter, result.GetOutputStream(),
                                  module);
             }
           }
         } else {
if (num_modules == 0) {
       return false;
     }
 
     if (command.GetArgumentCount() == 0) {
       // Dump all ASTs for all modules images
       result.GetOutputStream().Format("Dumping clang ast for {0} modules.\n",
                                       num_modules);
       for (ModuleSP module_sp : module_list.ModulesNoLocking()) {
-        if (GetDebugger().InterruptRequested())
+        if (INTERRUPT_REQUESTED(GetDebugger(), "Interrupted dumping clang ast"))
           break;
         if (SymbolFile *sf = module_sp->GetSymbolFile())
           sf->DumpClangAST(result.GetOutputStream());
       }
       result.SetStatus(eReturnStatusSuccessFinishResult);
       return true;
     }
 
     // Dump specified ASTs (by basename or fullpath)
     for (const Args::ArgEntry &arg : command.entries()) {
       ModuleList module_list;
       const size_t num_matches =
           FindModulesByName(target, arg.c_str(), module_list, true);
       if (num_matches == 0) {
         // Check the global list
         std::lock_guard<std::recursive_mutex> guard(
             Module::GetAllocationModuleCollectionMutex());
 
         result.AppendWarningWithFormat(
             "Unable to find an image that matches '%s'.\n", arg.c_str());
         continue;
       }
 
       for (size_t i = 0; i < num_matches; ++i) {
-        if (GetDebugger().InterruptRequested())
+        if (INTERRUPT_REQUESTED(GetDebugger(), 
+              "Interrupted in dump clang ast list with {0} of {1} dumped.",
+              i, num_matches))
           break;
+
         Module *m = module_list.GetModulePointerAtIndex(i);
         if (SymbolFile *sf = m->GetSymbolFile())
           sf->DumpClangAST(result.GetOutputStream());
       }
     }
     result.SetStatus(eReturnStatusSuccessFinishResult);
     return true;
   }
if (command.GetArgumentCount() == 0) {
       const size_t num_modules = target_modules.GetSize();
       if (num_modules == 0) {
         result.AppendError("the target has no associated executable images");
         return false;
       }
       result.GetOutputStream().Format(
           "Dumping debug symbols for {0} modules.\n", num_modules);
       for (ModuleSP module_sp : target_modules.ModulesNoLocking()) {
-        if (GetDebugger().InterruptRequested())
+        if (INTERRUPT_REQUESTED(GetDebugger(), "Interrupted in dumping all "
+                                "debug symbols with {0} of {1} modules dumped",
+                                 num_dumped, num_modules))
           break;
+
         if (DumpModuleSymbolFile(result.GetOutputStream(), module_sp.get()))
           num_dumped++;
       }
     } else {
       // Dump specified images (by basename or fullpath)
       const char *arg_cstr;
       for (int arg_idx = 0;
            (arg_cstr = command.GetArgumentAtIndex(arg_idx)) != nullptr;
            ++arg_idx) {
         ModuleList module_list;
         const size_t num_matches =
             FindModulesByName(target, arg_cstr, module_list, true);
         if (num_matches > 0) {
           for (size_t i = 0; i < num_matches; ++i) {
-            if (GetDebugger().InterruptRequested())
+            if (INTERRUPT_REQUESTED(GetDebugger(), "Interrupted dumping {0} "
+                                                   "of {1} requested modules",
+                                                   i, num_matches))
               break;
             Module *module = module_list.GetModulePointerAtIndex(i);
             if (module) {
               if (DumpModuleSymbolFile(result.GetOutputStream(), module))
                 num_dumped++;
             }
           }
         } else
if (command.GetArgumentCount() == 0) {
       const char *arg_cstr;
       for (int arg_idx = 0;
            (arg_cstr = command.GetArgumentAtIndex(arg_idx)) != nullptr;
            ++arg_idx) {
         FileSpec file_spec(arg_cstr);
 
         const ModuleList &target_modules = target->GetImages();
         std::lock_guard<std::recursive_mutex> guard(target_modules.GetMutex());
-        if (target_modules.GetSize() > 0) {
+        size_t num_modules = target_modules.GetSize();
+        if (num_modules > 0) {
           uint32_t num_dumped = 0;
           for (ModuleSP module_sp : target_modules.ModulesNoLocking()) {
-            if (GetDebugger().InterruptRequested())
+            if (INTERRUPT_REQUESTED(GetDebugger(), 
+                                    "Interrupted in dump all line tables with "
+                                    "{0} of {1} dumped", num_dumped, 
+                                    num_modules))
               break;
+
             if (DumpCompileUnitLineTable(
                     m_interpreter, result.GetOutputStream(), module_sp.get(),
                     file_spec,
                     m_options.m_verbose ? eDescriptionLevelFull
                                         : eDescriptionLevelBrief))
               num_dumped++;
           }
           if (num_dumped == 0)

lldb/source/Commands/CommandObjectThread.cpp

bool HandleOneThread(lldb::tid_t tid, CommandReturnObject &result) override {
     const bool stop_format = true;
     if (!thread->GetStatus(strm, m_options.m_start, m_options.m_count,
                            num_frames_with_source, stop_format, only_stacks)) {
       result.AppendErrorWithFormat(
           "error displaying backtrace for thread: \"0x%4.4x\"\n",
           thread->GetIndexID());
       return false;
     }
-    if (m_options.m_extended_backtrace && !GetDebugger().InterruptRequested()) {
+    if (m_options.m_extended_backtrace) { 
+      if (!INTERRUPT_REQUESTED(GetDebugger(), 
+                              "Interrupt skipped extended backtrace")) {
         DoExtendedBacktrace(thread, result);
       }
+    }
 
     return true;
   }
 
   CommandOptions m_options;
 };
 
 enum StepScope { eStepScopeSource, eStepScopeInstruction };

lldb/source/Core/Debugger.cpp

 using namespace lldb;
 using namespace lldb_private;
 
 static lldb::user_id_t g_unique_id = 1;
 static size_t g_debugger_event_thread_stack_bytes = 8 * 1024 * 1024;
 
 #pragma mark Static Functions
 
-typedef std::vector<DebuggerSP> DebuggerList;
 static std::recursive_mutex *g_debugger_list_mutex_ptr =
     nullptr; // NOTE: intentional leak to avoid issues with C++ destructor chain
-static DebuggerList *g_debugger_list_ptr =
+static Debugger::DebuggerList *g_debugger_list_ptr =
     nullptr; // NOTE: intentional leak to avoid issues with C++ destructor chain
 static llvm::ThreadPool *g_thread_pool = nullptr;
 
 static constexpr OptionEnumValueElement g_show_disassembly_enum_values[] = {
     {
         Debugger::eStopDisassemblyTypeNever,
         "never",
         "Never show disassembly when displaying a stop context.",

 }
 
 void Debugger::CancelInterruptRequest() {
   std::lock_guard<std::mutex> guard(m_interrupt_mutex);
   if (m_interrupt_requested > 0)
     m_interrupt_requested--;
 }
 
 bool Debugger::InterruptRequested() {
   // This is the one we should call internally.  This will return true either
   // if there's a debugger interrupt and we aren't on the IOHandler thread,
   // or if we are on the IOHandler thread and there's a CommandInterpreter
   // interrupt.
   if (!IsIOHandlerThreadCurrentThread()) {
     std::lock_guard<std::mutex> guard(m_interrupt_mutex);
     return m_interrupt_requested != 0;
   }
   return GetCommandInterpreter().WasInterrupted();
 }
 
+Debugger::InterruptionReport::InterruptionReport(std::string function_name, 
+    const llvm::formatv_object_base &payload) :  
+        m_function_name(function_name), 
+        m_interrupt_time(std::chrono::system_clock::now()),
+        m_thread_id(llvm::get_threadid()) {
+  llvm::raw_string_ostream desc(m_description);
+  desc << payload << "\n";
+}
+
+void Debugger::ReportInterruption(const InterruptionReport &report) {
+    // For now, just log the description:
+  Log *log = GetLog(LLDBLog::Host);
+  LLDB_LOG(log, "Interruption: {0}", report.m_description);
+}
+
+Debugger::DebuggerList Debugger::DebuggersRequestingInterruption() {
+  DebuggerList result;
+  if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
+    std::lock_guard<std::recursive_mutex> guard(*g_debugger_list_mutex_ptr);
+    for (auto debugger_sp : *g_debugger_list_ptr) {
+      if (debugger_sp->InterruptRequested())
+        result.push_back(debugger_sp);
+    }
+  }
+  return result;
+}
+
 size_t Debugger::GetNumDebuggers() {
   if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
     std::lock_guard<std::recursive_mutex> guard(*g_debugger_list_mutex_ptr);
     return g_debugger_list_ptr->size();
   }
   return 0;
 }
 

lldb/source/Core/Module.cpp

void Module::FindTypes(
     TypeMap &types) {
   // If a scoped timer is needed, place it in a SymbolFile::FindTypes override.
   // A timer here is too high volume for some cases, for example when calling
   // FindTypes on each object file.
   if (SymbolFile *symbols = GetSymbolFile())
     symbols->FindTypes(pattern, languages, searched_symbol_files, types);
 }
 
+static Debugger::DebuggerList 
+DebuggersOwningModuleRequestingInterruption(Module &module) {
+  Debugger::DebuggerList requestors 
+      = Debugger::DebuggersRequestingInterruption();
+  Debugger::DebuggerList interruptors;
+  if (requestors.empty())
+    return interruptors;
+    
+  for (auto debugger_sp : requestors) {
+    if (!debugger_sp->InterruptRequested())
+      continue;
+    if (debugger_sp->GetTargetList()
+        .AnyTargetContainsModule(module))
+      interruptors.push_back(debugger_sp);
+  }
+  return interruptors;
+}
+
 SymbolFile *Module::GetSymbolFile(bool can_create, Stream *feedback_strm) {
   if (!m_did_load_symfile.load()) {
     std::lock_guard<std::recursive_mutex> guard(m_mutex);
     if (!m_did_load_symfile.load() && can_create) {
+      Debugger::DebuggerList interruptors 
+          = DebuggersOwningModuleRequestingInterruption(*this);
+      if (!interruptors.empty()) {
+        for (auto debugger_sp : interruptors) {
+          REPORT_INTERRUPTION(*(debugger_sp.get()), 
+                              "Interrupted fetching symbols for module {0}", 
+                              this->GetFileSpec());
+        }
+        return nullptr;
+      }
       ObjectFile *obj_file = GetObjectFile();
       if (obj_file != nullptr) {
         LLDB_SCOPED_TIMER();
         m_symfile_up.reset(
             SymbolVendor::FindPlugin(shared_from_this(), feedback_strm));
         m_did_load_symfile = true;
       }
     }

lldb/source/Interpreter/CommandInterpreter.cpp

bool CommandInterpreter::HandleCommand(const char *command_line,
 
   Log *log = GetLog(LLDBLog::Commands);
   llvm::PrettyStackTraceFormat stack_trace("HandleCommand(command = \"%s\")",
                                    command_line);
 
   LLDB_LOGF(log, "Processing command: %s", command_line);
   LLDB_SCOPED_TIMERF("Processing command: %s.", command_line);
 
-  if (GetDebugger().InterruptRequested()) {
+  if (INTERRUPT_REQUESTED(GetDebugger(), "Interrupted initiating command")) {
     result.AppendError("... Interrupted");
     return false;
   }
 
   bool add_to_history;
   if (lazy_add_to_history == eLazyBoolCalculate)
     add_to_history = (m_command_source_depth == 0);
   else
while (!str.empty()) {
     {
       std::lock_guard<std::recursive_mutex> guard(io_handler.GetOutputMutex());
       stream->Write(line.data(), line.size());
       stream->Write("\n", 1);
     }
   }
 
   std::lock_guard<std::recursive_mutex> guard(io_handler.GetOutputMutex());
-  if (had_output && GetDebugger().InterruptRequested())
+  if (had_output && INTERRUPT_REQUESTED(GetDebugger(), 
+                                        "Interrupted dumping command output"))
     stream->Printf("\n... Interrupted.\n");
   stream->Flush();
 }
 
 bool CommandInterpreter::EchoCommandNonInteractive(
     llvm::StringRef line, const Flags &io_handler_flags) const {
   if (!io_handler_flags.Test(eHandleCommandFlagEchoCommand))
     return false;

lldb/source/Target/StackFrameList.cpp

 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/Unwind.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 #include "llvm/ADT/SmallPtrSet.h"
 
 #include <memory>
 
 //#define DEBUG_STACK_FRAMES 1
 
 using namespace lldb;
 using namespace lldb_private;
 
 // StackFrameList constructor
 StackFrameList::StackFrameList(Thread &thread,
                                const lldb::StackFrameListSP &prev_frames_sp,
if (idx == 0) {
         }
       } else {
         unwind_frame_sp = m_frames.front();
         cfa = unwind_frame_sp->m_id.GetCallFrameAddress();
       }
     } else {
       // Check for interruption when building the frames.
       // Do the check in idx > 0 so that we'll always create a 0th frame.
-      if (allow_interrupt && dbg.InterruptRequested()) {
-        Log *log = GetLog(LLDBLog::Host);
-        LLDB_LOG(log, "Interrupted %s", __FUNCTION__);
+      if (allow_interrupt 
+          && INTERRUPT_REQUESTED(dbg, "Interrupted having fetched {0} frames",
+                                 m_frames.size())) {
           was_interrupted = true;
           break;
       }
 
       const bool success =
           unwinder.GetFrameInfoAtIndex(idx, cfa, pc, behaves_like_zeroth_frame);
       if (!success) {
         // We've gotten to the end of the stack.
         SetAllFramesFetched();
         break;
if (selected_frame_marker != nullptr) {
       if (frame_sp == selected_frame_sp)
         marker = selected_frame_marker;
       else
         marker = unselected_marker;
     }
     // Check for interruption here.  If we're fetching arguments, this loop
     // can go slowly:
     Debugger &dbg = m_thread.GetProcess()->GetTarget().GetDebugger();
-    if (dbg.InterruptRequested()) {
-      Log *log = GetLog(LLDBLog::Host);
-      LLDB_LOG(log, "Interrupted %s", __FUNCTION__);
+    if (INTERRUPT_REQUESTED(dbg, 
+          "Interrupted dumping stack for thread {0:hex} with {1} shown.",
+          m_thread.GetID(), num_frames_displayed))
       break;
-    }
+
 
     if (!frame_sp->GetStatus(strm, show_frame_info,
                              num_frames_with_source > (first_frame - frame_idx),
                              show_unique, marker))
       break;
     ++num_frames_displayed;
   }
 
   strm.IndentLess();
   return num_frames_displayed;
 }

lldb/source/Target/Target.cpp

if (module_sp) {
             return true;
           });
         }
 
         // Preload symbols outside of any lock, so hopefully we can do this for
         // each library in parallel.
         if (GetPreloadSymbols())
           module_sp->PreloadSymbols();
-
         llvm::SmallVector<ModuleSP, 1> replaced_modules;
         for (ModuleSP &old_module_sp : old_modules) {
           if (m_images.GetIndexForModule(old_module_sp.get()) !=
               LLDB_INVALID_INDEX32) {
             if (replaced_modules.empty())
               m_images.ReplaceModule(old_module_sp, module_sp);
             else
               m_images.Remove(old_module_sp);

 }
 
 bool TargetProperties::SetPreferDynamicValue(lldb::DynamicValueType d) {
   const uint32_t idx = ePropertyPreferDynamic;
   return SetPropertyAtIndex(idx, d);
 }
 
 bool TargetProperties::GetPreloadSymbols() const {
+  if (INTERRUPT_REQUESTED(m_target->GetDebugger(), 
+      "Interrupted checking preload symbols")) {
+    return false;
+  }
   const uint32_t idx = ePropertyPreloadSymbols;
   return GetPropertyAtIndexAs<bool>(
       idx, g_target_properties[idx].default_uint_value != 0);
 }
 
 void TargetProperties::SetPreloadSymbols(bool b) {
   const uint32_t idx = ePropertyPreloadSymbols;
   SetPropertyAtIndex(idx, b);

lldb/source/Target/TargetList.cpp

if (error.Success() && exe_module_sp) {
               arch.GetArchitectureName());
         } else {
           error.SetErrorStringWithFormat("unsupported file type \"%s\"",
                                          file.GetPath().c_str());
         }
         return error;
       }
       target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target));
+      debugger.GetTargetList().RegisterInProcessTarget(target_sp);
       target_sp->SetExecutableModule(exe_module_sp, load_dependent_files);
       if (user_exe_path_is_bundle)
         exe_module_sp->GetFileSpec().GetPath(resolved_bundle_exe_path,
                                              sizeof(resolved_bundle_exe_path));
       if (target_sp->GetPreloadSymbols())
         exe_module_sp->PreloadSymbols();
     }
   } else {
     // No file was specified, just create an empty target with any arch if a
     // valid arch was specified
     target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target));
+    debugger.GetTargetList().RegisterInProcessTarget(target_sp);
   }
 
   if (!target_sp)
     return error;
 
   // Set argv0 with what the user typed, unless the user specified a
   // directory. If the user specified a directory, then it is probably a
   // bundle that was resolved and we need to use the resolved bundle path
uint32_t TargetList::GetIndexOfTarget(lldb::TargetSP target_sp) const {
   if (it != m_target_list.end())
     return std::distance(m_target_list.begin(), it);
   return UINT32_MAX;
 }
 
 void TargetList::AddTargetInternal(TargetSP target_sp, bool do_select) {
   lldbassert(!llvm::is_contained(m_target_list, target_sp) &&
              "target already exists it the list");
+  UnregisterInProcessTarget(target_sp);
   m_target_list.push_back(std::move(target_sp));
   if (do_select)
     SetSelectedTargetInternal(m_target_list.size() - 1);
 }
 
 void TargetList::SetSelectedTargetInternal(uint32_t index) {
   lldbassert(!m_target_list.empty());
   m_selected_target_idx = index < m_target_list.size() ? index : 0;

 }
 
 lldb::TargetSP TargetList::GetSelectedTarget() {
   std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
   if (m_selected_target_idx >= m_target_list.size())
     m_selected_target_idx = 0;
   return GetTargetAtIndex(m_selected_target_idx);
 }
+
+bool TargetList::AnyTargetContainsModule(Module &module) {
+  std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
+  for (const auto &target_sp : m_target_list) {
+    if (target_sp->GetImages().FindModule(&module))
+      return true;
+  }
+  for (const auto &target_sp: m_in_process_target_list) {
+    if (target_sp->GetImages().FindModule(&module))
+      return true;
+  }
+  return false;
+}
+
+  void TargetList::RegisterInProcessTarget(TargetSP target_sp) {
+    std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
+    std::unordered_set<TargetSP>::iterator iter;
+    bool was_added;
+    std::tie(iter, was_added) = m_in_process_target_list.insert(target_sp);
+    assert(was_added && "Target pointer was left in the in-process map");
+  }
+  
+  void TargetList::UnregisterInProcessTarget(TargetSP target_sp) {
+    std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
+    bool was_present = m_in_process_target_list.erase(target_sp);
+    assert(was_present && "Target pointer being removed was not registered");
+  }
+  
+  bool TargetList::IsTargetInProcess(TargetSP target_sp) {
+    std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
+    return m_in_process_target_list.count(target_sp) == 1; 
+  }