JDevlieghere updated this revision to Diff 310649.
JDevlieghere added a comment.

Change test to unit test.


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

https://reviews.llvm.org/D92820

Files:
  lldb/include/lldb/Utility/ReproducerInstrumentation.h
  lldb/source/Utility/ReproducerInstrumentation.cpp
  lldb/unittests/Utility/ReproducerInstrumentationTest.cpp

Index: lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
===================================================================
--- lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
+++ lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
@@ -576,8 +576,11 @@
   std::string str;
   llvm::raw_string_ostream os(str);
 
+  unsigned index = 0;
+
   Serializer serializer(os);
-  serializer.SerializeAll(static_cast<unsigned>(1), static_cast<unsigned>(2));
+  serializer.SerializeAll(index, static_cast<unsigned>(1));
+  serializer.SerializeAll(index, static_cast<unsigned>(2));
   serializer.SerializeAll(&foo, &bar);
 
   llvm::StringRef buffer(os.str());
@@ -597,8 +600,11 @@
   std::string str;
   llvm::raw_string_ostream os(str);
 
+  unsigned index = 0;
+
   Serializer serializer(os);
-  serializer.SerializeAll(static_cast<unsigned>(1), static_cast<unsigned>(2));
+  serializer.SerializeAll(index, static_cast<unsigned>(1));
+  serializer.SerializeAll(index, static_cast<unsigned>(2));
   serializer.SerializeAll(foo, bar);
 
   llvm::StringRef buffer(os.str());
@@ -1114,3 +1120,48 @@
     bar.Validate();
   }
 }
+
+TEST(RecordReplayTest, ValidIndex) {
+  std::string str;
+  llvm::raw_string_ostream os(str);
+
+  {
+    auto data = TestInstrumentationDataRAII::GetRecordingData(os);
+
+    unsigned index = 1;
+    int (*f)() = &lldb_private::repro::invoke<int (*)()>::method<
+        InstrumentedFoo::F>::record;
+    unsigned id = g_registry->GetID(uintptr_t(f));
+    g_serializer->SerializeAll(index, id);
+
+    unsigned result = 0;
+    g_serializer->SerializeAll(index, result);
+  }
+
+  TestingRegistry registry;
+  Deserializer deserializer(os.str());
+  registry.Replay(deserializer);
+}
+
+TEST(RecordReplayTest, InvalidIndex) {
+  std::string str;
+  llvm::raw_string_ostream os(str);
+
+  {
+    auto data = TestInstrumentationDataRAII::GetRecordingData(os);
+
+    unsigned index = 1;
+    int (*f)() = &lldb_private::repro::invoke<int (*)()>::method<
+        InstrumentedFoo::F>::record;
+    unsigned id = g_registry->GetID(uintptr_t(f));
+    g_serializer->SerializeAll(index, id);
+
+    unsigned result = 0;
+    unsigned invalid_index = 2;
+    g_serializer->SerializeAll(invalid_index, result);
+  }
+
+  TestingRegistry registry;
+  Deserializer deserializer(os.str());
+  EXPECT_DEATH(registry.Replay(deserializer), "");
+}
Index: lldb/source/Utility/ReproducerInstrumentation.cpp
===================================================================
--- lldb/source/Utility/ReproducerInstrumentation.cpp
+++ lldb/source/Utility/ReproducerInstrumentation.cpp
@@ -8,6 +8,7 @@
 
 #include "lldb/Utility/ReproducerInstrumentation.h"
 #include "lldb/Utility/Reproducer.h"
+#include <limits>
 #include <stdio.h>
 #include <stdlib.h>
 #include <thread>
@@ -84,6 +85,16 @@
   return r;
 }
 
