jingham updated this revision to Diff 187616.
jingham added a comment.

Trying to guess how thread list output is going to look is indeed a losing game.

I changed the stop hook to set a global variable in the target, then just print 
the value at the end.  If we ran the stop hook the right number of times, the 
variable will have the right value.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58394/new/

https://reviews.llvm.org/D58394

Files:
  include/lldb/Target/Process.h
  include/lldb/Target/Target.h
  lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit
  lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit
  lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit
  lit/ExecControl/StopHook/Inputs/stop-hook-threads-2.lldbinit
  lit/ExecControl/StopHook/Inputs/stop-hook-threads.cpp
  lit/ExecControl/StopHook/stop-hook-threads.test
  source/Commands/CommandObjectTarget.cpp
  source/Target/Process.cpp
  source/Target/Target.cpp

Index: source/Target/Target.cpp
===================================================================
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -2554,12 +2554,14 @@
 
   StopHookCollection::iterator pos, end = m_stop_hooks.end();
 
-  // If there aren't any active stop hooks, don't bother either:
+  // If there aren't any active stop hooks, don't bother either.
+  // Also see if any of the active hooks want to auto-continue.
   bool any_active_hooks = false;
-  for (pos = m_stop_hooks.begin(); pos != end; pos++) {
-    if ((*pos).second->IsActive()) {
+  bool auto_continue = false;
+  for (auto hook : m_stop_hooks) {
+    if (hook.second->IsActive()) {
       any_active_hooks = true;
-      break;
+      auto_continue |= hook.second->GetAutoContinue();
     }
   }
   if (!any_active_hooks)
@@ -2595,6 +2597,7 @@
   bool hooks_ran = false;
   bool print_hook_header = (m_stop_hooks.size() != 1);
   bool print_thread_header = (num_exe_ctx != 1);
+  bool did_restart = false;
 
   for (pos = m_stop_hooks.begin(); keep_going && pos != end; pos++) {
     // result.Clear();
@@ -2639,10 +2642,13 @@
         options.SetPrintResults(true);
         options.SetAddToHistory(false);
 
+        // Force Async:
+        bool old_async = GetDebugger().GetAsyncExecution();
+        GetDebugger().SetAsyncExecution(true);
         GetDebugger().GetCommandInterpreter().HandleCommands(
             cur_hook_sp->GetCommands(), &exc_ctx_with_reasons[i], options,
             result);
-
+        GetDebugger().SetAsyncExecution(old_async);
         // If the command started the target going again, we should bag out of
         // running the stop hooks.
         if ((result.GetStatus() == eReturnStatusSuccessContinuingNoResult) ||
@@ -2651,13 +2657,19 @@
           StopHookCollection::iterator tmp = pos;
           if (++tmp != end)
             result.AppendMessageWithFormat("\nAborting stop hooks, hook %" PRIu64
-                                           " set the program running.\n",
+                                           " set the program running.\n"
+                                           "  Consider using '-G true' to make "
+                                           "stop hooks auto-continue.\n",
                                            cur_hook_sp->GetID());
           keep_going = false;
+          did_restart = true;
         }
       }
     }
   }
+  // Finally, if auto-continue was requested, do it now:
+  if (!did_restart && auto_continue)
+    m_process_sp->PrivateResume();
 
   result.GetImmediateOutputStream()->Flush();
   result.GetImmediateErrorStream()->Flush();
@@ -3143,12 +3155,13 @@
 //--------------------------------------------------------------
 Target::StopHook::StopHook(lldb::TargetSP target_sp, lldb::user_id_t uid)
     : UserID(uid), m_target_sp(target_sp), m_commands(), m_specifier_sp(),
-      m_thread_spec_up(), m_active(true) {}
+      m_thread_spec_up() {}
 
 Target::StopHook::StopHook(const StopHook &rhs)
     : UserID(rhs.GetID()), m_target_sp(rhs.m_target_sp),
       m_commands(rhs.m_commands), m_specifier_sp(rhs.m_specifier_sp),
