JDevlieghere created this revision.
JDevlieghere added reviewers: labath, friss.
Herald added a subscriber: abidh.

Add synchronization for asynchronous events. This fixes an un expected packet 
during replay when an asynchronous event triggers a GDB packet. Consider the 
following example:

  $ ./bin/lldb --capture
  (lldb) gdb-remote 13000
  Process 770 stopped
  * thread #1, stop reason = signal SIGSTOP
      frame #0: 0x0000000105071000 dyld`_dyld_start
  (lldb) cont
  Process 770 resuming
  Process 770 exited with status = 0 (0x00000000)
  (lldb) reproducer generate
  
  $ ./bin/lldb --replay /path/to/reproducer
  (lldb) gdb-remote 13000
  (lldb) cont
  Reproducer expected packet: '$jThreadExtendedInfo:{"thread":14341}#01'
  Reproducer received packet: 'c'
  LLVM ERROR: Encountered unexpected packet during replay

The way the thread state is displayed at the prompt is asynchronous. It reacts 
to an event, in this case a process state change on connection.

I haven't bothered too much with naming. The same logic should be applicable to 
other situations that require synchronization.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D83446

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Utility/Reproducer.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Utility/Reproducer.cpp

Index: lldb/source/Utility/Reproducer.cpp
===================================================================
--- lldb/source/Utility/Reproducer.cpp
+++ lldb/source/Utility/Reproducer.cpp
@@ -13,6 +13,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h"
+#include <memory>
 
 using namespace lldb_private;
 using namespace lldb_private::repro;
@@ -305,12 +306,109 @@
     m_collector->addDirectory(dir);
 }
 
+std::unique_ptr<Synchronizer> SynchronizerProvider::GetNewSynchronizer() {
+  return std::make_unique<CaptureSynchronizer>(GetNewRecorder());
+}
+
+CaptureSynchronizer::CaptureSynchronizer(YamlRecorder *recorder)
+    : m_recorder(recorder) {
+  assert(recorder);
+}
+
+void CaptureSynchronizer::AddEvent() {
+  std::lock_guard<std::mutex> lock(m_mutex);
+  m_current.events++;
+  m_events_modified = true;
+  AddBarrier();
+}
+
+void CaptureSynchronizer::AddCommand() {
+  std::lock_guard<std::mutex> lock(m_mutex);
+  m_current.commands++;
+  m_commands_modified = true;
+  AddBarrier();
+}
+
+void CaptureSynchronizer::AddBarrier() {
+  // Caller locks.
+  if (m_commands_modified && m_events_modified) {
+    if (m_recorder)
+      m_recorder->Record(m_current);
+    m_commands_modified = false;
+    m_events_modified = false;
+  }
+}
+
+ReplaySynchronizer::ReplaySynchronizer(
+    std::vector<Synchronizer::Barrier> barriers)
+    : m_barriers(std::move(barriers)) {}
+
+void llvm::yaml::MappingTraits<Synchronizer::Barrier>::mapping(
+    IO &io, Synchronizer::Barrier &B) {
+  io.mapRequired("commands", B.commands);
+  io.mapRequired("events", B.events);
+}
+
+void ReplaySynchronizer::AddEvent() {
+  m_events++;
+  m_condition_variable.notify_one();
+}
+
+void ReplaySynchronizer::AddCommand() {
+  m_commands++;
+
+  if (m_barriers.empty())
+    return;
+
+  if (m_commands > m_barriers.back().commands) {
+    std::unique_lock<std::mutex> lock(m_mutex);
+    m_condition_variable.wait(
+        lock, [&] { return m_events >= m_barriers.back().events; });
+    m_barriers.pop_back();
+  }
+}
+
+std::unique_ptr<Synchronizer> lldb_private::repro::GetNewSynchronizer() {
+  auto &r = repro::Reproducer::Instance();
+
+  if (repro::Generator *g = r.GetGenerator()) {
+    return g->GetOrCreate<repro::SynchronizerProvider>().GetNewSynchronizer();
+  }
+
+  if (repro::Loader *l = r.GetLoader()) {
+    static std::unique_ptr<repro::MultiLoader<SynchronizerProvider>>
+        multi_loader = repro::MultiLoader<SynchronizerProvider>::Create(l);
+    if (!multi_loader)
+      return {};
+
+    llvm::Optional<std::string> file = multi_loader->GetNextFile();
+    if (!file)
+      return {};
+
+    auto buffer = llvm::MemoryBuffer::getFile(*file);
+    if (auto err = buffer.getError())
+      return {};
+
+    std::vector<Synchronizer::Barrier> barriers;
+    yaml::Input yin((*buffer)->getBuffer());
+    yin >> barriers;
+
+    /// The ReplaySynchronizer treats barriers like a stack.
+    std::reverse(barriers.begin(), barriers.end());
+
+    return std::make_unique<ReplaySynchronizer>(std::move(barriers));
+  }
+
+  return {};
+}
+
 void ProviderBase::anchor() {}
 char CommandProvider::ID = 0;
 char FileProvider::ID = 0;
 char ProviderBase::ID = 0;
 char VersionProvider::ID = 0;
 char WorkingDirectoryProvider::ID = 0;
