This revision was automatically updated to reflect the committed changes.
Closed by commit rGac25e8628c44: [lldb] Deal gracefully with concurrency in the
API instrumentation. (authored by JDevlieghere).
Herald added a project: LLDB.
Changed prior to commit:
https://reviews.llvm.org/D92820?vs=310649&id=310941#toc
Repository:
rG LLVM Github Monorepo
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 sequence = 123;
+
Serializer serializer(os);
- serializer.SerializeAll(static_cast<unsigned>(1), static_cast<unsigned>(2));
+ serializer.SerializeAll(sequence, static_cast<unsigned>(1));
+ serializer.SerializeAll(sequence, 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 sequence = 123;
+
Serializer serializer(os);
- serializer.SerializeAll(static_cast<unsigned>(1), static_cast<unsigned>(2));
+ serializer.SerializeAll(sequence, static_cast<unsigned>(1));
+ serializer.SerializeAll(sequence, static_cast<unsigned>(2));
serializer.SerializeAll(foo, bar);
llvm::StringRef buffer(os.str());
@@ -1114,3 +1120,48 @@
bar.Validate();
}
}
+
+TEST(RecordReplayTest, ValidSequence) {
+ std::string str;
+ llvm::raw_string_ostream os(str);
+
+ {
+ auto data = TestInstrumentationDataRAII::GetRecordingData(os);
+
+ unsigned sequence = 1;
+ int (*f)() = &lldb_private::repro::invoke<int (*)()>::method<
+ InstrumentedFoo::F>::record;
+ unsigned id = g_registry->GetID(uintptr_t(f));
+ g_serializer->SerializeAll(sequence, id);
+
+ unsigned result = 0;
+ g_serializer->SerializeAll(sequence, result);
+ }
+
+ TestingRegistry registry;
+ Deserializer deserializer(os.str());
+ registry.Replay(deserializer);
+}
+
+TEST(RecordReplayTest, InvalidSequence) {
+ std::string str;
+ llvm::raw_string_ostream os(str);
+
+ {
+ auto data = TestInstrumentationDataRAII::GetRecordingData(os);
+
+ unsigned sequence = 1;
+ int (*f)() = &lldb_private::repro::invoke<int (*)()>::method<
+ InstrumentedFoo::F>::record;
+ unsigned id = g_registry->GetID(uintptr_t(f));
+ g_serializer->SerializeAll(sequence, id);
+
+ unsigned result = 0;
+ unsigned invalid_sequence = 2;
+ g_serializer->SerializeAll(invalid_sequence, 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::CheckSequence(unsigned sequence) {
+ if (m_expected_sequence && *m_expected_sequence != sequence)
+ 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_sequence.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 sequence = 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.SetExpectedSequence(sequence);
GetReplayer(id)->operator()(deserializer);
}
@@ -181,21 +194,24 @@
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_sequence(std::numeric_limits<unsigned>::max()) {
if (!g_global_boundary) {
g_global_boundary = true;
m_local_boundary = true;
+ m_sequence = GetNextSequenceNumber();
}
}
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_sequence(std::numeric_limits<unsigned>::max()) {
if (!g_global_boundary) {
g_global_boundary = true;
m_local_boundary = true;
-
+ m_sequence = GetNextSequenceNumber();
LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "{0} ({1})",
m_pretty_func, m_pretty_args);
}
@@ -206,6 +222,11 @@
UpdateBoundary();
}
+unsigned Recorder::GetSequenceNumber() const {
+ assert(m_sequence != std::numeric_limits<unsigned>::max());
+ return m_sequence;
+}
+
void InstrumentationData::Initialize(Serializer &serializer,
Registry ®istry) {
InstanceImpl().emplace(serializer, registry);
@@ -228,3 +249,5 @@
}
thread_local bool lldb_private::repro::Recorder::g_global_boundary = false;
+std::atomic<unsigned> lldb_private::repro::Recorder::g_sequence;
+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) {
+ CheckSequence(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) {
+ CheckSequence(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) {
+ CheckSequence(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() {
+ CheckSequence(Deserialize<unsigned>());
unsigned result = Deserialize<unsigned>();
assert(result == 0);
(void)result;
@@ -369,6 +373,10 @@
return m_index_to_object.GetAllObjects();
}
+ void SetExpectedSequence(unsigned sequence) {
+ m_expected_sequence = sequence;
+ }
+
private:
template <typename T> T Read(ValueTag) {
assert(HasData(sizeof(T)));
@@ -410,11 +418,17 @@
return *(new UnderlyingT(Deserialize<UnderlyingT>()));
}
+ /// Verify that the given sequence number matches what we expect.
+ void CheckSequence(unsigned sequence);
+
/// Mapping of indices to objects.
IndexToObject m_index_to_object;
/// Buffer containing the serialized data.
llvm::StringRef m_buffer;
+
+ /// The result's expected sequence number.
+ llvm::Optional<unsigned> m_expected_sequence;
};
/// Partial specialization for C-style strings. We read the string value
@@ -745,12 +759,15 @@
if (!ShouldCapture())
return;
+ std::lock_guard<std::mutex> lock(g_mutex);
+ unsigned sequence = GetSequenceNumber();
unsigned id = registry.GetID(uintptr_t(f));
#ifdef LLDB_REPRO_INSTR_TRACE
Log(id);
#endif
+ serializer.SerializeAll(sequence);
serializer.SerializeAll(id);
serializer.SerializeAll(args...);
@@ -758,6 +775,7 @@
typename std::remove_reference<Result>::type>::type>::value) {
m_result_recorded = false;
} else {
+ serializer.SerializeAll(sequence);
serializer.SerializeAll(0);
m_result_recorded = true;
}
@@ -771,16 +789,20 @@
if (!ShouldCapture())
return;
+ std::lock_guard<std::mutex> lock(g_mutex);
+ unsigned sequence = GetSequenceNumber();
unsigned id = registry.GetID(uintptr_t(f));
#ifdef LLDB_REPRO_INSTR_TRACE
Log(id);
#endif
+ serializer.SerializeAll(sequence);
serializer.SerializeAll(id);
serializer.SerializeAll(args...);
// Record result.
+ serializer.SerializeAll(sequence);
serializer.SerializeAll(0);
m_result_recorded = true;
}
@@ -806,7 +828,9 @@
if (update_boundary)
UpdateBoundary();
if (m_serializer && ShouldCapture()) {
+ std::lock_guard<std::mutex> lock(g_mutex);
assert(!m_result_recorded);
+ m_serializer->SerializeAll(GetSequenceNumber());
m_serializer->SerializeAll(r);
m_result_recorded = true;
}
@@ -816,6 +840,7 @@
template <typename Result, typename T>
Result Replay(Deserializer &deserializer, Registry ®istry, uintptr_t addr,
bool update_boundary) {
+ deserializer.SetExpectedSequence(deserializer.Deserialize<unsigned>());
unsigned actual_id = registry.GetID(addr);
unsigned id = deserializer.Deserialize<unsigned>();
registry.CheckID(id, actual_id);
@@ -826,6 +851,7 @@
}
void Replay(Deserializer &deserializer, Registry ®istry, uintptr_t addr) {
+ deserializer.SetExpectedSequence(deserializer.Deserialize<unsigned>());
unsigned actual_id = registry.GetID(addr);
unsigned id = deserializer.Deserialize<unsigned>();
registry.CheckID(id, actual_id);
@@ -846,6 +872,9 @@
static void PrivateThread() { g_global_boundary = true; }
private:
+ static unsigned GetNextSequenceNumber() { return g_sequence++; }
+ unsigned GetSequenceNumber() const;
+
template <typename T> friend struct replay;
void UpdateBoundary() {
if (m_local_boundary)
@@ -871,8 +900,17 @@
/// Whether the return value was recorded explicitly.
bool m_result_recorded;
+ /// The sequence number for this pair of function and result.
+ unsigned m_sequence;
+
/// 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;
+
+ /// Unique, monotonically increasing sequence number.
+ static std::atomic<unsigned> g_sequence;
};
/// To be used as the "Runtime ID" of a constructor. It also invokes the
@@ -1014,6 +1052,7 @@
static Result replay(Recorder &recorder, Deserializer &deserializer,
Registry ®istry, char *str) {
+ deserializer.SetExpectedSequence(deserializer.Deserialize<unsigned>());
deserializer.Deserialize<unsigned>();
Class *c = deserializer.Deserialize<Class *>();
deserializer.Deserialize<const char *>();
@@ -1035,6 +1074,7 @@
static Result replay(Recorder &recorder, Deserializer &deserializer,
Registry ®istry, char *str) {
+ deserializer.SetExpectedSequence(deserializer.Deserialize<unsigned>());
deserializer.Deserialize<unsigned>();
Class *c = deserializer.Deserialize<Class *>();
deserializer.Deserialize<const char *>();
@@ -1055,6 +1095,7 @@
static Result replay(Recorder &recorder, Deserializer &deserializer,
Registry ®istry, char *str) {
+ deserializer.SetExpectedSequence(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