+void Deserializer::CheckIndex(unsigned idx) {
+  if (m_expected_idx && *m_expected_idx != idx)
+    llvm::report_fatal_error(
+        "The result does not match the preceding "
+        "function. This is probably the result of concurrent "
+        "use of the SB API during capture, which is currently not "
+        "supported.");
+  m_expected_idx.reset();
+}
+
 bool Registry::Replay(const FileSpec &file) {
   auto error_or_file = llvm::MemoryBuffer::getFile(file.GetPath());
   if (auto err = error_or_file.getError())
@@ -107,6 +118,7 @@
   setvbuf(stdout, nullptr, _IONBF, 0);
 
   while (deserializer.HasData(1)) {
+    unsigned idx = deserializer.Deserialize<unsigned>();
     unsigned id = deserializer.Deserialize<unsigned>();
 
 #ifndef LLDB_REPRO_INSTR_TRACE
@@ -115,6 +127,7 @@
     llvm::errs() << "Replaying " << id << ": " << GetSignature(id) << "\n";
 #endif
 
+    deserializer.SetExpectedIndex(idx);
     GetReplayer(id)->operator()(deserializer);
   }
 
@@ -181,21 +194,23 @@
 
 Recorder::Recorder()
     : m_serializer(nullptr), m_pretty_func(), m_pretty_args(),
-      m_local_boundary(false), m_result_recorded(true) {
+      m_local_boundary(false), m_result_recorded(true),
+      m_index(std::numeric_limits<unsigned>::max()) {
   if (!g_global_boundary) {
     g_global_boundary = true;
     m_local_boundary = true;
+    m_index = GetNextIndex();
   }
 }
 
 Recorder::Recorder(llvm::StringRef pretty_func, std::string &&pretty_args)
     : m_serializer(nullptr), m_pretty_func(pretty_func),
       m_pretty_args(pretty_args), m_local_boundary(false),
-      m_result_recorded(true) {
+      m_result_recorded(true), m_index(std::numeric_limits<unsigned>::max()) {
   if (!g_global_boundary) {
     g_global_boundary = true;
     m_local_boundary = true;
-
+    m_index = GetNextIndex();
     LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "{0} ({1})",
              m_pretty_func, m_pretty_args);
   }
@@ -228,3 +243,5 @@
 }
 
 thread_local bool lldb_private::repro::Recorder::g_global_boundary = false;
+std::atomic<unsigned> lldb_private::repro::Recorder::g_index;
+std::mutex lldb_private::repro::Recorder::g_mutex;
Index: lldb/include/lldb/Utility/ReproducerInstrumentation.h
===================================================================
--- lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -333,6 +333,7 @@
   }
 
   template <typename T> const T &HandleReplayResult(const T &t) {
+    CheckIndex(Deserialize<unsigned>());
     unsigned result = Deserialize<unsigned>();
     if (is_trivially_serializable<T>::value)
       return t;
@@ -342,6 +343,7 @@
 
   /// Store the returned value in the index-to-object mapping.
   template <typename T> T &HandleReplayResult(T &t) {
+    CheckIndex(Deserialize<unsigned>());
     unsigned result = Deserialize<unsigned>();
     if (is_trivially_serializable<T>::value)
       return t;
@@ -351,6 +353,7 @@
 
   /// Store the returned value in the index-to-object mapping.
   template <typename T> T *HandleReplayResult(T *t) {
+    CheckIndex(Deserialize<unsigned>());
     unsigned result = Deserialize<unsigned>();
     if (is_trivially_serializable<T>::value)
       return t;
@@ -360,6 +363,7 @@
   /// All returned types are recorded, even when the function returns a void.
   /// The latter requires special handling.
   void HandleReplayResultVoid() {
+    CheckIndex(Deserialize<unsigned>());
     unsigned result = Deserialize<unsigned>();
     assert(result == 0);
     (void)result;
@@ -369,6 +373,8 @@
     return m_index_to_object.GetAllObjects();
   }
 
+  void SetExpectedIndex(unsigned idx) { m_expected_idx = idx; }
+
 private:
   template <typename T> T Read(ValueTag) {
     assert(HasData(sizeof(T)));
@@ -410,11 +416,17 @@
     return *(new UnderlyingT(Deserialize<UnderlyingT>()));
   }
 
+  /// Verify that the given index matches the expected index if set.
+  void CheckIndex(unsigned idx);
+
   /// Mapping of indices to objects.
   IndexToObject m_index_to_object;
 
   /// Buffer containing the serialized data.
   llvm::StringRef m_buffer;
+
+  /// Expected index of the result.
+  llvm::Optional<unsigned> m_expected_idx;
 };
 
 /// Partial specialization for C-style strings. We read the string value
@@ -741,6 +753,7 @@
   template <typename Result, typename... FArgs, typename... RArgs>
   void Record(Serializer &serializer, Registry &registry, Result (*f)(FArgs...),
               const RArgs &... args) {
+    std::lock_guard<std::mutex> lock(g_mutex);
     m_serializer = &serializer;
     if (!ShouldCapture())
       return;
@@ -751,6 +764,7 @@
     Log(id);
 #endif
 
+    serializer.SerializeAll(m_index);
     serializer.SerializeAll(id);
     serializer.SerializeAll(args...);
 
@@ -758,6 +772,7 @@
             typename std::remove_reference<Result>::type>::type>::value) {
       m_result_recorded = false;
     } else {
+      serializer.SerializeAll(m_index);
       serializer.SerializeAll(0);
       m_result_recorded = true;
     }
@@ -771,16 +786,19 @@
     if (!ShouldCapture())
       return;
 
+    std::lock_guard<std::mutex> lock(g_mutex);
     unsigned id = registry.GetID(uintptr_t(f));
 
 #ifdef LLDB_REPRO_INSTR_TRACE
     Log(id);
 #endif
 
+    serializer.SerializeAll(m_index);
     serializer.SerializeAll(id);
     serializer.SerializeAll(args...);
 
     // Record result.
+    serializer.SerializeAll(m_index);
     serializer.SerializeAll(0);
     m_result_recorded = true;
   }
@@ -806,7 +824,9 @@
     if (update_boundary)
       UpdateBoundary();
     if (m_serializer && ShouldCapture()) {
+      std::lock_guard<std::mutex> lock(g_mutex);
       assert(!m_result_recorded);
+      m_serializer->SerializeAll(m_index);
       m_serializer->SerializeAll(r);
       m_result_recorded = true;
     }
@@ -816,6 +836,7 @@
   template <typename Result, typename T>
   Result Replay(Deserializer &deserializer, Registry &registry, uintptr_t addr,
                 bool update_boundary) {
+    deserializer.SetExpectedIndex(deserializer.Deserialize<unsigned>());
     unsigned actual_id = registry.GetID(addr);
     unsigned id = deserializer.Deserialize<unsigned>();
     registry.CheckID(id, actual_id);
@@ -826,6 +847,7 @@
   }
 
   void Replay(Deserializer &deserializer, Registry &registry, uintptr_t addr) {
+    deserializer.SetExpectedIndex(deserializer.Deserialize<unsigned>());
     unsigned actual_id = registry.GetID(addr);
     unsigned id = deserializer.Deserialize<unsigned>();
     registry.CheckID(id, actual_id);
@@ -846,6 +868,8 @@
   static void PrivateThread() { g_global_boundary = true; }
 
 private:
+  static uint64_t GetNextIndex() { return g_index++; }
+
   template <typename T> friend struct replay;
   void UpdateBoundary() {
     if (m_local_boundary)
@@ -871,8 +895,17 @@
   /// Whether the return value was recorded explicitly.
   bool m_result_recorded;
 
+  /// The current index.
+  unsigned m_index;
+
   /// Whether we're currently across the API boundary.
   static thread_local bool g_global_boundary;
+
+  /// Global mutex to protect concurrent access.
+  static std::mutex g_mutex;
+
+  /// Global index.
+  static std::atomic<unsigned> g_index;
 };
 
 /// To be used as the "Runtime ID" of a constructor. It also invokes the
@@ -1014,6 +1047,7 @@
 
     static Result replay(Recorder &recorder, Deserializer &deserializer,
                          Registry &registry, char *str) {
+      deserializer.SetExpectedIndex(deserializer.Deserialize<unsigned>());
       deserializer.Deserialize<unsigned>();
       Class *c = deserializer.Deserialize<Class *>();
       deserializer.Deserialize<const char *>();
@@ -1035,6 +1069,7 @@
 
     static Result replay(Recorder &recorder, Deserializer &deserializer,
                          Registry &registry, char *str) {
+      deserializer.SetExpectedIndex(deserializer.Deserialize<unsigned>());
       deserializer.Deserialize<unsigned>();
       Class *c = deserializer.Deserialize<Class *>();
       deserializer.Deserialize<const char *>();
@@ -1055,6 +1090,7 @@
 
     static Result replay(Recorder &recorder, Deserializer &deserializer,
                          Registry &registry, char *str) {
+      deserializer.SetExpectedIndex(deserializer.Deserialize<unsigned>());
       deserializer.Deserialize<unsigned>();
       deserializer.Deserialize<const char *>();
       size_t l = deserializer.Deserialize<size_t>();
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to