JDevlieghere created this revision.
JDevlieghere added reviewers: friss, labath.
Herald added a subscriber: jfb.
JDevlieghere requested review of this revision.
Prevent lldb from crashing when multiple threads are concurrently accessing the
SB API with reproducer capture enabled. The API instrumentation records both
the input arguments and the return value, but it cannot block for the duration
of the API call. Therefore we guarantee that the function id and its arguments
are emitted as a single unit and that the return value is emitted as a single
unit. Every API call is given a monotonically increasing index, which
correlates the function with its return value.
Using the index, we can detect situations where the return value does not
succeed the function call, in which case we print an error saying that
concurrency is not (currently) supported. In the future we might attempt to be
smarter and read ahead until we've found the return value matching the current
call.
https://reviews.llvm.org/D92820
Files:
lldb/include/lldb/Utility/ReproducerInstrumentation.h
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/source/Utility/ReproducerInstrumentation.cpp
lldb/test/API/functionalities/reproducers/concurrency/Makefile
lldb/test/API/functionalities/reproducers/concurrency/TestReproducerConcurrency.py
lldb/test/API/functionalities/reproducers/concurrency/driver.cpp
Index: lldb/test/API/functionalities/reproducers/concurrency/driver.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/reproducers/concurrency/driver.cpp
@@ -0,0 +1,55 @@
+#include "lldb/API/LLDB.h"
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+
+#include <csignal>
+#include <iostream>
+#include <thread>
+
+#define THREADS 10
+
+using namespace lldb;
+
+void f(uint64_t idx) {
+ SBDebugger debugger = lldb::SBDebugger::Create(false);
+ debugger.GetNumPlatforms();
+ debugger.GetNumAvailablePlatforms();
+ SBTarget target = debugger.GetDummyTarget();
+ SBCommandInterpreter interpreter = debugger.GetCommandInterpreter();
+ SBDebugger::Destroy(debugger);
+}
+
+int main(int argc, char **argv) {
+ if (argc < 2) {
+ std::cout << "missing argument: reproducer directory";
+ return 1;
+ }
+
+ if (const char *error = SBReproducer::Capture(argv[1])) {
+ std::cout << "reproducer capture failed: " << error << '\n';
+ return 1;
+ }
+
+ SBReproducer::SetAutoGenerate(true);
+
+ SBError error = SBDebugger::InitializeWithErrorHandling();
+ if (error.Fail()) {
+ std::cout << "initialization failed: " << error.GetCString() << '\n';
+ return 1;
+ }
+
+#if !defined(_MSC_VER)
+ signal(SIGPIPE, SIG_IGN);
+#endif
+
+ std::vector<std::thread> threads;
+ for (uint64_t i = 0; i < THREADS; i++)
+ threads.emplace_back(f, i);
+
+ for (auto &t : threads)
+ t.join();
+
+ SBDebugger::Terminate();
+ return 0;
+}
Index: lldb/test/API/functionalities/reproducers/concurrency/TestReproducerConcurrency.py
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/reproducers/concurrency/TestReproducerConcurrency.py
@@ -0,0 +1,53 @@
+from __future__ import print_function
+
+import os
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestReproducerConcurrency(TestBase):
+
+ mydir = TestBase.compute_mydir(__file__)
+
+ NO_DEBUG_INFO_TESTCASE = True
+
+ @skipIfNoSBHeaders
+ @skipIfWindows
+ def test_multiple_debuggers(self):
+ env = {self.dylibPath: self.getLLDBLibraryEnvVal()}
+
+ driver = self.getBuildArtifact("driver")
+ reproducer = self.getBuildArtifact("reproducer")
+
+ if os.path.exists(reproducer):
+ try:
+ shutil.rmtree(reproducer)
+ except OSError:
+ pass
+
+ self.buildDriver('driver.cpp', driver)
+ self.signBinary(driver)
+
+ if self.TraceOn():
+ check_call([driver, reproducer], env=env)
+ else:
+ with open(os.devnull, 'w') as fnull:
+ check_call([driver, reproducer],
+ env=env,
+ stdout=fnull,
+ stderr=fnull)
+
+ # Check that replay fails with the expected error.
+ replay = subprocess.Popen(
+ [lldbtest_config.lldbExec, '-replay', reproducer],
+ stdin=subprocess.PIPE,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
+ _, errs = replay.communicate()
+ errs = errs.decode('utf-8')
+ self.assertNotEqual(replay.returncode, 0)
+ self.assertIn('The result does not match the preceding function.',
+ errs)
Index: lldb/test/API/functionalities/reproducers/concurrency/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/reproducers/concurrency/Makefile
@@ -0,0 +1,5 @@
+MAKE_DSYM := NO
+ENABLE_THREADS := YES
+CXX_SOURCES := driver.cpp
+
+include Makefile.rules
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,15 @@
return r;
}
+void Deserializer::CheckIndex(unsigned idx) const {
+ 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.");
+}
+
bool Registry::Replay(const FileSpec &file) {
auto error_or_file = llvm::MemoryBuffer::getFile(file.GetPath());
if (auto err = error_or_file.getError())
@@ -107,6 +117,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 +126,7 @@
llvm::errs() << "Replaying " << id << ": " << GetSignature(id) << "\n";
#endif
+ deserializer.SetExpectedIndex(idx);
GetReplayer(id)->operator()(deserializer);
}
@@ -181,21 +193,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 +242,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/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -33,6 +33,7 @@
#include "lldb/Interpreter/CommandReturnObject.h"
#include "lldb/Target/Thread.h"
#include "lldb/Target/ThreadPlan.h"
+#include "lldb/Utility/ReproducerInstrumentation.h"
#include "lldb/Utility/Timer.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
Index: lldb/include/lldb/Utility/ReproducerInstrumentation.h
===================================================================
--- lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -333,6 +333,8 @@
}
template <typename T> const T &HandleReplayResult(const T &t) {
+ unsigned idx = Deserialize<unsigned>();
+ CheckIndex(idx);
unsigned result = Deserialize<unsigned>();
if (is_trivially_serializable<T>::value)
return t;
@@ -342,6 +344,8 @@
/// Store the returned value in the index-to-object mapping.
template <typename T> T &HandleReplayResult(T &t) {
+ unsigned idx = Deserialize<unsigned>();
+ CheckIndex(idx);
unsigned result = Deserialize<unsigned>();
if (is_trivially_serializable<T>::value)
return t;
@@ -351,6 +355,8 @@
/// Store the returned value in the index-to-object mapping.
template <typename T> T *HandleReplayResult(T *t) {
+ unsigned idx = Deserialize<unsigned>();
+ CheckIndex(idx);
unsigned result = Deserialize<unsigned>();
if (is_trivially_serializable<T>::value)
return t;
@@ -360,6 +366,8 @@
/// All returned types are recorded, even when the function returns a void.
/// The latter requires special handling.
void HandleReplayResultVoid() {
+ unsigned idx = Deserialize<unsigned>();
+ CheckIndex(idx);
unsigned result = Deserialize<unsigned>();
assert(result == 0);
(void)result;
@@ -369,6 +377,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 +420,17 @@
return *(new UnderlyingT(Deserialize<UnderlyingT>()));
}
+ /// Verify that the given index matches the expected index if set.
+ void CheckIndex(unsigned idx) const;
+
/// 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 +757,7 @@
template <typename Result, typename... FArgs, typename... RArgs>
void Record(Serializer &serializer, Registry ®istry, Result (*f)(FArgs...),
const RArgs &... args) {
+ std::lock_guard<std::mutex> lock(g_mutex);
m_serializer = &serializer;
if (!ShouldCapture())
return;
@@ -751,6 +768,7 @@
Log(id);
#endif
+ serializer.SerializeAll(m_index);
serializer.SerializeAll(id);
serializer.SerializeAll(args...);
@@ -758,6 +776,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 +790,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 +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(m_index);
m_serializer->SerializeAll(r);
m_result_recorded = true;
}
@@ -846,6 +870,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 +897,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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