+char SynchronizerProvider::ID = 0;
 const char *CommandProvider::Info::file = "command-interpreter.yaml";
 const char *CommandProvider::Info::name = "command-interpreter";
 const char *FileProvider::Info::file = "files.yaml";
@@ -319,3 +417,5 @@
 const char *VersionProvider::Info::name = "version";
 const char *WorkingDirectoryProvider::Info::file = "cwd.txt";
 const char *WorkingDirectoryProvider::Info::name = "cwd";
+const char *SynchronizerProvider::Info::file = "synchronizer.yaml";
+const char *SynchronizerProvider::Info::name = "synchronizer";
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===================================================================
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2773,22 +2773,25 @@
     if (WasInterrupted())
       return;
 
-  const bool is_interactive = io_handler.GetIsInteractive();
-  if (!is_interactive) {
-    // When we are not interactive, don't execute blank lines. This will happen
-    // sourcing a commands file. We don't want blank lines to repeat the
-    // previous command and cause any errors to occur (like redefining an
-    // alias, get an error and stop parsing the commands file).
-    if (line.empty())
-      return;
+    if (repro::Synchronizer *synchronizer = m_debugger.GetSynchronizer())
+      synchronizer->AddCommand();
+
+    const bool is_interactive = io_handler.GetIsInteractive();
+    if (!is_interactive) {
+      // When we are not interactive, don't execute blank lines. This will
+      // happen sourcing a commands file. We don't want blank lines to repeat
+      // the previous command and cause any errors to occur (like redefining an
+      // alias, get an error and stop parsing the commands file).
+      if (line.empty())
+        return;
 
-    // When using a non-interactive file handle (like when sourcing commands
-    // from a file) we need to echo the command out so we don't just see the
-    // command output and no command...
-    if (EchoCommandNonInteractive(line, io_handler.GetFlags()))
-      io_handler.GetOutputStreamFileSP()->Printf(
-          "%s%s\n", io_handler.GetPrompt(), line.c_str());
-  }
+      // When using a non-interactive file handle (like when sourcing commands
+      // from a file) we need to echo the command out so we don't just see the
+      // command output and no command...
+      if (EchoCommandNonInteractive(line, io_handler.GetFlags()))
+        io_handler.GetOutputStreamFileSP()->Printf(
+            "%s%s\n", io_handler.GetPrompt(), line.c_str());
+    }
 
   StartHandlingCommand();
 
Index: lldb/source/Core/Debugger.cpp
===================================================================
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -665,7 +665,8 @@
       m_io_handler_stack(), m_instance_name(), m_loaded_plugins(),
       m_event_handler_thread(), m_io_handler_thread(),
       m_sync_broadcaster(nullptr, "lldb.debugger.sync"),
