Skip to content

Commit 4510748

Browse files
vedantkvedantk
authored
Merge pull request #1440 from vedantk/eng/PR-65276251
Require SwiftASTContextReader to be valid at all times
2 parents ac19c36 + edb4f6e commit 4510748

File tree

13 files changed

+95
-57
lines changed

13 files changed

+95
-57
lines changed

‎lldb/include/lldb/Core/SwiftASTContextReader.h

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -91,17 +91,26 @@ struct ScopedSharedMutexReader {
9191
/// or even separate scratch contexts for each lldb::Module. But it
9292
/// will only do this if no client holds on to a read lock on \b
9393
/// m_scratch_typesystem_lock.
94-
struct SwiftASTContextReader : ScopedSharedMutexReader {
95-
SwiftASTContextForExpressions *m_ptr = nullptr;
96-
SwiftASTContextReader() : ScopedSharedMutexReader(nullptr) {}
97-
SwiftASTContextReader(SharedMutex &mutex, SwiftASTContextForExpressions *ctx)
98-
: ScopedSharedMutexReader(&mutex), m_ptr(ctx) {}
94+
class SwiftASTContextReader : ScopedSharedMutexReader {
95+
SwiftASTContextForExpressions *m_ptr;
96+
97+
public:
98+
SwiftASTContextReader(SharedMutex &mutex, SwiftASTContextForExpressions &ctx)
99+
: ScopedSharedMutexReader(&mutex), m_ptr(&ctx) {
100+
assert(m_ptr && "invalid context");
101+
}
102+
99103
SwiftASTContextReader(const SwiftASTContextReader &copy)
100104
: ScopedSharedMutexReader(copy.m_mutex), m_ptr(copy.m_ptr) {}
101-
SwiftASTContextForExpressions *get() { return m_ptr; }
102-
operator bool() const { return m_ptr; }
103-
SwiftASTContextForExpressions *operator->() { return m_ptr; }
104-
SwiftASTContextForExpressions &operator*() { return *m_ptr; }
105+
106+
SwiftASTContextForExpressions *get() {
107+
assert(m_ptr && "invalid context");
108+
return m_ptr;
109+
}
110+
111+
SwiftASTContextForExpressions *operator->() { return get(); }
112+
113+
SwiftASTContextForExpressions &operator*() { return *get(); }
105114
};
106115

107116
/// An RAII object that just acquires the reader lock.

‎lldb/include/lldb/Core/ValueObject.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,7 @@ class ValueObject : public UserID {
584584

585585
virtual bool HasSyntheticValue();
586586

587-
SwiftASTContextReader GetScratchSwiftASTContext();
587+
llvm::Optional<SwiftASTContextReader> GetScratchSwiftASTContext();
588588

589589
virtual bool IsSynthetic() { return false; }
590590

‎lldb/include/lldb/Target/Target.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1127,7 +1127,7 @@ class Target : public std::enable_shared_from_this<Target>,
11271127
return m_scratch_typesystem_lock;
11281128
}
11291129

1130-
SwiftASTContextReader
1130+
llvm::Optional<SwiftASTContextReader>
11311131
GetScratchSwiftASTContext(Status &error, ExecutionContextScope &exe_scope,
11321132
bool create_on_demand = true);
11331133

‎lldb/source/Core/ValueObject.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1697,10 +1697,10 @@ LanguageType ValueObject::GetObjectRuntimeLanguage() {
16971697
return lldb::eLanguageTypeUnknown;
16981698
}
16991699

1700-
SwiftASTContextReader ValueObject::GetScratchSwiftASTContext() {
1700+
llvm::Optional<SwiftASTContextReader> ValueObject::GetScratchSwiftASTContext() {
17011701
lldb::TargetSP target_sp(GetTargetSP());
17021702
if (!target_sp)
1703-
return {};
1703+
return llvm::None;
17041704
Status error;
17051705
ExecutionContext ctx = GetExecutionContextRef().Lock(false);
17061706
auto *exe_scope = ctx.GetBestExecutionContextScope();

‎lldb/source/Core/ValueObjectDynamicValue.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,5 +433,7 @@ bool ValueObjectDynamicValue::DynamicValueTypeInfoNeedsUpdate() {
433433
return false;
434434

435435
auto *cached_ctx = m_value.GetCompilerType().GetTypeSystem();
436-
return cached_ctx == GetScratchSwiftASTContext().get();
436+
llvm::Optional<SwiftASTContextReader> scratch_ctx =
437+
GetScratchSwiftASTContext();
438+
return scratch_ctx ? (cached_ctx == scratch_ctx->get()) : !cached_ctx;
437439
}

‎lldb/source/Expression/Materializer.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -927,9 +927,9 @@ class EntityResultVariable : public Materializer::Entity {
927927

928928
if (m_type.GetMinimumLanguage() == lldb::eLanguageTypeSwift) {
929929
Status status;
930-
auto type_system =
931-
target_sp->GetScratchSwiftASTContext(status, *exe_scope).get();
932-
if (type_system == nullptr) {
930+
llvm::Optional<SwiftASTContextReader> maybe_type_system =
931+
target_sp->GetScratchSwiftASTContext(status, *exe_scope);
932+
if (!maybe_type_system) {
933933
err.SetErrorStringWithFormat("Couldn't dematerialize a result variable: "
934934
"couldn't get the corresponding type "
935935
"system: %s", status.AsCString());

‎lldb/source/Plugins/ExpressionParser/Swift/SwiftExpressionParser.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,11 @@ SwiftExpressionParser::SwiftExpressionParser(
128128

129129
if (target_sp) {
130130
Status error;
131-
m_swift_ast_context = std::make_unique<SwiftASTContextReader>(
132-
target_sp->GetScratchSwiftASTContext(error, *exe_scope, true));
131+
llvm::Optional<SwiftASTContextReader> scratch_ctx =
132+
target_sp->GetScratchSwiftASTContext(error, *exe_scope, true);
133+
if (scratch_ctx)
134+
m_swift_ast_context =
135+
std::make_unique<SwiftASTContextReader>(scratch_ctx.getValue());
133136
}
134137
}
135138

‎lldb/source/Plugins/ExpressionParser/Swift/SwiftUserExpression.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,11 +195,13 @@ void SwiftUserExpression::ScanContext(ExecutionContext &exe_ctx, Status &err) {
195195

196196
// Make sure the target's SwiftASTContext has been setup before doing any
197197
// Swift name lookups.
198-
auto swift_ast_ctx = m_target->GetScratchSwiftASTContext(err, *frame);
199-
if (!swift_ast_ctx) {
198+
llvm::Optional<SwiftASTContextReader> maybe_swift_ast_ctx =
199+
m_target->GetScratchSwiftASTContext(err, *frame);
200+
if (!maybe_swift_ast_ctx) {
200201
LLDB_LOG(log, " [SUE::SC] NULL Swift AST Context");
201202
return;
202203
}
204+
SwiftASTContext *swift_ast_ctx = maybe_swift_ast_ctx->get();
203205

204206
if (!swift_ast_ctx->GetClangImporter()) {
205207
LLDB_LOG(log, " [SUE::SC] Swift AST Context has no Clang importer");

‎lldb/source/Plugins/Language/Swift/SwiftHashedContainer.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -294,13 +294,13 @@ HashedCollectionConfig::StorageObjectAtAddress(
294294
// same address.
295295
Status error;
296296
ExecutionContextScope *exe_scope = exe_ctx.GetBestExecutionContextScope();
297-
auto reader =
297+
llvm::Optional<SwiftASTContextReader> reader =
298298
process_sp->GetTarget().GetScratchSwiftASTContext(error, *exe_scope);
299-
SwiftASTContext *ast_ctx = reader.get();
300-
if (!ast_ctx)
299+
if (!reader)
301300
return nullptr;
302301
if (error.Fail())
303302
return nullptr;
303+
SwiftASTContext *ast_ctx = reader->get();
304304

305305
CompilerType rawStorage_type =
306306
ast_ctx->GetTypeFromMangledTypename(m_nativeStorageRoot_mangled);
@@ -445,10 +445,10 @@ NativeHashedStorageHandler::NativeHashedStorageHandler(
445445
m_value_stride = value_type_stride ? *value_type_stride : 0;
446446
if (TypeSystemSwift *swift_ast =
447447
llvm::dyn_cast_or_null<TypeSystemSwift>(key_type.GetTypeSystem())) {
448-
auto scratch_ctx_reader = nativeStorage_sp->GetScratchSwiftASTContext();
449-
auto scratch_ctx = scratch_ctx_reader.get();
450-
if (!scratch_ctx)
448+
llvm::Optional<SwiftASTContextReader> scratch_ctx_reader = nativeStorage_sp->GetScratchSwiftASTContext();
449+
if (!scratch_ctx_reader)
451450
return;
451+
SwiftASTContext *scratch_ctx = scratch_ctx_reader->get();
452452
auto *runtime = SwiftLanguageRuntime::Get(m_process);
453453
if (!runtime)
454454
return;

‎lldb/source/Plugins/Language/Swift/SwiftLanguage.cpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,9 +1013,11 @@ std::unique_ptr<Language::TypeScavenger> SwiftLanguage::GetTypeScavenger() {
10131013
if (target) {
10141014
const bool create_on_demand = false;
10151015
Status error;
1016-
auto ast_ctx = target->GetScratchSwiftASTContext(
1017-
error, *exe_scope, create_on_demand);
1018-
if (ast_ctx) {
1016+
llvm::Optional<SwiftASTContextReader> maybe_ast_ctx =
1017+
target->GetScratchSwiftASTContext(error, *exe_scope,
1018+
create_on_demand);
1019+
if (maybe_ast_ctx) {
1020+
SwiftASTContext *ast_ctx = maybe_ast_ctx->get();
10191021
ConstString cs_input{input};
10201022
Mangled mangled(cs_input);
10211023
if (mangled.GuessLanguage() == eLanguageTypeSwift) {
@@ -1071,9 +1073,11 @@ std::unique_ptr<Language::TypeScavenger> SwiftLanguage::GetTypeScavenger() {
10711073
Target *target = exe_scope->CalculateTarget().get();
10721074
const bool create_on_demand = false;
10731075
Status error;
1074-
auto ast_ctx = target->GetScratchSwiftASTContext(error, *exe_scope,
1075-
create_on_demand);
1076-
if (ast_ctx) {
1076+
llvm::Optional<SwiftASTContextReader> maybe_ast_ctx =
1077+
target->GetScratchSwiftASTContext(error, *exe_scope,
1078+
create_on_demand);
1079+
if (maybe_ast_ctx) {
1080+
SwiftASTContext *ast_ctx = maybe_ast_ctx->get();
10771081
auto iter = ast_ctx->GetModuleCache().begin(),
10781082
end = ast_ctx->GetModuleCache().end();
10791083

‎lldb/source/Target/SwiftLanguageRuntime.cpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -945,8 +945,10 @@ void SwiftLanguageRuntime::FindFunctionPointersInCall(
945945
Status error;
946946
Target &target = frame.GetThread()->GetProcess()->GetTarget();
947947
ExecutionContext exe_ctx(frame);
948-
auto swift_ast = target.GetScratchSwiftASTContext(error, frame);
949-
if (swift_ast) {
948+
llvm::Optional<SwiftASTContextReader> maybe_swift_ast =
949+
target.GetScratchSwiftASTContext(error, frame);
950+
if (maybe_swift_ast) {
951+
SwiftASTContext *swift_ast = maybe_swift_ast->get();
950952
CompilerType function_type = swift_ast->GetTypeFromMangledTypename(
951953
mangled_name.GetMangledName());
952954
if (error.Success()) {
@@ -1152,9 +1154,11 @@ SwiftLanguageRuntime::CalculateErrorValue(StackFrameSP frame_sp,
11521154
if (!exe_scope)
11531155
return error_valobj_sp;
11541156

1155-
auto ast_context = target->GetScratchSwiftASTContext(error, *frame_sp);
1156-
if (!ast_context || error.Fail())
1157+
llvm::Optional<SwiftASTContextReader> maybe_ast_context =
1158+
target->GetScratchSwiftASTContext(error, *frame_sp);
1159+
if (!maybe_ast_context || error.Fail())
11571160
return error_valobj_sp;
1161+
SwiftASTContext *ast_context = maybe_ast_context->get();
11581162

11591163
lldb::DataBufferSP buffer(new lldb_private::DataBufferHeap(
11601164
arg0->GetScalar().GetBytes(), arg0->GetScalar().GetByteSize()));
@@ -1483,9 +1487,9 @@ SwiftLanguageRuntimeImpl::GetBridgedSyntheticChildProvider(
14831487
ProjectionSyntheticChildren::TypeProjectionUP type_projection(
14841488
new ProjectionSyntheticChildren::TypeProjectionUP::element_type());
14851489

1486-
if (auto swift_ast_ctx = valobj.GetScratchSwiftASTContext()) {
1490+
if (auto maybe_swift_ast_ctx = valobj.GetScratchSwiftASTContext()) {
14871491
CompilerType swift_type =
1488-
swift_ast_ctx->GetTypeFromMangledTypename(type_name);
1492+
maybe_swift_ast_ctx->get()->GetTypeFromMangledTypename(type_name);
14891493

14901494
if (swift_type.IsValid()) {
14911495
ExecutionContext exe_ctx(m_process);

‎lldb/source/Target/SwiftLanguageRuntimeDynamicTypeResolution.cpp

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -513,18 +513,20 @@ SwiftLanguageRuntime::MetadataPromise::FulfillTypePromise(Status *error) {
513513
if (m_compiler_type.hasValue())
514514
return m_compiler_type.getValue();
515515

516-
auto swift_ast_ctx = m_for_object_sp->GetScratchSwiftASTContext();
517-
if (!swift_ast_ctx) {
516+
llvm::Optional<SwiftASTContextReader> maybe_swift_ast_ctx =
517+
m_for_object_sp->GetScratchSwiftASTContext();
518+
if (!maybe_swift_ast_ctx) {
518519
error->SetErrorString("couldn't get Swift scratch context");
519520
return CompilerType();
520521
}
522+
SwiftASTContext *swift_ast_ctx = maybe_swift_ast_ctx->get();
521523
auto &remote_ast = m_swift_runtime.GetRemoteASTContext(*swift_ast_ctx);
522524
swift::remoteAST::Result<swift::Type> result =
523525
remote_ast.getTypeForRemoteTypeMetadata(
524526
swift::remote::RemoteAddress(m_metadata_location));
525527

526528
if (result) {
527-
m_compiler_type = {swift_ast_ctx.get(), result.getValue().getPointer()};
529+
m_compiler_type = {swift_ast_ctx, result.getValue().getPointer()};
528530
if (log)
529531
log->Printf("[MetadataPromise] result is type %s",
530532
m_compiler_type->GetTypeName().AsCString());
@@ -543,10 +545,13 @@ SwiftLanguageRuntime::MetadataPromise::FulfillTypePromise(Status *error) {
543545
SwiftLanguageRuntime::MetadataPromiseSP
544546
SwiftLanguageRuntimeImpl::GetMetadataPromise(lldb::addr_t addr,
545547
ValueObject &for_object) {
546-
auto swift_ast_ctx = for_object.GetScratchSwiftASTContext();
547-
if (!swift_ast_ctx || swift_ast_ctx->HasFatalErrors())
548+
llvm::Optional<SwiftASTContextReader> maybe_swift_ast_ctx =
549+
for_object.GetScratchSwiftASTContext();
550+
if (!maybe_swift_ast_ctx)
551+
return nullptr;
552+
SwiftASTContext *swift_ast_ctx = maybe_swift_ast_ctx->get();
553+
if (swift_ast_ctx->HasFatalErrors())
548554
return nullptr;
549-
550555
if (addr == 0 || addr == LLDB_INVALID_ADDRESS)
551556
return nullptr;
552557

@@ -1038,9 +1043,11 @@ SwiftLanguageRuntimeImpl::DoArchetypeBindingForType(StackFrame &stack_frame,
10381043
auto &target = m_process.GetTarget();
10391044
assert(IsScratchContextLocked(target) &&
10401045
"Swift scratch context not locked ahead of archetype binding");
1041-
auto scratch_ctx = target.GetScratchSwiftASTContext(error, stack_frame);
1042-
if (!scratch_ctx)
1046+
llvm::Optional<SwiftASTContextReader> maybe_scratch_ctx =
1047+
target.GetScratchSwiftASTContext(error, stack_frame);
1048+
if (!maybe_scratch_ctx)
10431049
return base_type;
1050+
SwiftASTContext *scratch_ctx = maybe_scratch_ctx->get();
10441051
base_type = scratch_ctx->ImportType(base_type, error);
10451052

10461053
if (base_type.GetTypeInfo() & lldb::eTypeIsSwift) {
@@ -1425,11 +1432,13 @@ bool SwiftLanguageRuntimeImpl::GetDynamicTypeAndAddress_ClangType(
14251432
// Import the remangled dynamic name into the scratch context.
14261433
assert(IsScratchContextLocked(in_value.GetTargetSP()) &&
14271434
"Swift scratch context not locked ahead of dynamic type resolution");
1428-
auto scratch_ctx = in_value.GetScratchSwiftASTContext();
1429-
if (!scratch_ctx)
1435+
llvm::Optional<SwiftASTContextReader> maybe_scratch_ctx =
1436+
in_value.GetScratchSwiftASTContext();
1437+
if (!maybe_scratch_ctx)
14301438
return false;
14311439
CompilerType swift_type =
1432-
scratch_ctx->GetTypeFromMangledTypename(ConstString(remangled));
1440+
maybe_scratch_ctx->get()->GetTypeFromMangledTypename(
1441+
ConstString(remangled));
14331442

14341443
// Roll back the ObjC dynamic type resolution.
14351444
if (!swift_type)
@@ -1483,9 +1492,11 @@ bool SwiftLanguageRuntimeImpl::GetDynamicTypeAndAddress(
14831492
// use the scratch context where such operations are legal and safe.
14841493
assert(IsScratchContextLocked(in_value.GetTargetSP()) &&
14851494
"Swift scratch context not locked ahead of dynamic type resolution");
1486-
auto scratch_ctx = in_value.GetScratchSwiftASTContext();
1487-
if (!scratch_ctx)
1495+
llvm::Optional<SwiftASTContextReader> maybe_scratch_ctx =
1496+
in_value.GetScratchSwiftASTContext();
1497+
if (!maybe_scratch_ctx)
14881498
return false;
1499+
SwiftASTContextForExpressions *scratch_ctx = maybe_scratch_ctx->get();
14891500

14901501
auto retry_once = [&]() {
14911502
// Retry exactly once using the per-module fallback scratch context.

‎lldb/source/Target/Target.cpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2336,11 +2336,12 @@ Target::GetPersistentExpressionStateForLanguage(lldb::LanguageType language) {
23362336
SwiftPersistentExpressionState *
23372337
Target::GetSwiftPersistentExpressionState(ExecutionContextScope &exe_scope) {
23382338
Status error;
2339-
auto swift_ast_context = GetScratchSwiftASTContext(error, exe_scope, true);
2340-
if (!swift_ast_context)
2339+
auto maybe_swift_ast_context =
2340+
GetScratchSwiftASTContext(error, exe_scope, true);
2341+
if (!maybe_swift_ast_context)
23412342
return nullptr;
23422343
return (SwiftPersistentExpressionState *)
2343-
swift_ast_context->GetPersistentExpressionState();
2344+
maybe_swift_ast_context->get()->GetPersistentExpressionState();
23442345
}
23452346

23462347
UserExpression *Target::GetUserExpressionForLanguage(
@@ -2423,7 +2424,7 @@ ClangASTImporterSP Target::GetClangASTImporter() {
24232424
return ClangASTImporterSP();
24242425
}
24252426

2426-
SwiftASTContextReader Target::GetScratchSwiftASTContext(
2427+
llvm::Optional<SwiftASTContextReader> Target::GetScratchSwiftASTContext(
24272428
Status &error, ExecutionContextScope &exe_scope, bool create_on_demand) {
24282429
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_TARGET));
24292430

@@ -2504,7 +2505,9 @@ SwiftASTContextReader Target::GetScratchSwiftASTContext(
25042505
error);
25052506
}
25062507

2507-
return SwiftASTContextReader(GetSwiftScratchContextLock(), swift_ast_ctx);
2508+
if (!swift_ast_ctx)
2509+
return llvm::None;
2510+
return SwiftASTContextReader(GetSwiftScratchContextLock(), *swift_ast_ctx);
25082511
}
25092512

25102513
static SharedMutex *

0 commit comments

Comments
 (0)