-      m_thread_spec_up(), m_active(rhs.m_active) {
+      m_thread_spec_up(), m_active(rhs.m_active),
+      m_auto_continue(rhs.m_auto_continue) {
   if (rhs.m_thread_spec_up)
     m_thread_spec_up.reset(new ThreadSpec(*rhs.m_thread_spec_up));
 }
@@ -3175,6 +3188,9 @@
   else
     s->Indent("State: disabled\n");
 
+  if (m_auto_continue)
+    s->Indent("AutoContinue on\n");
+
   if (m_specifier_sp) {
     s->Indent();
     s->PutCString("Specifier:\n");
Index: source/Target/Process.cpp
===================================================================
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -1615,6 +1615,8 @@
   return error;
 }
 
+static const char *g_resume_sync_name = "lldb.Process.ResumeSynchronous.hijack";
+
 Status Process::ResumeSynchronous(Stream *stream) {
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_STATE |
                                                   LIBLLDB_LOG_PROCESS));
@@ -1628,7 +1630,7 @@
   }
 
   ListenerSP listener_sp(
-      Listener::MakeListener("lldb.Process.ResumeSynchronous.hijack"));
+      Listener::MakeListener(g_resume_sync_name));
   HijackProcessEvents(listener_sp);
 
   Status error = PrivateResume();
@@ -1652,6 +1654,11 @@
   return error;
 }
 
+bool Process::IsHijackedForSynchronousResume() {
+  const char *hijacker_name = GetHijackingListenerName();
+  return strcmp(hijacker_name, g_resume_sync_name) == 0;
+}
+
 StateType Process::GetPrivateState() { return m_private_state.GetValue(); }
 
 void Process::SetPrivateState(StateType new_state) {
@@ -4260,11 +4267,20 @@
         // public resume.
         process_sp->PrivateResume();
       } else {
-        // If we didn't restart, run the Stop Hooks here: They might also
-        // restart the target, so watch for that.
-        process_sp->GetTarget().RunStopHooks();
-        if (process_sp->GetPrivateState() == eStateRunning)
-          SetRestarted(true);
+        bool hijacked = 
+            process_sp->IsHijackedForEvent(eBroadcastBitStateChanged)
+            && !process_sp->IsHijackedForSynchronousResume();
+
+        if (!hijacked) {
+          // If we didn't restart, run the Stop Hooks here.
+          // Don't do that if state changed events aren't hooked up to the
+          // public (or SyncResume) broadcasters.  StopHooks are just for 
+          // real public stops.  They might also restart the target, 
+          // so watch for that.
+          process_sp->GetTarget().RunStopHooks();
+          if (process_sp->GetPrivateState() == eStateRunning)
+            SetRestarted(true);
+        }
       }
     }
   }
Index: source/Commands/CommandObjectTarget.cpp
===================================================================
--- source/Commands/CommandObjectTarget.cpp
+++ source/Commands/CommandObjectTarget.cpp
@@ -4555,7 +4555,7 @@
 
 static constexpr OptionDefinition g_target_stop_hook_add_options[] = {
     // clang-format off
-  { LLDB_OPT_SET_ALL, false, "one-liner",    'o', OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeOneLiner,                                         "Specify a one-line breakpoint command inline. Be sure to surround it with quotes." },
+  { LLDB_OPT_SET_ALL, false, "one-liner",    'o', OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeOneLiner,                                         "Add a command for the stop hook.  Can be specified more than once, and commands will be run in the order they appear." },
   { LLDB_OPT_SET_ALL, false, "shlib",        's', OptionParser::eRequiredArgument, nullptr, {}, CommandCompletions::eModuleCompletion, eArgTypeShlibName,    "Set the module within which the stop-hook is to be run." },
   { LLDB_OPT_SET_ALL, false, "thread-index", 'x', OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeThreadIndex,                                      "The stop hook is run only for the thread whose index matches this argument." },
   { LLDB_OPT_SET_ALL, false, "thread-id",    't', OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeThreadID,                                         "The stop hook is run only for the thread whose TID matches this argument." },
@@ -4566,6 +4566,7 @@
   { LLDB_OPT_SET_1,   false, "end-line",     'e', OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeLineNum,                                          "Set the end of the line range for which the stop-hook is to be run." },
   { LLDB_OPT_SET_2,   false, "classname",    'c', OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeClassName,                                        "Specify the class within which the stop-hook is to be run." },
   { LLDB_OPT_SET_3,   false, "name",         'n', OptionParser::eRequiredArgument, nullptr, {}, CommandCompletions::eSymbolCompletion, eArgTypeFunctionName, "Set the function name within which the stop hook will be run." },
+  { LLDB_OPT_SET_ALL, false, "auto-continue",'G', OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeBoolean,     "The breakpoint will auto-continue after running its commands." },
     // clang-format on
 };
 
