Skip to content

Commit db8c62e

Browse files
JDevlieghereJDevlieghere
authored
Merge pull request #7113 from augusto2112/thread-safe-refl-context
[lldb] Make access to ReflectionContext thread safe
2 parents fbc2eaa + ead9772 commit db8c62e

File tree

5 files changed

+132
-95
lines changed

5 files changed

+132
-95
lines changed

‎lldb/include/lldb/Core/ModuleList.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,9 @@ class ModuleList {
500500
bool AnyOf(
501501
std::function<bool(lldb_private::Module &module)> const &callback) const;
502502

503+
/// Atomically swaps the contents of this module list with \a other.
504+
void Swap(ModuleList &other);
505+
503506
protected:
504507
// Class typedefs.
505508
typedef std::vector<lldb::ModuleSP>

‎lldb/source/Core/ModuleList.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,3 +1295,10 @@ bool ModuleList::AnyOf(
12951295

12961296
return false;
12971297
}
1298+
1299+
1300+
void ModuleList::Swap(ModuleList &other) {
1301+
// scoped_lock locks both mutexes at once.
1302+
std::scoped_lock lock(m_modules_mutex, other.m_modules_mutex);
1303+
m_modules.swap(other.m_modules);
1304+
}

‎lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

Lines changed: 75 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -447,11 +447,41 @@ static bool HasReflectionInfo(ObjectFile *obj_file) {
447447
return hasReflectionSection;
448448
}
449449

450-
SwiftLanguageRuntimeImpl::ReflectionContextInterface *
450+
SwiftLanguageRuntimeImpl::ThreadSafeReflectionContext
451451
SwiftLanguageRuntimeImpl::GetReflectionContext() {
452-
if (!m_initialized_reflection_ctx)
453-
SetupReflection();
454-
return m_reflection_ctx.get();
452+
m_reflection_ctx_mutex.lock();
453+
SetupReflection();
454+
// SetupReflection can potentially fail.
455+
if (m_initialized_reflection_ctx)
456+
ProcessModulesToAdd();
457+
return {m_reflection_ctx.get(), m_reflection_ctx_mutex};
458+
}
459+
460+
void SwiftLanguageRuntimeImpl::ProcessModulesToAdd() {
461+
// A snapshot of the modules to be processed. This is necessary because
462+
// AddModuleToReflectionContext may recursively call into this function again.
463+
ModuleList modules_to_add_snapshot;
464+
modules_to_add_snapshot.Swap(m_modules_to_add);
465+
466+
if (modules_to_add_snapshot.IsEmpty())
467+
return;
468+
469+
auto &target = m_process.GetTarget();
470+
auto exe_module = target.GetExecutableModule();
471+
Progress progress(
472+
llvm::formatv("Setting up Swift reflection for '{0}'",
473+
exe_module->GetFileSpec().GetFilename().AsCString()),
474+
modules_to_add_snapshot.GetSize());
475+
476+
size_t completion = 0;
477+
478+
// Add all defered modules to reflection context that were added to
479+
// the target since this SwiftLanguageRuntime was created.
480+
modules_to_add_snapshot.ForEach([&](const ModuleSP &module_sp) -> bool {
481+
AddModuleToReflectionContext(module_sp);
482+
progress.Increment(++completion);
483+
return true;
484+
});
455485
}
456486

457487
SwiftMetadataCache *
@@ -463,79 +493,53 @@ SwiftLanguageRuntimeImpl::GetSwiftMetadataCache() {
463493

464494
void SwiftLanguageRuntimeImpl::SetupReflection() {
465495
LLDB_SCOPED_TIMER();
466-
467-
// SetupABIBit() iterates of the Target's images and thus needs to
468-
// acquire that ModuleList's lock. We need to acquire this before
469-
// locking m_add_module_mutex, since ModulesDidLoad can also be
470-
// called from a place where that lock is already held:
471-
// + lldb_private::DynamicLoaderDarwin::AddModulesUsingImageInfos()
472-
// + lldb_private::ModuleList::AppendIfNeeded()
473-
// + lldb_private::Target::NotifyModuleAdded()
474-
// + lldb_private::Target::ModulesDidLoad()
475496

497+
498+
std::lock_guard<std::recursive_mutex> lock(m_reflection_ctx_mutex);
499+
if (m_initialized_reflection_ctx)
500+
return;
501+
476502
// The global ABI bit is read by the Swift runtime library.
477503
SetupABIBit();
478504

479-
// A copy of the modules that should be added, to prevent mutating the list
480-
// while iterating over it.
481-
ModuleList modules_snapshot;
482-
483505
auto &target = m_process.GetTarget();
484506
auto exe_module = target.GetExecutableModule();
485-
{
486-
std::lock_guard<std::recursive_mutex> lock(m_add_module_mutex);
487-
if (m_initialized_reflection_ctx)
488-
return;
489-
490-
if (!exe_module) {
491-
LLDB_LOGF(GetLog(LLDBLog::Types), "%s: Failed to get executable module",
492-
LLVM_PRETTY_FUNCTION);
493-
m_initialized_reflection_ctx = false;
494-
return;
495-
}
496507

497-
bool objc_interop = (bool)findRuntime(m_process, RuntimeKind::ObjC);
498-
const char *objc_interop_msg =
499-
objc_interop ? "with Objective-C interopability" : "Swift only";
500-
501-
auto &triple = exe_module->GetArchitecture().GetTriple();
502-
if (triple.isArch64Bit()) {
503-
LLDB_LOGF(GetLog(LLDBLog::Types),
504-
"Initializing a 64-bit reflection context (%s) for \"%s\"",
505-
triple.str().c_str(), objc_interop_msg);
506-
m_reflection_ctx = ReflectionContextInterface::CreateReflectionContext64(
507-
this->GetMemoryReader(), objc_interop, GetSwiftMetadataCache());
508-
} else if (triple.isArch32Bit()) {
509-
LLDB_LOGF(GetLog(LLDBLog::Types),
510-
"Initializing a 32-bit reflection context (%s) for \"%s\"",
511-
triple.str().c_str(), objc_interop_msg);
512-
m_reflection_ctx = ReflectionContextInterface::CreateReflectionContext32(
513-
this->GetMemoryReader(), objc_interop, GetSwiftMetadataCache());
514-
} else {
515-
LLDB_LOGF(GetLog(LLDBLog::Types),
516-
"Could not initialize reflection context for \"%s\"",
517-
triple.str().c_str());
518-
}
519-
520-
modules_snapshot = std::move(m_modules_to_add);
521-
m_modules_to_add.Clear();
522-
m_initialized_reflection_ctx = true;
508+
auto *log = GetLog(LLDBLog::Types);
509+
if (!exe_module) {
510+
LLDB_LOGF(log, "%s: Failed to get executable module",
511+
LLVM_PRETTY_FUNCTION);
512+
m_initialized_reflection_ctx = false;
513+
return;
523514
}
524515

525-
Progress progress(
526-
llvm::formatv("Setting up Swift reflection for '{0}'",
527-
exe_module->GetFileSpec().GetFilename().AsCString()),
528-
modules_snapshot.GetSize());
529-
530-
size_t completion = 0;
531-
532-
// Add all defered modules to reflection context that were added to
533-
// the target since this SwiftLanguageRuntime was created.
534-
modules_snapshot.ForEach([&](const ModuleSP &module_sp) -> bool {
535-
AddModuleToReflectionContext(module_sp);
536-
progress.Increment(++completion);
537-
return true;
538-
});
516+
bool objc_interop = (bool)findRuntime(m_process, RuntimeKind::ObjC);
517+
const char *objc_interop_msg =
518+
objc_interop ? "with Objective-C interopability" : "Swift only";
519+
520+
auto &triple = exe_module->GetArchitecture().GetTriple();
521+
if (triple.isArch64Bit()) {
522+
LLDB_LOGF(log, "Initializing a 64-bit reflection context (%s) for \"%s\"",
523+
triple.str().c_str(), objc_interop_msg);
524+
m_reflection_ctx = ReflectionContextInterface::CreateReflectionContext64(
525+
this->GetMemoryReader(), objc_interop, GetSwiftMetadataCache());
526+
} else if (triple.isArch32Bit()) {
527+
LLDB_LOGF(log,
528+
"Initializing a 32-bit reflection context (%s) for \"%s\"",
529+
triple.str().c_str(), objc_interop_msg);
530+
m_reflection_ctx = ReflectionContextInterface::CreateReflectionContext32(
531+
this->GetMemoryReader(), objc_interop, GetSwiftMetadataCache());
532+
} else {
533+
LLDB_LOGF(log,
534+
"Could not initialize reflection context for \"%s\"",
535+
triple.str().c_str());
536+
}
537+
// We set m_initialized_reflection_ctx to true here because
538+
// AddModuleToReflectionContext can potentially call into SetupReflection
539+
// again (which will early exit). This is safe to do since every other thread
540+
// using reflection context will have to wait until all the modules are added,
541+
// since the thread performing the initialization locked the mutex.
542+
m_initialized_reflection_ctx = true;
539543
}
540544

541545
bool SwiftLanguageRuntimeImpl::IsABIStable() {
@@ -922,17 +926,9 @@ bool SwiftLanguageRuntimeImpl::AddModuleToReflectionContext(
922926
}
923927

924928
void SwiftLanguageRuntimeImpl::ModulesDidLoad(const ModuleList &module_list) {
925-
// If the reflection context hasn't been initialized, add them to
926-
// the list of deferred modules so they are added in
927-
// SetupReflection(), otherwise add them directly.
928-
std::lock_guard<std::recursive_mutex> lock(m_add_module_mutex);
929-
if (!m_initialized_reflection_ctx)
930-
m_modules_to_add.AppendIfNeeded(module_list);
931-
else
932-
module_list.ForEach([&](const ModuleSP &module_sp) -> bool {
933-
AddModuleToReflectionContext(module_sp);
934-
return true;
935-
});
929+
// The modules will be lazily processed on the next call to
930+
// GetReflectionContext.
931+
m_modules_to_add.AppendIfNeeded(module_list);
936932
}
937933

938934
std::string

‎lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeDynamicTypeResolution.cpp

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,8 @@ SwiftLanguageRuntimeImpl::GetMemoryReader() {
453453
if (!m_memory_reader_sp) {
454454
m_memory_reader_sp.reset(new LLDBMemoryReader(
455455
m_process, [&](swift::remote::RemoteAbsolutePointer pointer) {
456-
auto *reflection_context = GetReflectionContext();
456+
ThreadSafeReflectionContext reflection_context =
457+
GetReflectionContext();
457458
return reflection_context->stripSignedPointer(pointer);
458459
}));
459460
}
@@ -1104,7 +1105,7 @@ SwiftLanguageRuntimeImpl::GetNumChildren(CompilerType type,
11041105
if (!tr)
11051106
return {};
11061107

1107-
auto *reflection_ctx = GetReflectionContext();
1108+
ThreadSafeReflectionContext reflection_ctx = GetReflectionContext();
11081109
auto &builder = reflection_ctx->getBuilder();
11091110
auto tc = swift::reflection::TypeConverter(builder);
11101111
LLDBTypeInfoProvider tip(*this, *ts);
@@ -1331,7 +1332,7 @@ SwiftLanguageRuntimeImpl::GetIndexOfChildMemberWithName(
13311332
exe_ctx, omit_empty_base_classes,
13321333
child_indexes);
13331334
case ReferenceKind::Strong: {
1334-
auto *reflection_ctx = GetReflectionContext();
1335+
ThreadSafeReflectionContext reflection_ctx = GetReflectionContext();
13351336
auto &builder = reflection_ctx->getBuilder();
13361337
TypeConverter tc(builder);
13371338
LLDBTypeInfoProvider tip(*this, *ts);
@@ -1511,7 +1512,7 @@ CompilerType SwiftLanguageRuntimeImpl::GetChildCompilerTypeAtIndex(
15111512
llvm::StringRef type_name = TypeSystemSwiftTypeRef::GetBaseName(
15121513
ts->CanonicalizeSugar(dem, type_node));
15131514

1514-
auto *reflection_ctx = GetReflectionContext();
1515+
ThreadSafeReflectionContext reflection_ctx = GetReflectionContext();
15151516
if (!reflection_ctx)
15161517
return {};
15171518
CompilerType instance_type = valobj->GetCompilerType();
@@ -1625,7 +1626,7 @@ CompilerType SwiftLanguageRuntimeImpl::GetChildCompilerTypeAtIndex(
16251626

16261627
bool SwiftLanguageRuntimeImpl::ForEachSuperClassType(
16271628
ValueObject &instance, std::function<bool(SuperClassType)> fn) {
1628-
auto *reflection_ctx = GetReflectionContext();
1629+
ThreadSafeReflectionContext reflection_ctx = GetReflectionContext();
16291630
if (!reflection_ctx)
16301631
return false;
16311632
CompilerType instance_type = instance.GetCompilerType();
@@ -1692,7 +1693,7 @@ bool SwiftLanguageRuntimeImpl::GetDynamicTypeAndAddress_Pack(
16921693
lldb::DynamicValueType use_dynamic, TypeAndOrName &pack_type_or_name,
16931694
Address &address, Value::ValueType &value_type) {
16941695
Log *log(GetLog(LLDBLog::Types));
1695-
auto *reflection_ctx = GetReflectionContext();
1696+
ThreadSafeReflectionContext reflection_ctx = GetReflectionContext();
16961697
if (!reflection_ctx)
16971698
return false;
16981699

@@ -2011,7 +2012,7 @@ bool SwiftLanguageRuntimeImpl::GetDynamicTypeAndAddress_Class(
20112012
return false;
20122013
}
20132014
Log *log(GetLog(LLDBLog::Types));
2014-
auto *reflection_ctx = GetReflectionContext();
2015+
ThreadSafeReflectionContext reflection_ctx = GetReflectionContext();
20152016
const auto *typeref = reflection_ctx->readTypeFromInstance(instance_ptr);
20162017
if (!typeref) {
20172018
LLDB_LOGF(log,
@@ -2234,7 +2235,7 @@ bool SwiftLanguageRuntimeImpl::GetDynamicTypeAndAddress_Protocol(
22342235

22352236
swift::remote::RemoteAddress remote_existential(existential_address);
22362237

2237-
auto *reflection_ctx = GetReflectionContext();
2238+
ThreadSafeReflectionContext reflection_ctx = GetReflectionContext();
22382239
auto pair = reflection_ctx->projectExistentialAndUnwrapClass(
22392240
remote_existential, *protocol_typeref);
22402241
if (use_local_buffer)
@@ -2365,7 +2366,7 @@ CompilerType SwiftLanguageRuntimeImpl::BindGenericTypeParameters(
23652366
auto ts =
23662367
unbound_type.GetTypeSystem().dyn_cast_or_null<TypeSystemSwift>();
23672368
Status error;
2368-
auto *reflection_ctx = GetReflectionContext();
2369+
ThreadSafeReflectionContext reflection_ctx = GetReflectionContext();
23692370
if (!reflection_ctx) {
23702371
LLDB_LOG(GetLog(LLDBLog::Types),
23712372
"No reflection context available.");
@@ -2433,7 +2434,7 @@ SwiftLanguageRuntimeImpl::BindGenericTypeParameters(StackFrame &stack_frame,
24332434
using namespace swift::Demangle;
24342435

24352436
Status error;
2436-
auto *reflection_ctx = GetReflectionContext();
2437+
ThreadSafeReflectionContext reflection_ctx = GetReflectionContext();
24372438
if (!reflection_ctx) {
24382439
LLDB_LOG(GetLog(LLDBLog::Expressions | LLDBLog::Types),
24392440
"No reflection context available.");
@@ -2893,7 +2894,7 @@ SwiftLanguageRuntimeImpl::GetValueType(ValueObject &in_value,
28932894
// Read the value witness table and check if the data is inlined in
28942895
// the existential container or not.
28952896
swift::remote::RemoteAddress remote_existential(existential_address);
2896-
auto *reflection_ctx = GetReflectionContext();
2897+
ThreadSafeReflectionContext reflection_ctx = GetReflectionContext();
28972898
llvm::Optional<bool> is_inlined =
28982899
reflection_ctx->isValueInlinedInExistentialContainer(
28992900
remote_existential);
@@ -3372,7 +3373,7 @@ SwiftLanguageRuntimeImpl::GetTypeRef(CompilerType type,
33723373
return nullptr;
33733374

33743375
// Build a TypeRef.
3375-
auto *reflection_ctx = GetReflectionContext();
3376+
ThreadSafeReflectionContext reflection_ctx = GetReflectionContext();
33763377
if (!reflection_ctx)
33773378
return nullptr;
33783379

@@ -3447,7 +3448,7 @@ SwiftLanguageRuntimeImpl::GetSwiftRuntimeTypeInfo(
34473448
if (out_tr)
34483449
*out_tr = type_ref;
34493450

3450-
auto *reflection_ctx = GetReflectionContext();
3451+
ThreadSafeReflectionContext reflection_ctx = GetReflectionContext();
34513452
if (!reflection_ctx)
34523453
return nullptr;
34533454

‎lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeImpl.h

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,29 @@ class SwiftLanguageRuntimeImpl {
256256
stripSignedPointer(swift::remote::RemoteAbsolutePointer pointer) = 0;
257257
};
258258

259+
/// A wrapper around TargetReflectionContext, which holds a lock to ensure
260+
/// exclusive access.
261+
struct ThreadSafeReflectionContext {
262+
ThreadSafeReflectionContext(ReflectionContextInterface *reflection_ctx,
263+
std::recursive_mutex &mutex)
264+
: m_reflection_ctx(reflection_ctx), m_lock(mutex, std::adopt_lock) {}
265+
266+
ReflectionContextInterface *operator->() const {
267+
return m_reflection_ctx;
268+
}
269+
270+
operator bool() const {
271+
return m_reflection_ctx != nullptr;
272+
}
273+
274+
private:
275+
ReflectionContextInterface *m_reflection_ctx;
276+
// This lock operates on a recursive mutex because the initialization
277+
// of ReflectionContext recursive calls itself (see
278+
// SwiftLanguageRuntimeImpl::SetupReflection).
279+
std::lock_guard<std::recursive_mutex> m_lock;
280+
};
281+
259282
protected:
260283
static void
261284
ForEachGenericParameter(swift::Demangle::NodePointer node,
@@ -384,8 +407,13 @@ class SwiftLanguageRuntimeImpl {
384407
/// Lazily initialize and return \p m_dynamic_exclusivity_flag_addr.
385408
llvm::Optional<lldb::addr_t> GetDynamicExclusivityFlagAddr();
386409

387-
/// Lazily initialize the reflection context. Return \p nullptr on failure.
388-
ReflectionContextInterface *GetReflectionContext();
410+
/// Lazily initialize the reflection context. Returns a
411+
/// ThreadSafeReflectionContext with a \p nullptr on failure.
412+
ThreadSafeReflectionContext GetReflectionContext();
413+
414+
// Add the modules in m_modules_to_add to the Reflection Context. The
415+
// ModulesDidLoad() callback appends to m_modules_to_add.
416+
void ProcessModulesToAdd();
389417

390418
/// Lazily initialize and return \p m_SwiftNativeNSErrorISA.
391419
llvm::Optional<lldb::addr_t> GetSwiftNativeNSErrorISA();
@@ -409,12 +437,14 @@ class SwiftLanguageRuntimeImpl {
409437
/// \{
410438
std::unique_ptr<ReflectionContextInterface> m_reflection_ctx;
411439

440+
/// Mutex guarding accesses to the reflection context.
441+
std::recursive_mutex m_reflection_ctx_mutex;
442+
412443
SwiftMetadataCache m_swift_metadata_cache;
413444

414445
/// Record modules added through ModulesDidLoad, which are to be
415446
/// added to the reflection context once it's being initialized.
416447
ModuleList m_modules_to_add;
417-
std::recursive_mutex m_add_module_mutex;
418448

419449
/// Add the image to the reflection context.
420450
/// \return true on success.

0 commit comments

Comments
 (0)