Skip to content

Commit 7db154f

Browse files
committed
Implement private discriminator lookup for debug map symbol files.
It turns out that this wasn;t implemented at all and the existing tests were working by coincidence because RegisterAllVariables() also injects local copies of global variables, which bypasses the discriminator matching. After properly implementing discriminator support a couple of additional bugs got uncovered which should all be fixed by this commit and the matching Swift compiler update to serialize the isTopLevelGlobal bit on VarDecls. <rdar://problem/58846302>
1 parent ec323e9 commit 7db154f

File tree

6 files changed

+65
-37
lines changed

6 files changed

+65
-37
lines changed

lldb/packages/Python/lldbsuite/test/lang/swift/private_var/main.swift

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,9 @@ func doSomething(b: Int) {
1616
a += b //% self.expect("expr a", substrs=['Int', '= 1'])
1717
}
1818

19-
doSomething(b:2)
19+
func withLocalShadow() {
20+
let a = 23
21+
doSomething(b: a) //% self.expect("log enable lldb expr");self.expect("expr a", substrs=['Int', '= 23'])
22+
}
23+
24+
withLocalShadow()

lldb/source/Plugins/ExpressionParser/Swift/SwiftASTManipulator.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,6 +1109,7 @@ bool SwiftASTManipulator::AddExternalVariables(
11091109
swift::VarDecl(is_static, introducer, is_capture_list, loc, name,
11101110
&m_source_file);
11111111
redirected_var_decl->setInterfaceType(var_type);
1112+
redirected_var_decl->setTopLevelGlobal(true);
11121113

11131114
swift::TopLevelCodeDecl *top_level_code =
11141115
new (ast_context) swift::TopLevelCodeDecl(&m_source_file);
@@ -1222,6 +1223,9 @@ bool SwiftASTManipulator::AddExternalVariables(
12221223
redirected_var_decl->setInterfaceType(interface_type);
12231224
redirected_var_decl->setDebuggerVar(true);
12241225
redirected_var_decl->setImplicit(true);
1226+
// This avoids having local variables filtered out by
1227+
// swift::namelookup::filterForDiscriminator().
1228+
redirected_var_decl->overwriteAccess(swift::AccessLevel::Public);
12251229

12261230
swift::PatternBindingDecl *pattern_binding =
12271231
GetPatternBindingForVarDecl(redirected_var_decl, containing_function);

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

Lines changed: 33 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,6 @@ class LLDBExprNameLookup : public LLDBNameLookup {
413413
}
414414
}
415415
}
416-
417416
return swift::Identifier();
418417
}
419418
};
@@ -742,45 +741,44 @@ static void RegisterAllVariables(
742741
// The module scoped variables are stored at the CompUnit level, so
743742
// after we go through the current context, then we have to take one
744743
// more pass through the variables in the CompUnit.
745-
bool handling_globals = false;
746744
VariableList variables;
747745

748746
// Proceed from the innermost scope outwards, adding all variables
749747
// not already shadowed by an inner declaration.
750748
llvm::SmallDenseSet<const char *, 8> processed_names;
751-
while (true) {
752-
if (!handling_globals) {
753-
constexpr bool can_create = true;
754-
constexpr bool get_parent_variables = false;
755-
constexpr bool stop_if_block_is_inlined_function = true;
756-
757-
block->AppendVariables(
758-
can_create, get_parent_variables, stop_if_block_is_inlined_function,
759-
[](Variable *) { return true; }, &variables);
760-
} else {
761-
if (sc.comp_unit) {
762-
lldb::VariableListSP globals_sp = sc.comp_unit->GetVariableList(true);
763-
if (globals_sp)
764-
variables.AddVariables(globals_sp.get());
765-
}
766-
}
767-
768-
// Process all variables in this scope.
769-
for (size_t vi = 0, ve = variables.GetSize(); vi != ve; ++vi)
770-
AddVariableInfo({variables.GetVariableAtIndex(vi)}, stack_frame_sp,
771-
ast_context, language_runtime, processed_names,
772-
local_variables);
773-
774-
if (!handling_globals) {
775-
if (block == top_block)
776-
// Now add the containing module block, that's what holds the
777-
// module globals:
778-
handling_globals = true;
779-
else
780-
block = block->GetParent();
781-
} else
782-
break;
783-
}
749+
bool done = false;
750+
do {
751+
// Iterate over all parent contexts *including* the top_block.
752+
if (block == top_block)
753+
done = true;
754+
bool can_create = true;
755+
bool get_parent_variables = false;
756+
bool stop_if_block_is_inlined_function = true;
757+
758+
block->AppendVariables(
759+
can_create, get_parent_variables, stop_if_block_is_inlined_function,
760+
[](Variable *) { return true; }, &variables);
761+
762+
if (!done)
763+
block = block->GetParent();
764+
} while (block && !done);
765+
766+
// Also add local copies of globals. This is in many cases redundant
767+
// work because the globals would also be found in the expression
768+
// context's Swift module, but it allows a limited form of
769+
// expression evaluation to work even if the Swift module failed to
770+
// load, as long as the module isn't necessary to resolve the type
771+
// or aother symbols in the expression.
772+
if (sc.comp_unit) {
773+
lldb::VariableListSP globals_sp = sc.comp_unit->GetVariableList(true);
774+
if (globals_sp)
775+
variables.AddVariables(globals_sp.get());
776+
}
777+
778+
for (size_t vi = 0, ve = variables.GetSize(); vi != ve; ++vi)
779+
AddVariableInfo({variables.GetVariableAtIndex(vi)}, stack_frame_sp,
780+
ast_context, language_runtime, processed_names,
781+
local_variables);
784782
}
785783