@@ -4606,6 +4607,17 @@
         m_sym_ctx_specified = true;
         break;
 
+      case 'G': {
+        bool value, success;
+        value = OptionArgParser::ToBoolean(option_arg, false, &success);
+        if (success) {
+          m_auto_continue = value;
+        } else
+          error.SetErrorStringWithFormat(
+              "invalid boolean value '%s' passed for -G option",
+              option_arg.str().c_str());
+      }
+      break;
       case 'l':
         if (option_arg.getAsInteger(0, m_line_start)) {
           error.SetErrorStringWithFormat("invalid start line number: \"%s\"",
@@ -4661,7 +4673,7 @@
 
       case 'o':
         m_use_one_liner = true;
-        m_one_liner = option_arg;
+        m_one_liner.push_back(option_arg);
         break;
 
       default:
@@ -4690,6 +4702,7 @@
 
       m_use_one_liner = false;
       m_one_liner.clear();
+      m_auto_continue = false;
     }
 
     std::string m_class_name;
@@ -4708,7 +4721,8 @@
     bool m_thread_specified;
     // Instance variables to hold the values for one_liner options.
     bool m_use_one_liner;
-    std::string m_one_liner;
+    std::vector<std::string> m_one_liner;
+    bool m_auto_continue;
   };
 
   CommandObjectTargetStopHookAdd(CommandInterpreter &interpreter)
@@ -4833,10 +4847,13 @@
 
         new_hook_sp->SetThreadSpecifier(thread_spec);
       }