-      m_forward_listener_sp(), m_clear_once() {
+      m_forward_listener_sp(), m_synchronizer_up(repro::GetNewSynchronizer()),
+      m_clear_once() {
   char instance_cstr[256];
   snprintf(instance_cstr, sizeof(instance_cstr), "debugger_%d", (int)GetID());
   m_instance_name.SetCString(instance_cstr);
@@ -1467,6 +1468,9 @@
 
         if (m_forward_listener_sp)
           m_forward_listener_sp->AddEvent(event_sp);
+
+        if (m_synchronizer_up)
+          m_synchronizer_up->AddEvent();
       }
     }
   }
Index: lldb/include/lldb/Utility/Reproducer.h
===================================================================
--- lldb/include/lldb/Utility/Reproducer.h
+++ lldb/include/lldb/Utility/Reproducer.h
@@ -457,7 +457,76 @@
   unsigned m_index = 0;
 };
 
+class Synchronizer {
+public:
+  virtual ~Synchronizer() = default;
+  virtual void AddEvent() = 0;
+  virtual void AddCommand() = 0;
+
+  struct Barrier {
+    uint64_t commands = 0;
+    uint64_t events = 0;
+  };
+};
+
+class SynchronizerProvider
+    : public MultiProvider<YamlRecorder, SynchronizerProvider> {
+public:
+  struct Info {
+    static const char *name;
+    static const char *file;
+  };
+
+  SynchronizerProvider(const FileSpec &directory) : MultiProvider(directory) {}
+
+  std::unique_ptr<Synchronizer> GetNewSynchronizer();
+
+  static char ID;
+};
+
+class CaptureSynchronizer : public Synchronizer {
+public:
+  CaptureSynchronizer(YamlRecorder *recorder);
+  void AddEvent() override;
+  void AddCommand() override;
+
+private:
+  void AddBarrier();
+
+  std::mutex m_mutex;
+  YamlRecorder *m_recorder;
+  Barrier m_current;
+  bool m_commands_modified;
+  bool m_events_modified;
+};
+
+class ReplaySynchronizer : public Synchronizer {
+public:
+  ReplaySynchronizer(std::vector<Synchronizer::Barrier> barriers);
+  void AddEvent() override;
+  void AddCommand() override;
+
+private:
+  std::mutex m_mutex;
+  std::condition_variable m_condition_variable;
+  std::vector<Synchronizer::Barrier> m_barriers;
+  uint64_t m_commands = 0;
+  uint64_t m_events = 0;
+};
+
+std::unique_ptr<Synchronizer> GetNewSynchronizer();
+
 } // namespace repro
 } // namespace lldb_private
 
+LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(lldb_private::repro::Synchronizer::Barrier)
+
+namespace llvm {
+namespace yaml {
+template <> struct MappingTraits<lldb_private::repro::Synchronizer::Barrier> {
+  static void mapping(IO &io, lldb_private::repro::Synchronizer::Barrier &B);
+};
+} // namespace yaml
+} // namespace llvm
+
 #endif // LLDB_UTILITY_REPRODUCER_H
Index: lldb/include/lldb/Core/Debugger.h
===================================================================
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -27,6 +27,7 @@
 #include "lldb/Utility/Broadcaster.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/UserID.h"
 #include "lldb/lldb-defines.h"
@@ -339,6 +340,8 @@
     return m_broadcaster_manager_sp;
   }
 
+  repro::Synchronizer *GetSynchronizer() { return m_synchronizer_up.get(); }
+
 protected:
   friend class CommandInterpreter;
   friend class REPL;
@@ -430,6 +433,7 @@
   HostThread m_io_handler_thread;
   Broadcaster m_sync_broadcaster;
   lldb::ListenerSP m_forward_listener_sp;
+  std::unique_ptr<repro::Synchronizer> m_synchronizer_up;
   llvm::once_flag m_clear_once;
   lldb::TargetSP m_dummy_target_sp;
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to