786784
static void ResolveSpecialNames(

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3099,9 +3099,16 @@ bool SymbolFileDWARF::GetCompileOption(const char *option, std::string &value,
30993099
const uint32_t num_compile_units = GetNumCompileUnits();
31003100

31013101
if (cu) {
3102-
DWARFUnit *dwarf_cu = GetDWARFCompileUnit(cu);
3102+
auto *dwarf_cu =
3103+
llvm::dyn_cast_or_null<DWARFCompileUnit>(GetDWARFCompileUnit(cu));
31033104

31043105
if (dwarf_cu) {
3106+
// GetDWARFCompileUnit() only looks up by CU#. Make sure that
3107+
// this is actually the correct SymbolFile by converting it
3108+
// back to a CompileUnit.
3109+
if (GetCompUnitForDWARFCompUnit(*dwarf_cu) != cu)
3110+
return false;
3111+
31053112
const DWARFBaseDIE die = dwarf_cu->GetUnitDIEOnly();
31063113
if (die) {
31073114
const char *flags =

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1254,6 +1254,17 @@ CompilerDeclContext SymbolFileDWARFDebugMap::FindNamespace(
12541254
return matching_namespace;
12551255
}
12561256

1257+
bool SymbolFileDWARFDebugMap::GetCompileOption(const char *option,
1258+
std::string &value,
1259+
CompileUnit *cu) {
1260+
bool success = false;
1261+
ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool {
1262+
success |= oso_dwarf->GetCompileOption(option, value, cu);
1263+
return success;
1264+
});
1265+
return success;
1266+
}
1267+
12571268
void SymbolFileDWARFDebugMap::DumpClangAST(Stream &s) {
12581269
ForEachSymbolFile([&s](SymbolFileDWARF *oso_dwarf) -> bool {
12591270
oso_dwarf->DumpClangAST(s);

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,9 @@ class SymbolFileDWARFDebugMap : public lldb_private::SymbolFile {
145145

146146
void DumpClangAST(lldb_private::Stream &s) override;
147147

148+
bool GetCompileOption(const char *option, std::string &value,
149+
lldb_private::CompileUnit *cu) override;
150+
148151
// PluginInterface protocol
149152
lldb_private::ConstString GetPluginName() override;
150153

0 commit comments

Comments
 (0)