+      
+      new_hook_sp->SetAutoContinue(m_options.m_auto_continue);
       if (m_options.m_use_one_liner) {
-        // Use one-liner.
-        new_hook_sp->GetCommandPointer()->AppendString(
-            m_options.m_one_liner.c_str());
+        // Use one-liners.
+        for (auto cmd : m_options.m_one_liner)
+          new_hook_sp->GetCommandPointer()->AppendString(
+            cmd.c_str());
         result.AppendMessageWithFormat("Stop hook #%" PRIu64 " added.\n",
                                        new_hook_sp->GetID());
       } else {
Index: lit/ExecControl/StopHook/stop-hook-threads.test
===================================================================
--- lit/ExecControl/StopHook/stop-hook-threads.test
+++ lit/ExecControl/StopHook/stop-hook-threads.test
@@ -12,23 +12,22 @@
 
 # CHECK: Hook: 1
 # CHECK-NEXT:  State: enabled
+# CHECK-NO-FILTER-NEXT: AutoContinue on
 # CHECK-FILTER-NEXT:  Thread
 # CHECK-FILTER-NEXT:  index: 2
 # CHECK-NEXT:  Commands: 
-# CHECK-NEXT:    frame variable
+# CHECK-NEXT:    expr lldb_val += 1
+# CHECK-NEXT:    thread list
 
 # CHECK-FILTER: Hook: 2
 # CHECK-FILTER-NEXT:  State: enabled
+# CHECK-FILTER-NEXT:  AutoContinue on
 # CHECK-FILTER-NEXT:  Commands: 
-# CHECK-FILTER-NEXT:    continue
+# CHECK-FILTER-NEXT:    script print('Hit stop hook')
 
 # Get the threads going
 continue
 
-# When we filter per thread, we expect exactly 4 identical "frame var" results
-# CHECK-FILTER: (uint32_t) thread_index = [[THREAD_INDEX:[0-9]*]]
-# CHECK-FILTER-COUNT-3: (uint32_t) thread_index = [[THREAD_INDEX]]
-# CHECK-FILTER-NOT: thread_index
-
-# When we don't filter, we expect to count 12 stopped threads in the thread list output
-# CHECK-NO-FILTER-COUNT-12: at stop-hook-threads.cpp{{.*}} stop reason = breakpoint
\ No newline at end of file
+# Now make sure we hit the command the right number of times:
+# CHECK-NO-FILTER: lldb_val was set to: 15.
+# CHECK-FILTER: lldb_val was set to: 5.
Index: lit/ExecControl/StopHook/Inputs/stop-hook-threads.cpp
===================================================================
--- lit/ExecControl/StopHook/Inputs/stop-hook-threads.cpp
+++ lit/ExecControl/StopHook/Inputs/stop-hook-threads.cpp
@@ -16,6 +16,7 @@
 std::uniform_int_distribution<> g_distribution{0, 3000};
 
 uint32_t g_val = 0;
+uint32_t lldb_val = 0;
 
 uint32_t
 access_pool (bool flag = false)
@@ -62,7 +63,8 @@
 int main (int argc, char const *argv[])
 {
     std::thread threads[3];
-
+    // Break here to set up the stop hook
+    printf("Stop hooks engaged.\n");
     // Create 3 threads
     for (auto &thread : threads)
         thread = std::thread{thread_func, std::distance(threads, &thread)};
@@ -71,5 +73,7 @@
     for (auto &thread : threads)
         thread.join();
 
+    // print lldb_val so we can check it here.
+    printf ("lldb_val was set to: %d.\n", lldb_val);
     return 0;
 }
Index: lit/ExecControl/StopHook/Inputs/stop-hook-threads-2.lldbinit
===================================================================
--- lit/ExecControl/StopHook/Inputs/stop-hook-threads-2.lldbinit
+++ lit/ExecControl/StopHook/Inputs/stop-hook-threads-2.lldbinit
@@ -1,4 +1,5 @@
+break set -f stop-hook-threads.cpp -p "Break here to set up the stop hook"
 break set -f stop-hook-threads.cpp -p "Break here to test that the stop-hook"
 run
-target stop-hook add -x 2 -o "frame variable thread_index"
-target stop-hook add -o continue
+target stop-hook add -x 2 -o "expr lldb_val += 1" -o "thread list"
+target stop-hook add -G true -o "script print('Hit stop hook') 
Index: lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit
===================================================================
--- lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit
+++ lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit
@@ -1,7 +1,7 @@
+break set -f stop-hook-threads.cpp -p "Break here to set up the stop hook"
 break set -f stop-hook-threads.cpp -p "Break here to test that the stop-hook"
 run
-target stop-hook add
-frame variable --show-globals g_val
+target stop-hook add -G true
+expr lldb_val += 1
 thread list
-continue
 DONE
Index: lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit
===================================================================
--- lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit
+++ lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit
@@ -1,3 +1,3 @@
 target stop-hook add -f stop-hook.c -l 29 -e 34
 expr ptr
-DONE
\ No newline at end of file
+DONE
Index: lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit
===================================================================
--- lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit
+++ lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit
@@ -1 +1 @@
-target stop-hook add -f stop-hook.c -l 29 -e 34 -o "expr ptr"
\ No newline at end of file
+target stop-hook add -f stop-hook.c -l 29 -e 34 -o "expr ptr"
Index: include/lldb/Target/Target.h
===================================================================
--- include/lldb/Target/Target.h
+++ include/lldb/Target/Target.h
@@ -1153,6 +1153,10 @@
 
     void SetIsActive(bool is_active) { m_active = is_active; }
 
+    void SetAutoContinue(bool auto_continue) {m_auto_continue = auto_continue;}
+
+    bool GetAutoContinue() const { return m_auto_continue; }
+
     void GetDescription(Stream *s, lldb::DescriptionLevel level) const;
 
   private:
@@ -1160,7 +1164,8 @@
     StringList m_commands;
     lldb::SymbolContextSpecifierSP m_specifier_sp;
     std::unique_ptr<ThreadSpec> m_thread_spec_up;
-    bool m_active;
+    bool m_active = true;
+    bool m_auto_continue = false;
 
     // Use CreateStopHook to make a new empty stop hook. The GetCommandPointer
     // and fill it with commands, and SetSpecifier to set the specifier shared
Index: include/lldb/Target/Process.h
===================================================================
--- include/lldb/Target/Process.h
+++ include/lldb/Target/Process.h
@@ -2519,6 +2519,8 @@
   ///
   //------------------------------------------------------------------
   void RestoreProcessEvents();
+  
+  bool IsHijackedForSynchronousResume();
 
   const lldb::ABISP &GetABI();
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to